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

Fix server error on user profile vote summary tab if there are orphaned votes #1491

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

Oaphi
Copy link
Member

@Oaphi Oaphi commented Dec 16, 2024

Closes #1488

This PR fixes the issue with orphan votes (with post_id but no actual post) by:

  • making the vote_summary view resilient to entries with missing posts;
  • making the UsersController#vote_summary filter out votes without associated posts;
  • adding a CleanupVotes daily job that removes orphaned votes (those that do not correspond to any post but still have a post_id assigned to them);
  • making Vote model more resilient to post not being present;

@Oaphi Oaphi self-assigned this Dec 16, 2024
@Oaphi Oaphi requested review from cellio and a team December 16, 2024 14:27
@Oaphi Oaphi added area: ruby Changes to server-side code complexity: average Not particularly hard, not particularly trivial. complexity: easy Issues that should take limited effort to resolve/fix/build. and removed complexity: average Not particularly hard, not particularly trivial. labels Dec 16, 2024
@cellio
Copy link
Member

cellio commented Dec 16, 2024

LGTM based on inspection (one paranoid request). I appreciate the extra resilience in the view and controller.

Someone with more knowledge of the scripts/daily jobs should also review this.

How do we test it? Can we inject some bad data on the dev server and test there before going to production?

@Oaphi
Copy link
Member Author

Oaphi commented Dec 16, 2024

How do we test it?

There are several ways of doing so. A simple prerequisite is voting on something, then intentionally breaking the database integrity by updating the record in the votes table to contain a non-existent post_id. If done right, you will see an entry in the target user's vote summary tab that says "Post not found".

After that's done, it's just a matter of executing the job. The simplest approach is to change the associated script file to call perform_now instead of perform_later and simply run it as a script via bin/rails runner <path_to_script> (in fact, this is basically what whenever does under the hood). There is no reason to test the schedule as it's managed by whenever (it sets up a cron job), although that can be done too

@cellio cellio requested a review from ArtOfCode- December 26, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ruby Changes to server-side code complexity: easy Issues that should take limited effort to resolve/fix/build.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trying to get votes summary for a user might result in server error if one of the posts are somehow destroyed
2 participants