Refactor RDS#1589
Conversation
|
I'll update the title and description just for clarity |
Co-authored-by: Victoria Lee <victorialee@dhcp-172-31-039-225.mobile.uci.edu>
There was a problem hiding this comment.
5 issues found across 15 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/antalmanac/src/backend/lib/rds/schedules-rds.ts">
<violation number="1" location="apps/antalmanac/src/backend/lib/rds/schedules-rds.ts:74">
P2: The null-check for `account` is placed after `account.userId` is accessed, so it cannot prevent a crash and is effectively dead in this position.</violation>
</file>
<file name="apps/antalmanac/src/stores/SessionStore.ts">
<violation number="1" location="apps/antalmanac/src/stores/SessionStore.ts:56">
P2: When `sessionResult` is null, only part of session state is cleared, leaving stale user profile fields in the store.</violation>
</file>
<file name="apps/antalmanac/src/backend/lib/rds/notification-rds.ts">
<violation number="1" location="apps/antalmanac/src/backend/lib/rds/notification-rds.ts:1">
P2: This file duplicates an existing `NotificationRDS` implementation, creating two copies of the same data-access logic that can drift out of sync.</violation>
</file>
<file name="apps/antalmanac/src/backend/lib/rds/users-rds.ts">
<violation number="1" location="apps/antalmanac/src/backend/lib/rds/users-rds.ts:82">
P2: Handle the `findIndex` miss case; returning `-1` as `scheduleIndex` can propagate an invalid index to callers.</violation>
<violation number="2" location="apps/antalmanac/src/backend/lib/rds/users-rds.ts:169">
P1: Filtering by `accounts.accountType = 'GUEST'` makes this lookup fail now that `GUEST` accounts are removed, so imported users may never be flagged.
(Based on your team's feedback about treating 'GUEST' account type as removed from the system.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/antalmanac/src/backend/lib/rds/notification-rds.ts">
<violation number="1" location="apps/antalmanac/src/backend/lib/rds/notification-rds.ts:19">
P2: The newly added `stage` argument is unused across notification read/update/delete methods, so stage-specific calls are silently ignored. This creates dead API surface and can cause cross-environment notification operations to act on the wrong rows.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/antalmanac/src/backend/lib/rds/notification-rds.ts">
<violation number="1" location="apps/antalmanac/src/backend/lib/rds/notification-rds.ts:25">
P1: Environment-filtered queries were added, but upsert conflict keys still ignore environment, which can make notifications disappear across environments after cross-environment writes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
the extra files other than the rds stuff are formatting from the merge commit which makes the PR bloated asf. mmm a rebase might be risky now considering i have commits on top of them.. the commits i have are just fixes/responses to cubic |
Summary
Reverted PR #1577
Closes #1158