diff --git a/hasura/migrations/default/1737035548134_recalculate_app_ratings/down.sql b/hasura/migrations/default/1737035548134_recalculate_app_ratings/down.sql new file mode 100644 index 000000000..201282039 --- /dev/null +++ b/hasura/migrations/default/1737035548134_recalculate_app_ratings/down.sql @@ -0,0 +1,9 @@ +-- Could not auto-generate a down migration. +-- Please write an appropriate down migration for the SQL below: +-- UPDATE app +-- SET +-- rating_count = ( +-- SELECT COUNT(*) +-- FROM app_reviews +-- WHERE app_reviews.app_id = app.id +-- ); diff --git a/hasura/migrations/default/1737035548134_recalculate_app_ratings/up.sql b/hasura/migrations/default/1737035548134_recalculate_app_ratings/up.sql new file mode 100644 index 000000000..86d0c2104 --- /dev/null +++ b/hasura/migrations/default/1737035548134_recalculate_app_ratings/up.sql @@ -0,0 +1,7 @@ +UPDATE app +SET + rating_count = ( + SELECT COUNT(*) + FROM app_reviews + WHERE app_reviews.app_id = app.id + ); diff --git a/web/api/v2/app/submit-app-review/graphql/update-review-counter.generated.ts b/web/api/v2/app/submit-app-review/graphql/update-review-counter.generated.ts index b88d8091f..7b176e4f5 100644 --- a/web/api/v2/app/submit-app-review/graphql/update-review-counter.generated.ts +++ b/web/api/v2/app/submit-app-review/graphql/update-review-counter.generated.ts @@ -7,6 +7,7 @@ type GraphQLClientRequestHeaders = RequestOptions["requestHeaders"]; export type UpdateAppRatingSumMutationMutationVariables = Types.Exact<{ app_id: Types.Scalars["String"]["input"]; rating: Types.Scalars["Int"]["input"]; + rating_count_inc: Types.Scalars["Int"]["input"]; }>; export type UpdateAppRatingSumMutationMutation = { @@ -18,10 +19,14 @@ export type UpdateAppRatingSumMutationMutation = { }; export const UpdateAppRatingSumMutationDocument = gql` - mutation UpdateAppRatingSumMutation($app_id: String!, $rating: Int!) { + mutation UpdateAppRatingSumMutation( + $app_id: String! + $rating: Int! + $rating_count_inc: Int! + ) { update_app( where: { id: { _eq: $app_id } } - _inc: { rating_count: 1, rating_sum: $rating } + _inc: { rating_sum: $rating, rating_count: $rating_count_inc } ) { affected_rows } diff --git a/web/api/v2/app/submit-app-review/graphql/update-review-counter.graphql b/web/api/v2/app/submit-app-review/graphql/update-review-counter.graphql index 3c06aa67c..c4670da6f 100644 --- a/web/api/v2/app/submit-app-review/graphql/update-review-counter.graphql +++ b/web/api/v2/app/submit-app-review/graphql/update-review-counter.graphql @@ -1,7 +1,11 @@ -mutation UpdateAppRatingSumMutation($app_id: String!, $rating: Int!) { +mutation UpdateAppRatingSumMutation( + $app_id: String! + $rating: Int! + $rating_count_inc: Int! +) { update_app( where: { id: { _eq: $app_id } } - _inc: { rating_count: 1, rating_sum: $rating } + _inc: { rating_sum: $rating, rating_count: $rating_count_inc } ) { affected_rows } diff --git a/web/api/v2/app/submit-app-review/index.ts b/web/api/v2/app/submit-app-review/index.ts index 7ec8bd51f..760ac238c 100644 --- a/web/api/v2/app/submit-app-review/index.ts +++ b/web/api/v2/app/submit-app-review/index.ts @@ -120,16 +120,22 @@ export const POST = async (req: NextRequest) => { }); } - // Anchor: Update App Review Count - let ratingChange = parsedParams.rating; + // Calculate the rating sum and count increment + let ratingSumIncrement = parsedParams.rating; + let ratingCountIncrement = 1; if (app_reviews.length) { - ratingChange = parsedParams.rating - app_reviews[0].rating; + // If we already have an existing row for this user, only adjust for the difference + ratingSumIncrement = parsedParams.rating - app_reviews[0].rating; + ratingCountIncrement = 0; } + + // Anchor: Update App Review Count const { update_app } = await updateReviewCount( serviceClient, ).UpdateAppRatingSumMutation({ app_id: app_id, - rating: ratingChange, + rating: ratingSumIncrement, + rating_count_inc: ratingCountIncrement, }); if (!update_app) { diff --git a/web/tests/api/v2/app/submit-app-review.test.ts b/web/tests/api/v2/app/submit-app-review.test.ts index 5435a5683..f579a755b 100644 --- a/web/tests/api/v2/app/submit-app-review.test.ts +++ b/web/tests/api/v2/app/submit-app-review.test.ts @@ -49,6 +49,10 @@ const validBody = { }; describe("/api/v2/app/submit-app-review", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it("Can successfully submit an app review", async () => { const mockReq = new NextRequest( "https://cdn.test.com/api/v2/app/submit-app-review", @@ -77,14 +81,63 @@ describe("/api/v2/app/submit-app-review", () => { }); GetAppReview.mockResolvedValue({ - app_reviews: [ - { - rating: 3, - }, - ], + app_reviews: [], }); const res = await POST(mockReq); expect(res.status).toBe(200); + expect(UpdateAppRatingSumMutation).toHaveBeenCalledTimes(1); + expect(UpdateAppRatingSumMutation).toHaveBeenCalledWith( + expect.objectContaining({ + rating_count_inc: 1, + rating: 3, // rating increment + app_id: validBody.app_id, + }), + ); + }); + + it("Does not increment rating_count if user updates existing rating", async () => { + const validBody = { + ...appReviewMockProof, + rating: 4, + country: "us", + app_id: "app_staging_e396fd19c804e62f657767ccaa78885c", + }; + const mockReq = new NextRequest( + "https://cdn.test.com/api/v2/app/submit-app-review", + { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify(validBody), + }, + ); + GetAppReview.mockResolvedValue({ + app_reviews: [{ rating: 3 }], + }); + UpsertAppReview.mockResolvedValue({ + insert_app_reviews_one: { + id: "review_123", + rating: 4, + app_id: validBody.app_id, + country: validBody.country, + }, + }); + UpdateAppRatingSumMutation.mockResolvedValue({ + update_app: { + affected_rows: 1, + }, + }); + const res = await POST(mockReq); + expect(res.status).toBe(200); + expect(UpdateAppRatingSumMutation).toHaveBeenCalledTimes(1); + expect(UpdateAppRatingSumMutation).toHaveBeenCalledWith( + expect.objectContaining({ + rating_count_inc: 0, + rating: 1, // rating increment + app_id: validBody.app_id, + }), + ); }); });