-
Notifications
You must be signed in to change notification settings - Fork 221
feat(website): Add search suggestions to OSV database #3660
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
feat(website): Add search suggestions to OSV database #3660
Conversation
I'll resolve the conflicts and look into this, but I have a feeling this could be related to Turbo's cache, did you try a hard reload? |
I'm facing the same issue actually but it goes away when I do a hard reload or try in incognito somehow which is strange XD - I will investigate this further and also try to add the dropdown feature to the navbar search too. |
- Address page initialization errors by applying retry and defensive mechanisms - Add orphaned DOM element cleanup to prevent layout interference - Refactor search modules
I've pushed a fix (hopefully), implemented a few retry mechanisms and fallbacks to ensure the page loads well on the first try. Also restructured the modules by exporting the Search Suggestions class to |
Awesome! My small nit would be - would it be possible to show the ids in the search suggestion in uppercase instead of lower? - most vuln IDs present as uppercase :). I'll review the rest of the code when I get the chance! |
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.
Thanks, very nice! Made a bunch of comments.
I do have a question on why the searchSuggestionsManager need to be cleanedup and recreated so much (at least code length wise), is it just because of the "turbo" load?
@another-rex thanks for the review, I'm going through them now and will update! Yes, I've added those retries because of the Turbo cache bug, it seemed that this was the way I got it to stabilize and not load unstyled without needing a forced reload. Weird bug but that seemed to work :) |
Applied the required linter formatting. The CI check should now pass. |
Looks amazing! |
/gcbrun |
LGTM! Great work! We will aim to merge this on Tuesday (Sydney time) after this week's release so we have a week to observe it in the test instance, and make sure the database changes happen smoothly :) |
/gcbrun |
Ah it looks like the worker tests are failing because of the added |
Thanks for the heads up, I'll run the tests locally and update you shortly! |
@jess-lowe I've updated the test data and the tests are passing for me now. Let me know if this works! |
/gcbrun |
It's now live on test.osv.dev! Only some records are showing up as we populate the search_tags. I did find a bug though, if you scroll down a little bit and try to search, the search dropdown will be offset in the wrong spot. (I think one of the offset calculations in javascript has a sign flipped?) |
Awesome, Thanks for the update! Yes it seems like this is happening on both dropdowns, I will look into this and patch it shortly. |
Checked the code, calculations look correct, it just didnt account for the relative position of the document. |
This PR introduces a new
/api/search_suggestions
endpoint that aims to provide real-time search suggestions for the OSV vulnerability database.This PR is part of a larger effort to enhance the search experience across the website (see PR #3657)
Addresses issue #382