-
Notifications
You must be signed in to change notification settings - Fork 543
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
AO3-6669 Search for noncanonical tags for autocomplete via Elasticsearch #4717
Conversation
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.
Two minor style things I noticed!
.where(["canonical = 0 AND name LIKE ?", | ||
'%' + search_param + '%']).limit(10).map(&:name)) | ||
one_tag = tag_class.find_by(canonical: false, name: params[:term]) | ||
# Is there a tag which is just right (this is really for testing) |
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.
Could you explain how this is just for testing? I see match
is used in rendering the output so I am slightly confused.
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.
@zz9pzza please correct me if I'm wrong, but this is hit in if a user types the whole tag into autocomplete without choosing one of the suggestions first. The tests definitely do that, but in theory a real user could as well (although that seems potentially unlikely)
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.
If this is used where I think it's used (the Synonyms field on canonical tags' edit pages), there's actually a pretty good choice the wrangler will be entering the name of the exact tag they're looking for, fwiw!
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.
The code will not find exact matches naturally, adding the check for the exact match match makes this use case work and the elasticsearch code finds good matches for the autocomplete.
Pull Request Checklist
as the first thing in your pull request title (e.g.
AO3-1234 Fix thing
)until they are reviewed and merged before creating new pull requests.
Issue
https://otwarchive.atlassian.net/browse/AO3-6669
Purpose
What does this PR do?
Testing Instructions
How can the Archive's QA team verify that this is working as you intended?
If you have a Jira account with access, please update or comment on the issue
with any new or missing testing instructions instead.
References
Are there other relevant issues/pull requests/mailing list discussions?
Credit
What name and pronouns should we use to credit you in the Archive of Our Own's Release Notes?
If you have a Jira account, please include the same name in the "Full name"
field on your Jira profile, so we can assign you the issues you're working on.
Please note that if you do not fill in this section, we will use your GitHub account name and
they/them pronouns.