feat(calendar+retro): all-owned-calendar sync, retrospective caps, server-derived clock format

[thomasluizon]

The orbit-api half of a paired, cross-repo session change (sibling: the orbit-ui-mobile PR linked in the comments). Three fixes land together.

1. Read all owned Google Calendars (the "Rotina" bug)

Sync previously read only the user's primary calendar, so events kept in a secondary calendar (e.g. "Rotina") never appeared.

• Enumerate CalendarList; include every calendar with accessRole == "owner" && !deleted && !hidden (holidays/birthdays/shared come back as reader, so they're excluded automatically).
• Events.list per calendar with pageToken pagination, merged; per-calendar recurring-master dedup so two calendars can't collide.
• Each event is tagged with calendarId + calendarName (additive contract fields).
• New User.GoogleCalendarSelectedIds preference (+ migration) for the settings picker: GET /api/calendar/calendars → {id,name,accessRole,primary,backgroundColor,isSynced}[], PUT /api/calendar/selected-calendars ← {calendarIds}. The fetcher uses the saved selection when set, else the default owned set.
• calendar.readonly already grants all of this — no re-consent.

2. Fix the retrospective completion math

• Cap per-habit, aggregate, and weekly completion at 100% (over-logging used to read 200–300%).
• Flag one-time tasks (IsOneTime) so clients render done/not-done instead of a misleading "0%".
• The AI-narrative prompt receives the capped numbers too (no more "200% de completude").

3. Server-derived 12h/24h clock

ProfileResponse.Uses24HourClock, derived from the user's IANA timezone via a small tested resolver (TimeFormatResolver) — clients render the right clock format by region with no client-side timezone table.

Tests

dotnet test → 3719 pass, 0 failed. New: GoogleCalendarEventFetcherTests, GetUserCalendarsQueryHandlerTests, SetSelectedCalendarsCommandHandlerTests, TimeFormatResolverTests, plus retrospective IsOneTime and profile Uses24HourClock cases.

Compatibility

All contract changes are additive (new optional fields / new endpoints); old mobile clients are unaffected. Deploy before clients depend on the new fields.

🤖 Generated with Claude Code

[thomasluizon]

Paired with thomasluizon/orbit-ui-mobile#306 (the client half of this session's calendar + time-picker + retrospective change).

[sonarqubecloud]

Quality Gate failed

Failed conditions
71.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #249

Scope: PR #249 — feat(calendar+retro): all-owned-calendar sync, retrospective caps, server-derived clock format
Recommendation: APPROVE

Summary

PR #249 lands three improvements: multi-calendar sync (fixing the "Rotina" bug where only primary was fetched), retrospective percentage caps at 100 %, and a server-derived Uses24HourClock field in the profile. The architecture is clean — the Google SDK is extracted behind an IGoogleCalendarApi seam that unlocks the new GoogleCalendarEventFetcherTests suite, the new CQRS handlers follow established patterns, and all contract changes are additive (new optional fields with safe defaults; no existing fields removed or renamed). Test coverage is thorough across every new handler, validator, resolver, and the fetcher's cross-calendar dedup logic. No Critical or High findings.

Findings

Critical

None.

High

None.

Medium

[MEDIUM] SetSelectedCalendarsCommandValidator — no per-ID length cap

• dimension: Security / Validation (#12)
• location: src/Orbit.Application/Calendar/Validators/SetSelectedCalendarsCommandValidator.cs:13–16
• issue: RuleForEach rejects blank IDs but does not cap their length. A caller may send 50 IDs each containing very large strings. The domain method serializes the result to the GoogleCalendarSelectedIds text column (no HasMaxLength constraint in OrbitDbContext), so the theoretical ceiling is the 10 MB Kestrel body limit per request.
• risk: An authenticated user can write up to ~10 MB of arbitrary text into their own user row on every PUT, and the read path deserializes it on every calendar fetch. Google Calendar IDs are in practice short email-format strings, but the lack of a length guard is a defense-in-depth gap.
• fix: Add MaximumLength(1024) to the RuleForEach block and a matching HasMaxLength on the EF column in OrbitDbContext.ConfigureUserEntity.

RuleForEach(x => x.CalendarIds)
    .NotEmpty().WithMessage("Calendar ids must be non-empty.")
    .MaximumLength(1024).WithMessage("Calendar id is too long.");

• reference: orbit-api hard rule — Validation; CLAUDE.md "The backend is the source of truth"

---

[MEDIUM] GetUserCalendarsQueryHandler — unconditional SaveChangesAsync before the Google API call

• dimension: Correctness (#1)
• location: src/Orbit.Application/Calendar/Queries/GetUserCalendarsQuery.cs:44
• issue: unitOfWork.SaveChangesAsync is called unconditionally after GetValidAccessTokenAsync but before the Google list call. The existing GetCalendarEventsQueryHandler uses a dedicated ResolveAccessTokenAsync helper that only saves when a token was actually refreshed or invalidated. The new handler issues a DB write on every GET /api/calendar/calendars call, even when no token was refreshed.
• risk: EF change-tracking will no-op if nothing is dirty, so correctness is not broken today. But the pattern couples a read endpoint to an unconditional DB write and diverges from the established two-path token refresh pattern, making failure diagnosis harder.
• fix: Mirror the conditional pattern from GetCalendarEventsQueryHandler.ResolveAccessTokenAsync — call SaveChangesAsync only when the token service actually mutated the user entity (refresh succeeded or token was invalidated).
• reference: Clean-architecture query/command separation; CLAUDE.md "No workarounds"

Low / Info

None.

Subagents

Agent	Verdict
security-reviewer	PASS (0 Critical, 0 High; 2 Medium — per-ID length cap reported above; rate limiting on new calendar endpoints is a follow-up)
contract-aligner	NOT VERIFIABLE IN CI — orbit-ui-mobile not checked out; all new fields are additive with safe defaults; no regressions on API side

Validation

Check	Result
Build (dotnet)	N/A (skipped — separate required CI check)
Tests (dotnet)	N/A (skipped — separate required CI check; PR body reports 3719 pass, 0 failed)

What's good

• SDK seam extraction: IGoogleCalendarApi correctly isolates the Google SDK from the fetcher logic, enabling GoogleCalendarEventFetcherTests without live network calls — a meaningful clean-architecture improvement.
• Per-calendar dedup: Using a fresh seenRecurringMasterIds HashSet per calendar lets the same recurring-event master appear in two different calendars while still deduplicating within each. The FetchAsync_RecurringMasterDedup_IsPerCalendar test is particularly well-crafted.
• Fail-open per calendar: Catching exceptions in FetchCalendarEvents and returning [] for the failing calendar (with LogCalendarFetchSkipped) is the right resilience trade-off — one broken calendar does not blank the entire sync.
• Backward compatibility: All contract additions use safe positional defaults (CalendarId = "", CalendarName = "", IsOneTime = false, Uses24HourClock = true). Old mobile clients are unaffected.
• TimeFormatResolver: Clean static lookup with a CLDR-sourced rationale in the XML doc. Defaulting null/unknown to 24h is the correct safe default for the global majority.
• Retrospective caps: Three independent Math.Min(100, …) sites (per-habit, aggregate, weekly consistency) plus the AI prompt — thorough and consistent.
• Test coverage: New GoogleCalendarEventFetcherTests, GetUserCalendarsQueryHandlerTests, SetSelectedCalendarsCommandHandlerTests, and TimeFormatResolverTests cover edge cases (deleted/hidden calendars, explicit vs default selection, reconnect-required path, empty-list-clears-selection).

Recommendation

APPROVE. Two Medium findings that are straightforward to fix before or after merge:

1. Add MaximumLength(1024) to RuleForEach in SetSelectedCalendarsCommandValidator (+ matching HasMaxLength on the EF column).
2. Make the SaveChangesAsync in GetUserCalendarsQueryHandler conditional, mirroring the established pattern in GetCalendarEventsQueryHandler.

Neither blocks the feature's correctness.

🤖 Generated with Claude Code