Skip to content

Commit

Permalink
Fix app rating upsert issue (#1136)
Browse files Browse the repository at this point in the history
* Write a test case

* Fix app rating upsert issue

* Add migration script

* Remove rating sum from migration
  • Loading branch information
Takaros999 authored Jan 16, 2025
1 parent 292f84f commit e8b8dcc
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -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
-- );
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
UPDATE app
SET
rating_count = (
SELECT COUNT(*)
FROM app_reviews
WHERE app_reviews.app_id = app.id
);
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
Expand Down
14 changes: 10 additions & 4 deletions web/api/v2/app/submit-app-review/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
63 changes: 58 additions & 5 deletions web/tests/api/v2/app/submit-app-review.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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,
}),
);
});
});

0 comments on commit e8b8dcc

Please sign in to comment.