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, ); });