Add a sectors-style age-range editor to the person form#1872
Conversation
| <%= render "shared/sectorable_item_fields", f: sfi, show_admin_flags: true %> | ||
| <% end %> | ||
| <!-- Sectors + Age ranges (side by side; age ranges is the thinner column) --> | ||
| <% age_type, age_cats = (@person_categories_grouped || {}).find { |type, _| type.name == "AgeRange" } %> |
There was a problem hiding this comment.
🤖 From Claude: AgeRange is pulled out of the shared categories loop here and rejected from the other_category_types block below (line 174), so the primary-age hidden field and "mark the primary ones" hint move into this column with it. Other profile-specific types (Workshop Settings) keep their original full-width rendering.
| if params[key].key?(:managed_category_type_ids) | ||
| # The form only edits certain category types (e.g. age ranges + workshop | ||
| # settings). Preserve taggings of every other type the form never shows so | ||
| # saving can't silently drop them — and assign the union so the join rows |
There was a problem hiding this comment.
🤖 From Claude: This is the core safety fix: the form sends managed_category_type_ids (age ranges + workshop settings), so we only replace those types and union-preserve every other type the form never shows. Because preserved categories stay in the assigned set, their join rows aren't destroyed/recreated — is_primary and legacy_id survive. Org keeps the old full-replace path since it doesn't send the key.
| # profile-specific type shown above. assign_associations preserves taggings | ||
| # of any other type, so saving the form can't drop a person's non-AgeRange | ||
| # category connections (e.g. art types tagged elsewhere). | ||
| @managed_category_type_ids = ([ @age_range_type&.id ] + |
There was a problem hiding this comment.
🤖 From Claude: Note: category_ids was also dropped from person_params. Otherwise #update's assign_attributes(person_params) would full-replace categories= and save before assign_associations runs its scoped/preserving assignment, destroying the non-AgeRange join rows. assign_associations reads category_ids raw from params, so the contract is unchanged.
| choose(event) { | ||
| const select = event.target | ||
| const chip = select.closest("[data-age-range-picker-target='chip']") | ||
| chip.dataset.categoryId = select.value |
There was a problem hiding this comment.
🤖 From Claude: A new chip uses a select for its id, but the primary star submits person[primary_age_category_ids][] keyed by id — so on select we copy the chosen id onto the star's value. That keeps the multi-primary contract identical to persisted chips (which render the id directly) without changing the controller params.
|
|
||
| // Single-select primary: lighting one star clears the others and floats that | ||
| // chip to the front, mirroring primary_sector#selectPrimary. | ||
| togglePrimary(event) { |
There was a problem hiding this comment.
🤖 From Claude: Single-select to match sectors. The server (apply_primary_age_groups!) still accepts a set, but registration's primary age group is a single-select dropdown so the data never has >1 primary — the UI now enforces the same. Persisted chips' stars carry the primaryToggle target too, so clearing-others sees them.
| # name (sectors have no position column, so name is the position-equivalent). | ||
| # Unlike sectorable_items_primary_first, the primary is NOT floated to the top, | ||
| # so starring a sector on the form doesn't reshuffle the chips. | ||
| def sectorable_items_ordered |
There was a problem hiding this comment.
🤖 From Claude: Edit-form ordering is intentionally split from display ordering: the form uses this (alpha, primary not floated) so starring doesn't reshuffle chips, while profile/recipients/dashboard keep sectorable_items_primary_first. Sectors have no position column, so name is the position-equivalent — same idea as the age-range chips, which do have positions.
| # categorizable_items (AgeRange categories only) so the form's add/remove and | ||
| # primary toggle round-trip as nested attributes — the is_primary flag splits | ||
| # primary vs additional, no separate primary_age_category_ids param needed. | ||
| has_many :age_range_categorizable_items, |
There was a problem hiding this comment.
🤖 From Claude: Scoped association so age ranges edit via cocoon nested attributes (is_primary on the join row is the primary flag — no separate param). Heads-up: AgeGroupTaggable had a private helper of the same name; I renamed it to age_range_items_relation to avoid the concern shadowing this reader (it was being included below the generated association methods, so the private one won).
| # editor. Reads the same association the form's nested attributes build into, so | ||
| # unsaved picks survive a failed save (and aren't primary-first — starring | ||
| # shouldn't reshuffle them). Display surfaces lead with the primary instead. | ||
| def age_range_items_ordered |
There was a problem hiding this comment.
🤖 From Claude: The retention fix: read the same age_range_categorizable_items association the form's nested attributes build into. The old version read the general categorizable_items, whose in-memory target doesn't include the freshly-built (unsaved) items after a failed save — so a picked range vanished on re-render. Sectors never had this because sectorable_items is a single association.
|
|
||
| # Count the in-memory set (not a DB query): nested attributes build the items in | ||
| # one transaction, so a row-level check would see none persisted yet. | ||
| def at_most_one_primary_age_range |
There was a problem hiding this comment.
🤖 From Claude: Person-only on purpose (not in the shared concern like sectors): organizations aggregate several affiliated members' primary age groups via tag_age_groups, so an org legitimately has >1 primary. Sectors avoid this because registration tags orgs with primary_ids: [] (orgs never get a primary sector).
| # Keep one tagging per age-range category. Prefer the persisted row, fold any | ||
| # duplicate's primary flag onto the keeper, and drop the extras (destroy if | ||
| # persisted, otherwise remove from the unsaved set). | ||
| def dedupe_age_range_items |
There was a problem hiding this comment.
🤖 From Claude: Fixes RecordNotUnique on the categorizable_items unique index. Two new rows for the same age range both pass CategorizableItem's uniqueness validation (neither is persisted yet at validation time) and then collide on INSERT. This collapses them before validation — keeper prefers the persisted row and inherits any duplicate's primary flag. Note: sectors have the same theoretical in-memory-dup gap (separate SectorableItem uniqueness); not seen in practice, left as-is unless it surfaces.
There was a problem hiding this comment.
Pull request overview
Adds a sectors-style, cocoon-backed AgeRange editor to the Person form, unifies the “single primary” star behavior across sectors + age ranges via a shared Stimulus controller, and fixes category-assignment scoping so saving the person form doesn’t wipe taggings for category types the form doesn’t manage.
Changes:
- Introduces
Person#age_range_categorizable_itemsnested attributes + new_age_range_item_fieldspartial to edit age ranges via cocoon chips (with server-side single-primary + dedupe safeguards). - Replaces the sector-only
primary_sectorStimulus controller with a sharedprimary_tagcontroller (configurable highlight classes; no chip reordering). - Updates tag assignment (
TagAssignable) to only replace categories for explicitly-managed category types, preserving all other category connections.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/requests/people_age_ranges_spec.rb | Adds request coverage for the new age-range editor behavior (save/remove/dedupe/validation/rerender preservation). |
| spec/requests/events/professional_field_identifiers_spec.rb | Updates expectations to match the new age-range chip UI instead of checkbox UI. |
| spec/models/concerns/sectors_taggable_spec.rb | Adds specs for the new “ordered vs primary-first” sector display/edit ordering helpers. |
| app/views/shared/_sectorable_item_fields.html.erb | Switches chip targets/actions to the shared primary-tag controller. |
| app/views/shared/_age_range_item_fields.html.erb | New cocoon chip field partial for age-range taggings, mirroring sectors UX. |
| app/views/people/show.html.erb | Adjusts profile display layout to show sectors + (compact) age ranges side-by-side. |
| app/views/people/_form.html.erb | Adds age-range editor, switches sectors to stable ordering, and wires managed_category_type_ids for scoped category saves. |
| app/models/person.rb | Adds nested association + ordering + single-primary validation + dedupe for age-range categorizable items. |
| app/models/concerns/sectors_taggable.rb | Adds sectorable_items_ordered for edit-form stable ordering (no primary-first reshuffle). |
| app/models/concerns/age_group_taggable.rb | Renames internal query helper to avoid colliding with Person’s new association. |
| app/frontend/javascript/controllers/primary_tag_controller.js | New shared Stimulus controller implementing single-primary star + configurable highlighting. |
| app/frontend/javascript/controllers/primary_sector_controller.js | Removed in favor of primary_tag_controller. |
| app/frontend/javascript/controllers/index.js | Registers primary-tag controller instead of primary-sector. |
| app/controllers/people_controller.rb | Populates age-range collections + managed category type IDs; permits age-range nested attrs. |
| app/controllers/concerns/tag_assignable.rb | Preserves non-managed category types when managed_category_type_ids is provided. |
| AGENTS.md | Documents the new primary_tag Stimulus controller. |
| <%= hidden_field_tag "person[category_ids][]", "" %> | ||
| <% (@managed_category_type_ids || []).each do |type_id| %> | ||
| <%= hidden_field_tag "person[managed_category_type_ids][]", type_id %> | ||
| <% end %> |
| def dedupe_age_range_items | ||
| live = age_range_categorizable_items.reject(&:marked_for_destruction?) | ||
| live.group_by(&:category_id).each_value do |items| | ||
| next if items.size <= 1 | ||
|
|
||
| keeper = items.find(&:persisted?) || items.first | ||
| keeper.is_primary = true if items.any?(&:is_primary?) | ||
| (items - [ keeper ]).each do |dup| | ||
| dup.persisted? ? dup.mark_for_destruction : age_range_categorizable_items.delete(dup) | ||
| end | ||
| end | ||
| end |
Age ranges previously sat in the generic profile-specific categories block, separated from sectors. Pull the AgeRange type out so it edits as a column next to sectors, and on the profile render the two as side-by- side columns with age ranges as the thinner one — reclaiming the empty space age groups left when it wrapped to its own full-width row. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AgeRange isn't a profile_specific category type, so the person edit form never actually showed an age-range editor — age groups could only be set via registration. Add one that mirrors the sector chip UI: add/remove chips with a primary star, but multi-select primaries (a person serves several primary age groups) and no leader flag. On the profile, sectors and age ranges render as two columns; on the form they split 50/50 and wrap as the viewport narrows. Saving the form previously did `categories = submitted ids`, which would wipe every category type the form doesn't show. Scope replacement to the types the form actually edits (via managed_category_type_ids) so a person's non-AgeRange taggings — and their is_primary/legacy_id — survive untouched. Drop category_ids from strong params so assign_attributes no longer pre-wipes categories before assign_associations runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make the two boxes equal height by stretching each column and letting the card grow (flex-col + flex-1 + content-start). Replace the age-range add <select> with an "Add age range" button styled and behaving like cocoon's "Add Sector": it inserts a chip containing a picker, the controller keys the chip's primary star to the chosen id and stops a range being added twice, and disables the button once every range is in use. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match primary_sector: lighting one star clears the others and prepends that chip, so a person has at most one primary age range. Registration's "primary age group" is itself a single-select dropdown, so the data never has more than one primary — the edit UI now reflects that. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the move-to-front on starring a primary age range and render the selected ranges merged in category position order instead of primary-first, so chips don't jump around as you star/unstar. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mirror the age-range change for sectors: the edit form now renders sector chips alphabetically (sectors have no position column) via a new sectorable_items_ordered, and the primary-sector controller no longer moves the starred chip to the front — so starring doesn't reshuffle them. Profile, recipients, and dashboard views keep leading with the primary via sectorable_items_primary_first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Age ranges now edit through cocoon nested fields like sectors, via a scoped age_range_categorizable_items association with accepts_nested_ attributes_for, instead of the bespoke category_ids/template Stimulus picker. Both pickers share one primary_tag controller (renamed from primary_sector) whose highlight colors come from the Classes API, so the single-primary star behaves identically. Drop the age-range-specific JS controller and partials. Because age ranges no longer ride category_ids, they're removed from managed_category_type_ids — category_ids now carries only workshop settings, and nested attributes manage the age taggings directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two fixes for the cocoon age picker: - age_range_items_ordered now reads the age_range_categorizable_items association the nested attributes build into (not the general categorizable_items), so a range picked before a failed save survives the re-render — its chip and primary star come back. Mirrors how sectors read sectorable_items. - Add a Person at_most_one_primary_age_range validation mirroring SectorsTaggable, so posting two primary age ranges is a form error. Person-only: organizations aggregate several members' primary age groups, so they legitimately have more than one. Update professional_field_identifiers_spec to assert the cocoon chips instead of the old age checkboxes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The picker can submit the same age range twice as two new rows. The CategorizableItem uniqueness validation can't catch it — both are unsaved, so each sees no DB conflict and both INSERT, hitting the unique index (RecordNotUnique). Collapse duplicates in a before_validation on Person: keep one row per category (preferring the persisted one), fold any duplicate's primary flag onto it, and drop the extras. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
463db33 to
76f0dc6
Compare
With no profile-specific category types, the per-type hidden fields render nothing, so the browser omits the managed_category_type_ids key entirely and assign_associations replaces categories with the (empty) category_ids — wiping the person's age ranges and other-type taggings. Request specs missed it because passing an empty array still sends the key; the browser does not. Emit an always-present blank field like category_ids so the preserve branch always runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🤖 PR, suggested 👤 review level: 🔬 Inspect — new editor UI, a category-assignment scoping fix, and a shared cocoon mechanism
Why
The person edit form never had an age-range editor — AgeRange isn't a
profile_specificcategory type, so it was absent from the categories section. Age groups could only be set via registration. This adds an editor, and unifies it with the sector editor.What
Person#age_range_categorizable_itemsassociation +accepts_nested_attributes_for, rendered byshared/_age_range_item_fieldswithlink_to_add_association/link_to_remove_association. Theis_primaryjoin-row flag is the primary marker — no separate param.primary_tagcontroller (renamed fromprimary_sector) drives the single-select primary star for both pickers; highlight colors come from the Stimulus Classes API. Lighting one star clears the others; no reorder — chips keep alphabetical (sectors) / position (age) order. The bespokeage_range_pickercontroller + chip partials are gone.Person#at_most_one_primary_age_rangemirrorsSectorsTaggable— posting two primaries is a form error. Person-only, because orgs aggregate several members' primary age groups.flex-wrap+min-w-[20rem]), stacking as the viewport narrows; on the profile they're two columns with age ranges thinner.TagAssignable): saving used tocategories = submitted_ids, wiping every type the form doesn't show. The form sendsmanaged_category_type_ids(workshop settings only — age ranges go through nested attributes); only those are replaced, so other taggings survive.category_idsdropped from strong params. The form now emits an always-present blankmanaged_category_type_idsfield (likecategory_ids): without it, a person with no profile-specific types renders zero managed fields, the browser drops the key, and saving falls back to the replace branch — wiping the person's age ranges and other-type taggings.Tests
spec/requests/people_age_ranges_spec.rb— cocoon render, tag/primary via nested attributes,_destroy, position order, single-primary validation (request + model), retention after a validation error, non-AgeRange preservation, and the always-present blank managed field (guards the key-drop data loss above).spec/models/concerns/sectors_taggable_spec.rb—sectorable_items_orderedvssectorable_items_primary_first.spec/requests/events/professional_field_identifiers_spec.rb— updated to assert the cocoon chips instead of the old age checkboxes.