ProjectSidewalk / SidewalkWebpage

Project Sidewalk web page
http://projectsidewalk.org
MIT License
84 stars 25 forks source link

On /labelmap/validate endpoint, delete label's validation if already exists for user #3632

Closed jsomeara closed 2 months ago

jsomeara commented 2 months ago

Resolves #3629

Before, the /labelmap/validate endpoint did not check if a user already had a validation for the given label. As a result, multiple validations could be created, leading to a variety of issues. This fix adds a check in the endpoint to see if a validation has already been made by the user. If yes, then it deletes the old validation and creates a new validation. If no, then it simply creates a new validation. This matches the behavior in processValidationTaskSubmissions, which did already have the check implemented correctly.

Video

https://github.com/user-attachments/assets/7ae4bf08-1d63-4552-992c-f5e957a8b65f

Testing instructions
  1. Go to the gallery
  2. Find a label and switch back and forth between agree and disagree. Previously, this would create multiple validations (see issue for more details).
  3. Refresh and go to that same label. Instead of the previous multiple validations, you should see only one now.
Things to check before submitting the PR
misaugstad commented 2 months ago

I think that the code here is looking good to prevent this issue going forward, now we just want to retroactively fix this for old duplicate validations! The way that we'll do this is through a "database migration", called "database evolutions" in our framework.

You'll create a new file in the conf/evolutions directory (they are named with incrementing numbers). You can look through our other evolutions to get a sense for what they're about. There's an "Ups" section that is applied automatically once on page load. Then there is a "Downs" section that is saved and the SQL in the Downs section should revert the changes in the Ups. Most likely your Downs section will be empty though, since we can't really get back our deletes (we'll just need to be careful before pushing to prod) :)

In addition, if you make a change to your evolutions file and reload the site, it will apply the Downs from the file as it was written when you loaded the page to apply the Ups, then it will apply the Ups from your newly edited file. Similarly, if you edit an older evolutions file, it applies the Downs for each evolutions (in reverse order) up to the one that was changed, then reapplies the Ups.

In summary, the Ups are what you want to change in the db, and the Downs are how you undo it!

If you ever get into a sticky spot, you can manually edit the Ups/Downs as saved in the database by editing the play_evolutions table. This probably won't be an issue for you since you likely won't be using Downs, but worth mentioning. Feel free to ask follow-up questions over Slack!

It's probably easiest to start with the simple deleting from the label_validation table as you're learning SQL. Once you've got that working, you can look into what all is going on in the deleteLabelValidation function, since we may need to mess with the label_history and label tables as well! But do let me know when you get to this point, and we can talk through it! And I can check the prod dbs to see if this is even an issue that we need to deal with. If no one's edited the tags/severity through /newValidateBeta and then changed their validation in Gallery, we might not need to do the complicated part!

jsomeara commented 2 months ago

Ready for review!

jsomeara commented 2 months ago

Implemented the requested changes!