From ea2ba227eba5c42da2c395c19d9599fefa143900 Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Mon, 5 Jan 2026 13:18:56 +0530 Subject: [PATCH] filter: Use `convert_suggestion_to_term` to validate suggestions. This will allow to perform additional operations on search suggestions before converting them to narrow term and then performing the required validation. Another benefit of this is we can use the validated term directly in the caller code without doing any other check. (coming in future commits) --- web/src/filter.ts | 33 +++++++++++++++------------------ web/src/search_pill.ts | 4 ++-- web/src/search_suggestion.ts | 2 +- web/tests/filter.test.cjs | 9 +++++---- 4 files changed, 23 insertions(+), 25 deletions(-) diff --git a/web/src/filter.ts b/web/src/filter.ts index a84a3fe17b..0509511c2e 100644 --- a/web/src/filter.ts +++ b/web/src/filter.ts @@ -468,31 +468,28 @@ export class Filter { }; } - static convert_suggestion_to_term(suggestion: NarrowTermSuggestion): NarrowTerm | undefined { + static convert_suggestion_to_term( + suggestion: NarrowTermSuggestion, + ): NarrowCanonicalTerm | undefined { + // We don't want to raise exceptions in this function; just + // return undefined for invalid terms. + // // NOTE: We will add more logic here once `NarrowTerm` // operand has different type based on operator. - const potential_narrow_term: NarrowTerm = { - operator: suggestion.operator, - operand: suggestion.operand, - negated: suggestion.negated, - }; + try { + const potential_narrow_term: NarrowCanonicalTerm = Filter.canonicalize_term(suggestion); - if (!Filter.is_valid_search_term(potential_narrow_term)) { + if (!Filter.is_valid_canonical_term(potential_narrow_term)) { + return undefined; + } + + return potential_narrow_term; + } catch { return undefined; } - - return potential_narrow_term; } - static is_valid_search_term(term: NarrowTerm): boolean { - // We don't want to raise exceptions in this function; just - // return false for invalid terms. - try { - term = Filter.canonicalize_term(term); - } catch { - return false; - } - + static is_valid_canonical_term(term: NarrowCanonicalTerm): boolean { switch (term.operator) { case "has": return ["image", "link", "attachment", "reaction"].includes(term.operand); diff --git a/web/src/search_pill.ts b/web/src/search_pill.ts index 7a9592b8da..951588f181 100644 --- a/web/src/search_pill.ts +++ b/web/src/search_pill.ts @@ -349,7 +349,7 @@ export function set_search_bar_contents( continue; } - if (!Filter.is_valid_search_term(term)) { + if (Filter.convert_suggestion_to_term(term) === undefined) { invalid_inputs.push(input); continue; } @@ -357,7 +357,7 @@ export function set_search_bar_contents( if (user_pill_operators.has(term.operator) && term.operand !== "") { const users = term.operand.split(",").map((email) => { // This is definitely not undefined, because we just validated it - // with `Filter.is_valid_search_term`. + // with `Filter.convert_suggestion_to_term`. const user = people.get_by_email(email)!; return user; }); diff --git a/web/src/search_suggestion.ts b/web/src/search_suggestion.ts index f079348109..58da1622a1 100644 --- a/web/src/search_suggestion.ts +++ b/web/src/search_suggestion.ts @@ -1117,7 +1117,7 @@ export function get_search_result( attacher.push([...attacher.base, ...suggestion_line]); } else if ( all_search_terms.length > 0 && - all_search_terms.every((term) => Filter.is_valid_search_term(term)) + all_search_terms.every((term) => Filter.convert_suggestion_to_term(term) !== undefined) ) { suggestion_line = get_default_suggestion_line(all_search_terms); attacher.push(suggestion_line); diff --git a/web/tests/filter.test.cjs b/web/tests/filter.test.cjs index cf3f5fe00e..4c8b14cb6c 100644 --- a/web/tests/filter.test.cjs +++ b/web/tests/filter.test.cjs @@ -2089,7 +2089,7 @@ test("first_valid_id_from", ({override}) => { assert.equal(filter.first_valid_id_from(msg_ids), 20); }); -test("is_valid_search_term", () => { +test("convert_suggestion_to_term", () => { const denmark = { stream_id: 100, name: "Denmark", @@ -2118,16 +2118,17 @@ test("is_valid_search_term", () => { ]; for (const [search_term_string, expected_is_valid] of test_data) { assert.equal( - Filter.is_valid_search_term(Filter.parse(search_term_string)[0]), + Filter.convert_suggestion_to_term(Filter.parse(search_term_string)[0]) !== undefined, expected_is_valid, ); } + // Invalid operator. assert.equal( - Filter.is_valid_search_term({ + Filter.convert_suggestion_to_term({ operator: "foo", operand: "bar", - }), + }) !== undefined, false, ); });