-
-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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.
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: fix negated suggestions not working in some cases #9477
Conversation
Partially fixes zulip#9461. Negated suggestion for both operand and operators are handle in get_special_filter_suggestions. A bug is get_operator_suggestions causing the removal of `-` symbol from the operand was also fixed.
`get_containing_suggestions` was used to get the operand suggestions for the `has` operator. `get_special_filter_suggestions` is now used to get both the operand and operator suggestions for `has`.
Hello @zulip/server-search members, this pull request was labeled with the "area: search" label, so you may want to check it out! |
query = 'hel'; | ||
suggestions = search.get_suggestions(query); | ||
expected = [ | ||
"hel", | ||
"stream:dev+help", | ||
]; | ||
assert.deepEqual(suggestions.strings, expected); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this was accidentally removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timabbott updated, thanks for the review.
attach_suggestions(result, base, suggestions); | ||
|
||
suggestions = get_person_suggestions(persons, last, base_operators, 'sender'); | ||
suggestions = get_person_suggestions(persons, last, base_operators, 'pm-with'); | ||
attach_suggestions(result, base, suggestions); | ||
|
||
suggestions = get_person_suggestions(persons, last, base_operators, 'from'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Structurally, we want to remove below from:
as well, right, since that's just another spelling for "sender:"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as for the case of not suggesting from
queries below when we are suggesting sender
queries, this has been taken by the following line of code here.
// Be especially strict about the less common "from" operator.
if (autocomplete_operator === 'from' && last.operator !== 'from') {
return [];
}
The above ensures that from
suggestions are shown only when from
has been decide as an operator. i.e query starts with from:
. But removing the fromsuggestions from below would mean that we will not show a
fromsuggestion even when the user types
from:` in the query bar which we don't want I think.
Fixes zulip#9461. Adds negated suggestions for stream filters when the query is negated which were previously being returned empty.
Merged, after marking the last commit to fix #9313. Thanks @shubham-padia! |
fixes #9461.
fixes #9313.
negated special filter operator suggestions
negated special filter operand suggestions
negated stream suggestions:
negated sent_by_me suggestions:
previously sent by me suggestion was not occurring in the below query: