WAIT: Pluralize the additional age groups field identifier#1838
Open
maebeale wants to merge 4 commits into
Open
WAIT: Pluralize the additional age groups field identifier#1838maebeale wants to merge 4 commits into
maebeale wants to merge 4 commits into
Conversation
maebeale
commented
Jun 22, 2026
| ADDITIONAL_AGE_GROUP_FIELD_IDENTIFIERS = %w[additional_age_groups additional_age_group].freeze | ||
| AGE_GROUP_FIELD_IDENTIFIERS = [ PRIMARY_AGE_GROUP_FIELD_IDENTIFIER, *ADDITIONAL_AGE_GROUP_FIELD_IDENTIFIERS ].freeze | ||
|
|
||
| # The payment-method field. Its answer options ("Credit card (now)", etc.) are |
Collaborator
Author
There was a problem hiding this comment.
🤖 From Claude: Current name first, legacy singular last — same ordering convention as ADDITIONAL_SECTOR_FIELD_IDENTIFIERS. Both keys are also added to DYNAMIC_FIELD_CATEGORY_TYPES so every .key?/[id] lookup resolves either name.
maebeale
commented
Jun 22, 2026
| sector_ids = FormField::SECTOR_FIELD_IDENTIFIERS.flat_map { |id| collect_ids_from_checkboxes(id) } | ||
| primary_age_ids = collect_ids_from_checkboxes("primary_age_group") | ||
| additional_age_ids = collect_ids_from_checkboxes("additional_age_group") | ||
| additional_age_ids = FormField::ADDITIONAL_AGE_GROUP_FIELD_IDENTIFIERS.flat_map { |id| collect_ids_from_checkboxes(id) } |
Collaborator
Author
There was a problem hiding this comment.
🤖 From Claude: Collects across both identifiers (flat_map) exactly like the sector line above, so registrations submitted against legacy singular fields still tag correctly.
The "additional age groups" form field is multi-select, so its identifier should be plural to match what it stores and the existing instance method. The legacy singular name is kept as an alias so existing form data and old form fields keep resolving, mirroring the sector identifier pattern. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
maebeale
force-pushed
the
maebeale/pluralize-additional-age-groups
branch
from
6fa850e to
fdeee6e
Compare
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The constant mirrors the labels each section builds and a spec asserts the two agree, so the field-label rename has to update it too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
maebeale
changed the title
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.
🤖 PR, suggested 👤 review level: 📖 Read — light-logic: a field-identifier rename with a legacy alias, no schema or data changes
What is the goal of this PR and why is this important?
The "additional age groups" form field is multi-select, so its identifier should be plural (
additional_age_groups) to match what it stores and the existingadditional_age_groupsinstance method — the singularadditional_age_groupread as if it held one value.How did you approach the change?
Made
additional_age_groupsthe canonical identifier and kept the singular as a legacy alias, mirroring the existing*_SECTOR_FIELD_IDENTIFIERSpattern: the constant is now a current-first/legacy-last array,DYNAMIC_FIELD_CATEGORY_TYPESmaps both names toAgeRange, and consumers iterate the array so old form data and existing form fields keep resolving. No DB columns or migration are involved — additional age groups live incategorizable_items(is_primary: false) — so this only affects newly built forms.Anything else to add?
Existing in-database form fields keep their singular identifier and still work via the alias; they are not auto-renamed. A backfill migration could rename them if we want the stored data to match.