From 4e878cf22dd0d7c16e08d016062a554d2f638808 Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 1 Aug 2026 07:45:25 -0400 Subject: [PATCH] Replace org Windows-audience dropdown with age-range chip picker The Windows-audience select edited windows_type_id, which no other org flow sets and which duplicated concepts already captured by age ranges. Swap it for the same cocoon age-range chip picker the person form uses, sharing the chip mechanics through AgeGroupTaggable so both models stay in step. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/controllers/organizations_controller.rb | 17 +++- app/models/concerns/age_group_taggable.rb | 40 ++++++++ app/models/person.rb | 43 +-------- app/views/organizations/_form.html.erb | 69 +++++++++----- .../requests/organizations_age_ranges_spec.rb | 95 +++++++++++++++++++ .../views/organizations/edit.html.erb_spec.rb | 13 ++- 6 files changed, 212 insertions(+), 65 deletions(-) create mode 100644 spec/requests/organizations_age_ranges_spec.rb diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index a5c7c1e17d..429b73a3d9 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -185,6 +185,20 @@ 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 @org_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 = @organization.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 an organization's other category connections. + @managed_category_type_ids = @org_categories_grouped.map { |type, _| type.id } end def set_index_variables @@ -221,7 +235,7 @@ def organization_params params.require(:organization).permit( :name, :description, :start_date, :end_date, :mission_vision_values, :agency_type, :agency_type_other, :filemaker_code, :logo, :notes, :email, :website_url, - :organization_status_id, :location_id, :windows_type_id, + :organization_status_id, :location_id, :profile_show_sectors, :profile_show_email, :profile_show_phone, :profile_show_website, :profile_show_description, :profile_show_workshops, :profile_show_stories, :profile_show_events_registered, :profile_show_workshop_logs, @@ -231,6 +245,7 @@ def organization_params :sector_id, :_destroy ], + age_range_categorizable_items_attributes: [ :id, :category_id, :is_primary, :_destroy ], affiliations_attributes: [ :id, :person_id, diff --git a/app/models/concerns/age_group_taggable.rb b/app/models/concerns/age_group_taggable.rb index 95c7083b3d..a87561e4ec 100644 --- a/app/models/concerns/age_group_taggable.rb +++ b/app/models/concerns/age_group_taggable.rb @@ -8,6 +8,30 @@ module AgeGroupTaggable AGE_RANGE_CATEGORY_TYPE = "AgeRange" + included do + # 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. The Person and Organization chip editors are identical. + has_many :age_range_categorizable_items, + -> { joins(category: :category_type).where(category_types: { name: 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 + 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 + age_range_categorizable_items.sort_by { |item| [ item.category&.position || 0, item.category&.name.to_s ] } + end + # AgeRange categories tagged on this record, split by the primary flag and # returned in display order. Filters the categorizable_items association in # Ruby (like the sectors index does with sectorable_items) so the result rides @@ -53,6 +77,22 @@ def tag_age_groups(primary_ids:, additional_ids:) private + # 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 + 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 + def age_range_categories(primary:) categorizable_items .select { |item| item.is_primary? == primary && age_range_item?(item) } diff --git a/app/models/person.rb b/app/models/person.rb index 0d2032be50..571bf863c1 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -83,19 +83,8 @@ 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, - -> { 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 + # The age-range chip editor (age_range_categorizable_items nested attributes, + # dedupe, ordering) is shared with Organization via AgeGroupTaggable. accepts_nested_attributes_for :user, update_only: true accepts_nested_attributes_for :affiliations, allow_destroy: true, reject_if: proc { |attrs| attrs["organization_id"].blank? } @@ -319,18 +308,12 @@ 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 - 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. + # one transaction, so a row-level check would see none persisted yet. Person-only: + # organizations aggregate several members' primary age groups (see AgeGroupTaggable), + # so their own tags don't carry this single-primary rule. def at_most_one_primary_age_range primary_count = age_range_categorizable_items.reject(&:marked_for_destruction?).count(&:is_primary?) return if primary_count <= 1 @@ -338,22 +321,6 @@ def at_most_one_primary_age_range 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 - 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 - def other_form_responses(identifiers) form_submissions .joins(form_answers: :form_field) diff --git a/app/views/organizations/_form.html.erb b/app/views/organizations/_form.html.erb index 490e115f1c..22b15f795a 100644 --- a/app/views/organizations/_form.html.erb +++ b/app/views/organizations/_form.html.erb @@ -23,8 +23,8 @@ - -
+ +
Sectors @@ -49,15 +49,33 @@
-
- <%= f.association :windows_type, - label: "Windows audience", - include_blank: true, - required: false, - input_html: { - class: "rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring focus:ring-blue-200" - } %> -
+ + <% if @age_ranges_collection.present? %> +
+
+ Age ranges +
+
+ <% age_owner = f.object.respond_to?(:object) ? f.object.object : f.object %> + <%= f.simple_fields_for :age_range_categorizable_items, age_owner.age_range_items_ordered do |afi| %> + <%= render "shared/age_range_item_fields", f: afi %> + <% end %> + + <%= link_to_add_association "➕ Add age range", + f, + :age_range_categorizable_items, + partial: "shared/age_range_item_fields", + render_options: { + locals: { collection: (@age_ranges_collection || []) + .reject { |_, id| (@current_age_range_category_ids || []).include?(id) } } }, + class: "btn btn-secondary-outline" %> +
+
+ <% end %>
@@ -68,26 +86,29 @@ - - <% if @org_categories_grouped.present? %> - <% primary_age_ids = @organization.primary_age_category_ids %> + <%# Tells the controller which category types this form edits via category_ids + (workshop settings); saving replaces only those and preserves any other type + the form never shows (age ranges, edited as nested attributes above). Always + submit the blank keys so unchecking every box still posts them. %> + <%= hidden_field_tag "organization[category_ids][]", "" %> + <%= hidden_field_tag "organization[managed_category_type_ids][]", "" %> + <% (@managed_category_type_ids || []).each do |type_id| %> + <%= hidden_field_tag "organization[managed_category_type_ids][]", type_id %> + <% end %> + + + <% other_category_types = (@org_categories_grouped || {}).reject { |type, _| type.name == "AgeRange" } %> + <% if other_category_types.present? %>
- <%# Ensures the primary-age param is always submitted so unchecking every - toggle clears the primary flags. %> - <%= hidden_field_tag "organization[primary_age_category_ids][]", "" %> - <% @org_categories_grouped.each do |type, cats| %> - <% is_age = type.name == "AgeRange" %> + <% other_category_types.each do |type, cats| %>
<%= type.display_label %>
- <% if is_age %> -

