-
Notifications
You must be signed in to change notification settings - Fork 195
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
Limit duplicate search creation #4687
Limit duplicate search creation #4687
Conversation
@rtibbles I'm not sure if this is intended behavior but using the saved search entry seems to duplicate the entry. Everything else looks fine! Screen.Recording.2024-08-30.at.15.26.52.mov |
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.
Code changes and manual QA seem to checkout. Open question for me is raised here otherwise we should be good to merge
Ah, no - that is not intended, and actually probably more likely the original origin of the bug reported, rather than "save search" being clicked more than once. Will update. |
…id is read only. Return uuid in plain hex format.
Disable when there are no saved searches to view.
@akolson I have replicated the issue you reported (which I think was the root issue). I have also resolved a couple of other issues with the search functionality in the process, as they were simple to fix and impeded my 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.
LGTM. Issue with duplication has been fixed and code changes make sense to me. Thanks @rtibbles
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Summary
Description of the change(s) you made
Manual verification steps performed
Created a saved search
Confirmed I could not save it again
Refreshed page, confirmed it was still the case
Wait for sync to resolve, reload page, see that there is no duplication
Clear search, see that it navigates back to page
Delete all saved searches, see that it is not possible to show the saved search modal
Screenshots (if applicable)
References
Fixes #4655
Fixes #4287
Fixes #3118
Comments
Contributor's Checklist
PR process:
CHANGELOG
label been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocs
label has been added if this introduces a change that needs to be updated in the user docs?requirements.txt
files also included in this PRStudio-specifc:
notranslate
class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages
,components
, andlayouts
directories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarn
andpip
)