Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ end
- `paginated_fields` β€” Client-side pagination of nested fields
- `password_toggle` β€” Show/hide password fields
- `prefetch_lazy` β€” Prefetch lazy-loaded content
- `primary_tag` β€” Shared single-primary star for the sector and age-range cocoon chip editors (clears other stars, highlights via configurable classes, no reorder)
- `print_options` β€” Print options toggle for analytics
- `reminder_preview` β€” Live-preview a custom message in the reminder email as the admin types it on the bulk-reminder page
- `remote_select` β€” AJAX-powered select dropdown
Expand Down
14 changes: 13 additions & 1 deletion app/controllers/concerns/tag_assignable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,19 @@ def assign_associations(record, param_key: nil)
key = param_key || record.model_name.param_key

selected_category_ids = Array(params[key][:category_ids]).reject(&:blank?).map(&:to_i)
record.categories = Category.where(id: selected_category_ids)
selected = Category.where(id: selected_category_ids).to_a

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

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: 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.

# for preserved categories stay intact (is_primary/legacy_id untouched).
managed_type_ids = Array(params[key][:managed_category_type_ids]).reject(&:blank?).map(&:to_i)
preserved = record.categories.reject { |category| managed_type_ids.include?(category.category_type_id) }
record.categories = (preserved + selected).uniq
else
record.categories = selected
end

if params[key].key?(:sector_ids)
selected_sector_ids = Array(params[key][:sector_ids]).reject(&:blank?).map(&:to_i)
Expand Down
17 changes: 16 additions & 1 deletion app/controllers/people_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,21 @@ def set_form_variables
.group_by(&:category_type)
.select { |type, _| type&.profile_specific? }
.sort_by { |type, _| type&.name.to_s.downcase }

# Age ranges edit as their own sectors-style cocoon chip picker (AgeRange
# isn't a profile_specific type, so it never appears in
# @person_categories_grouped). Tagged via age_range_categorizable_items nested
# attributes, not category_ids.
@age_range_type = CategoryType.find_by(name: AgeGroupTaggable::AGE_RANGE_CATEGORY_TYPE)
age_ranges = @age_range_type ? @age_range_type.categories.published.order(:position, :name) : Category.none
@age_ranges_collection = age_ranges.pluck(:name, :id)
@current_age_range_category_ids = @person.age_range_categorizable_items.map(&:category_id)

# The category types this form edits via category_ids β€” the profile-specific
# types shown below (workshop settings). assign_associations preserves taggings
# of any other type (age ranges included, handled by nested attributes), so
# saving the form can't drop a person's other category connections.
@managed_category_type_ids = @person_categories_grouped.map { |type, _| type.id }
end