Check every age group served, then mark the primary ones.

- <% end %>
<% cats.each do |category| %> <%= render "shared/category_checkbox", param_key: "organization", category: category, checked: @organization.category_ids.include?(category.id), - is_age: is_age, - primary_checked: is_age && primary_age_ids.include?(category.id) %> + is_age: false, + primary_checked: false %> <% end %>
diff --git a/spec/requests/organizations_age_ranges_spec.rb b/spec/requests/organizations_age_ranges_spec.rb new file mode 100644 index 0000000000..20b9827f3a --- /dev/null +++ b/spec/requests/organizations_age_ranges_spec.rb @@ -0,0 +1,95 @@ +require "rails_helper" + +RSpec.describe "Organization age ranges", type: :request do + let(:admin) { create(:user, :admin) } + let!(:organization_status) { create(:organization_status, name: "Active") } + let(:organization) { create(:organization, organization_status: organization_status) } + + # AgeGroupTaggable matches the type by the exact name "AgeRange". + let(:age_type) { create(:category_type, :published, name: "AgeRange") } + # Created in order so the positioned gem assigns positions children < teens < adults. + let!(:children) { create(:category, :published, category_type: age_type, name: "Children (0-12)") } + let!(:teens) { create(:category, :published, category_type: age_type, name: "Teens (13-17)") } + let!(:adults) { create(:category, :published, category_type: age_type, name: "Adults (18+)") } + + # A profile-specific category the form edits via category_ids (workshop settings). + let(:workshop_type) { create(:category_type, :published, name: "WorkshopEnvironment", profile_specific: true) } + let!(:in_person) { create(:category, :published, category_type: workshop_type, name: "In person") } + + before { sign_in admin } + + # Age ranges save as age_range_categorizable_items nested attributes (the cocoon + # chip picker), not category_ids — mirroring the person form. + def update_org(age_items:, category_ids: [ "" ]) + patch organization_path(organization), params: { + organization: { + name: organization.name, + organization_status_id: organization_status.id, + category_ids: category_ids, + managed_category_type_ids: [ workshop_type.id ], + age_range_categorizable_items_attributes: age_items + } + } + end + + describe "edit form" do + it "renders the cocoon age-range chip picker, not the windows dropdown" do + get edit_organization_path(organization) + + expect(response.body).to include("primary-tag") + expect(response.body).to include("Add age range") + expect(response.body).to include("Children (0-12)") + expect(response.body).not_to include("organization[windows_type_id]") + end + end + + describe "saving age ranges" do + it "tags the selected age ranges and marks the chosen one primary" do + update_org(age_items: [ + { category_id: children.id, is_primary: "1" }, + { category_id: adults.id, is_primary: "0" } + ]) + + organization.reload + expect(organization.primary_age_groups).to contain_exactly(children) + expect(organization.additional_age_groups).to contain_exactly(adults) + end + + it "removes an age range via _destroy" do + organization.categories << children + item = organization.categorizable_items.find_by(category: children) + + update_org(age_items: [ { id: item.id, category_id: children.id, _destroy: "1" } ]) + + organization.reload + expect(organization.primary_age_groups).to be_empty + expect(organization.additional_age_groups).to be_empty + end + + it "dedupes duplicate selections instead of raising RecordNotUnique" do + expect { + update_org(age_items: [ + { category_id: children.id, is_primary: "0" }, + { category_id: children.id, is_primary: "1" } + ]) + }.not_to raise_error + + organization.reload + expect(organization.categorizable_items.where(category: children).count).to eq(1) + expect(organization.primary_age_groups).to contain_exactly(children) + end + end + + describe "preserving non-AgeRange category connections" do + it "keeps the organization's workshop-setting taggings when saving age ranges" do + organization.categories << in_person + + update_org(age_items: [ { category_id: children.id, is_primary: "1" } ], + category_ids: [ "", in_person.id.to_s ]) + + organization.reload + expect(organization.categories).to include(in_person) + expect(organization.primary_age_groups).to contain_exactly(children) + end + end +end diff --git a/spec/views/organizations/edit.html.erb_spec.rb b/spec/views/organizations/edit.html.erb_spec.rb index c706dd03a3..a7861bbff0 100644 --- a/spec/views/organizations/edit.html.erb_spec.rb +++ b/spec/views/organizations/edit.html.erb_spec.rb @@ -27,12 +27,21 @@ it "renders the edit organization form" do render assert_select "form[action=?][method=?]", organization_path(organization), "post" do - assert_select "select[name=?]", "organization[windows_type_id]" - assert_select "textarea[name=?]", "organization[name]" assert_select "textarea[name=?]", "organization[description]" end + # The Windows audience dropdown was replaced by the age-range chip picker. + assert_select "select[name=?]", "organization[windows_type_id]", false + end + + it "renders the cocoon age-range chip picker instead of the windows dropdown" do + assign(:age_ranges_collection, [ [ "Children (0-12)", 1 ], [ "Adults (18+)", 2 ] ]) + assign(:current_age_range_category_ids, []) + render + expect(rendered).to include("primary-tag") + expect(rendered).to include("Add age range") + expect(rendered).to include("Children (0-12)") end describe "status select visibility" do