Skip to content

Extract rota display into reusable partials#214

Merged
peterdrier merged 2 commits intomainfrom
sprint/2026-04-10/batch-10
Apr 15, 2026
Merged

Extract rota display into reusable partials#214
peterdrier merged 2 commits intomainfrom
sprint/2026-04-10/batch-10

Conversation

@peterdrier
Copy link
Copy Markdown
Owner

Summary

  • Extract inline rota rendering from Views/Shifts/Index.cshtml into three shared partials: _RotaHeader.cshtml (name, badges, tags, description), _BuildStrikeRotaTable.cshtml (all-day shift table with collapsible date ranges and range signup), and _EventRotaTable.cshtml (timed shift table with individual signup)
  • Add typed view models (RotaHeaderViewModel, BuildStrikeRotaTableViewModel, EventRotaTableViewModel) for clean partial contracts
  • Reduce volunteer browse view from 586 to 259 lines with no visual or behavioral changes

Issues

Test plan

  • Browse /Shifts — verify rota headers display correctly (name, priority badges, period badges, tags, visibility, description, practical info)
  • Verify build/strike rotas show all-day shift table with collapsible date ranges
  • Verify event rotas show timed shift table with signup buttons
  • Test range signup form on build/strike rotas
  • Test individual signup on event rotas
  • Verify date range expand/collapse still works (chevron toggle)
  • Verify Signed up / Full / Needs help badges display correctly
  • Confirm no visual differences from before the refactor

peterdrier added a commit that referenced this pull request Apr 13, 2026
Two regressions from the rota partial extraction (PR #214):

- _BuildStrikeRotaTable: restore the "all slots signed up but some
  still understaffed" info alert that was dropped when extracting
  from Views/Shifts/Index.cshtml.
- _BuildStrikeRotaTable: restore the hidden filter inputs
  (departmentId, fromDate, toDate, period, tags[]) on the range
  signup form so SignUpRange redirects preserve filter state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@peterdrier
Copy link
Copy Markdown
Owner Author

PR Review — 2026-04-13

Spec: closes #198. The issue author paused the ShiftAdmin half of the refactor on 2026-03-25 (view slated for replacement in Vol→Camps transition), so only extracting from Views/Shifts/Index.cshtml is correct.

Findings (verified against base fd4a66c):

  1. Localization dropped in partialsnot a regression. Only Shifts_SignedUp and Shifts_Pending exist in Resources/SharedResource*.resx; every other Shifts_* key in the pre-refactor view (Essential, Important, Filled, SlotsFull, Start, End, Date, Days, Total, etc.) fell back to the literal key string. The refactor replaced broken keys with readable English — strictly an improvement. No fix applied.
  2. Shifts_SlotsFull info alert dropped — real regression. The else if (allDayShifts.All(...) && ...Any(... < MinVolunteers)) branch that rendered the "all signed up but some still understaffed" alert was lost in the extraction. Restored in _BuildStrikeRotaTable.cshtml with English text matching the refactor style.
  3. Filter hidden inputs dropped from range signup form — real regression. departmentId, fromDate, toDate, period, tags[] were emitted as hidden inputs so SignUpRange could preserve filter state on redirect. Added Filter* properties to BuildStrikeRotaTableViewModel, plumbed from Index.cshtml, re-emit in the partial.

Fix commit: b536c5e. dotnet build clean, 0 warnings, 0 errors.

Bottom line: Refactor is structurally correct and ShiftAdmin is correctly left alone per issue author. With b536c5e the two functional regressions are restored. Ready to merge after QA spot-check of the /Shifts page.


Review by Claude Code `/pr-review`

Extracts inline rota rendering from Views/Shifts/Index.cshtml into three
shared partials with typed view models:

- _RotaHeader: rota name, preference star, priority/period/visibility
  badges, tags, description, practical info.
- _BuildStrikeRotaTable: all-day shift table with collapsible date ranges,
  range signup form (preserving filter state), SlotsFull info alert.
- _EventRotaTable: timed shift table with per-shift signup form (preserving
  filter state), phase badges, signup lists.

Reduces volunteer browse view rota loop from ~350 lines to ~30 lines of
partial dispatch. All Localizer calls, filter hidden inputs on both signup
forms, and the SlotsFull alert are preserved from the pre-refactor view.

Closes #198

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@peterdrier peterdrier force-pushed the sprint/2026-04-10/batch-10 branch from b536c5e to 4206676 Compare April 13, 2026 16:10
@peterdrier
Copy link
Copy Markdown
Owner Author

Rebased onto main — 2026-04-13

The PR branch was cut from 898ad10a, which predates the i18n commit (fce4152 Add i18n localization for user-facing views). Main has since added 64 `Shifts_*` resource entries and filter-state hidden inputs on the single-shift signup form. That's why GitHub reported a merge conflict.

Rebased to origin/main (fd4a66c) and reapplied the refactor from scratch, preserving everything from main's current Index.cshtml:

  • All `@Localizer["Shifts_*"]` calls (Title, BrowseTitle, Essential, Important, Filled, Date, Start, End, Full, NeedsHelp, SlotsFull, SignedUpAllDays, SignedUpPartial, DayNeedsHelp/DaysNeedHelp, ClickToExpand, Days, Total, AllDay, FullDay, Shift, Phase, Time, Duration, SignUpButton, SignUpForDates, SetUp, Event, Strike, AllPhases, HiddenBadge, MatchesPrefs, etc.) — all now inside the partials.
  • `Shifts_SlotsFull` info alert branch inside `_BuildStrikeRotaTable`.
  • Filter hidden inputs (`departmentId`, `fromDate`, `toDate`, `period`, `tags[]`) on both forms — the range signup in `_BuildStrikeRotaTable` and the single-shift signup in `_EventRotaTable` (missed by my earlier fix, because the pre-i18n tree I reviewed didn't have them either).
  • `GetPhase` / `GetPhaseBadge` `@functions` block moved into `_EventRotaTable` (only consumer).

Build: `dotnet build Humans.Web` → 0 warnings, 0 errors.

History: force-pushed (`b536c5e` → `4206676`). The branch is now 1 commit ahead of `main`. My previous fix commit (b536c5e) and the original refactor commit (1e18ec1) were squashed into a single clean commit on top of main since there was no sensible way to replay them through the i18n additions.

Ready to merge pending QA spot-check of `/Shifts`.


Review by Claude Code `/pr-review`

@peterdrier peterdrier closed this Apr 14, 2026
@peterdrier peterdrier reopened this Apr 14, 2026
NuGet published five advisories against 14.11.1 earlier today
(GHSA-26qp-ffjh-2x4v, GHSA-cr67-pvmx-2pp2, GHSA-fwvm-ggf6-2p4x,
GHSA-v67w-737x-v2c9, GHSA-x9h5-r9v2-vcww — ImageMagick stack overflow
in DestroyXMLTree() and related). TreatWarningsAsErrors promotes NU1902
/NU1903 to build-breaking errors, so restore was failing on main.
14.12.0 is the advertised fix for all five.

Also update the About page NuGet list per the post-update maintenance
rule if present.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@peterdrier peterdrier merged commit e808f15 into main Apr 15, 2026
3 checks passed
@peterdrier peterdrier deleted the sprint/2026-04-10/batch-10 branch April 15, 2026 01:09
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