From 6b617f893d73fb9e8304d228336cf574c29992a3 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 12 Nov 2022 16:48:01 +0100 Subject: [PATCH 1/5] SearchResults: remove computed search prop It is only used in one location, and not from the template. In other words we should inline it to make the code simpler. --- client/components/Windows/SearchResults.vue | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/client/components/Windows/SearchResults.vue b/client/components/Windows/SearchResults.vue index a9dbdb8f..b6265e2b 100644 --- a/client/components/Windows/SearchResults.vue +++ b/client/components/Windows/SearchResults.vue @@ -129,13 +129,14 @@ export default defineComponent({ const oldScrollTop = ref(0); const oldChatHeight = ref(0); - const search = computed(() => store.state.messageSearchResults); const messages = computed(() => { - if (!search.value) { + const results = store.state.messageSearchResults?.results; + + if (!results) { return []; } - return search.value.results; + return results; }); const chan = computed(() => { @@ -307,7 +308,6 @@ export default defineComponent({ loadMoreButton, messages, moreResultsAvailable, - search, network, channel, route, From dca202427aa543d43d18fb72ae10ffa51b3b6c60 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 12 Nov 2022 16:59:57 +0100 Subject: [PATCH 2/5] SearchResults: Fix search progess upon search When we hit doSearch, we always reset the offset value to 0, meaning we always hit the conditional (!0) and always set the messageSearchInProgress flag to undefined. This is wrong, we do want to set this flag when we initiate a search. --- client/components/Windows/SearchResults.vue | 5 ----- 1 file changed, 5 deletions(-) diff --git a/client/components/Windows/SearchResults.vue b/client/components/Windows/SearchResults.vue index b6265e2b..c4d799ed 100644 --- a/client/components/Windows/SearchResults.vue +++ b/client/components/Windows/SearchResults.vue @@ -189,11 +189,6 @@ export default defineComponent({ const doSearch = () => { offset.value = 0; store.commit("messageSearchInProgress", true); - - if (!offset.value) { - store.commit("messageSearchInProgress", undefined); // Only reset if not getting offset - } - socket.emit("search", { networkUuid: network.value?.uuid, channelName: channel.value?.name, From 8095d9e88a0018d2ac559ab01488d2736b4fe5e6 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 12 Nov 2022 22:50:56 +0100 Subject: [PATCH 3/5] SearchQuery: offset is always a number Fix type confusion that specified offset to be a string, it is always a number. --- server/plugins/messageStorage/sqlite.ts | 11 +++++------ server/plugins/messageStorage/types.d.ts | 2 +- test/plugins/sqlite.ts | 8 ++++++-- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index f2a3b526..50eae3c1 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -224,7 +224,7 @@ class SqliteMessageStorage implements ISqliteMessageStorage { let select = 'SELECT msg, type, time, network, channel FROM messages WHERE type = "message" AND json_extract(msg, "$.text") LIKE ? ESCAPE \'@\''; - const params = [`%${escapedSearchTerm}%`]; + const params: any[] = [`%${escapedSearchTerm}%`]; if (query.networkUuid) { select += " AND network = ? "; @@ -239,9 +239,8 @@ class SqliteMessageStorage implements ISqliteMessageStorage { const maxResults = 100; select += " ORDER BY time DESC LIMIT ? OFFSET ? "; - params.push(maxResults.toString()); - query.offset = parseInt(query.offset as string, 10) || 0; - params.push(String(query.offset)); + params.push(maxResults); + params.push(query.offset); return new Promise((resolve, reject) => { this.database.all(select, params, (err, rows) => { @@ -252,8 +251,8 @@ class SqliteMessageStorage implements ISqliteMessageStorage { searchTerm: query.searchTerm, target: query.channelName, networkUuid: query.networkUuid, - offset: query.offset as number, - results: parseSearchRowsToMessages(query.offset as number, rows).reverse(), + offset: query.offset, + results: parseSearchRowsToMessages(query.offset, rows).reverse(), }; resolve(response); } diff --git a/server/plugins/messageStorage/types.d.ts b/server/plugins/messageStorage/types.d.ts index 6dfa20db..9e81929e 100644 --- a/server/plugins/messageStorage/types.d.ts +++ b/server/plugins/messageStorage/types.d.ts @@ -26,7 +26,7 @@ export type SearchQuery = { searchTerm: string; networkUuid: string; channelName: string; - offset: number | string; + offset: number; }; export type SearchResponse = diff --git a/test/plugins/sqlite.ts b/test/plugins/sqlite.ts index 00211339..bd27141f 100644 --- a/test/plugins/sqlite.ts +++ b/test/plugins/sqlite.ts @@ -167,7 +167,9 @@ describe("SQLite Message Storage", function () { .search({ searchTerm: "msg", networkUuid: "retrieval-order-test-network", - } as any) + channelName: "", + offset: 0, + }) .then((messages) => { // @ts-expect-error Property 'results' does not exist on type '[]'. expect(messages.results).to.have.lengthOf(100); @@ -192,7 +194,9 @@ describe("SQLite Message Storage", function () { .search({ searchTerm: query, networkUuid: "this-is-a-network-guid2", - } as any) + channelName: "", + offset: 0, + }) .then((messages) => { // @ts-expect-error Property 'results' does not exist on type '[]'. expect(messages.results.map((i) => i.text)).to.deep.equal(expected); From 51c9ce078d15efafd677cff525b681dcec51fdd5 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 12 Nov 2022 23:16:16 +0100 Subject: [PATCH 4/5] Search: fix off by one offset error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Offset is eventually passed to sqlite as an OFFSET clause. This works as follows: sqlite> select num from seq limit 5 offset 0; ┌─────┐ │ num │ ├─────┤ │ 1 │ │ 2 │ │ 3 │ │ 4 │ │ 5 │ └─────┘ sqlite> select num from seq limit 5 offset 5; ┌─────┐ │ num │ ├─────┤ │ 6 │ │ 7 │ │ 8 │ │ 9 │ │ 10 │ └─────┘ However, the code currently emits a request for offset + 1, which ends up skipping a message sqlite> select num from seq limit 5 offset 5+1; ┌─────┐ │ num │ ├─────┤ │ 7 │ │ 8 │ │ 9 │ │ 10 │ │ 11 │ └─────┘ --- client/components/Windows/SearchResults.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/components/Windows/SearchResults.vue b/client/components/Windows/SearchResults.vue index c4d799ed..322a0173 100644 --- a/client/components/Windows/SearchResults.vue +++ b/client/components/Windows/SearchResults.vue @@ -212,7 +212,7 @@ export default defineComponent({ networkUuid: network.value?.uuid, channelName: channel.value?.name, searchTerm: String(route.query.q || ""), - offset: offset.value + 1, + offset: offset.value, }); }; From 83e11b0143e599a40924cab856636beeca6df27c Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 12 Nov 2022 23:51:07 +0100 Subject: [PATCH 5/5] Search: Clear earlier searches when a new one is executed Fixes: https://github.com/thelounge/thelounge/issues/4637 --- client/components/Windows/SearchResults.vue | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/client/components/Windows/SearchResults.vue b/client/components/Windows/SearchResults.vue index 322a0173..f7c98f79 100644 --- a/client/components/Windows/SearchResults.vue +++ b/client/components/Windows/SearchResults.vue @@ -186,9 +186,14 @@ export default defineComponent({ return new Date(previousMessage.time).getDay() !== new Date(message.time).getDay(); }; - const doSearch = () => { + const clearSearchState = () => { offset.value = 0; - store.commit("messageSearchInProgress", true); + store.commit("messageSearchInProgress", false); + store.commit("messageSearchResults", null); + }; + + const doSearch = () => { + clearSearchState(); // this is a new search, so we need to clear anything before that socket.emit("search", { networkUuid: network.value?.uuid, channelName: channel.value?.name, @@ -296,6 +301,7 @@ export default defineComponent({ onUnmounted(() => { eventbus.off("escapekey", closeSearch); eventbus.off("re-search", doSearch); + clearSearchState(); }); return {