Skip to content

feat(notification): implement MarkAllNotificationsAsRead API - #2762

Merged
hvn2k1 merged 1 commit into
mainfrom
mark-all-noti-api
Aug 7, 2026
Merged

feat(notification): implement MarkAllNotificationsAsRead API#2762
hvn2k1 merged 1 commit into
mainfrom
mark-all-noti-api

Conversation

@hvn2k1

@hvn2k1 hvn2k1 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Follows #2751

What this PR does

Implements the MarkAllNotificationsAsRead RPC, the last remaining stub of the notification center backend: any authenticated console user can mark every published notification as read in one call.

Background / Why this PR is needed

The inbox's "mark all as read" action needs a bulk endpoint; looping mark_as_read from the client would be racy and slow. With this, all RFC 0047 backend APIs are implemented.

Points

  • The whole operation is a single idempotent INSERT ... SELECT upserting read markers for every status = PUBLISHED AND deleted = false notification (INSERT IGNORE on MySQL, ON CONFLICT DO NOTHING on Postgres) — no per-row loop, and existing markers keep their original read_at.
  • The account-created bound is intentionally NOT applied here: marking literally all published notifications keeps the read flag consistent across every tab, while the unread count/tab remain bounded as before.
  • With no stubs left, the unused statusNotImplemented sentinel and the proto's "intentionally left empty" NOTE are removed; no generated files change (the comment never reached them).

Copilot AI review requested due to automatic review settings August 7, 2026 06:40
@hvn2k1
hvn2k1 requested a review from Ubisoft-potato August 7, 2026 06:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Implements the final missing notification-center backend RPC, MarkAllNotificationsAsRead, enabling an authenticated console user to mark every published, non-deleted notification as read in a single bulk upsert.

Changes:

  • Add bulk “mark all as read” SQL (INSERT … SELECT with conflict-ignore semantics) for both Postgres and MySQL, and expose it via NotificationStorage.
  • Implement NotificationService.MarkAllNotificationsAsRead with authentication + transactional execution.
  • Remove the old “not implemented” sentinel and the proto NOTE that is no longer accurate.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
proto/notification/service.proto Removes outdated NOTE about empty request/response bodies.
pkg/notification/storage/postgres/sql/insert_all_notification_reads.sql Adds Postgres bulk upsert to create missing read markers for all published notifications.
pkg/notification/storage/postgres/notification.go Embeds and exposes MarkAllNotificationsAsRead in Postgres storage implementation.
pkg/notification/storage/postgres/notification_test.go Adds unit test coverage for the new Postgres storage method (contains a gomock expectation issue).
pkg/notification/storage/notification.go Extends NotificationStorage interface with MarkAllNotificationsAsRead.
pkg/notification/storage/mysql/sql/insert_all_notification_reads.sql Adds MySQL bulk insert-ignore for read markers for all published notifications.
pkg/notification/storage/mysql/notification.go Embeds and exposes MarkAllNotificationsAsRead in MySQL storage implementation.
pkg/notification/storage/mysql/notification_test.go Adds unit test coverage for the new MySQL storage method (contains a gomock expectation issue).
pkg/notification/storage/mock/notification.go Updates the gomock storage mock with the new interface method.
pkg/notification/api/error.go Removes now-unused statusNotImplemented and its imports.
pkg/notification/api/api.go Implements the MarkAllNotificationsAsRead RPC with auth + transaction + storage call.
pkg/notification/api/api_test.go Adds service-level unit tests for the new RPC behavior.
Files not reviewed (1)
  • pkg/notification/storage/mock/notification.go: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/notification/storage/postgres/notification_test.go
Comment thread pkg/notification/storage/mysql/notification_test.go
@hvn2k1
hvn2k1 marked this pull request as ready for review August 7, 2026 06:44

@Ubisoft-potato Ubisoft-potato left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice work for notifation! 👍
Thanks!

@hvn2k1
hvn2k1 merged commit 0516403 into main Aug 7, 2026
12 checks passed
@hvn2k1
hvn2k1 deleted the mark-all-noti-api branch August 7, 2026 10:40
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.

3 participants