Skip to content

Further fixes for /v6/my-reviews for big accounts - #142

Merged
jmgasper merged 2 commits into
masterfrom
develop
Nov 2, 2025
Merged

Further fixes for /v6/my-reviews for big accounts#142
jmgasper merged 2 commits into
masterfrom
develop

Conversation

@jmgasper

@jmgasper jmgasper commented Nov 2, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

@jmgasper
jmgasper merged commit d8bb50d into master Nov 2, 2025
6 checks passed
rv."resourceId",
COUNT(*) AS "pendingAppealCount",
now()
FROM reviews.review rv

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
The now() function is used multiple times in the SQL statements. Consider using a single variable to store the current timestamp to ensure consistency across the transaction, especially if the transaction takes a non-trivial amount of time to execute.

AFTER INSERT OR UPDATE OR DELETE ON reviews."appealResponse"
FOR EACH ROW
EXECUTE FUNCTION reviews.handle_appeal_response_change();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[💡 style]
There is no newline at the end of the file. While this is a minor 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.

Comment thread prisma/schema.prisma
}

model reviewPendingSummary {
resourceId String @id

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider adding a @db.VarChar annotation to resourceId to explicitly define its length in the database schema. This can help prevent potential issues with varying string lengths across different database systems.

hasIncompleteReviews: boolean | null;
incompletePhaseName: string | null;
hasPendingAppealResponses: boolean | null;
hasPendingAppealResponses: boolean;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[❗❗ correctness]
The type change from boolean | null to boolean for hasPendingAppealResponses may introduce issues if the previous logic depended on distinguishing between false and null. Ensure that the rest of the codebase correctly handles this change.

deliverable_reviews."hasIncompleteReviews" AS "hasIncompleteReviews",
deliverable_reviews."incompletePhaseName" AS "incompletePhaseName",
pending_appeals."hasPendingAppealResponses" AS "hasPendingAppealResponses",
COALESCE(pending_appeals."pendingAppealCount" > 0, FALSE) AS "hasPendingAppealResponses",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
The use of COALESCE(pending_appeals."pendingAppealCount" > 0, FALSE) assumes that pendingAppealCount is always non-null. If pendingAppealCount can be null, this logic may not behave as expected. Consider verifying the nullability of pendingAppealCount.

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.

1 participant