Skip to content

fix: exclude retired notifications from inbox counts - #774

Merged
thomasluizon merged 1 commit into
mainfrom
thomasluizon/ticket-1551-retired-notification-count
Oct 19, 2026
Merged

thomasluizon merged 1 commit into
mainfrom
thomasluizon/ticket-1551-retired-notification-count

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

Fixes thomasluizon/orbit-tickets#1551.

Accounts with 51 unread retired notifications could have an empty client inbox and an unread badge they could not clear. GetNotificationsQueryHandler now excludes URLs starting with /social, /public-profile or /u/ from items, unread count and total count before database paging.

The shared expression and its prefix list live in src/Orbit.Application/Notifications/Queries/GetNotificationsQuery.cs, with a linked comment naming the client constant RETIRED_NOTIFICATION_URL_PREFIXES. Keeping the filter in this read handler preserves mark-all-read and delete-all behavior for every account row. Null URLs remain visible. Stored rows and response contracts are unchanged; no migration is needed.

tests/Orbit.Application.Tests/Queries/Notifications/GetNotificationsQueryHandlerTests.cs covers the 51-row regression for each prefix, active unread rows beyond the first page, filtered totals, offset/limit paging, account ownership, soft deletes and retained active URLs. The tests use the real repository with EF InMemory. A separate test runs all three query predicates and paging through the installed Npgsql translator without opening a database connection.

Assumptions

  • Kept the shared visibility expression in the existing query handler rather than adding a global repository filter, because bulk actions must still reach retired notifications.
  • Interpreted both locales as English (en_US.UTF-8) and Brazilian Portuguese (pt_BR.UTF-8), matching LocaleHelper, rather than additional regional variants.

Test evidence

At baseline a7d1cf2e, before changing tests or implementation, dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~GetNotificationsQueryHandlerTests --verbosity minimal passed all 5 existing tests with the defect present.

With the new regression tests and the unchanged implementation, the same command failed 5 of 17 cases. Handle_FiftyOneUnreadRetiredNotifications_ReturnsEmptyInbox failed for all three prefixes: unread and total counts were 51 instead of 0, and retired items were returned. Handle_MixedNotifications_CountsActiveUnreadRowsBeyondThePage reported unread 56 instead of 53 and total 58 instead of 55. Handle_OffsetAndLimit_PageOverActiveNotifications returned /chat, /public-profile/user instead of /progress, /profile.

After the fix, the same command passed all 17 cases. After adding Handle_FiltersAndPaging_TranslateToPostgresWithoutConnecting, the following focused command passed all 22 cases, including the existing bulk-action tests:

dotnet test tests/Orbit.Application.Tests --filter 'FullyQualifiedName~GetNotificationsQueryHandlerTests|FullyQualifiedName~MarkAllNotificationsReadCommandHandlerTests|FullyQualifiedName~DeleteAllNotificationsCommandHandlerTests' --verbosity minimal

Both builds exited 0 with zero errors:

LANG=en_US.UTF-8 LC_ALL=en_US.UTF-8 DOTNET_CLI_UI_LANGUAGE=en dotnet build Orbit.slnx --verbosity minimal
LANG=pt_BR.UTF-8 LC_ALL=pt_BR.UTF-8 DOTNET_CLI_UI_LANGUAGE=pt-BR dotnet build Orbit.slnx --verbosity minimal

Generated OpenAPI documents remain unchanged. Builds report existing dependency, obsolete API and versioning warnings.

Each command below passed under both locale environments above, with 10,103 tests per locale:

dotnet test tests/Orbit.Domain.Tests --no-build --verbosity minimal
dotnet test tests/Orbit.Application.Tests --no-build --verbosity minimal
dotnet test tests/Orbit.Infrastructure.Tests --no-build --verbosity minimal
dotnet test tests/Orbit.Analyzers.Tests --no-build --verbosity minimal
Suite English passed Brazilian Portuguese passed
Domain 742 742
Application 5,569 5,569
Infrastructure 3,760 3,760
Analyzers 32 32

No external response fields were introduced. PostgreSQL translation is exercised directly by the installed provider in the focused test above. No server, browser, live database or mutation testing was started.

@pullfrog pullfrog Bot 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.

✅ No new issues found.

Reviewed changes in the notification query handler and regression tests, including the matching retirement policy in orbit-ui-mobile.

  • Consistent visibility: Items, unread counts, and total counts share the same account-scoped filter for retired URLs, applied before pagination while retaining null URLs.
  • Preserved behavior: Response contracts, stored rows, soft-delete filtering, and bulk mark-read/delete behavior remain unchanged.
  • Regression coverage: Verified the 51-row case, mixed notification counts, filtered paging, retained URLs, and PostgreSQL query translation. All 22 focused Application tests and 6 relational repository tests passed; no live PostgreSQL execution or full-suite rerun was performed.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

@sonarqubecloud

Copy link
Copy Markdown

@thomasluizon
thomasluizon merged commit f5df366 into main Oct 19, 2026
22 checks passed
@thomasluizon
thomasluizon deleted the thomasluizon/ticket-1551-retired-notification-count branch October 19, 2026 07:55
thomasluizon added a commit that referenced this pull request Oct 19, 2026
…d cancellations and the Play renewal notice (#746) (#778)

* Deprecate AI memory HTTP contracts for ORB-229 (part 1) (#771)

(cherry picked from commit f8abd02)

* Deprecate challenge and accountability routes for ORB-212 (#772)

(cherry picked from commit 572b8de)

* Deprecate social API routes and required profile fields (ORB-233 part 1) (#773)

(cherry picked from commit 9c414c7)

* fix: exclude retired notifications from inbox counts (#774)

(cherry picked from commit f5df366)

* fix: retain owed cancellations after account erasure (#775)

* fix: retain provider cancellation obligations after account erasure

* fix: preserve erasure for accounts without billing obligations

* fix: claim owed cancellations before provider calls

* fix: alert owed cancellation exhaustion only after a failed attempt

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit c369535)

* fix: notify returning Play subscribers about renewal (#776)

* fix: explain Play renewal recovery on account reactivation

* test: register reactivation service in sign-in retry fixture

* fix: verify Play renewal before reactivation notice

(cherry picked from commit 96023f9)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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