def find_duplicate_people(first_name, last_name, email, legal_first_name: nil, email_2: nil)
Expand Down Expand Up @@ -465,8 +480,8 @@ def person_params
:youtube_url,
:twitter_url,
:created_by_id, :updated_by_id,
category_ids: [],
sectorable_items_attributes: [ :id, :sector_id, :is_leader, :is_primary, :_destroy ],
age_range_categorizable_items_attributes: [ :id, :category_id, :is_primary, :_destroy ],
addresses_attributes: [
:id,
:address_type,
Expand Down
4 changes: 2 additions & 2 deletions app/frontend/javascript/controllers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ application.register("searchable-select", SearchableSelectController)
import SectionFilterController from "./section_filter_controller"
application.register("section-filter", SectionFilterController)

import PrimarySectorController from "./primary_sector_controller"
application.register("primary-sector", PrimarySectorController)
import PrimaryTagController from "./primary_tag_controller"
application.register("primary-tag", PrimaryTagController)

import FormSectionToggleController from "./form_section_toggle_controller"
application.register("form-section-toggle", FormSectionToggleController)
Expand Down
43 changes: 0 additions & 43 deletions app/frontend/javascript/controllers/primary_sector_controller.js

This file was deleted.

37 changes: 37 additions & 0 deletions app/frontend/javascript/controllers/primary_tag_controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { Controller } from "@hotwired/stimulus"

// Drives a chip editor with a single-select "primary" star, shared by the sector
// and age-range pickers on the person/organization form. Lighting one star clears
// the others; the configurable primary/default classes highlight the starred chip.
// Chips are NOT reordered β€” they keep their rendered (alphabetical / position)
// order, so starring doesn't reshuffle them. Profile/recipients/dashboard views
// still lead with the primary on display. The sector chip's leader (crown) flag is
// independent and CSS-only, so it needs no JS here.
export default class extends Controller {
static targets = ["chip", "primary"]
static classes = ["primary", "default"]

connect() {
this.style()
}

selectPrimary(event) {
if (event.target.checked) {
this.primaryTargets.forEach((checkbox) => {
if (checkbox !== event.target) checkbox.checked = false
})
}
this.style()
}

// Reflect each chip's primary state: highlight the starred chip, reset the rest.
style() {
this.primaryTargets.forEach((checkbox) => {
const chip = checkbox.closest("[data-primary-tag-target='chip']")
if (!chip) return
const primary = checkbox.checked
this.primaryClasses.forEach((klass) => chip.classList.toggle(klass, primary))
this.defaultClasses.forEach((klass) => chip.classList.toggle(klass, !primary))
})
}
}
6 changes: 4 additions & 2 deletions app/models/concerns/age_group_taggable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def primary_age_category_ids
# only updates existing items rather than creating them.
def apply_primary_age_groups!(primary_category_ids)
primary = sanitize_age_ids(primary_category_ids).to_set
age_range_categorizable_items.includes(:category).find_each do |item|
age_range_items_relation.includes(:category).find_each do |item|
desired = primary.include?(item.category_id)
item.update!(is_primary: desired) if item.is_primary? != desired
end
Expand Down Expand Up @@ -64,7 +64,9 @@ def age_range_item?(item)
item.category&.category_type&.name == AGE_RANGE_CATEGORY_TYPE
end

def age_range_categorizable_items
# Query relation of this record's AgeRange categorizable_items. Named to avoid
# colliding with Person's age_range_categorizable_items nested association.
def age_range_items_relation
categorizable_items
.joins(category: :category_type)
.where(category_types: { name: AGE_RANGE_CATEGORY_TYPE })
Expand Down
11 changes: 10 additions & 1 deletion app/models/concerns/sectors_taggable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,20 @@ module SectorsTaggable
# Sectorable items ordered for display: the primary sector first, then the
# rest alphabetically by sector name. Sorts the in-memory association rather
# than issuing a query, so it stays correct when a form re-renders its
# unsaved items after a failed save.
# unsaved items after a failed save. Used by profile, recipients, and dashboard
# views, where the primary should always lead.
def sectorable_items_primary_first
sectorable_items.sort_by { |item| [ item.is_primary? ? 0 : 1, item.sector&.name.to_s.downcase ] }
end

# Sectorable items in stable order for the edit form: alphabetically by sector
# 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

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: 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.

sectorable_items.sort_by { |item| item.sector&.name.to_s.downcase }
end

# Additively tag sectors as primary/additional without disturbing other
# taggings β€” used by registration, where a respondent names a single primary
# sector (the dropdown) plus any number of additional sectors (the checkboxes).
Expand Down
51 changes: 51 additions & 0 deletions app/models/person.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ class Person < ApplicationRecord
CONTACT_TYPES = [ "work", "personal" ].freeze
validates :email_type, inclusion: { in: %w[work personal] }, allow_blank: true
validates :email_2_type, inclusion: { in: %w[work personal] }, allow_blank: true
# Mirrors SectorsTaggable's single-primary rule for age ranges β€” the chip
# editor's single-star JS is the first line of defense, this guards imports,
# the console, and bad form posts. Person-only: organizations aggregate
# several members' primary age groups, so they legitimately have more than one.
validate :at_most_one_primary_age_range
# TODO: add validation for zip code containing only numbers
# TODO: add validation on STATE
# TODO: add validation on phone number type
Expand All @@ -67,6 +72,19 @@ class Person < ApplicationRecord
accepts_nested_attributes_for :contact_methods, allow_destroy: true, reject_if: :all_blank
accepts_nested_attributes_for :sectorable_items, allow_destroy: true,
reject_if: proc { |attrs| attrs["sector_id"].blank? }
# Age ranges edit through cocoon nested fields like sectors. A scoped view of
# 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,

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: 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).

-> { joins(category: :category_type).where(category_types: { name: AgeGroupTaggable::AGE_RANGE_CATEGORY_TYPE }) },
class_name: "CategorizableItem", as: :categorizable, inverse_of: :categorizable
accepts_nested_attributes_for :age_range_categorizable_items, allow_destroy: true,
reject_if: proc { |attrs| attrs["category_id"].blank? }
# The picker can submit the same age range twice (two new rows), which the
# CategorizableItem uniqueness validation can't catch β€” both are unsaved, so
# both INSERT and hit the DB unique index. Collapse duplicates before validation.
before_validation :dedupe_age_range_items
accepts_nested_attributes_for :user, update_only: true
accepts_nested_attributes_for :affiliations, allow_destroy: true,
reject_if: proc { |attrs| attrs["organization_id"].blank? }
Expand Down Expand Up @@ -268,8 +286,41 @@ def other_workshop_setting_responses
other_form_responses(OTHER_WORKSHOP_SETTING_IDENTIFIERS)
end

# The age-range nested items in category position order for the cocoon chip
# 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

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: 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.

age_range_categorizable_items.sort_by { |item| [ item.category&.position || 0, item.category&.name.to_s ] }
end

private

# 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

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: 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).

primary_count = age_range_categorizable_items.reject(&:marked_for_destruction?).count(&:is_primary?)
return if primary_count <= 1

errors.add(:base, "Only one age range can be marked as primary")
end

# 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

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: 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.

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
Comment on lines +311 to +322

def other_form_responses(identifiers)
form_submissions
.joins(form_answers: :form_field)
Expand Down
Loading