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 app rating upsert issue #1136

Merged
merged 4 commits into from
Jan 16, 2025
Merged

Conversation

Takaros999
Copy link
Contributor

@Takaros999 Takaros999 commented Jan 15, 2025

PR Type

  • Regular Task
  • Bug Fix
  • QA Tests

Description

This PR fixes an issue with app ratings and adds a migration script to recalculate app ratings.

Issue

Currently, we're using a rolling sum to update app ratings. We allow the user to upsert his review and apply the rating difference to the app's rating sum. The issue is that we're updating the rating count each time the user updates his review, this means that if multiple user decide to change their app rating they can affect the total app rating score, because the rating count will increase while app reviews stay the same.

Migration script

Added a migration script to recalculate app ratings.

Checklist

  • I have self-reviewed this PR.
  • I have left comments in the code for clarity.
  • I have added necessary unit tests.
  • I have updated the documentation as needed.

@Takaros999 Takaros999 self-assigned this Jan 15, 2025
@Takaros999 Takaros999 marked this pull request as ready for review January 16, 2025 13:58
Copy link
Contributor

@andy-t-wang andy-t-wang left a comment

Choose a reason for hiding this comment

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

Make that change and we should be good. I would watch the DB though it could need to be scaled up for this

FROM app_reviews
WHERE app_reviews.app_id = app.id
),
rating_count = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch we only need to update rating_count though. No need to do sum as well. Will make this operation slightly cheaper

@andy-t-wang andy-t-wang merged commit e8b8dcc into main Jan 16, 2025
12 checks passed
@andy-t-wang andy-t-wang deleted the dev-1320-fix-app-rating-upsert-issue branch January 16, 2025 21:55
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