Skip to content

JM: Surface onboarding day-attendance checkmarks on the registration edit form#1932

Draft
maebeale wants to merge 4 commits into
mainfrom
maebeale/attendance-checkmarks-edit
Draft

JM: Surface onboarding day-attendance checkmarks on the registration edit form#1932
maebeale wants to merge 4 commits into
mainfrom
maebeale/attendance-checkmarks-edit

Conversation

@maebeale

@maebeale maebeale commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

🤖 suggested review level: 3 Read 📖 small view + Stimulus change; day fields already permitted, status derivation mirrors existing model logic

What is the goal of this PR and why is this important?

  • The per-day attendance checkmarks (Day 1…N) only existed on the event's Onboarding matrix. Surface them on the single-registration edit form too, so an admin can mark days without switching pages.

How did you approach the change?

  • Added the day checkboxes beside the status dropdown in event_registrations/_form.html.erb. DAY_FIELDS were already permitted params, so they save with the form.
  • Extended the attendance-status Stimulus controller: toggling a day derives the status the same way onboarding does (registered → incomplete_attendance → attended), and never overrides a deliberate inactive state (cancelled/no_show/transferred_out) — mirrors EventRegistration#sync_attendance_status_to_days!.
  • Status still only persists on save ("Unsaved" hint), and the admin can override the dropdown afterward.

Anything else to add?

  • Only admins reach this edit form (edit?manage?), matching the admin-only day-field params.
  • Covered by new system specs (render/persist, status derivation, inactive-status guard).

Copilot AI review requested due to automatic review settings July 4, 2026 18:51
// Recompute the status from how many day checkboxes are checked, then reflect it
// in the dropdown (which re-renders the icon and the unsaved hint via update()).
deriveFromDays() {
if (!this.activeStatusesValue.includes(this.selectTarget.value)) return

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 From Claude: Guards the days→status derivation to active statuses so a deliberate cancelled/no_show/transferred_out isn't silently rolled back by toggling a day — mirrors the server-side sync_attendance_status_to_days!.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Surfaces per-day attendance checkmarks (Day 1…N) on the single EventRegistration edit form, reusing the same day-count semantics as the Onboarding matrix and keeping the attendance status dropdown in sync via the existing attendance-status Stimulus controller.

Changes:

  • Add per-day attendance checkbox UI to event_registrations/_form, saving via already-permitted DAY_FIELDS.
  • Extend attendance-status Stimulus controller to derive an active attendance status (registeredincomplete_attendanceattended) from checked days, while leaving inactive/manual statuses untouched.
  • Add system specs covering rendering, persistence, status derivation, and the inactive-status guard.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
spec/system/event_registration_edit_spec.rb Adds system coverage for day checkbox rendering/persistence and status-derivation behavior.
app/views/event_registrations/_form.html.erb Renders per-day attendance checkboxes and wires them into the Stimulus controller with day count + active statuses.
app/frontend/javascript/controllers/attendance_status_controller.js Adds day targets/values and deriveFromDays() to keep the status dropdown consistent with day checkboxes.

@maebeale maebeale marked this pull request as ready for review July 4, 2026 19:16
Copilot AI review requested due to automatic review settings July 4, 2026 19:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread spec/system/event_registration_edit_spec.rb
@maebeale maebeale force-pushed the maebeale/attendance-checkmarks-edit branch 3 times, most recently from 32beabd to fd6d743 Compare July 4, 2026 19:45
Copilot AI review requested due to automatic review settings July 4, 2026 19:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread app/frontend/javascript/controllers/attendance_status_controller.js
Copilot AI review requested due to automatic review settings July 4, 2026 20:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment on lines 39 to 40
update() {
const status = this.selectTarget.value
@maebeale maebeale changed the title Surface onboarding day-attendance checkmarks on the registration edit form JM: Surface onboarding day-attendance checkmarks on the registration edit form Jul 5, 2026
@maebeale maebeale force-pushed the maebeale/attendance-checkmarks-edit branch from 9ab1d7d to 2bd2f52 Compare July 5, 2026 02:40
maebeale and others added 4 commits July 4, 2026 22:45
… form

The per-day attendance checkboxes only lived on the Onboarding matrix, so an
admin editing a single registration couldn't mark days without leaving the page.
Surface the same checkmarks beside the status dropdown, and mirror onboarding's
behavior so toggling a day rolls the attendance status forward/back (client-side,
respecting deliberate inactive states). DAY_FIELDS were already permitted params,
so the boxes save with the form.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Group them inline with the "View ticket" / "View submission" links instead of
under the status dropdown. The attendance-status controller now spans the whole
card so the checkmarks still drive the status select.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Push it to the right edge of the meta strip and stack the label over the day
chips, per the requested layout.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The derivation spec reloaded the record immediately after clicking Save, racing
the submit round-trip and occasionally reading the pre-save status. Assert the
redirect landed first, matching the sibling persistence spec.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 5, 2026 02:45
@maebeale maebeale force-pushed the maebeale/attendance-checkmarks-edit branch from 2bd2f52 to 67e272f Compare July 5, 2026 02:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@maebeale maebeale marked this pull request as draft July 5, 2026 03:10
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.

3 participants