Shifts: audience and shift-list options on the coordinator emails - #1792
Conversation
Per-rota email (`/Teams/{slug}/Shifts/Rotas/{id}/Email`) gets an
include-shifts checkbox, ticked by default. Cleared, the body drops the
shift section lead-in and all rather than stranding "your shifts are:"
over nothing — so a thank-you reads as one.
Team-wide email (`/Teams/{slug}/Shifts/Email`) gets the same checkbox plus
an audience selection: upcoming rotas only (the previous behaviour, still
the default) or all rotas in the active event, narrowed by Build / Event /
Strike, all ticked. That makes "thank everyone who worked the department"
reachable once the event is over. The period boxes are hidden under
upcoming and `TeamRotasAudienceFilter.Includes` ignores them there, so a
hidden cleared box can never narrow an upcoming send; a `RotaPeriod.All`
rota rides on any ticked period.
Changing an audience control re-posts the form with `intent=refresh` so
the recipient preview and the Send button's count track the selection,
leaving the half-written message and its validation alone. The refresh
button is the no-JS fallback.
Surfaces: six cultures (new keys plus the copy that assumed "upcoming");
authorization unchanged (`ResolveDepartmentManagementAsync`); audit rows
unchanged, one per dispatch; no schema change, so no migration; entry
points unchanged; Shifts.md and the email-a-rota feature doc updated;
tests for the filter, both templates, both dispatch paths and the refresh
branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MBDudxvE4SPpBrEP1WFVeb
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Reviewed commit ed09475 — no issues found. |
PR Surface ReportCompared Summary: 23 changed file(s) | EF migrations: 0 added file(s), max 0/1 per context Reforge Surface Score
Section Deltas
Section Size & Complexity Deltas
Rule Deltas
Corpus Size & Complexity
At head: largest class Published Write Surface14 of 48 sections publish write capability, 23 interfaces (0). Interface SurfaceNo new interfaces or interface methods. Diff Size
New Files
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed09475b91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The audience controls drove the recipient preview through a scripted re-post, so with the script off or broken a coordinator could widen the selection and press Send while the form still showed the old count and names — an irreversible dispatch whose displayed scope was stale. The form now carries the `TeamRotasAudienceFilter.Key` its recipient list was built from. A send whose posted selection differs is turned back: the form re-renders against the new audience with a notice, and sending again dispatches it. The script is cosmetic now — its failure costs a click, never a wrong send. Six cultures for the new notice; authorization, audit and GDPR paths unchanged (the turn-back sends nothing, so it writes no audit row); Shifts.md and the email-a-rota doc updated. Review-round: 1 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBz5pZi8nZaweSw8G1agzw
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01adb03010
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The audience guard re-rendered the form after assigning the new `TeamRotasAudienceFilter.Key` to the model, but the hidden field is an `asp-for` input and the tag helper prefers ModelState's attempted value over the model. Model binding had already stored the key the browser posted, so the re-render emitted the stale one and the next Send posted it again — turning back forever on exactly the scriptless path the guard exists to protect. The refresh branch escaped only by way of its `ModelState.Clear()`. The posted entry is now dropped where the model's key becomes the authoritative one, ahead of the assignment rather than inside the guard, so every return-view path below renders the audience the recipient list was just built from. The turn-back test asserted the model property, which was always right; it now seeds ModelState the way binding does and asserts the entry is gone — the half the model assertion could not see. Behaviour matches what Shifts.md and the email-a-rota doc already describe, so neither drifts; no schema, authorization, audit or localization surface moves. Review-round: 2 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBz5pZi8nZaweSw8G1agzw
2 participants
What
Both coordinator email compose forms get an include-shifts checkbox (ticked by default), and the team-wide one gets an audience selection: Upcoming rotas only (default, the previous behaviour) or All rotas in this event, narrowed by Build / Event / Strike (all ticked).
Why
A thank-you after the event needs to reach everyone who held a shift with the department, not just whoever still has one ahead — and it should not carry a schedule. Today the team-wide form silently means "upcoming", and both forms always append the recipient's shift list.
Existing surface checked
IRotaCoordinatorMessageService— its three methods took the new options as parameters rather than gaining siblings;BuildTeamRotaGroupsAsyncalready did the rota-by-rota admission, so the filter slots in there.CoordinatorRotaMessageRequest/CoordinatorTeamRotasMessageRequest— oneIncludeShiftsflag each rather than a second request shape.ShiftsEmails— one privateShiftSectionhelper shared by both templates.RotaPeriod(Shifts.Contracts) — reused as the period vocabulary; no new enum.EmailComposerViewModel/_EmailComposer— unchanged; the new controls sit outside it since they are audience, not composition.TeamRotasAudienceFilter(internal to Shifts). Four loose bools threaded through three methods and a view model read worse, and theUpcomingOnlyshort-circuit needs one place to live.UI changes / screenshots
No preview deploy screenshots yet — the PR's own preview at
https://{pr_id}.n.burn.campis the place to check both forms. Rendered markup changes:EmailRota.cshtml: one checkbox below the composer.EmailTeamRotas.cshtml: the same checkbox, a two-radio scope fieldset, a period fieldset hidden under "upcoming", and an "Update recipients" button beside Send.Checklist
mainonpeterdrier/Humans(the QA fork).origin/main.nobodies-collective/Humans#732is already qualified.EF migrations— no schema change.NuGet packages updated?— none.New project rule?— none surfaced.dotnet build Humans.slnx -v quiet0 errors;dotnet test Humans.slnx -v quietgreen across 49 projects (Integration self-skips, as designed).EmailTeamRotasViewModel.RefreshIntent, shared by view and controller; resource keys follow the section's existingEmailRota_/EmailTeamRotas_/Shifts_Email_families.Other surfaces walked:
ResolveDepartmentManagementAsync, and the newintent=refreshbranch sits after it.Docs/Shifts.mdandDocs/features/email-a-rota.mdupdated in this PR.Reviewer notes
falseand silently empty an upcoming-scoped audience.TeamRotasAudienceFilter.Includesshort-circuits onUpcomingOnly;GetTeamRotasRecipientPreviewAsync_IgnoresPeriodFlags_WhenUpcomingOnlypins it.RotaPeriod.Allrota is admitted by any ticked period. That is a judgment call, not something the existing model dictated.ModelState.Clear()on that branch is what keeps a half-written message from showing a validation error while the coordinator is still picking an audience.formnovalidatehandles the client side.intentis a plain action parameter, not a view-model property, so it cannot be smuggled in by model binding from a normal send.Closes
No issue — raised directly by Peter.
🤖 Generated with Claude Code
https://claude.ai/code/session_01MBDudxvE4SPpBrEP1WFVeb
Generated by Claude Code