RFC 5545 .ics calendar invites for coaching session emails - #384
Merged
Conversation
…actions query (phase 3)
…ites # Conflicts: # domain/src/coaching_session.rs # migration/src/lib.rs
Rescheduling a series (PUT /coaching_session_series/:id) now re-sends the recurring invite to both participants so their calendar event updates in place instead of going stale. update_rule increments ical_sequence in the same UPDATE that replaces the rule. Its only caller is the reschedule flow, and the route accepts only start_at, recurrence, and duration_minutes, so every rule replacement is a calendar move. send_recurring_sessions_scheduled_email becomes generic over the notification type (send_series_invite_email<N>), mirroring the single-session refactor. The create and reschedule paths differ only by template and the session_or_series variable. SeriesRescheduled shares the rescheduled_email_template_id flag with SessionRescheduled. build_series_invite_ics needed no changes: it already reads the sequence off the series and derives the UID from its id. Also gates two mock-only test helpers behind cfg(feature = "mock"). They were dead without the feature, which had been failing `cargo clippy --all-targets -- -D warnings` (a CI step) since phase 4a.
Plan: mark Phase 5 done (5a + 5b), record the 5b design and the pre-existing clippy break fixed alongside it, and document the accepted limitation that a series reschedule drops past occurrences from calendars while the DB keeps them. Test plan: add HR3-HR5 and RS3-RS4 for series reschedule, and correct the header, which still claimed reschedule was unimplemented.
Deleting a coaching session or a series now sends a METHOD:CANCEL invite to both participants so the event leaves their calendars. The .ics keeps the invite's UID and DTSTART so clients can match the cancellation to the event they hold, and carries SEQUENCE + 1. That bump is in-memory only: the row is being deleted, so persisting it would write to a row that is about to vanish. Cancellations fire after the delete commits, not before. The models are already in hand, so there is no need to announce a cancellation that a failing delete would then contradict. Two guards keep the emails honest: deleting an already-past session is housekeeping rather than a cancellation, and a series whose future set is empty has nothing to cancel. Cancellation emails carry no session link. The row is gone by the time a recipient could click, so the two markers keep the trait's None URL template rather than overriding it. They also load no topics, goals, or actions: identifying the event is enough. Splits rescheduled_email_template_id into session_ and series_ variants. Resend templates have no conditional syntax and the two reschedule paths send disjoint variables, so one shared template cannot render both. Adds session_cancelled_ and series_cancelled_ alongside them. Deployment passthrough for all four lands in phase 7.
Plan: mark Phase 6 done, record the three decisions that changed from the original text (fire after the delete, in-memory SEQUENCE bump, no link in a cancellation), and retire Phase 7's shared-template decision now that Resend is confirmed to have no conditional syntax. Test plan: add HC1-HC2 and SC1-SC4 for cancellation, and correct every reference to the old single shared reschedule flag.
Per-feature implementation-plan docs are process artifacts, not shippable product. The living plan moves outside the repo; the manual test plan under docs/test-plans/ stays, since that is a legitimate committed home.
Covers the two symptoms Phase 8 fixes, so a tester knows what a regression looks like: a per-occurrence cancel that silently does nothing, and a per-occurrence reschedule that duplicates the event. SO1 pins the RECURRENCE-ID address not drifting across repeated moves.
Invites are sent from hello@mail.myrefactor.com while ORGANIZER names the coach, so the two addresses disagree. Gmail labels such a message External, and live testing showed Google will let a user manually add the first invite but then silently refuse to apply a later update, even with a matching UID and a higher SEQUENCE. Allowing otherwise would let anyone who learned a UID move meetings on someone's calendar. SENT-BY (RFC 5545 3.2.18) is the property for exactly this: it says an agent is acting on the organizer's behalf rather than impersonating them. The value is quoted because a cal-address contains a colon. Whether a given client honours it is empirical, so this is worth sending regardless of what Google does with it: the previous output claimed an organizer it could not substantiate. All six builders pass the From address; the parameter is omitted entirely when sent_by is None, leaving existing output byte-identical.
This reverts commit 651ff66.
…ndees Live testing against Gmail showed an RFC-correct .ics is not enough to update an event. iTIP has an authorization model, and clients decide by identity: we sent From hello@mail.myrefactor.com while claiming the coach as ORGANIZER, so Google tagged the mail External, gated the invite behind a trust prompt, and ignored every reschedule despite a matching UID and a higher SEQUENCE. It had matched the event and declined to mutate it. A probe settled it. Pointing a fixture coach at the sending address, with no code change, made the friction vanish and moved a rescheduled event on its own. So the organizer is now the platform, whose address is the one we actually send from. Both humans become attendees rather than the coach being replaced, so they still see each other, and the coach gains updates on their own calendar that were previously impossible: an account's own events are authoritative and no external message can rewrite them. The anchor timezone still comes from the coach. It was read off the organizer, and following that rename would have anchored every invite to UTC and quietly undone the VTIMEZONE work. Attendees are appended as multi-properties. A component keys properties by name, so appending ATTENDEE twice keeps only the last and one participant disappears without any error. SENT-BY was tried first and reverted: it made Gmail stop recognizing the message as an invitation at all.
Documents why ORGANIZER is the platform rather than the coach, since that is what makes updates apply at all, and the two consequences a tester will hit: the coach RSVPs to their own session, and every person sees a one-time add-to-calendar prompt. Also records what was verified against real Gmail and Google Calendar, and warns that sessions created before the change can never be updated by us, so update testing needs a fresh session.
A reschedule email showed only the new time, so a recipient could not tell what had moved. Both reschedule templates now receive session_when and previous_session_when, each rendered in the recipient's own timezone. The value is one variable rather than a date and a time, because Resend supports plain substitution with no conditionals: the literal "at" between two variables always renders, so an unchanged start would read "Unchanged at Unchanged". Folding the connector into the value lets the whole phrase vary. previous_session_when reads Unchanged when the start did not move. That case is reachable because a reschedule notice also fires for title, meeting URL, and duration edits, which must keep sending so a stale Join link never sits on someone's calendar. The previous start needs no storage and no extra query. The single-session path already holds the pre-update model to detect a calendar-relevant change, and the series controller still holds the pre-update series whose rule carries the old start. Both variables are always supplied on the reschedule paths rather than sent optionally, since Resend fails a send outright when a declared variable is missing. The scheduled and cancelled payloads are unchanged and asserted so.
Series emails gave a first date, a last date, and a count, so a recipient could not tell weekly from biweekly. Live testing made the gap sharper: a reschedule that changed only the frequency reported "has rescheduled your recurring series" with an unchanged start and no other difference, so the one thing that changed was the one thing not shown. Recurrence::summary renders the cadence, e.g. Weekly or Every 2 weeks on Monday and Wednesday. It lives next to the type rather than in the email module because it describes the recurrence, not the email. Series create carries recurrence_summary. Series reschedule also carries previous_recurrence_summary, which reads Unchanged when the cadence held, matching how previous_session_when already behaves. A start-only move leaves the cadence untouched, so that case is reachable. The summary and the RRULE both derive an effective interval, and Biweekly doubling it is easy to get wrong in one place only. Rather than refactor verified .ics code to share the derivation, a table test pins the two together so any future divergence fails loudly. Single-session and cancellation payloads are unchanged and asserted so.
emails.rs had grown to 4,629 lines with roughly 60 percent of it tests, which buried the production code it was meant to sit beside. The tests move to emails_tests.rs, wired with the path attribute this crate already uses in user.rs, user_role.rs, coaching_session_topic.rs, and gateway/ical.rs. emails.rs itself already extracted one test module this way, so the split follows a pattern the file had started. It also lets the test file be made read-only for frozen-test work without locking the source next to it. Pure move: the diff to emails.rs is the three wiring lines and the deletion, no production code touched, and the domain suite reports the same 302 tests before and after.
Three correctness fixes surfaced by PR review. increment_ical_sequence and update_rule both read the current SEQUENCE in one statement and wrote back N+1 in another. Two overlapping edits would read the same N and both write N+1, and a calendar client treats a repeated SEQUENCE as a duplicate of the invite it already has and silently ignores it, which is the exact failure this feature exists to prevent. Both now increment as a column expression in a single UPDATE. The session-level edit and its bump also commit in one transaction, so a failed bump can no longer leave the row mutated behind a failed request. A series PUT carrying an unchanged rule re-materialized the whole series: it deleted every future session (taking notes and goal links with it), recreated an identical schedule under new ids, burned a SEQUENCE, and re-invited both people to a meeting that never moved. reschedule now short-circuits when the requested rule matches the stored one and reports that through Rescheduled, so the controller knows not to send invites. find_open_by_coaching_session_id loaded every action for a session and dropped the completed ones in Rust. It runs on the invite path for every single-session email, so the filter moves into the query. Status::COMPLETED keeps the open/completed split defined in one place for both the predicate and is_completed.
… builders The .ics attachment was base64 and never decoded, so nothing pinned the occurrence-vs-standalone dispatch in either the invite or the cancel path. Swapping those arms changed no assertion. Tests now capture the request Resend receives, decode the attachment, and assert the UID and RECURRENCE-ID that distinguish addressing one occurrence from addressing the whole series. Both swaps were confirmed to fail before the assertions were kept. Adds the missing per-occurrence reschedule test, and scopes the RRULE check to the VEVENT since a spliced VTIMEZONE carries its own RRULE for DST. The body matcher is a subset match, so a renamed template variable was invisible even though Resend fails such a send with 422. The two reschedule flows now pin their exact variable set. description_full_single used a due date whose UTC and New York calendar dates agreed, so it passed with the timezone conversion deleted. Moved to a time near midnight that genuinely differs. Removes three duplications behind anchor_tz, ics_uid, and session_summary (20 sites), and gives rrule_value and Recurrence::summary one shared effective_interval so the emitted rule and the human phrase cannot disagree. Consolidates three empty-slice guards, one of which used .expect in production code, into series_bounds. Drops the stale module-wide dead_code allow (nothing in the module is dead) and the phase-0 spike example, which duplicated ical.rs with no test covering it.
…by meaning Extracting anchor_tz inserted it above platform_organizer's doc comment, which left anchor_tz carrying two unrelated paragraphs and stripped platform_organizer of the note explaining that calendar clients only apply updates when ORGANIZER matches the sending address. That is the hardest-won constraint in this feature and it belongs on the function that encodes it. The no-op reschedule guard compared serialized JSON, which is a comparison of representation rather than of meaning: a stored rule carrying a field this version does not know about, or an optional that serializes as null rather than being skipped, read as "changed" and re-materialized the whole series. It now parses both sides and compares SeriesRule values. An unreadable stored rule still falls through to re-materializing, so the failure direction is unchanged.
The plan still described phases 0-8 and told the tester to run the phase-0 spike example, which no longer exists. It also stated the opposite of current behavior for series reschedules, which now short-circuit on an unchanged rule. Adds RS5 and RS6 for the two behaviors that are invisible in an email: a re-submitted identical series rule must leave the existing session rows alone (the destructive case, since a reschedule otherwise deletes and recreates them), and rapid successive edits must each land a distinct SEQUENCE. Adds HE1 to HE5 and SE1 for the reschedule email content from phases 10 and 11. Records the 2026-08-12 verification against real Postgres, and adds guidance on which cases a browser-driven run can assert and which need a mail inbox and a calendar client. Drops the em dashes throughout.
…coverage A local backend on the dev .env carries live Resend template ids and mails both participants on every create, reschedule and cancel. That is not obvious from a local run, and a first pass through this plan generated an estimated 60 to 90 real sends before anyone noticed. Documents the mock-Resend approach as the default instead, which needs no code change since RESEND_BASE_URL is already configurable. Flags the series section as blocked for any series carrying a note, action or agreement (refactor-platform-fe#446, pre-existing on main), and notes that this inverts RS5's premise: the foreign key makes such a series impossible to reschedule at all rather than destroying its notes. Records the frontend Playwright coverage and the concurrency caveat it found, where five simultaneous edits advance SEQUENCE correctly but the stored date settles on the fourth rather than the fifth.
jhodapp
marked this pull request as ready for review
August 12, 2026 16:23
Contributor
Greptile SummaryThe PR adds RFC 5545 calendar attachments and lifecycle notifications for single and recurring coaching sessions, backed by persistent sequence and recurrence-address fields.
Confidence Score: 4/5The PR does not yet appear safe to merge because a concurrent series reschedule can leave future replacement sessions alive after the series is cancelled. Cancellation still snapshots future sessions before opening its transaction and deletes only those captured IDs; a concurrent reschedule can replace them first, after which deleting the series merely detaches the replacements through the SET NULL foreign key. Files Needing Attention: domain/src/coaching_session_series.rs
|
| Filename | Overview |
|---|---|
| domain/src/coaching_session.rs | Adds post-commit invite notifications and atomically couples calendar-relevant edits or deletion with sequence increments. |
| domain/src/coaching_session_series.rs | Adds recurring invite orchestration, meaningful no-op detection, sequence handling, and future-session lifecycle coordination. |
| domain/src/emails.rs | Centralizes calendar-aware notification composition and replaces argument-limit suppressions with recipient and participant context types. |
| domain/src/gateway/ical.rs | Implements reusable RFC 5545 generation for requests, cancellations, recurrence rules, occurrence overrides, and timezone-aware timestamps. |
| domain/src/gateway/resend.rs | Extends Resend payload construction with base64-encoded calendar attachments and method-specific content types. |
| migration/src/m20260812_000000_add_ical_sequence.rs | Adds sequence counters and nullable recurrence identifiers needed for stable calendar lifecycle updates. |
| entity_api/src/coaching_session_series.rs | Adds atomic column-expression sequence increments to series update and cancellation operations. |
Sequence Diagram
sequenceDiagram
participant Client
participant Domain
participant DB
participant Email as Resend
participant Calendar
Client->>Domain: Create or modify session/series
Domain->>DB: Persist mutation and SEQUENCE
DB-->>Domain: Commit updated model
Domain->>Email: Send lifecycle email with invite.ics
Email->>Calendar: Deliver REQUEST or CANCEL
Calendar->>Calendar: Match UID and apply higher SEQUENCE
Reviews (6): Last reviewed commit: "refactor(emails): fire every session not..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Replaces the four `#[allow(clippy::too_many_arguments)]` suppressions with the context-struct pattern the coding standards call for, and whose worked example (`ActionEmailContext`) already lives in this file. The standard is explicit that a function over the limit should bundle related parameters "instead of adding an `#[allow]` attribute", so these were a documented violation rather than taste. Two groupings do the work. `Recipient` carries the addressee, the other party, and the label the copy uses for that other party: three values that always travel together and are meaningless apart, since `other_user_role` describes `other_user` rather than the recipient. `Participants` carries the coach and coachee as one named value. `Participants` is about readability at the call site, not type safety. Both fields are `&users::Model`, so a transposed pair still compiles; naming them makes the swap visible where two adjacent bare arguments were not. Confirmed by transposing them at the series notify site: the suite still passes, because that call site cannot be mocked. The struct narrows where the mistake can hide, it does not eliminate it. No behavior change. Mock suite 305 / 287 / 162, both clippy runs clean.
…le read Found by review. A cancellation built its SEQUENCE as `model.ical_sequence + 1` from a model read before the delete, so an edit committing in that window could claim the same number. A calendar client that already applied the edit sees a cancellation whose SEQUENCE it has seen, treats it as a duplicate, and drops it. The session vanishes from the app and stays on both participants' calendars, which is the one outcome cancellation exists to prevent. The window is not theoretical: on the single-session path a Tiptap HTTP call sits between the read and the delete. Both delete paths now bump in SQL inside the delete transaction and send the value the DB returns. The bump takes the row lock, so a competing edit either lands first with a lower SEQUENCE or blocks and then finds no row. This is the same fix already applied to the update path; the cancel path was the sibling case that got missed, so the pattern now holds everywhere a SEQUENCE is issued. Consequently all six builders take the sequence as given, rather than three adding one in memory and three not. Adds delete_bumps_ical_sequence_in_sql_before_deleting, asserting on the emitted SQL that the bump is self-referential and precedes the DELETE. Restoring the old in-memory bump makes it fail with a log showing only a SELECT and a DELETE. The three builder tests now pin pass-through, since the guarantee they protected has moved to the caller.
Member
Author
|
Confirmed and fixed in The bug as described was real. Cancellation built its SEQUENCE as Fix. Both delete paths now bump in SQL inside the delete transaction and send the value the DB returns. The bump takes the row lock, so a competing edit either lands first with a lower SEQUENCE or blocks and then finds no row. All six builders now take the sequence as given, instead of three adding one in memory and three not. Worth noting this is the same defect class already fixed on the update path earlier in this PR. I fixed the instance that was pointed at rather than the pattern, and the cancel path was the sibling that got missed. It now holds everywhere a SEQUENCE is issued. Test. Mock suite 306 / 287 / 162, both clippy invocations clean, fmt clean. |
…production
The file was named for the day the branch started, not the day it ships, which
put it at position 101 in lib.rs, ahead of two migrations already applied in
production. Applying is unaffected, since pending selection is a set difference.
Rollback is not.
`down -n 1` rolls back the LAST entry in lib.rs, not the most recently applied
migration: get_migration_with_status builds its list from get_migration_files
(the registration order) and only marks status, then exec_down reverses that.
So a one-step rollback during a bad deploy of this feature would have dropped
m20260807_users_lower_email_index, a users index, rather than the columns the
operator meant to remove.
Renamed to m20260812 and moved to the end of the registration list. Verified
against real Postgres: `down -n 1` now rolls back the ical migration, drops all
three columns, and leaves the users index in place; `up` reapplies cleanly.
The filename is the identity recorded in seaql_migrations, so any database that
already ran the old name sees the renamed file as pending and fails on "column
already exists". Production has never applied it, so it is clean there. Local
and PR-preview databases need the version string updated in place:
UPDATE refactor_platform.seaql_migrations
SET version = 'm20260812_000000_add_ical_sequence'
WHERE version = 'm20260702_000000_add_ical_sequence';
is_calendar_relevant_change lived on the session module but was a statement about invite content, not about sessions. Its own doc comment gave it away: it had to explain that `title` counts only because title rides in the .ics DESCRIPTION, reaching into another module to justify itself. Now emails::affects_invite, alongside the builders whose output it describes. Anyone adding a field to an invite now has the list in front of them rather than a module away, which is the failure that produced the known gap the doc comment now records: the DESCRIPTION also carries topics, goals and open actions, edited through their own endpoints, so a before/after comparison of the session row cannot see them and those edits do not re-send. Pure and synchronous by requirement, not by preference, and the doc says so. The caller runs it inside the update transaction to decide whether to bump SEQUENCE, which commits with the edit; the email that follows is best-effort and cannot be what makes that decision. No behavior change: the same four fields, same call site, same result. The test moves with it and now names the .ics property each field maps to, with updated_at as the control.
The rename commit staged the file move but not the lib.rs edit that points at it, so the pushed tree declared `mod m20260702_000000_add_ical_sequence` against a file that no longer existed and the build failed with E0583. Local gates passed because the working tree was correct; only the commit was not.
Every other gateway imports exactly one thing from the app, crate::error. ical.rs also reached for entity::users::Model, via Participant::from_user, which this PR added during the organizer rework. That contradicted the module's own design: DescriptionParts was deliberately built from plain data so the builder would not know the schema, and half-decoupling is worse than either consistent choice because a reader cannot tell which convention applies. The user-to-participant mapping moves to emails.rs, which already owns entity knowledge, as a small `participant` helper. The builder keeps only Participant::new. The gateway's tests carried the same coupling: a 15-field users::Model fixture existed only to supply a name and an email. They now use plain named constants, so the builder can be exercised without constructing an entity at all, and the file no longer imports entity or sea_orm. Recurrence stays. It is a value type rather than an entity, and the emitted RRULE is defined in terms of it. No behavior change.
This PR had split them. Session create and both series notifications fired from controllers, while reschedule and the three cancellations fired from domain functions, so series reschedule and series cancel, the same entity and the same lifecycle, sat on opposite sides of the boundary. A reader could not answer "where do session emails get sent" from one place. There was a real constraint underneath: the domain-side ones need state that only exists mid-call, the pre-update date or a model bumped inside the delete transaction, which a controller cannot see. That made the split defensible but left it undocumented and looking arbitrary. Resolved toward domain, which already announces its own side effects there via publish_coaching_session_title_updated. All six now fire after their commit, best-effort. The series reschedule notification in particular belongs here: the previous rule is only in scope inside `reschedule`, so the caller was reaching for something the domain already had. Controllers no longer reference the emails module at all; the import is gone from both. create_with_sessions takes &Config to match its siblings. No behavior change. The notify functions, their ordering relative to commit, and their best-effort semantics are untouched.
github-project-automation
Bot
moved this from Review
to ✅ Done
in Refactor Coaching Platform
jhodapp
added a commit
that referenced
this pull request
Aug 13, 2026
Review question: the invite and cancellation orchestrators were carrying the same three-arm match verbatim, differing only in which pair of builders it called. Extracted as build_session_ics, which takes the two builders as closures. It owns the whole decision in one place: a session inside a series is addressed as an override, a standalone one by its own UID, and a series member predating ical_recurrence_id has neither so it goes out with no attachment. `description` is threaded through to whichever builder runs, so it moves exactly once rather than needing a clone to satisfy both closures. Also finishes an inconsistency from #384: send_session_cancelled_email_to_recipient still took recipient, other_user and other_user_role as three positional arguments while its sibling took a Recipient. It only escaped the earlier change because it sat one argument under the clippy limit, not because the transposition hazard was any smaller. Both now take Recipient. No behavior change.
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
1 participant
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.
Description
Attaches RFC 5545
.icscalendar invites to coaching-session lifecycle emails, so a session or series that is scheduled, rescheduled, or cancelled shows up correctly in Google Calendar, Apple Calendar, and Outlook without anyone copying times by hand.One reusable builder (
domain/src/gateway/ical.rs) serves all six flows. Invites are attached to the existing create emails; reschedule and cancel emails are net-new. Every invite for a given session or series shares a stableUIDand carries an incrementingSEQUENCE, which is what lets a calendar client update or remove the event in place rather than accumulating duplicates.GitHub Issue: Closes #333
Changes
.icsbuilder (domain/src/gateway/ical.rs):VCALENDAR/VEVENTwithMETHOD,STATUS,UID,SEQUENCE,RRULE, organizer/attendee, andLOCATION/URL/CONFERENCEfrom the meeting URL. Pure and clock-free:dtstampis injected so output is deterministic and unit-testable.VTIMEZONEsplicing: real IANA zone definitions are spliced in fromvtimezones-rsso strict clients (Outlook) render the coach's local time instead of floating time. Falls back to UTCZtimes when the zone is UTC or unknown.domain/src/gateway/resend.rs): base64-inlineinvite.icswith the correcttext/calendar; method=REQUEST|CANCELcontent type.UID, bumpedSEQUENCE. Single-session sends only fire when a calendar-relevant field actually changed (date,duration_minutes,meeting_url,title).METHOD:CANCEL+STATUS:CANCELLED, fired after the delete commits. Skipped for already-past sessions and for series with nothing upcoming.UIDplus aRECURRENCE-IDnaming that occurrence's original start.m20260812_000000_add_ical_sequence: addsical_sequence INTEGER NOT NULL DEFAULT 0tocoaching_sessionsandcoaching_session_series, plus a nullableical_recurrence_id TIMESTAMPoncoaching_sessions.SEQUENCEis incremented atomically. The bump is a column expression in a singleUPDATE ... RETURNING, and for a single-session edit it shares one transaction with the edit itself. A read-then-write let two overlapping edits both land on the same nextSEQUENCE, and a calendar client discards a repeatedSEQUENCEas a duplicate of the invite it already has, which defeats the entire point of the feature precisely when someone adjusts a time twice in quick succession.Status::COMPLETEDso theCompleted | WontDosplit lives in one place for bothis_completed()and the query, andfind_open_by_coaching_session_idfilters in SQL rather than loading every action and discarding most of them.Database Migration
m20260812_000000_add_ical_sequence— additive only.NOT NULL DEFAULT 0sequence counters on two tables, plus a nullableical_recurrence_idoncoaching_sessions. Non-breaking, no backfill, no manual steps. Existing rows get0, so the first invite sent after deploy is treated as the original.Series sessions materialized before this migration have a
NULLical_recurrence_idand therefore no valid occurrence address. Per-occurrence edits on those skip the email and log, rather than guess and address the wrong instance. New series populate the column at materialization.Deployment prerequisites
Four new env vars are read by the backend and are wired through all four deployment layers in
this PR: both compose files and both deploy workflows. They are non-secret template ids, so they
travel as
vars.rather thansecrets.and need nosecrets:declaration.SESSION_RESCHEDULED_EMAIL_TEMPLATE_IDRECURRING_SESSIONS_RESCHEDULED_EMAIL_TEMPLATE_IDSESSION_CANCELLED_EMAIL_TEMPLATE_IDRECURRING_SESSIONS_CANCELLED_EMAIL_TEMPLATE_IDAll four Resend templates exist, and all four variables are already populated in the GitHub
productionenvironment. Nothing is outstanding for deployment.Testing Strategy
Automated. Full mock suite green (domain 305 / entity_api 287 / web 162), both clippy invocations clean,
cargo fmt --checkclean. Coverage splits deliberately:.icsstructure is asserted on the pure builders with a fixeddtstamp, while wiring is asserted with mockito against the Resend payload. Because the email orchestrators swallow send errors by design, every mockito test ends in.assert_async()— without it the tests pass even when nothing is sent.Manual.
docs/test-plans/ics-calendar-invites-manual-test-plan.mdhas step-by-step happy and sad paths: H1-H6, S1-S9, reschedule HR1-HR5 / RS1-RS6, reschedule email content HE1-HE5 / SE1, cancellation HC1-HC2 / SC1-SC4, and per-occurrence HO1-HO3 / SO1-SO2. It also flags which cases a browser-driven run can assert and which need a real inbox and calendar client.Verified live against real Gmail and Google Calendar (real Resend delivery, coach in
America/New_York, coachee inAmerica/Chicago): create, reschedule applied in place with no duplicate, and cancel removing the event, all on both the coach and coachee accounts. Also confirmed in that run: all six templates render, per-recipient timezone rendering,VTIMEZONEhonoured by Google and Apple, andCONFERENCErendering as a real Join with Google Meet button. Earlier Apple checks (file import) covered series create, the per-occurrenceRECURRENCE-IDmove, and series cancel.Verified against real Postgres, for the behaviors that are invisible in an email: a single reschedule bumps
SEQUENCEinside one transaction; ten concurrent reschedules of the same session advanced it by exactly ten with no lost updates; a series reschedule rewrites the rule and bumps correctly; an identical series request leaves the existing session rows untouched; a real change after that still applies; and the open-actions filter runs clean with completed rows present. For contrast, the previous read-then-write bump was replayed at the same concurrency and lost nine of ten updates.Frontend e2e coverage
Three Playwright specs in the frontend repo, 13 tests, green against this branch on real Postgres. They point the backend at a mock Resend (
RESEND_BASE_URLwas already configurable, so no backend change was needed) and assert on the decoded invite, not just app state. Covered: create, RS1 (proven as zero emails sent), HR1, HC1, RS6, and H2 confirming the DST-aware zone conversion is correct rather than merely present. RS5 passed including a payload shape the compare-by-meaning guard was needed for: the frontend omitsintervalwhen it is 1 while the stored rule normalizes tointerval: 1.Contract
CalendarInviteFieldsv1 pinsical_sequenceandical_recurrence_idas read-stable, sinceical_sequenceis the only app-visible evidence that an invite was or was not re-issued.Known blocker for series testing (pre-existing, not from this PR)
refactor-platform-fe#446:notes,actions, andagreementsreferencecoaching_sessions.idwithON DELETE NO ACTION, so deleting a session that has content returns 503. Series reschedule and cancel both bulk-delete future sessions and therefore inherit it, which means a series stops being reschedulable the moment real coaching happens in it.This predates this branch and exists on
maintoday: the delete path is unchanged here and the foreign keys come from older shared migrations. Running this PR's test plan is simply what surfaced it. It needs a cascade-or-block product decision and is deliberately out of scope here. Practical effect: every series case in the test plan can only be exercised on a content-free series, and all series-side e2e coverage is blocked behind it.Concerns
ORGANIZERis the platform, not the coach, and that is load-bearing. Live testing against real Gmail and Google Calendar showed an RFC-correct.icsis necessary but not sufficient: iTIP has an authorization model. While the invite named the coach as organizer and we sent fromhello@mail.myrefactor.com, Google tagged the mail External and silently ignored every reschedule despite a matchingUIDand a higherSEQUENCE.ORGANIZER;SENT-BY=...was tried and made it worse, and has been reverted. With the organizer set to the sending address and both humans as attendees, the full lifecycle now applies in place on both accounts. Two accepted costs: coaches RSVP to their own sessions, and each person sees a one-time "Add to calendar" prompt. Sessions created before this change can never be updated or cancelled by us, since their invites named a coach as organizer.UID. Rescheduling or cancelling the whole series replaces or removes the entire recurring event, including occurrences that already happened. The app keeps those past sessions and their notes; calendars do not show them. Per-occurrence edits no longer have this problem, but series-level ones do. Test cases HR5 and SC4 exist to get a decision on whether that is acceptable.DESCRIPTIONbut does not currently re-send, because those are edited through separate endpoints. Known follow-up.build_*_icsfunctions still spell out the same 16-fieldIcsInviteliteral, and notification calls fire from the domain layer for single sessions but from controllers for series. Both are real, neither changes behavior, and both are cheaper to do against code that is not simultaneously being verified live. (The four#[allow(clippy::too_many_arguments)]that were also listed here have since been removed in9fd7b194, replaced by theRecipientandParticipantscontext types the coding standards call for.)SEQUENCEadvanced correctly and without gaps, but the stored date settled on the fourth rather than the fifth. The invite always matches whatever the row ends up holding, so no calendar diverges from the app; it is the row itself that has no last-write-wins guarantee. Pre-existing behavior of the update path, surfaced by this PR's testing rather than introduced by it.