Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

search: Add search suggestion for "unresolved topics". #31758

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+26 −2
Open

search: Add search suggestion for "unresolved topics". #31758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
search: Add search suggestion for "unresolved topics". 
Previously, the search suggestion only featured "resolved topics" and
filtering for "unresolved topics" was not as intuitive. Users would have
to use the negate operator "-" to filter for unresolved topics and it is
not possible to edit pills.

This change adds "unresolved topics" to the search with support to cancel
negations.

Fixes: #31725.
  • Loading branch information
@Joelute
Joelute committed Sep 26, 2024
commit 5be7bc12e29b478fc714961a4a9f32f1edfac69f
26 changes: 24 additions & 2 deletions web/src/search_suggestion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,20 +597,31 @@ function get_special_filter_suggestions(

const last_string = Filter.unparse([last]).toLowerCase();
suggestions = suggestions.filter((s) => {
let search_string = s.search_string;
if (match_criteria(terms, s.incompatible_patterns)) {
return false;
}
if (last_string === "") {
return true;
}

// If the suggestion is already negated and the search query is also negated,
// we should cancel out the negation while maintaining a suggestion match.
if (
(last.negated === true || is_search_operand_negated) &&
search_string.startsWith("--")
) {
search_string = search_string.slice(1);
s.search_string = s.search_string.slice(2);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain a bit more why you're keeping two copies of the search string here, and why you're slicing them on different indices? Maybe you can share an example of what would happen here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of - with the suggestion of -is:resolved, we would get a suggestion string of --is:resolved.
One copy of the string is to used to store the string that the typeahead should return. is:resolved
The other copy of the string is used for pattern matching -is:resolved

}

// returns the substring after the ":" symbol.
const suggestion_operand = s.search_string.slice(s.search_string.indexOf(":") + 1);
const suggestion_operand = search_string.slice(search_string.indexOf(":") + 1);
// e.g for `att` search query, `has:attachment` should be suggested.
const show_operator_suggestions =
last.operator === "search" && suggestion_operand.toLowerCase().startsWith(last_string);
return (
s.search_string.toLowerCase().startsWith(last_string) ||
search_string.toLowerCase().startsWith(last_string) ||
show_operator_suggestions ||
s.description_html.toLowerCase().startsWith(last_string)
);
Expand Down Expand Up @@ -710,6 +721,17 @@ function get_is_filter_suggestions(last: NarrowTerm, terms: NarrowTerm[]): Sugge
{operator: "dm-including"},
],
},
{
search_string: "-is:resolved",
description_html: "unresolved topics",
is_people: false,
incompatible_patterns: [
{operator: "is", operand: "resolved"},
{operator: "is", operand: "dm"},
{operator: "dm"},
{operator: "dm-including"},
],
},
];
const special_filtered_suggestions = get_special_filter_suggestions(last, terms, suggestions);
// Suggest "is:dm" to anyone with "is:private" in their muscle memory
Expand Down
2 changes: 2 additions & 0 deletions web/tests/search_suggestion.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ test("empty_query_suggestions", () => {
"is:alerted",
"is:unread",
"is:resolved",
"-is:resolved",
"sender:myself@zulip.com",
`channel:${devel_id}`,
`channel:${office_id}`,
Expand Down Expand Up @@ -504,6 +505,7 @@ test("check_is_suggestions", ({override, mock_template}) => {
"-is:alerted",
"-is:unread",
"-is:resolved",
"is:resolved",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this being included in a query that starts with a -?

Copy link
Collaborator Author

@Joelute Joelute Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an agreement to include it in CZO. Due to the confusion, I'm wondering if we should change course and just remove that from being suggested for -.

];
assert.deepEqual(suggestions.strings, expected);

Expand Down