Conversation
|
/review -claude |
There was a problem hiding this comment.
Review: in-product notifications
Solid, well-structured feature. The stale-response guarding (requestSequence / unreadRequestSequence) and the isCurrentNotification() membership+timestamp check make the optimistic mark-as-read and user-switch races robust, and the tests cover them directly. Vue reactivity of the mutated array items is correct, teardown of the timer and visibilitychange listener is clean, and the OpenAPI contract matches the frontend types (field names, readAt nullability, optional actionPath, 1-based pageIndex).
Findings below are mostly low-severity; the unreadCount overwrite on "Load more" (P2) is the one worth confirming as intentional. No blocking issues.
Also verified as fine (no change needed): the const: true on the PATCH read body is valid OpenAPI 3.1; the unconditional interval timer is harmless for signed-out users since refreshUnreadCount early-returns; the poll + visibility overlap is deduped by the sequence guard; and notification.body/title are rendered with escaped {{ }} interpolation (no v-html).
Summary
hasUnreadfrom the notification list response and refresh on sign-in and notification-center open without pollingBackend: goplus/builder-backend#355
This PR provides reusable notification UI. Feedback replies for #1789 will become the first producer in a separate PR.
Testing
npm run type-checknpm run lint -- --no-fix