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)
This commit is contained in:
Aman Agrawal 2026-01-05 13:18:56 +05:30 committed by Tim Abbott
parent 35a2ad7bfd
commit ea2ba227eb
4 changed files with 23 additions and 25 deletions

View File

@ -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);

View File

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

View File

@ -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);

View File

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