Skip to content

fix(api): localize calendar-sync notifications - #279

Merged
thomasluizon merged 1 commit into
mainfrom
fix/qa-followups
Jul 3, 2026
Merged

fix(api): localize calendar-sync notifications#279
thomasluizon merged 1 commit into
mainfrom
fix/qa-followups

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Pairs with thomasluizon/orbit-ui-mobile#381 (no contract coupling; merge order free). User report: notification bell showed "2 new calendar events / Tap to review and import" in a pt-BR app.

Both hardcoded-English sites in RunCalendarAutoSyncCommand fixed via the house LocaleHelper.IsPortuguese(user.Language) pattern: the suggestions notification (proper pt pluralization) and the Google-disconnected reconnect notification (copy matches the clients' existing calendarSync.reconnect strings). Exhaustive 17-site audit of every notification/push creation site: all others already localized. Already-persisted rows keep their frozen text.

Tests: 32 passed on the handler suite (5 new: en/pt plural + singular suggestion, en/pt reconnect). Build clean, no bare comments.

🤖 Generated with Claude Code

Both notification sites in RunCalendarAutoSyncCommand hardcoded English:
the new-suggestions notification (with count pluralization) and the
Google-disconnected reconnect notification now localize via LocaleHelper +
User.Language, matching every other notification site (17-site audit: all
others already localized). Existing stored rows keep their frozen text.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Jul 3, 2026

Copy link
Copy Markdown

@claude claude 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.

Code Review: PR #279

Scope: PR #279 in thomasluizon/orbit-api — "fix(api): localize calendar-sync notifications (pt-BR/en)"
Recommendation: APPROVE

Summary

The diff replaces two hardcoded-English notification strings in RunCalendarAutoSyncCommandHandler (src/Orbit.Application/Calendar/Commands/RunCalendarAutoSyncCommand.cs) — the new-suggestions notification and the Google-disconnected/reconnect notification — with pt-BR/English variants gated on LocaleHelper.IsPortuguese(user.Language). This is the same helper already used at 15+ other call sites across the codebase (Referrals, Accountability, Social, Gamification, Reminder/StreakFreeze/GoalDeadline/ResendEmail services), so the fix is a drop-in application of an existing house pattern, not a new mechanism. 5 new unit tests cover en/pt × singular/plural for the suggestions notification and en/pt for the reconnect notification, and the fixed TimeProvider in the test fixture (2026-04-09 14:00 UTC, UTC timezone) correctly lands inside the existing quiet-hours window so the notification-creation path is actually exercised.

Findings

Critical: None.
High: None.
Medium: None.

Low / Info:

  • Grepped every Notification.Create(...) call site under src/ — all others already route title/body through locale-aware helpers or pre-localized variables. The PR body's claim of an "exhaustive 17-site audit" with no remaining hardcoded-English site in this file checks out.
  • The CreateSuggestionNotification signature change (Guid userIdUser user) is the minimal change needed to reach user.Language; its single call site in FetchAndProcessLocked was updated in the same diff, no orphaned overload left behind.
  • Portuguese pluralization is handled correctly (novo/novos agreement), not a naive string concat.

Subagents

Agent Verdict
security-reviewer PASS — diff only swaps fixed string literals behind a boolean locale branch; count is an int, no user-controlled string is interpolated into notification title/body, no new endpoint/auth/DTO surface
contract-aligner N/A — no DTO, Controller route, or packages/shared type touched

Validation

Check Result
Build (dotnet) N/A — covered by the separate required CI Build check
Tests (dotnet) N/A — covered by the separate required CI Unit Tests check

What's good

  • Reuses LocaleHelper.IsPortuguese(user.Language) exactly as used elsewhere rather than inventing new localization plumbing.
  • Test coverage added for every new branch in the same PR, per tests/CLAUDE.md.
  • Already-persisted notification rows are explicitly left frozen — correct call, avoids an out-of-scope backfill/migration.

Recommendation

No changes requested. Safe to merge.

@thomasluizon
thomasluizon merged commit 85d59fa into main Jul 3, 2026
8 checks passed
@thomasluizon
thomasluizon deleted the fix/qa-followups branch July 3, 2026 19:42
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