Further fixes for /v6/my-reviews for big accounts #141
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| rv."resourceId", | ||
| COUNT(*) AS "pendingAppealCount", | ||
| now() | ||
| FROM reviews.review rv |
There was a problem hiding this comment.
[correctness]
The use of now() for updatedAt in the INSERT INTO statement could lead to inconsistencies if the transaction is long-running. Consider using CURRENT_TIMESTAMP to ensure consistency with the default value of the column.
| ON CONFLICT ("resourceId") | ||
| DO UPDATE SET | ||
| "pendingAppealCount" = EXCLUDED."pendingAppealCount", | ||
| "updatedAt" = now(); |
There was a problem hiding this comment.
[correctness]
Using now() here could lead to inconsistencies if the transaction is long-running. Consider using CURRENT_TIMESTAMP to ensure consistency with the default value of the column.
|
|
||
| IF pending_count > 0 THEN | ||
| INSERT INTO reviews.review_pending_summary ("resourceId", "pendingAppealCount", "updatedAt") | ||
| VALUES (p_resource_id, pending_count, now()) |
There was a problem hiding this comment.
[correctness]
The use of now() for updatedAt in the VALUES clause could lead to inconsistencies if the transaction is long-running. Consider using CURRENT_TIMESTAMP to ensure consistency with the default value of the column.
| ON CONFLICT ("resourceId") | ||
| DO UPDATE SET | ||
| "pendingAppealCount" = EXCLUDED."pendingAppealCount", | ||
| "updatedAt" = now(); |
There was a problem hiding this comment.
[correctness]
Using now() here could lead to inconsistencies if the transaction is long-running. Consider using CURRENT_TIMESTAMP to ensure consistency with the default value of the column.
| FOR EACH ROW | ||
| EXECUTE FUNCTION reviews.handle_appeal_response_change(); | ||
|
|
||
| COMMIT; No newline at end of file |
There was a problem hiding this comment.
[💡 style]
The file does not end with a newline character. While this is not a critical issue, it is generally a good practice to include a newline at the end of files to avoid potential issues with some tools and to adhere to POSIX standards.
| } | ||
|
|
||
| model reviewPendingSummary { | ||
| resourceId String @id |
There was a problem hiding this comment.
[💡 maintainability]
Consider adding a @db.VarChar annotation to resourceId to specify the maximum length for consistency with other string fields in the schema.
| hasIncompleteReviews: boolean | null; | ||
| incompletePhaseName: string | null; | ||
| hasPendingAppealResponses: boolean | null; | ||
| hasPendingAppealResponses: boolean; |
There was a problem hiding this comment.
[❗❗ correctness]
Changing hasPendingAppealResponses from boolean | null to boolean in the ChallengeSummaryRow interface may lead to issues if there are existing code paths that rely on the null value to represent a specific state. Ensure that all usages of this property are updated accordingly to handle the absence of a pending appeal response correctly.
| deliverable_reviews."hasIncompleteReviews" AS "hasIncompleteReviews", | ||
| deliverable_reviews."incompletePhaseName" AS "incompletePhaseName", | ||
| pending_appeals."hasPendingAppealResponses" AS "hasPendingAppealResponses", | ||
| COALESCE(pending_appeals."pendingAppealCount" > 0, FALSE) AS "hasPendingAppealResponses", |
There was a problem hiding this comment.
[correctness]
The use of COALESCE(pending_appeals."pendingAppealCount" > 0, FALSE) assumes that pendingAppealCount is never NULL. If there are scenarios where pendingAppealCount could be NULL, this logic might not behave as expected. Consider verifying that pendingAppealCount is always initialized or handle the NULL case explicitly.
No description provided.