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 error when trying to smile already smiled records #1685

Merged
merged 8 commits into from
Mar 19, 2024

Conversation

pixeldesu
Copy link
Member

Fixes #1673

Multiple things that went wrong here:

  • The scoped subquery thing never really worked (apparently), ActiveRecord never maps the as specified selects to the accessors, so they always are false (or nil)
    • This has been fixed with trying to pull the values we managed to fetch from self.attributes, because they are IN THERE.
  • Reactions didn't have a scoped uniqueness validation, so we always ran into a Postgres exception with the indices.
  • Not sure if just locally, but at least noticed here, for some reason User coercion sometimes fails in use cases and breaks so badly the app needs to restart?
    • Fixed this with passing IDs into the Reaction use cases now and letting the use case resolve the source user.

Otherwise it'd say "Parent already taken" instead of "Target already smiled"
This has to be done because using multiple queries in a select statement does not automatically map into an ActiveRecord model and the values we want wait in the `self.attributes` hash.
For some wild reason this locally sometimes causes coercion errors in the user instance check, restarting fixes it (temporarily?) so letting the UseCase resolve users is a cleaner solution here.
@github-actions github-actions bot added the bugfix Pull Requests that fix a bug label Mar 19, 2024
Copy link

codecov bot commented Mar 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.86%. Comparing base (87458f4) to head (6708209).
Report is 26 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1685      +/-   ##
==========================================
+ Coverage   95.84%   95.86%   +0.01%     
==========================================
  Files         179      179              
  Lines        2770     2779       +9     
==========================================
+ Hits         2655     2664       +9     
  Misses        115      115              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pixeldesu pixeldesu merged commit b160fa2 into main Mar 19, 2024
11 of 12 checks passed
@pixeldesu pixeldesu deleted the bugfix/reactions-be-broken branch March 19, 2024 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Pull Requests that fix a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Internal Server Error
1 participant