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.

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 delete warnings #662

Merged
merged 13 commits into from
Jul 20, 2022
Merged

Search delete warnings #662

merged 13 commits into from
Jul 20, 2022

Conversation

shapiromatron
Copy link
Owner

@shapiromatron shapiromatron commented Jul 19, 2022

Add more guards to prevent cascading delete of reference imports

  1. Force user to write "delete" instead of clicking a button
  2. Instead of hypothetical data loss, show exactly what will be cascade deleted if a user proceeds

Make it very clear when a user is about to remove a search that has data related to it. Instead of a hypothetical warning, show exact numbers for data removed from this deletion.

Before:
Screen Shot 2022-07-19 at 8 28 46 AM

After:
image

In addition, add guard on PubMed imports to also not delete removed references which have related studies.

@shapiromatron shapiromatron requested a review from caseyhans July 20, 2022 17:25
@shapiromatron
Copy link
Owner Author

@casey1173 ready for review. I made two big changes:

  1. used a template instead of string manipulation to render the message
  2. updated numbers to be exact, to show what will be deleted if a user proceeds. We're using the same calculation in the warning message is what's actually done when a user deletes.

@shapiromatron shapiromatron marked this pull request as ready for review July 20, 2022 17:28
Copy link
Collaborator

@caseyhans caseyhans left a comment

Choose a reason for hiding this comment

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

Changed some wording slightly for personal clarity--feel free to change back if you'd like.
I also changed the comma-separated list to a semicolon-separated list since most studies have commas in the name (e.g., Butterworth, 1998, 1023693).

@shapiromatron shapiromatron merged commit 2d452e4 into main Jul 20, 2022
@shapiromatron shapiromatron deleted the search-delete-warnings branch July 20, 2022 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants