feat(notification): implement MarkAllNotificationsAsRead API - #2762
Merged
Conversation
There was a problem hiding this comment.
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 … SELECTwith conflict-ignore semantics) for both Postgres and MySQL, and expose it viaNotificationStorage. - Implement
NotificationService.MarkAllNotificationsAsReadwith 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.
hvn2k1
marked this pull request as ready for review
August 7, 2026 06:44
Ubisoft-potato
approved these changes
Aug 7, 2026
Ubisoft-potato
left a comment
Collaborator
There was a problem hiding this comment.
Nice work for notifation! 👍
Thanks!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows #2751
What this PR does
Implements the
MarkAllNotificationsAsReadRPC, 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
INSERT ... SELECTupserting read markers for everystatus = PUBLISHED AND deleted = falsenotification (INSERT IGNOREon MySQL,ON CONFLICT DO NOTHINGon Postgres) — no per-row loop, and existing markers keep their originalread_at.readflag consistent across every tab, while the unread count/tab remain bounded as before.statusNotImplementedsentinel and the proto's "intentionally left empty" NOTE are removed; no generated files change (the comment never reached them).