Skip to content

Project safe calendar BYDAY recurrences (#569) - #545

Merged
thomasluizon merged 6 commits into
mainfrom
fix/ticket-569-byday-projection
Sep 25, 2026
Merged

thomasluizon merged 6 commits into
mainfrom
fix/ticket-569-byday-projection

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Closes thomasluizon/orbit-tickets#569

Change

Return daily and weekly calendar events with plain BYDAY when every Google expanded occurrence in the existing 60 day feed keeps one date shift and displayed start time. Shift BYDAY to the account weekdays. For alternate week rules, shift WKST too, including its Monday default, so the week cycle stays aligned. Keep ordinal weekdays, BYSETPOS, BYMONTHDAY, and nonuniform projections omitted. Store the expanded starts with sync suggestions so both import feeds make the same decision. These evidence fields remain absent from the response contract.

Assumptions

  • Google's complete 60 day expanded event list is the bounded proof window; rejected a second RRULE expander built from one sampled instance.
  • An alternate week rule needs its week start shifted with BYDAY; rejected changing only the weekday tokens because that changes week parity.
  • A stored suggestion lacking expanded occurrence evidence must refresh through auto sync; rejected treating one legacy sample as proof of a shifted series.
    Weekly interval-one rules become equivalent daily rules with BYDAY; multi-day alternate-week rules remain withheld. PR Project safe calendar BYDAY recurrences (#569) #545 currently says Closes #569, which points to the wrong repository.
  • The three conflict resolutions preserve both branches' assertions and behaviour.

External interface evidence

The installed Google.Apis.Calendar.v3 1.75.0.4206 assembly was inspected with reflection. Event.OriginalStartTime and Event.Start are EventDateTime; Event.RecurringEventId, Event.Status, and Event.Summary are strings; EventDateTime.DateTimeDateTimeOffset is nullable DateTimeOffset. To reproduce, inspect those properties with typeof(Event).GetProperty(name)?.PropertyType after referencing that package version. Its installed Google.Apis.Calendar.v3.xml describes OriginalStartTime as the recurrence-defined start even when an instance moves. GoogleCalendarApi.ListEventsAsync already requests expanded instances for 60 days and follows all pages.

Test evidence

  • Before the change, LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Application.Tests/Orbit.Application.Tests.csproj --filter 'FullyQualifiedName~Handle_RecurringByDayEventCrossingAccountDate_OmitsEvent' -v normal passed with the event omitted.
  • With the assertion strengthened and implementation still unchanged, LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Application.Tests/Orbit.Application.Tests.csproj --filter 'FullyQualifiedName~Handle_RecurringByDayEventCrossingAccountDate_ShiftsWeekdays' --no-build -v minimal failed because the result was empty.
  • After the fix, LC_ALL=en_US.UTF-8 dotnet test tests/Orbit.Application.Tests/Orbit.Application.Tests.csproj --filter 'FullyQualifiedName~Handle_RecurringByDayEventCrossingAccountDate_ShiftsWeekdays' --no-restore -v minimal passed. The calendar group passed 225 tests before the final alternate week test was added; that focused test then passed.
  • LC_ALL=en_US.UTF-8 dotnet build Orbit.slnx -v quiet passed with 0 errors. LC_ALL=en_US.UTF-8 dotnet test -v quiet passed 6,387 tests.
  • The ordinal, BYSETPOS, BYMONTHDAY, and nonuniform omission cases could not fail before implementation because the baseline already omitted shifted BYDAY events. They now have explicit controls with expanded occurrence evidence.
  • One full suite run hit ReminderSchedulerServiceTests.CheckAndSendReminders_TwoSameDayScheduledReminders_PersistsBothWithoutUniqueViolation at 00:00 UTC, before its 00:01 reminder was due. The full suite passed after 00:01 UTC without code changes.
  • Unchanged weekly tests passed 4/4 with the defect present. Strengthened tests then failed 9/14 before the fix.
  • The inconsistent anchor test failed 1/1 before its guard. Afterward, all 235 calendar tests passed.
  • LC_ALL=en_US.UTF-8 dotnet build Orbit.slnx -v quiet: 0 errors.
  • LC_ALL=en_US.UTF-8 dotnet test -v quiet: 6,411 passed, 0 failed.
  • Merge-forward after orbit-api 544: three conflicts resolved so both sides hold (StoredCalendarEventJson.cs keeps ExpandedOccurrencesUtc and the RecurrenceTimeZone projection; the serialization test now asserts Europe/Lisbon appears only as recurrenceTimeZone; the fetcher test keeps both sides' assertions).
  • dotnet build Orbit.slnx: 0 errors. Focused: 238 calendar tests and 16 fetcher tests pass.
  • Full suite: 6,463 of 6,463 pass under LANG=en_US.UTF-8 LC_ALL=en_US.UTF-8. Under this Mac's default locale nine formatting tests fail, none in the merged files; CI runs en_US.

Manual steps

None.

@thomasluizon

Copy link
Copy Markdown
Owner Author

Approach for #569: keep Google's expanded occurrence starts from the 60 day event list on each recurring item. In src/Orbit.Application/Calendar/Queries/GetCalendarEventsQuery.cs, allow a shifted rule only for daily or weekly plain BYDAY rules when every expanded occurrence proves the same local date shift and displayed clock. Rewrite only the BYDAY values after that proof. Preserve the existing refusal for ordinal tokens, BYSETPOS, BYMONTHDAY, and changing offsets. Carry the occurrence evidence through src/Orbit.Infrastructure/Services/GoogleCalendarEventFetcher.cs and src/Orbit.Application/Calendar/StoredCalendarEventJson.cs so event and suggestion feeds agree. Add focused handler, fetcher, and stored JSON tests. This uses Google's bounded expansion instead of a whole rule regex shift or a second RRULE engine.

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

Important

The current projection can expose recurrences that installed clients cannot import faithfully and can approve schedules that drift after the 60-day evidence window.

Reviewed changes I reviewed the full three-commit diff for calendar projection, internal recurrence evidence storage, and its focused coverage.

  • Expanded occurrence evidence — Collects recurrence-defined starts from Google's expanded feed and carries them through live events and stored suggestions.
  • RRULE projection — Shifts plain daily and weekly BYDAY tokens across account dates, including WKST for alternate-week rules.
  • Compatibility storage — Keeps the new evidence out of the response DTO and refreshes legacy timed recurrence rows that lack it.
  • Tests — Covers serialization, live and suggestion feed agreement, unsupported rule forms, seasonal clock changes, and alternate-week projection.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using GPT Sol | 𝕏

Comment thread src/Orbit.Application/Calendar/Queries/GetCalendarEventsQuery.cs Outdated
Comment thread src/Orbit.Application/Calendar/Queries/GetCalendarEventsQuery.cs
pullfrog[bot]
pullfrog Bot previously approved these changes Sep 25, 2026

ghost 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 I reviewed the importer compatibility and timezone stability fixes added since the prior Pullfrog review.

  • Normalized importer-facing rules: Rewrote weekly interval-one BYDAY rules as equivalent daily rules that installed clients can import with weekday filters.
  • Restricted admitted RRULEs: Rejected unsupported selectors, multi-day interval rules, and inconsistent recurrence anchors while preserving representable single-day interval rules.
  • Proved year-round stability: Required shifted rules to keep one projected clock and date offset across the full timezone adjustment cycle in addition to the bounded provider evidence.
  • Strengthened regression coverage: Added exact tests for normalized cadence, unsupported rule terms, inconsistent anchors, and drift beyond the provider fetch window.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

@sonarqubecloud

ghost commented Sep 25, 2026

Copy link
Copy Markdown

ghost 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 I reviewed the merge-forward integration added since the prior Pullfrog review.

  • Preserved recurrence evidence: Kept expanded occurrence starts in stored suggestion rows and restored them when those rows are read.
  • Integrated recurrence timezone responses: Reconstructed recurrenceTimeZone from the recurring master's stored source timezone for timed recurring suggestions while keeping server-only evidence off the response.
  • Strengthened serialization coverage: Verified the public response exposes only recurrenceTimeZone and stored rows retain both recurrence evidence and the response field.
  • Retained fetcher coverage: Preserved assertions for expanded recurrence-defined starts alongside the merged recurrence timezone behavior.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

@thomasluizon
thomasluizon merged commit 44b1e3b into main Sep 25, 2026
@thomasluizon
thomasluizon deleted the fix/ticket-569-byday-projection branch September 25, 2026 12:12
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