From 54c1109be457e8452e3273b3e7118116c50c7a65 Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 4 Jul 2026 21:48:05 -0400 Subject: [PATCH 01/14] Add OtherResponse model to capture free-text "Other" sector answers Free-text "Other" sector values were derived on the fly from form answers and shown as read-only chips with nowhere to hang a curation decision. Materialize them as records so a curator can promote a recurring value into a real Sector, keep it as a free-text chip, or dismiss it (hide from profile/edit). Data layer only: migration, model, factory, specs. `kind` leaves room for the identical workshop-setting responses to migrate here later. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/models/other_response.rb | 50 +++++++++++++++ .../20260705014639_create_other_responses.rb | 28 +++++++++ db/schema.rb | 19 ++++++ spec/factories/other_responses.rb | 21 +++++++ spec/models/other_response_spec.rb | 62 +++++++++++++++++++ 5 files changed, 180 insertions(+) create mode 100644 app/models/other_response.rb create mode 100644 db/migrate/20260705014639_create_other_responses.rb create mode 100644 spec/factories/other_responses.rb create mode 100644 spec/models/other_response_spec.rb diff --git a/app/models/other_response.rb b/app/models/other_response.rb new file mode 100644 index 000000000..b4607aa2f --- /dev/null +++ b/app/models/other_response.rb @@ -0,0 +1,50 @@ +class OtherResponse < ApplicationRecord + # The free-text "Other" a person typed on a tag-backed form question. Only + # sectors are wired up today; the `kind` column leaves room for the identical + # workshop-setting responses to move here later. + KINDS = %w[sector].freeze + + # A response starts life as `pending` (awaiting a curator's decision) and is + # then either promoted into a real tag, kept as a free-text chip, or dismissed + # (hidden from the person's profile and edit form). + STATUSES = %w[pending kept promoted dismissed].freeze + + # Statuses that still surface as an "(other)" chip on the person's pages. + VISIBLE_STATUSES = %w[pending kept].freeze + + belongs_to :person + belongs_to :promotable, polymorphic: true, optional: true + belongs_to :source_form_answer, class_name: "FormAnswer", optional: true + + before_validation :set_normalized_text + + validates :text, presence: true + validates :kind, inclusion: { in: KINDS } + validates :status, inclusion: { in: STATUSES } + validates :normalized_text, uniqueness: { scope: [ :person_id, :kind ] } + + scope :sectors, -> { where(kind: "sector") } + scope :visible, -> { where(status: VISIBLE_STATUSES) } + scope :pending, -> { where(status: "pending") } + scope :promotable_now, -> { where.not(status: "dismissed") } + + # Case/whitespace-insensitive key used both for the unique index and for + # grouping the same typed value across many people on the review page. + def self.normalize(value) + value.to_s.strip.downcase + end + + def dismiss! + update!(status: "dismissed") + end + + def keep! + update!(status: "kept") + end + + private + + def set_normalized_text + self.normalized_text = self.class.normalize(text) + end +end diff --git a/db/migrate/20260705014639_create_other_responses.rb b/db/migrate/20260705014639_create_other_responses.rb new file mode 100644 index 000000000..84272654f --- /dev/null +++ b/db/migrate/20260705014639_create_other_responses.rb @@ -0,0 +1,28 @@ +class CreateOtherResponses < ActiveRecord::Migration[8.1] + # Materializes the free-text "Other" answers people type on tag-backed form + # questions (sectors today; workshop settings could follow via `kind`). These + # can't be Sector/Category records, so they were previously derived on the fly + # from form answers. Capturing them as records lets a curator promote a + # recurring value into a real tag, keep it as a free-text chip, or dismiss it. + def up + return if table_exists?(:other_responses) + + create_table :other_responses do |t| + t.references :person, null: false, foreign_key: true + t.string :kind, null: false + t.string :text, null: false + t.string :normalized_text, null: false + t.string :status, null: false, default: "pending" + t.references :promotable, polymorphic: true, null: true + t.references :source_form_answer, null: true, foreign_key: { to_table: :form_answers } + t.timestamps + end + + add_index :other_responses, [ :person_id, :kind, :normalized_text ], + unique: true, name: "index_other_responses_on_person_kind_text" + end + + def down + drop_table :other_responses, if_exists: true + end +end diff --git a/db/schema.rb b/db/schema.rb index ee633774d..11ad49dad 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -834,6 +834,23 @@ t.index ["windows_type_id"], name: "index_organizations_on_windows_type_id" end + create_table "other_responses", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| + t.datetime "created_at", null: false + t.string "kind", null: false + t.string "normalized_text", null: false + t.bigint "person_id", null: false + t.bigint "promotable_id" + t.string "promotable_type" + t.bigint "source_form_answer_id" + t.string "status", default: "pending", null: false + t.string "text", null: false + t.datetime "updated_at", null: false + t.index ["person_id", "kind", "normalized_text"], name: "index_other_responses_on_person_kind_text", unique: true + t.index ["person_id"], name: "index_other_responses_on_person_id" + t.index ["promotable_type", "promotable_id"], name: "index_other_responses_on_promotable" + t.index ["source_form_answer_id"], name: "index_other_responses_on_source_form_answer_id" + end + create_table "pay_charges", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.integer "amount", null: false t.integer "amount_refunded" @@ -1738,6 +1755,8 @@ add_foreign_key "organizations", "locations" add_foreign_key "organizations", "organization_statuses" add_foreign_key "organizations", "windows_types" + add_foreign_key "other_responses", "form_answers", column: "source_form_answer_id" + add_foreign_key "other_responses", "people" add_foreign_key "pay_charges", "pay_customers", column: "customer_id" add_foreign_key "pay_charges", "pay_subscriptions", column: "subscription_id" add_foreign_key "pay_payment_methods", "pay_customers", column: "customer_id" diff --git a/spec/factories/other_responses.rb b/spec/factories/other_responses.rb new file mode 100644 index 000000000..1693fd49d --- /dev/null +++ b/spec/factories/other_responses.rb @@ -0,0 +1,21 @@ +FactoryBot.define do + factory :other_response do + person + kind { "sector" } + sequence(:text) { |n| "Equine therapy #{n}" } + status { "pending" } + + trait :kept do + status { "kept" } + end + + trait :dismissed do + status { "dismissed" } + end + + trait :promoted do + status { "promoted" } + association :promotable, factory: :sector + end + end +end diff --git a/spec/models/other_response_spec.rb b/spec/models/other_response_spec.rb new file mode 100644 index 000000000..5f3c9182a --- /dev/null +++ b/spec/models/other_response_spec.rb @@ -0,0 +1,62 @@ +require "rails_helper" + +RSpec.describe OtherResponse, type: :model do + describe "validations" do + it "has a valid factory" do + expect(build(:other_response)).to be_valid + end + + it "requires text" do + expect(build(:other_response, text: "")).not_to be_valid + end + + it "rejects an unknown kind" do + expect(build(:other_response, kind: "nonsense")).not_to be_valid + end + + it "rejects an unknown status" do + expect(build(:other_response, status: "nonsense")).not_to be_valid + end + + it "is unique per person + kind + normalized text" do + person = create(:person) + create(:other_response, person: person, kind: "sector", text: "Equine therapy") + dup = build(:other_response, person: person, kind: "sector", text: " equine therapy ") + + expect(dup).not_to be_valid + end + + it "allows the same text for a different person" do + create(:other_response, kind: "sector", text: "Equine therapy") + expect(build(:other_response, kind: "sector", text: "Equine therapy")).to be_valid + end + end + + describe "normalization" do + it "derives normalized_text from text on save" do + response = create(:other_response, text: " Equine Therapy ") + expect(response.normalized_text).to eq("equine therapy") + end + end + + describe "scopes" do + it ".visible returns only pending and kept" do + pending = create(:other_response) + kept = create(:other_response, :kept) + create(:other_response, :dismissed) + create(:other_response, :promoted) + + expect(OtherResponse.visible).to contain_exactly(pending, kept) + end + end + + describe "#dismiss! / #keep!" do + it "transitions status" do + response = create(:other_response) + response.dismiss! + expect(response.reload.status).to eq("dismissed") + response.keep! + expect(response.reload.status).to eq("kept") + end + end +end From e04a97498980bae08ddf90874c22ededae13aa45 Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 4 Jul 2026 21:57:28 -0400 Subject: [PATCH 02/14] Capture, display, and curate "Other" sector responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Capture free-text "Other" sector answers as OtherResponse records at registration time (PublicRegistration), de-duped per person. - Person#other_sector_responses now reads the visible (pending/kept) records; profile + edit render them via a records-based partial. The edit chip gains an × that dismisses (hides) a response. Submission view is untouched (audit). - Admin review page (/other_responses) groups the same value across people with a count; promote maps them all to an existing sector or mints a new published one and tags every non-dismissed person. Reachable from the Sectors index. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 1 + app/controllers/other_responses_controller.rb | 75 +++++++++++++++++++ app/models/person.rb | 9 ++- app/policies/other_response_policy.rb | 8 ++ .../public_registration.rb | 22 ++++++ app/views/other_responses/index.html.erb | 63 ++++++++++++++++ app/views/people/_form.html.erb | 2 +- .../people/_other_sector_responses.html.erb | 20 +++++ app/views/people/show.html.erb | 2 +- app/views/sectors/index.html.erb | 3 + config/routes.rb | 5 ++ spec/models/person_spec.rb | 22 ++---- .../professional_field_identifiers_spec.rb | 2 +- spec/requests/other_responses_spec.rb | 72 ++++++++++++++++++ spec/requests/people_other_responses_spec.rb | 18 ++++- spec/views/page_bg_class_alignment_spec.rb | 1 + 16 files changed, 301 insertions(+), 24 deletions(-) create mode 100644 app/controllers/other_responses_controller.rb create mode 100644 app/policies/other_response_policy.rb create mode 100644 app/views/other_responses/index.html.erb create mode 100644 app/views/people/_other_sector_responses.html.erb create mode 100644 spec/requests/other_responses_spec.rb diff --git a/AGENTS.md b/AGENTS.md index 12d579d53..5fa8f5cec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,6 +102,7 @@ This codebase (Rails 8.1) | `Story` | Editorial content with facilitators, primary/gallery assets | | `Resource` | Handouts, toolkits, templates with downloadable assets | | `Person` | Organization affiliates with contacts, addresses, sectors | +| `OtherResponse` | A free-text "Other" a person typed on a tag-backed form question (`kind: "sector"` today), captured at registration so a curator can `promote` it into a real `Sector` tag, `keep` it as a chip, or `dismiss` it (hide from profile/edit). Reviewed at `/other_responses` | | `Organization` | Groups with affiliations, addresses, logos via ActiveStorage | | `Grant` | Donated funds (polymorphic `donor`: Organization or Person) with eligibility criteria, tasks, deadlines; parent of `Scholarship`. Scholarship totals cannot exceed the grant amount | | `Scholarship` | Award to a `Person`; optionally drawn from a `Grant`, syncs to event registration `Allocation` | diff --git a/app/controllers/other_responses_controller.rb b/app/controllers/other_responses_controller.rb new file mode 100644 index 000000000..45af4b5a9 --- /dev/null +++ b/app/controllers/other_responses_controller.rb @@ -0,0 +1,75 @@ +class OtherResponsesController < ApplicationController + before_action :set_other_response, only: :update + + # Review page: the same free-text "Other" sector value typed across many + # people, grouped with a count so a curator can decide what to promote. + def index + authorize! + responses = OtherResponse + .sectors.where(status: OtherResponse::VISIBLE_STATUSES) + .includes(:person) + + @groups = responses.group_by(&:normalized_text).map do |_normalized, rows| + { + display_text: rows.first.text, + normalized_text: rows.first.normalized_text, + count: rows.size, + status_counts: rows.each_with_object(Hash.new(0)) { |r, h| h[r.status] += 1 } + } + end.sort_by { |group| [ -group[:count], group[:display_text].downcase ] } + + @sectors = Sector.excluding_other.order(:name) + end + + # Curate a single response — the profile-edit "×" dismisses, and the review + # page can keep an individual person's response. + def update + authorize! @other_response + status = params.dig(:other_response, :status) + @other_response.update!(status: status) if OtherResponse::STATUSES.include?(status) + + if params[:return_to] == "person_edit" + redirect_to edit_person_path(@other_response.person), status: :see_other + else + redirect_to other_responses_path, status: :see_other + end + end + + # Promote every non-dismissed person who typed this value into a real Sector + # tag — mapping to an existing sector or minting a new (published) one — and + # mark those responses promoted so they stop showing as free-text chips. + def promote + authorize! to: :promote? + sector = target_sector + return redirect_to other_responses_path, alert: "Pick or name a sector to promote to." unless sector + + responses = OtherResponse.sectors.promotable_now + .where(normalized_text: OtherResponse.normalize(params[:normalized_text])) + + responses.includes(:person).find_each do |response| + response.person.tag_sectors(primary_ids: [], additional_ids: [ sector.id ]) + response.update!(status: "promoted", promotable: sector) + end + + redirect_to other_responses_path, status: :see_other, + notice: "Promoted #{responses.size} response(s) to “#{sector.name}”." + end + + private + + def set_other_response + @other_response = OtherResponse.find(params[:id]) + end + + # The promote target: an existing sector by id, or a newly minted published one + # from a typed name. Returns nil when neither was supplied. + def target_sector + if params[:sector_id].present? + Sector.find_by(id: params[:sector_id]) + elsif params[:new_sector_name].present? + Sector.find_or_create_by!(name: params[:new_sector_name].strip) do |sector| + sector.published = true + end + end + end +end diff --git a/app/models/person.rb b/app/models/person.rb index 66c166e3a..9d5fe9609 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -20,6 +20,7 @@ class Person < ApplicationRecord has_many :categorizable_items, inverse_of: :categorizable, as: :categorizable, dependent: :destroy has_many :notifications, as: :noticeable, dependent: :destroy has_many :sectorable_items, as: :sectorable, dependent: :destroy + has_many :other_responses, dependent: :destroy has_many :stories_as_spotlighted_facilitator, inverse_of: :spotlighted_facilitator, class_name: "Story", dependent: :restrict_with_error has_many :stories_as_author, inverse_of: :author, class_name: "Story", foreign_key: :author_id, @@ -305,10 +306,12 @@ def remote_search_label # profile fields shown on the edit page. OTHER_WORKSHOP_SETTING_IDENTIFIERS = %w[primary_age_group additional_age_group].freeze - # Free-text "Other" sectors the person typed on registration forms. - # They can't be Sector records, so they're surfaced beside the sector tags. + # Free-text "Other" sectors the person typed on registration forms, captured + # as OtherResponse records (see EventRegistrationServices::PublicRegistration). + # They can't be Sector records, so they're surfaced beside the sector tags — + # only while pending or explicitly kept (dismissed/promoted ones drop off). def other_sector_responses - other_form_responses(FormField::SECTOR_FIELD_IDENTIFIERS) + other_responses.sectors.visible.order(:text) end # Free-text "Other" workshop settings (category-backed fields) from forms. diff --git a/app/policies/other_response_policy.rb b/app/policies/other_response_policy.rb new file mode 100644 index 000000000..fb48adc03 --- /dev/null +++ b/app/policies/other_response_policy.rb @@ -0,0 +1,8 @@ +class OtherResponsePolicy < ApplicationPolicy + # Curating free-text "Other" responses (reviewing, promoting, keeping, + # dismissing) is an admin-only task. index?/update? fall back to manage?. + + def promote? + admin? + end +end diff --git a/app/services/event_registration_services/public_registration.rb b/app/services/event_registration_services/public_registration.rb index 1719e172d..99bec853e 100644 --- a/app/services/event_registration_services/public_registration.rb +++ b/app/services/event_registration_services/public_registration.rb @@ -442,6 +442,7 @@ def invoice_requested? def create_form_submission(person) submission = FormSubmission.create!(person: person, form: @registration_form, event: @event, role: "registration") save_form_answers(submission) + capture_other_sector_responses(submission) submission end @@ -450,9 +451,30 @@ def update_form_submission(person) record.event = @event end save_form_answers(submission) + capture_other_sector_responses(submission) submission end + # Materialize the free-text "Other" sector answers as OtherResponse records so + # they can be curated (promoted/kept/dismissed). Reuses OtherOption.texts, the + # same extraction used to display them, and de-dupes on the person's normalized + # value so a repeat registration of the same text doesn't create a second row. + def capture_other_sector_responses(submission) + submission.form_answers + .joins(:form_field) + .where(form_fields: { field_identifier: FormField::SECTOR_FIELD_IDENTIFIERS }) + .each do |answer| + OtherOption.texts(answer.submitted_answer).each do |text| + submission.person.other_responses.find_or_create_by!( + kind: "sector", normalized_text: OtherResponse.normalize(text) + ) do |response| + response.text = text + response.source_form_answer = answer + end + end + end + end + def save_form_answers(submission) @form_params.each do |field_id, raw_value| field = @registration_form.form_fields.find_by(id: field_id) diff --git a/app/views/other_responses/index.html.erb b/app/views/other_responses/index.html.erb new file mode 100644 index 000000000..181ed856f --- /dev/null +++ b/app/views/other_responses/index.html.erb @@ -0,0 +1,63 @@ +<% content_for(:page_bg_class, "admin-only bg-blue-100") %> +<%= link_to sectors_path, class: "inline-flex items-center gap-1.5 text-sm text-gray-500 hover:text-gray-700 px-2 py-1 mb-2" do %> + Sectors +<% end %> +
+
+ +
+
+

Review “Other” sectors

+

+ Free-text sectors people typed on registration forms, grouped by value. + Promote a recurring one into a real sector tag for everyone who typed it. +

+
+
+ +
+ <% if @groups.any? %> + + + + + + + + + + <% @groups.each do |group| %> + + + + + + <% end %> + +
ResponsePeoplePromote to a sector
+ <%= group[:display_text] %> + <% if group[:status_counts]["kept"].to_i.positive? %> + (<%= group[:status_counts]["kept"] %> kept) + <% end %> + <%= group[:count] %> + <%= form_with url: promote_other_responses_path, method: :post, + class: "flex flex-wrap items-center gap-2" do %> + <%= hidden_field_tag :normalized_text, group[:normalized_text] %> + <%= select_tag :sector_id, + options_from_collection_for_select(@sectors, :id, :name), + include_blank: "Existing sector…", + class: "rounded-md border-gray-300 text-sm" %> + or + <%= text_field_tag :new_sector_name, group[:display_text], + placeholder: "New sector name", + class: "rounded-md border-gray-300 text-sm" %> + <%= submit_tag "Promote", class: "btn btn-primary-outline text-sm", + data: { turbo_confirm: "Promote “#{group[:display_text]}” for all #{group[:count]} people?" } %> + <% end %> +
+ <% else %> +

No “Other” sector responses to review.

+ <% end %> +
+
+
diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index e2c8ff349..fa8934913 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -160,7 +160,7 @@ .reject { |_, id| (@current_sector_ids || []).include?(id) }, show_admin_flags: true } }, class: "btn btn-secondary-outline" %> - <%= render "people/other_responses", responses: @person.other_sector_responses %> + <%= render "people/other_sector_responses", responses: @person.other_sector_responses, dismissable: true %> diff --git a/app/views/people/_other_sector_responses.html.erb b/app/views/people/_other_sector_responses.html.erb new file mode 100644 index 000000000..3a1e27c4d --- /dev/null +++ b/app/views/people/_other_sector_responses.html.erb @@ -0,0 +1,20 @@ +<%# Chips for a person's free-text "Other" sector responses (OtherResponse + records). Read-only on the profile; on the edit form (dismissable: true) + each chip carries an × that dismisses it (hides it from the profile/edit). + Renders bare chips — the caller supplies the surrounding flex container. %> +<% responses ||= [] %> +<% dismissable = local_assigns.fetch(:dismissable, false) %> +<% responses.each do |response| %> + + <%= response.text %> + (other) + <% if dismissable %> + <%= link_to "×", + other_response_path(response, other_response: { status: "dismissed" }, return_to: "person_edit"), + data: { turbo_method: :patch }, + class: "ml-2 -mr-1 px-1 leading-none text-gray-400 hover:text-red-600", + title: "Hide this response from the profile" %> + <% end %> + +<% end %> diff --git a/app/views/people/show.html.erb b/app/views/people/show.html.erb index a6ae611b7..f98eb2132 100644 --- a/app/views/people/show.html.erb +++ b/app/views/people/show.html.erb @@ -167,7 +167,7 @@ display_leader: true, is_leader: si.is_leader %> <% end %> - <%= render "people/other_responses", responses: other_sectors %> + <%= render "people/other_sector_responses", responses: other_sectors %> <% else %>

None selected.

diff --git a/app/views/sectors/index.html.erb b/app/views/sectors/index.html.erb index e72ce0e70..d078db12a 100644 --- a/app/views/sectors/index.html.erb +++ b/app/views/sectors/index.html.erb @@ -8,6 +8,9 @@ Sectors (<%= @count_display %>)
+ <%= link_to "Review “Other”", + other_responses_path, + class: "text-sm text-gray-500 hover:text-gray-700 px-2 py-1" %> <%= link_to "Dedupe", dedupe_index_sectors_path, class: "text-sm text-gray-500 hover:text-gray-700 px-2 py-1" %> diff --git a/config/routes.rb b/config/routes.rb index 00eea12ff..a792bbfac 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -172,6 +172,11 @@ resources :comments, only: [ :index, :create, :update ] end resources :faqs + resources :other_responses, only: [ :index, :update ] do + collection do + post :promote + end + end resources :notifications, only: [ :index, :show, :update ] do member do post :resend diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 8538717d7..5a231eeaa 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -421,23 +421,17 @@ def answer(identifier, value) end describe "#other_sector_responses" do - # Exercises the legacy "service area" field identifiers on purpose — they - # must still resolve via FormField::SECTOR_FIELD_IDENTIFIERS. - it "returns free-text Other values from primary sector fields" do - answer("primary_service_area", "5, Other: Equine therapy") - answer("primary_service_area_single", "Other: Music therapy") + it "returns the person's visible sector OtherResponses" do + create(:other_response, person: person, kind: "sector", text: "Equine therapy") + create(:other_response, :kept, person: person, kind: "sector", text: "Music therapy") - expect(person.other_sector_responses).to contain_exactly("Equine therapy", "Music therapy") + expect(person.other_sector_responses.map(&:text)) + .to contain_exactly("Equine therapy", "Music therapy") end - it "ignores answers without an Other value" do - answer("primary_service_area", "5, 12") - - expect(person.other_sector_responses).to be_empty - end - - it "does not pull from unrelated fields" do - answer("primary_age_group", "Other: School") + it "omits dismissed and promoted responses" do + create(:other_response, :dismissed, person: person, kind: "sector", text: "Hidden") + create(:other_response, :promoted, person: person, kind: "sector", text: "Promoted") expect(person.other_sector_responses).to be_empty end diff --git a/spec/requests/events/professional_field_identifiers_spec.rb b/spec/requests/events/professional_field_identifiers_spec.rb index c5d832613..47dab03b1 100644 --- a/spec/requests/events/professional_field_identifiers_spec.rb +++ b/spec/requests/events/professional_field_identifiers_spec.rb @@ -105,7 +105,7 @@ it "tags the person with the primary/additional split and captures the Other free text" do expect(primary_sector_of(registrant)).to eq(sector_education) expect(additional_sectors_of(registrant)).to contain_exactly(sector_mh) - expect(registrant.other_sector_responses).to include("Equine therapy") + expect(registrant.other_sector_responses.map(&:text)).to include("Equine therapy") expect(registrant.primary_age_groups).to contain_exactly(age_adults) expect(registrant.additional_age_groups).to contain_exactly(age_teens, age_children) end diff --git a/spec/requests/other_responses_spec.rb b/spec/requests/other_responses_spec.rb new file mode 100644 index 000000000..03c1ae381 --- /dev/null +++ b/spec/requests/other_responses_spec.rb @@ -0,0 +1,72 @@ +require "rails_helper" + +RSpec.describe "OtherResponses", type: :request do + let(:admin) { create(:user, :admin) } + + describe "GET /other_responses" do + it "requires an admin" do + get other_responses_path + expect(response).not_to have_http_status(:ok) + end + + it "groups the same value across people with a count" do + sign_in admin + alice = create(:person) + bob = create(:person) + create(:other_response, person: alice, kind: "sector", text: "Equine therapy") + create(:other_response, person: bob, kind: "sector", text: "equine therapy") + create(:other_response, :dismissed, person: bob, kind: "sector", text: "Hidden one") + + get other_responses_path + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Equine therapy") + expect(response.body).not_to include("Hidden one") + end + end + + describe "PATCH /other_responses/:id (dismiss)" do + it "dismisses the response and returns to the person edit page" do + sign_in admin + response_record = create(:other_response, kind: "sector", text: "Equine therapy") + + patch other_response_path(response_record), + params: { other_response: { status: "dismissed" }, return_to: "person_edit" } + + expect(response).to redirect_to(edit_person_path(response_record.person)) + expect(response_record.reload.status).to eq("dismissed") + end + end + + describe "POST /other_responses/promote" do + it "tags every non-dismissed person and marks the responses promoted" do + sign_in admin + sector = create(:sector, name: "Equine Therapy") + kept = create(:other_response, person: create(:person), kind: "sector", text: "Equine therapy") + dismissed = create(:other_response, :dismissed, person: create(:person), kind: "sector", text: "Equine therapy") + + post promote_other_responses_path, + params: { normalized_text: "equine therapy", sector_id: sector.id } + + expect(kept.reload.status).to eq("promoted") + expect(kept.person.sectors).to include(sector) + expect(dismissed.reload.status).to eq("dismissed") + expect(dismissed.person.sectors).not_to include(sector) + end + + it "mints a new published sector when given a name" do + sign_in admin + person = create(:person) + create(:other_response, person: person, kind: "sector", text: "Equine therapy") + + expect { + post promote_other_responses_path, + params: { normalized_text: "equine therapy", new_sector_name: "Equine therapy" } + }.to change(Sector, :count).by(1) + + sector = Sector.find_by(name: "Equine therapy") + expect(sector.published).to be(true) + expect(person.sectors).to include(sector) + end + end +end diff --git a/spec/requests/people_other_responses_spec.rb b/spec/requests/people_other_responses_spec.rb index e7480c9af..c29d1a2ef 100644 --- a/spec/requests/people_other_responses_spec.rb +++ b/spec/requests/people_other_responses_spec.rb @@ -14,9 +14,9 @@ def answer(identifier, value) before { sign_in admin } describe "profile page" do - it "shows the Other service area as a free-text chip, not the primary sector" do + it "shows the Other sector as a free-text chip, not the primary sector" do person.update!(profile_show_sectors: true) - answer("primary_service_area", "Other: Equine therapy") + create(:other_response, person: person, kind: "sector", text: "Equine therapy") get person_path(person) @@ -26,15 +26,25 @@ def answer(identifier, value) equine_chip = response.body[/Equine therapy.{0,80}/m] expect(equine_chip).to include("(other)") end + + it "hides a dismissed Other sector" do + person.update!(profile_show_sectors: true) + create(:other_response, :dismissed, person: person, kind: "sector", text: "Equine therapy") + + get person_path(person) + + expect(response.body).not_to include("Equine therapy") + end end describe "edit page" do - it "shows the Other service area in the sectors section" do - answer("primary_service_area_single", "Other: Music therapy") + it "shows the Other sector in the sectors section with a dismiss control" do + create(:other_response, person: person, kind: "sector", text: "Music therapy") get edit_person_path(person) expect(response.body).to include("Music therapy") + expect(response.body).to include("Hide this response from the profile") end it "shows the Other workshop setting near the category checkboxes" do diff --git a/spec/views/page_bg_class_alignment_spec.rb b/spec/views/page_bg_class_alignment_spec.rb index 5cd9ed7c6..9b9730636 100644 --- a/spec/views/page_bg_class_alignment_spec.rb +++ b/spec/views/page_bg_class_alignment_spec.rb @@ -99,6 +99,7 @@ "app/views/taggings/matrix.html.erb" => "admin-only bg-blue-100", # index "app/views/allocations/index.html.erb" => "admin-only bg-blue-100", + "app/views/other_responses/index.html.erb" => "admin-only bg-blue-100", "app/views/banners/index.html.erb" => "admin-only bg-blue-100", "app/views/comments/index.html.erb" => "admin-only bg-blue-100", "app/views/bookmarks/index.html.erb" => "admin-only bg-blue-100", From 88dd079d9931c2dda177b5235416e4a7a8f1f303 Mon Sep 17 00:00:00 2001 From: maebeale Date: Sat, 4 Jul 2026 22:53:06 -0400 Subject: [PATCH 03/14] Add bulk keep/dismiss and status filter to the "Other" review queue The review page could only promote; add Keep all / Dismiss all bulk actions per grouped value, and a status filter (All / Pending / Kept) so kept responses stay visible in the queue but can be filtered out. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/controllers/other_responses_controller.rb | 23 +++++++++- app/views/other_responses/index.html.erb | 16 +++++++ config/routes.rb | 1 + spec/requests/other_responses_spec.rb | 46 +++++++++++++++++++ 4 files changed, 85 insertions(+), 1 deletion(-) diff --git a/app/controllers/other_responses_controller.rb b/app/controllers/other_responses_controller.rb index 45af4b5a9..bac63e263 100644 --- a/app/controllers/other_responses_controller.rb +++ b/app/controllers/other_responses_controller.rb @@ -5,8 +5,10 @@ class OtherResponsesController < ApplicationController # people, grouped with a count so a curator can decide what to promote. def index authorize! + @status_filter = params[:status].presence_in(OtherResponse::VISIBLE_STATUSES) + statuses = @status_filter ? [ @status_filter ] : OtherResponse::VISIBLE_STATUSES responses = OtherResponse - .sectors.where(status: OtherResponse::VISIBLE_STATUSES) + .sectors.where(status: statuses) .includes(:person) @groups = responses.group_by(&:normalized_text).map do |_normalized, rows| @@ -21,6 +23,25 @@ def index @sectors = Sector.excluding_other.order(:name) end + # Bulk keep/dismiss every visible person who typed this value, from the review + # queue. Keep leaves it as a free-text chip; dismiss hides it from profiles. + def curate + authorize! to: :update? + status = params[:status] + unless %w[kept dismissed].include?(status) + return redirect_to other_responses_path, alert: "Choose keep or dismiss." + end + + scope = OtherResponse.sectors.where(status: OtherResponse::VISIBLE_STATUSES) + .where(normalized_text: OtherResponse.normalize(params[:normalized_text])) + count = scope.count + scope.find_each { |response| response.update!(status: status) } + + verb = status == "kept" ? "Kept" : "Dismissed" + redirect_to other_responses_path(status: params[:return_status].presence), + status: :see_other, notice: "#{verb} #{count} response(s)." + end + # Curate a single response — the profile-edit "×" dismisses, and the review # page can keep an individual person's response. def update diff --git a/app/views/other_responses/index.html.erb b/app/views/other_responses/index.html.erb index 181ed856f..b66994c0e 100644 --- a/app/views/other_responses/index.html.erb +++ b/app/views/other_responses/index.html.erb @@ -15,6 +15,15 @@
+ +
+ <% [ [ "All", nil ], [ "Pending", "pending" ], [ "Kept", "kept" ] ].each do |label, value| %> + <% active = @status_filter == value %> + <%= link_to label, other_responses_path(status: value), + class: "px-3 py-1 rounded-full #{active ? "bg-gray-800 text-white" : "bg-white text-gray-600 border border-gray-300 hover:bg-gray-100"}" %> + <% end %> +
+
<% if @groups.any? %> @@ -50,6 +59,13 @@ <%= submit_tag "Promote", class: "btn btn-primary-outline text-sm", data: { turbo_confirm: "Promote “#{group[:display_text]}” for all #{group[:count]} people?" } %> <% end %> +
+ <%= button_to "Keep all", curate_other_responses_path(normalized_text: group[:normalized_text], status: "kept", return_status: @status_filter), + class: "text-gray-500 hover:text-gray-700" %> + <%= button_to "Dismiss all", curate_other_responses_path(normalized_text: group[:normalized_text], status: "dismissed", return_status: @status_filter), + class: "text-gray-500 hover:text-red-600", + form: { data: { turbo_confirm: "Hide “#{group[:display_text]}” from all #{group[:count]} profiles?" } } %> +
<% end %> diff --git a/config/routes.rb b/config/routes.rb index a792bbfac..e32bb7041 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -175,6 +175,7 @@ resources :other_responses, only: [ :index, :update ] do collection do post :promote + post :curate end end resources :notifications, only: [ :index, :show, :update ] do diff --git a/spec/requests/other_responses_spec.rb b/spec/requests/other_responses_spec.rb index 03c1ae381..265eca60b 100644 --- a/spec/requests/other_responses_spec.rb +++ b/spec/requests/other_responses_spec.rb @@ -23,6 +23,52 @@ expect(response.body).to include("Equine therapy") expect(response.body).not_to include("Hidden one") end + + it "filters to a single status" do + sign_in admin + create(:other_response, kind: "sector", text: "Pending value") + create(:other_response, :kept, kind: "sector", text: "Kept value") + + get other_responses_path(status: "kept") + + expect(response.body).to include("Kept value") + expect(response.body).not_to include("Pending value") + end + end + + describe "POST /other_responses/curate (bulk)" do + it "keeps every visible person who typed the value" do + sign_in admin + one = create(:other_response, kind: "sector", text: "Equine therapy") + two = create(:other_response, kind: "sector", text: "equine therapy") + + post curate_other_responses_path, + params: { normalized_text: "equine therapy", status: "kept" } + + expect(one.reload.status).to eq("kept") + expect(two.reload.status).to eq("kept") + end + + it "dismisses every visible person who typed the value" do + sign_in admin + response_record = create(:other_response, :kept, kind: "sector", text: "Equine therapy") + + post curate_other_responses_path, + params: { normalized_text: "equine therapy", status: "dismissed" } + + expect(response_record.reload.status).to eq("dismissed") + end + + it "rejects an unsupported status" do + sign_in admin + response_record = create(:other_response, kind: "sector", text: "Equine therapy") + + post curate_other_responses_path, + params: { normalized_text: "equine therapy", status: "promoted" } + + expect(response).to redirect_to(other_responses_path) + expect(response_record.reload.status).to eq("pending") + end end describe "PATCH /other_responses/:id (dismiss)" do From ca7c6f2ed1cf0eb579cada8476f9731e2ac54093 Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 7 Jul 2026 21:21:06 -0400 Subject: [PATCH 04/14] Update app/controllers/other_responses_controller.rb Co-authored-by: Justin <16829344+jmilljr24@users.noreply.github.com> --- app/controllers/other_responses_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/other_responses_controller.rb b/app/controllers/other_responses_controller.rb index bac63e263..183ac37ef 100644 --- a/app/controllers/other_responses_controller.rb +++ b/app/controllers/other_responses_controller.rb @@ -60,7 +60,7 @@ def update # tag — mapping to an existing sector or minting a new (published) one — and # mark those responses promoted so they stop showing as free-text chips. def promote - authorize! to: :promote? + authorize! sector = target_sector return redirect_to other_responses_path, alert: "Pick or name a sector to promote to." unless sector From 175e3931f4afa12b6ff95c6eb5a3dfab765c79ec Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 7 Jul 2026 21:24:13 -0400 Subject: [PATCH 05/14] Spell out OtherResponse policy rules instead of the manage? fallback Per review: define index?/update?/curate?/promote? explicitly (all admin?) rather than leaning on ApplicationPolicy's inherited manage? rule, matching how the other policies are written. Controller uses plain authorize! so each action resolves its own rule. Adds non-admin rejection specs for curate and promote. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/controllers/other_responses_controller.rb | 2 +- app/policies/other_response_policy.rb | 17 +++++++++++++++-- spec/requests/other_responses_spec.rb | 13 +++++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/app/controllers/other_responses_controller.rb b/app/controllers/other_responses_controller.rb index 183ac37ef..a5361e8f5 100644 --- a/app/controllers/other_responses_controller.rb +++ b/app/controllers/other_responses_controller.rb @@ -26,7 +26,7 @@ def index # Bulk keep/dismiss every visible person who typed this value, from the review # queue. Keep leaves it as a free-text chip; dismiss hides it from profiles. def curate - authorize! to: :update? + authorize! status = params[:status] unless %w[kept dismissed].include?(status) return redirect_to other_responses_path, alert: "Choose keep or dismiss." diff --git a/app/policies/other_response_policy.rb b/app/policies/other_response_policy.rb index fb48adc03..4b82329b4 100644 --- a/app/policies/other_response_policy.rb +++ b/app/policies/other_response_policy.rb @@ -1,6 +1,19 @@ class OtherResponsePolicy < ApplicationPolicy - # Curating free-text "Other" responses (reviewing, promoting, keeping, - # dismissing) is an admin-only task. index?/update? fall back to manage?. + # Reviewing and curating free-text "Other" responses (promote/keep/dismiss) is + # an admin-only task. Defined explicitly rather than leaning on the inherited + # manage? fallback, matching how the other policies spell out their rules. + + def index? + admin? + end + + def update? + admin? + end + + def curate? + admin? + end def promote? admin? diff --git a/spec/requests/other_responses_spec.rb b/spec/requests/other_responses_spec.rb index 265eca60b..192ec772e 100644 --- a/spec/requests/other_responses_spec.rb +++ b/spec/requests/other_responses_spec.rb @@ -37,6 +37,12 @@ end describe "POST /other_responses/curate (bulk)" do + it "requires an admin" do + create(:other_response, kind: "sector", text: "Equine therapy") + post curate_other_responses_path, params: { normalized_text: "equine therapy", status: "dismissed" } + expect(response).not_to have_http_status(:ok) + end + it "keeps every visible person who typed the value" do sign_in admin one = create(:other_response, kind: "sector", text: "Equine therapy") @@ -85,6 +91,13 @@ end describe "POST /other_responses/promote" do + it "requires an admin" do + sector = create(:sector, name: "Equine Therapy") + create(:other_response, kind: "sector", text: "Equine therapy") + post promote_other_responses_path, params: { normalized_text: "equine therapy", sector_id: sector.id } + expect(response).not_to have_http_status(:ok) + end + it "tags every non-dismissed person and marks the responses promoted" do sign_in admin sector = create(:sector, name: "Equine Therapy") From 302b47f264b02e8a40b1d830488089afa712b469 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 09:38:44 -0400 Subject: [PATCH 06/14] Capture "Other" answers from every question, not just sectors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generalize the capture so any question's free-text "Other" is materialized as an OtherResponse, on all three submission paths (registration, scholarship, bulk payment) via a shared OtherResponses::CaptureFromSubmission service. - OtherResponse gains field_identifier (which question) and derives kind: "sector" (promotable into a Sector, shown on the profile) or "generic" (auxiliary data, admin-review only). Unique per person + field + value. - Org-owned agency_type "Other" is excluded — it belongs to the organization (synced by #1875) and will get its own promotable path when OrganizationType becomes a model. - Review page groups by question (sectors bucketed together, each other question on its own), shows Promote only for sector groups; keep/dismiss for all. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 6 +- app/controllers/other_responses_controller.rb | 78 +++++++++++++------ app/models/other_response.rb | 45 +++++++++-- .../bulk_payment.rb | 1 + .../public_registration.rb | 26 +------ .../capture_from_submission.rb | 54 +++++++++++++ app/views/other_responses/index.html.erb | 54 +++++++------ .../20260705014639_create_other_responses.rb | 5 +- db/schema.rb | 3 +- spec/factories/other_responses.rb | 7 +- spec/models/other_response_spec.rb | 41 +++++++--- spec/requests/other_responses_spec.rb | 77 +++++++++++------- .../capture_from_submission_spec.rb | 57 ++++++++++++++ 13 files changed, 335 insertions(+), 119 deletions(-) create mode 100644 app/services/other_responses/capture_from_submission.rb create mode 100644 spec/services/other_responses/capture_from_submission_spec.rb diff --git a/AGENTS.md b/AGENTS.md index 5fa8f5cec..b3ec64f1c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,7 +102,7 @@ This codebase (Rails 8.1) | `Story` | Editorial content with facilitators, primary/gallery assets | | `Resource` | Handouts, toolkits, templates with downloadable assets | | `Person` | Organization affiliates with contacts, addresses, sectors | -| `OtherResponse` | A free-text "Other" a person typed on a tag-backed form question (`kind: "sector"` today), captured at registration so a curator can `promote` it into a real `Sector` tag, `keep` it as a chip, or `dismiss` it (hide from profile/edit). Reviewed at `/other_responses` | +| `OtherResponse` | A free-text "Other" a person typed on any form question, captured at submission time (registration, scholarship, bulk payment). `field_identifier` records the question; `kind` is derived — `sector` (promotable into a `Sector`, shown on the profile) or `generic` (auxiliary data, admin-review only). Curated at `/other_responses`: `promote` (sectors only), `keep`, or `dismiss`. Org-type "Other" is excluded (owned by the org). | | `Organization` | Groups with affiliations, addresses, logos via ActiveStorage | | `Grant` | Donated funds (polymorphic `donor`: Organization or Person) with eligibility criteria, tasks, deadlines; parent of `Scholarship`. Scholarship totals cannot exceed the grant amount | | `Scholarship` | Award to a `Person`; optionally drawn from a `Grant`, syncs to event registration `Allocation` | @@ -213,6 +213,10 @@ end - `AffiliationServices::CreateFromRegistration` — On registration / org linking, creates a "job affiliation" with the typed title (when present) plus a standing "Facilitator" affiliation, in one transaction. Skips the facilitator one only when the person already has an active-or-pending affiliation titled exactly "Facilitator" with that org (a current one or one dated to a future training); an ended facilitator affiliation gets a fresh second one. Dedupe is by title + org + dates, so a job title like "Lead Facilitator" still gets its own Facilitator affiliation. Accepts an optional `organization_address:` and sets it on every affiliation it creates (the registrant's typed agency address, upserted onto the org); when an affiliation already exists and is skipped, it backfills that address onto the existing one only if it has none (an admin-set address is never overwritten) +### Other responses + +- `OtherResponses::CaptureFromSubmission` — Materializes a form submission's free-text "Other" answers as `OtherResponse` records. Runs over every answered field (uses `OtherOption.texts`, which keys strictly on the `Other:` prefix, so named specify options and the CE `Yes: N` box are ignored); excludes org-owned `agency_type`; de-dupes per person + question. Shared by the registration, scholarship, and bulk-payment submission paths + ### Organizations - `OrganizationServices::UpsertAddress` — Find-or-create an organization's "work" address from a registrant's submitted agency fields (street/city/state/zip/country). Updates the matching city/state address in place, else adds a new one; never demotes the org's existing primary (a registrant's address becomes primary only when the org has none yet). Returns nil when no city is given. Shared by `PublicRegistration` and the admin org-linking actions so both build the org address identically before linking the affiliation to it diff --git a/app/controllers/other_responses_controller.rb b/app/controllers/other_responses_controller.rb index a5361e8f5..bd5d2a2bd 100644 --- a/app/controllers/other_responses_controller.rb +++ b/app/controllers/other_responses_controller.rb @@ -1,30 +1,26 @@ class OtherResponsesController < ApplicationController before_action :set_other_response, only: :update - # Review page: the same free-text "Other" sector value typed across many - # people, grouped with a count so a curator can decide what to promote. + # Review page: the same free-text "Other" value typed across many people, + # grouped so a curator can decide what to do. Sector "Other"s are bucketed + # together (they all promote into the one Sector catalog); every other question + # is grouped on its own, since its "Other"s are unrelated auxiliary data. def index authorize! @status_filter = params[:status].presence_in(OtherResponse::VISIBLE_STATUSES) statuses = @status_filter ? [ @status_filter ] : OtherResponse::VISIBLE_STATUSES - responses = OtherResponse - .sectors.where(status: statuses) - .includes(:person) - - @groups = responses.group_by(&:normalized_text).map do |_normalized, rows| - { - display_text: rows.first.text, - normalized_text: rows.first.normalized_text, - count: rows.size, - status_counts: rows.each_with_object(Hash.new(0)) { |r, h| h[r.status] += 1 } - } - end.sort_by { |group| [ -group[:count], group[:display_text].downcase ] } + responses = OtherResponse.where(status: statuses).includes(:person, :source_form_answer) + + @groups = responses + .group_by { |response| [ group_bucket(response), response.normalized_text ] } + .map { |_key, rows| build_group(rows) } + .sort_by { |group| [ group[:promotable] ? 0 : 1, group[:question_label].downcase, -group[:count], group[:display_text].downcase ] } @sectors = Sector.excluding_other.order(:name) end - # Bulk keep/dismiss every visible person who typed this value, from the review - # queue. Keep leaves it as a free-text chip; dismiss hides it from profiles. + # Bulk keep/dismiss every visible person in a group, from the review queue. + # Keep leaves the value in place; dismiss hides it. def curate authorize! status = params[:status] @@ -32,8 +28,7 @@ def curate return redirect_to other_responses_path, alert: "Choose keep or dismiss." end - scope = OtherResponse.sectors.where(status: OtherResponse::VISIBLE_STATUSES) - .where(normalized_text: OtherResponse.normalize(params[:normalized_text])) + scope = group_scope.where(status: OtherResponse::VISIBLE_STATUSES) count = scope.count scope.find_each { |response| response.update!(status: status) } @@ -56,17 +51,16 @@ def update end end - # Promote every non-dismissed person who typed this value into a real Sector - # tag — mapping to an existing sector or minting a new (published) one — and - # mark those responses promoted so they stop showing as free-text chips. + # Promote every non-dismissed person in a sector group into a real Sector tag — + # mapping to an existing sector or minting a new (published) one — and mark + # those responses promoted so they stop showing as free-text chips. Only sector + # groups are promotable, enforced by the .sectors scope. def promote authorize! sector = target_sector return redirect_to other_responses_path, alert: "Pick or name a sector to promote to." unless sector - responses = OtherResponse.sectors.promotable_now - .where(normalized_text: OtherResponse.normalize(params[:normalized_text])) - + responses = group_scope.sectors.promotable_now responses.includes(:person).find_each do |response| response.person.tag_sectors(primary_ids: [], additional_ids: [ sector.id ]) response.update!(status: "promoted", promotable: sector) @@ -82,6 +76,42 @@ def set_other_response @other_response = OtherResponse.find(params[:id]) end + # Group key: sector "Other"s share one bucket (kind), every other question is + # keyed by its field so distinct questions never merge. + def group_bucket(response) + response.promotable? ? response.kind : "field:#{response.field_identifier}" + end + + def build_group(rows) + first = rows.first + { + kind: first.kind, + field_identifier: first.field_identifier, + promotable: first.promotable?, + question_label: question_label_for(first), + display_text: first.text, + normalized_text: first.normalized_text, + count: rows.size, + status_counts: rows.each_with_object(Hash.new(0)) { |r, h| h[r.status] += 1 } + } + end + + def question_label_for(response) + return "Sectors" if response.promotable? + response.source_form_answer&.question_name_when_answered.presence || response.field_identifier.humanize + end + + # The set of responses a curate/promote action targets, matching how the group + # was bucketed: sectors by kind, everything else by field. + def group_scope + scope = OtherResponse.where(normalized_text: OtherResponse.normalize(params[:normalized_text])) + if params[:kind] == "sector" + scope.sectors + else + scope.where(field_identifier: params[:field_identifier]) + end + end + # The promote target: an existing sector by id, or a newly minted published one # from a typed name. Returns nil when neither was supplied. def target_sector diff --git a/app/models/other_response.rb b/app/models/other_response.rb index b4607aa2f..fd84c34ae 100644 --- a/app/models/other_response.rb +++ b/app/models/other_response.rb @@ -1,39 +1,64 @@ class OtherResponse < ApplicationRecord - # The free-text "Other" a person typed on a tag-backed form question. Only - # sectors are wired up today; the `kind` column leaves room for the identical - # workshop-setting responses to move here later. - KINDS = %w[sector].freeze + # A free-text "Other" a person typed on any tag/option-backed form question, + # captured at submission time so a curator can promote / keep / dismiss it. + # `field_identifier` records which question it came from; `kind` is the coarse + # category that decides whether it can be promoted into a real tag. + # + # - "sector" → the additional-sectors question; promotable into a Sector, and + # shown on the person's profile beside the sector tags. + # - "generic" → any other question's "Other"; auxiliary data reviewed by admins + # (keep/dismiss only), never shown on a profile. + # + # Organization-type "Other" is deliberately NOT captured here — it belongs to + # the organization (see PublicRegistration#sync_agency_type) and will get its + # own promotable path once OrganizationType is a model. + KINDS = %w[sector generic].freeze + + # Kinds that can be promoted into a real tag record. + PROMOTABLE_KINDS = %w[sector].freeze # A response starts life as `pending` (awaiting a curator's decision) and is - # then either promoted into a real tag, kept as a free-text chip, or dismissed - # (hidden from the person's profile and edit form). + # then either promoted into a real tag, kept as auxiliary data, or dismissed. STATUSES = %w[pending kept promoted dismissed].freeze - # Statuses that still surface as an "(other)" chip on the person's pages. + # Statuses that still surface (as an "(other)" chip for sectors, or a live row + # in the review queue). VISIBLE_STATUSES = %w[pending kept].freeze belongs_to :person belongs_to :promotable, polymorphic: true, optional: true belongs_to :source_form_answer, class_name: "FormAnswer", optional: true + before_validation :set_kind before_validation :set_normalized_text + validates :field_identifier, presence: true validates :text, presence: true validates :kind, inclusion: { in: KINDS } validates :status, inclusion: { in: STATUSES } - validates :normalized_text, uniqueness: { scope: [ :person_id, :kind ] } + validates :normalized_text, uniqueness: { scope: [ :person_id, :field_identifier ] } scope :sectors, -> { where(kind: "sector") } scope :visible, -> { where(status: VISIBLE_STATUSES) } scope :pending, -> { where(status: "pending") } scope :promotable_now, -> { where.not(status: "dismissed") } + # The coarse category for a question. Sector fields promote into Sectors; + # everything else is generic auxiliary data. + def self.kind_for(field_identifier) + field_identifier.to_s.in?(FormField::SECTOR_FIELD_IDENTIFIERS) ? "sector" : "generic" + end + # Case/whitespace-insensitive key used both for the unique index and for # grouping the same typed value across many people on the review page. def self.normalize(value) value.to_s.strip.downcase end + def promotable? + kind.in?(PROMOTABLE_KINDS) + end + def dismiss! update!(status: "dismissed") end @@ -44,6 +69,10 @@ def keep! private + def set_kind + self.kind = self.class.kind_for(field_identifier) if field_identifier.present? + end + def set_normalized_text self.normalized_text = self.class.normalize(text) end diff --git a/app/services/event_registration_services/bulk_payment.rb b/app/services/event_registration_services/bulk_payment.rb index cd6041ea2..34bce257c 100644 --- a/app/services/event_registration_services/bulk_payment.rb +++ b/app/services/event_registration_services/bulk_payment.rb @@ -133,6 +133,7 @@ def create_phone_contact(person) def create_form_submission(person) submission = FormSubmission.create!(person: person, form: @form, event: @event, role: "bulk_payment") save_form_answers(submission) + OtherResponses::CaptureFromSubmission.call(submission) submission end diff --git a/app/services/event_registration_services/public_registration.rb b/app/services/event_registration_services/public_registration.rb index 99bec853e..53ca1d7d7 100644 --- a/app/services/event_registration_services/public_registration.rb +++ b/app/services/event_registration_services/public_registration.rb @@ -442,7 +442,7 @@ def invoice_requested? def create_form_submission(person) submission = FormSubmission.create!(person: person, form: @registration_form, event: @event, role: "registration") save_form_answers(submission) - capture_other_sector_responses(submission) + OtherResponses::CaptureFromSubmission.call(submission) submission end @@ -451,30 +451,10 @@ def update_form_submission(person) record.event = @event end save_form_answers(submission) - capture_other_sector_responses(submission) + OtherResponses::CaptureFromSubmission.call(submission) submission end - # Materialize the free-text "Other" sector answers as OtherResponse records so - # they can be curated (promoted/kept/dismissed). Reuses OtherOption.texts, the - # same extraction used to display them, and de-dupes on the person's normalized - # value so a repeat registration of the same text doesn't create a second row. - def capture_other_sector_responses(submission) - submission.form_answers - .joins(:form_field) - .where(form_fields: { field_identifier: FormField::SECTOR_FIELD_IDENTIFIERS }) - .each do |answer| - OtherOption.texts(answer.submitted_answer).each do |text| - submission.person.other_responses.find_or_create_by!( - kind: "sector", normalized_text: OtherResponse.normalize(text) - ) do |response| - response.text = text - response.source_form_answer = answer - end - end - end - end - def save_form_answers(submission) @form_params.each do |field_id, raw_value| field = @registration_form.form_fields.find_by(id: field_id) @@ -518,6 +498,8 @@ def save_scholarship_submission(person) record = submission.form_answers.find_or_initialize_by(form_field: field) record.update!(submitted_answer: text, question_name_when_answered: field.name) end + + OtherResponses::CaptureFromSubmission.call(submission) end def save_continuing_education_submission(person) diff --git a/app/services/other_responses/capture_from_submission.rb b/app/services/other_responses/capture_from_submission.rb new file mode 100644 index 000000000..4f9f111bf --- /dev/null +++ b/app/services/other_responses/capture_from_submission.rb @@ -0,0 +1,54 @@ +module OtherResponses + # Materializes the free-text "Other" answers on a form submission as + # OtherResponse records so they can be curated (promoted/kept/dismissed). + # + # Runs over every answered field — not just sectors — because any question can + # offer an "Other" option. OtherOption.texts keys strictly on the "Other:" + # prefix, so named specify options ("Word of Mouth: …") and the CE "Yes: 3" + # box are ignored. De-dupes on the person's normalized value per question, so + # re-submitting the same answer never creates a second row. + # + # Shared by the registration, scholarship, and bulk-payment submission paths. + class CaptureFromSubmission + # Fields whose "Other" belongs to the organization, not the person, so they + # must not land in the person's review queue. Organization type is synced + # onto the org (PublicRegistration#sync_agency_type) and will get its own + # promotable path when OrganizationType becomes a model. + EXCLUDED_FIELD_IDENTIFIERS = %w[agency_type].freeze + + def self.call(submission) + new(submission).call + end + + def initialize(submission) + @submission = submission + end + + def call + answers.each do |answer| + field_identifier = answer.form_field&.field_identifier + next if field_identifier.blank? || field_identifier.in?(EXCLUDED_FIELD_IDENTIFIERS) + + OtherOption.texts(answer.submitted_answer).each do |text| + capture(field_identifier, text, answer) + end + end + end + + private + + def answers + @submission.form_answers.includes(:form_field) + end + + def capture(field_identifier, text, answer) + @submission.person.other_responses.find_or_create_by!( + field_identifier: field_identifier, + normalized_text: OtherResponse.normalize(text) + ) do |response| + response.text = text + response.source_form_answer = answer + end + end + end +end diff --git a/app/views/other_responses/index.html.erb b/app/views/other_responses/index.html.erb index b66994c0e..abbb4d00b 100644 --- a/app/views/other_responses/index.html.erb +++ b/app/views/other_responses/index.html.erb @@ -7,10 +7,10 @@
-

Review “Other” sectors

+

Review “Other” responses

- Free-text sectors people typed on registration forms, grouped by value. - Promote a recurring one into a real sector tag for everyone who typed it. + Free-text “Other” answers people typed on forms, grouped by question and value. + Promote a recurring sector into a real tag, or keep/dismiss any value.

@@ -29,14 +29,16 @@
+ - - + + <% @groups.each do |group| %> + @@ -72,7 +80,7 @@
Question ResponsePeoplePromote to a sectorPeopleAction
<%= group[:question_label] %> <%= group[:display_text] %> <% if group[:status_counts]["kept"].to_i.positive? %> @@ -45,26 +47,32 @@ <%= group[:count] %> - <%= form_with url: promote_other_responses_path, method: :post, - class: "flex flex-wrap items-center gap-2" do %> - <%= hidden_field_tag :normalized_text, group[:normalized_text] %> - <%= select_tag :sector_id, - options_from_collection_for_select(@sectors, :id, :name), - include_blank: "Existing sector…", - class: "rounded-md border-gray-300 text-sm" %> - or - <%= text_field_tag :new_sector_name, group[:display_text], - placeholder: "New sector name", - class: "rounded-md border-gray-300 text-sm" %> - <%= submit_tag "Promote", class: "btn btn-primary-outline text-sm", - data: { turbo_confirm: "Promote “#{group[:display_text]}” for all #{group[:count]} people?" } %> + <% if group[:promotable] %> + <%= form_with url: promote_other_responses_path, method: :post, + class: "flex flex-wrap items-center gap-2" do %> + <%= hidden_field_tag :kind, group[:kind] %> + <%= hidden_field_tag :field_identifier, group[:field_identifier] %> + <%= hidden_field_tag :normalized_text, group[:normalized_text] %> + <%= select_tag :sector_id, + options_from_collection_for_select(@sectors, :id, :name), + include_blank: "Existing sector…", + class: "rounded-md border-gray-300 text-sm" %> + or + <%= text_field_tag :new_sector_name, group[:display_text], + placeholder: "New sector name", + class: "rounded-md border-gray-300 text-sm" %> + <%= submit_tag "Promote", class: "btn btn-primary-outline text-sm", + data: { turbo_confirm: "Promote “#{group[:display_text]}” for all #{group[:count]} people?" } %> + <% end %> <% end %> -
- <%= button_to "Keep all", curate_other_responses_path(normalized_text: group[:normalized_text], status: "kept", return_status: @status_filter), +
text-sm"> + <%= button_to "Keep all", + curate_other_responses_path(kind: group[:kind], field_identifier: group[:field_identifier], normalized_text: group[:normalized_text], status: "kept", return_status: @status_filter), class: "text-gray-500 hover:text-gray-700" %> - <%= button_to "Dismiss all", curate_other_responses_path(normalized_text: group[:normalized_text], status: "dismissed", return_status: @status_filter), + <%= button_to "Dismiss all", + curate_other_responses_path(kind: group[:kind], field_identifier: group[:field_identifier], normalized_text: group[:normalized_text], status: "dismissed", return_status: @status_filter), class: "text-gray-500 hover:text-red-600", - form: { data: { turbo_confirm: "Hide “#{group[:display_text]}” from all #{group[:count]} profiles?" } } %> + form: { data: { turbo_confirm: "Dismiss “#{group[:display_text]}” for all #{group[:count]} people?" } } %>
<% else %> -

No “Other” sector responses to review.

+

No “Other” responses to review.

<% end %>
diff --git a/db/migrate/20260705014639_create_other_responses.rb b/db/migrate/20260705014639_create_other_responses.rb index 84272654f..cf5d14d9f 100644 --- a/db/migrate/20260705014639_create_other_responses.rb +++ b/db/migrate/20260705014639_create_other_responses.rb @@ -9,6 +9,7 @@ def up create_table :other_responses do |t| t.references :person, null: false, foreign_key: true + t.string :field_identifier, null: false t.string :kind, null: false t.string :text, null: false t.string :normalized_text, null: false @@ -18,8 +19,8 @@ def up t.timestamps end - add_index :other_responses, [ :person_id, :kind, :normalized_text ], - unique: true, name: "index_other_responses_on_person_kind_text" + add_index :other_responses, [ :person_id, :field_identifier, :normalized_text ], + unique: true, name: "index_other_responses_on_person_field_text" end def down diff --git a/db/schema.rb b/db/schema.rb index 11ad49dad..a0b5cdd47 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -836,6 +836,7 @@ create_table "other_responses", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t| t.datetime "created_at", null: false + t.string "field_identifier", null: false t.string "kind", null: false t.string "normalized_text", null: false t.bigint "person_id", null: false @@ -845,7 +846,7 @@ t.string "status", default: "pending", null: false t.string "text", null: false t.datetime "updated_at", null: false - t.index ["person_id", "kind", "normalized_text"], name: "index_other_responses_on_person_kind_text", unique: true + t.index ["person_id", "field_identifier", "normalized_text"], name: "index_other_responses_on_person_field_text", unique: true t.index ["person_id"], name: "index_other_responses_on_person_id" t.index ["promotable_type", "promotable_id"], name: "index_other_responses_on_promotable" t.index ["source_form_answer_id"], name: "index_other_responses_on_source_form_answer_id" diff --git a/spec/factories/other_responses.rb b/spec/factories/other_responses.rb index 1693fd49d..8a20b089d 100644 --- a/spec/factories/other_responses.rb +++ b/spec/factories/other_responses.rb @@ -1,9 +1,14 @@ FactoryBot.define do factory :other_response do person - kind { "sector" } + field_identifier { "additional_sectors" } sequence(:text) { |n| "Equine therapy #{n}" } status { "pending" } + # kind is derived from field_identifier (sector fields → "sector"). + + trait :generic do + field_identifier { "how_did_you_hear" } + end trait :kept do status { "kept" } diff --git a/spec/models/other_response_spec.rb b/spec/models/other_response_spec.rb index 5f3c9182a..ffdabe9a5 100644 --- a/spec/models/other_response_spec.rb +++ b/spec/models/other_response_spec.rb @@ -10,25 +10,39 @@ expect(build(:other_response, text: "")).not_to be_valid end - it "rejects an unknown kind" do - expect(build(:other_response, kind: "nonsense")).not_to be_valid + it "requires a field_identifier" do + expect(build(:other_response, field_identifier: "")).not_to be_valid end - it "rejects an unknown status" do - expect(build(:other_response, status: "nonsense")).not_to be_valid + it "is unique per person + field + normalized text" do + person = create(:person) + create(:other_response, person: person, field_identifier: "additional_sectors", text: "Equine therapy") + dup = build(:other_response, person: person, field_identifier: "additional_sectors", text: " equine therapy ") + + expect(dup).not_to be_valid end - it "is unique per person + kind + normalized text" do + it "allows the same text on a different question" do person = create(:person) - create(:other_response, person: person, kind: "sector", text: "Equine therapy") - dup = build(:other_response, person: person, kind: "sector", text: " equine therapy ") + create(:other_response, person: person, field_identifier: "additional_sectors", text: "Equine therapy") + other_question = build(:other_response, person: person, field_identifier: "how_did_you_hear", text: "Equine therapy") - expect(dup).not_to be_valid + expect(other_question).to be_valid end it "allows the same text for a different person" do - create(:other_response, kind: "sector", text: "Equine therapy") - expect(build(:other_response, kind: "sector", text: "Equine therapy")).to be_valid + create(:other_response, field_identifier: "additional_sectors", text: "Equine therapy") + expect(build(:other_response, field_identifier: "additional_sectors", text: "Equine therapy")).to be_valid + end + end + + describe "kind derivation" do + it "is sector for a sector field" do + expect(create(:other_response, field_identifier: "additional_sectors").kind).to eq("sector") + end + + it "is generic for any other field" do + expect(create(:other_response, field_identifier: "how_did_you_hear").kind).to eq("generic") end end @@ -39,7 +53,7 @@ end end - describe "scopes" do + describe "scopes and #promotable?" do it ".visible returns only pending and kept" do pending = create(:other_response) kept = create(:other_response, :kept) @@ -48,6 +62,11 @@ expect(OtherResponse.visible).to contain_exactly(pending, kept) end + + it "only sector responses are promotable" do + expect(create(:other_response).promotable?).to be(true) + expect(create(:other_response, :generic).promotable?).to be(false) + end end describe "#dismiss! / #keep!" do diff --git a/spec/requests/other_responses_spec.rb b/spec/requests/other_responses_spec.rb index 192ec772e..d33f9d17d 100644 --- a/spec/requests/other_responses_spec.rb +++ b/spec/requests/other_responses_spec.rb @@ -9,13 +9,13 @@ expect(response).not_to have_http_status(:ok) end - it "groups the same value across people with a count" do + it "groups the same sector value across people with a count" do sign_in admin alice = create(:person) bob = create(:person) - create(:other_response, person: alice, kind: "sector", text: "Equine therapy") - create(:other_response, person: bob, kind: "sector", text: "equine therapy") - create(:other_response, :dismissed, person: bob, kind: "sector", text: "Hidden one") + create(:other_response, person: alice, text: "Equine therapy") + create(:other_response, person: bob, text: "equine therapy") + create(:other_response, :dismissed, person: bob, text: "Hidden one") get other_responses_path @@ -24,10 +24,20 @@ expect(response.body).not_to include("Hidden one") end + it "shows generic (non-sector) responses under their question" do + sign_in admin + create(:other_response, :generic, text: "A friend") + + get other_responses_path + + expect(response.body).to include("A friend") + expect(response.body).to include("How did you hear") + end + it "filters to a single status" do sign_in admin - create(:other_response, kind: "sector", text: "Pending value") - create(:other_response, :kept, kind: "sector", text: "Kept value") + create(:other_response, text: "Pending value") + create(:other_response, :kept, text: "Kept value") get other_responses_path(status: "kept") @@ -38,39 +48,42 @@ describe "POST /other_responses/curate (bulk)" do it "requires an admin" do - create(:other_response, kind: "sector", text: "Equine therapy") - post curate_other_responses_path, params: { normalized_text: "equine therapy", status: "dismissed" } + create(:other_response, text: "Equine therapy") + post curate_other_responses_path, params: { kind: "sector", normalized_text: "equine therapy", status: "dismissed" } expect(response).not_to have_http_status(:ok) end - it "keeps every visible person who typed the value" do + it "keeps every visible person in a sector group" do sign_in admin - one = create(:other_response, kind: "sector", text: "Equine therapy") - two = create(:other_response, kind: "sector", text: "equine therapy") + one = create(:other_response, text: "Equine therapy") + two = create(:other_response, text: "equine therapy") post curate_other_responses_path, - params: { normalized_text: "equine therapy", status: "kept" } + params: { kind: "sector", normalized_text: "equine therapy", status: "kept" } expect(one.reload.status).to eq("kept") expect(two.reload.status).to eq("kept") end - it "dismisses every visible person who typed the value" do + it "dismisses a generic group scoped to its question" do sign_in admin - response_record = create(:other_response, :kept, kind: "sector", text: "Equine therapy") + generic = create(:other_response, :generic, text: "A friend") + sector = create(:other_response, text: "A friend") post curate_other_responses_path, - params: { normalized_text: "equine therapy", status: "dismissed" } + params: { field_identifier: "how_did_you_hear", normalized_text: "a friend", status: "dismissed" } - expect(response_record.reload.status).to eq("dismissed") + expect(generic.reload.status).to eq("dismissed") + # The identically-typed sector value on a different question is untouched. + expect(sector.reload.status).to eq("pending") end it "rejects an unsupported status" do sign_in admin - response_record = create(:other_response, kind: "sector", text: "Equine therapy") + response_record = create(:other_response, text: "Equine therapy") post curate_other_responses_path, - params: { normalized_text: "equine therapy", status: "promoted" } + params: { kind: "sector", normalized_text: "equine therapy", status: "promoted" } expect(response).to redirect_to(other_responses_path) expect(response_record.reload.status).to eq("pending") @@ -80,7 +93,7 @@ describe "PATCH /other_responses/:id (dismiss)" do it "dismisses the response and returns to the person edit page" do sign_in admin - response_record = create(:other_response, kind: "sector", text: "Equine therapy") + response_record = create(:other_response, text: "Equine therapy") patch other_response_path(response_record), params: { other_response: { status: "dismissed" }, return_to: "person_edit" } @@ -93,19 +106,19 @@ describe "POST /other_responses/promote" do it "requires an admin" do sector = create(:sector, name: "Equine Therapy") - create(:other_response, kind: "sector", text: "Equine therapy") - post promote_other_responses_path, params: { normalized_text: "equine therapy", sector_id: sector.id } + create(:other_response, text: "Equine therapy") + post promote_other_responses_path, params: { kind: "sector", normalized_text: "equine therapy", sector_id: sector.id } expect(response).not_to have_http_status(:ok) end it "tags every non-dismissed person and marks the responses promoted" do sign_in admin sector = create(:sector, name: "Equine Therapy") - kept = create(:other_response, person: create(:person), kind: "sector", text: "Equine therapy") - dismissed = create(:other_response, :dismissed, person: create(:person), kind: "sector", text: "Equine therapy") + kept = create(:other_response, person: create(:person), text: "Equine therapy") + dismissed = create(:other_response, :dismissed, person: create(:person), text: "Equine therapy") post promote_other_responses_path, - params: { normalized_text: "equine therapy", sector_id: sector.id } + params: { kind: "sector", normalized_text: "equine therapy", sector_id: sector.id } expect(kept.reload.status).to eq("promoted") expect(kept.person.sectors).to include(sector) @@ -116,16 +129,28 @@ it "mints a new published sector when given a name" do sign_in admin person = create(:person) - create(:other_response, person: person, kind: "sector", text: "Equine therapy") + create(:other_response, person: person, text: "Equine therapy") expect { post promote_other_responses_path, - params: { normalized_text: "equine therapy", new_sector_name: "Equine therapy" } + params: { kind: "sector", normalized_text: "equine therapy", new_sector_name: "Equine therapy" } }.to change(Sector, :count).by(1) sector = Sector.find_by(name: "Equine therapy") expect(sector.published).to be(true) expect(person.sectors).to include(sector) end + + it "does not promote a generic group" do + sign_in admin + generic = create(:other_response, :generic, text: "A friend") + sector = create(:sector, name: "A Friend") + + post promote_other_responses_path, + params: { field_identifier: "how_did_you_hear", normalized_text: "a friend", sector_id: sector.id } + + expect(generic.reload.status).to eq("pending") + expect(generic.person.sectors).not_to include(sector) + end end end diff --git a/spec/services/other_responses/capture_from_submission_spec.rb b/spec/services/other_responses/capture_from_submission_spec.rb new file mode 100644 index 000000000..cb3a48f91 --- /dev/null +++ b/spec/services/other_responses/capture_from_submission_spec.rb @@ -0,0 +1,57 @@ +require "rails_helper" + +RSpec.describe OtherResponses::CaptureFromSubmission do + let(:person) { create(:person) } + let(:form) { create(:form) } + let(:submission) { create(:form_submission, person: person, form: form) } + + def answer(identifier, value) + field = create(:form_field, form: form, field_identifier: identifier) + create(:form_answer, form_submission: submission, form_field: field, submitted_answer: value) + end + + it "captures Other answers from any question, tagged with the field and kind" do + answer("additional_sectors", "5, Other: Equine therapy") + answer("how_did_you_hear", "Other: A friend") + + described_class.call(submission) + + responses = person.other_responses.order(:field_identifier) + expect(responses.map { |r| [ r.field_identifier, r.text, r.kind ] }).to contain_exactly( + [ "additional_sectors", "Equine therapy", "sector" ], + [ "how_did_you_hear", "A friend", "generic" ] + ) + end + + it "ignores answers with no Other free text and named specify options" do + answer("additional_sectors", "5, 12") + answer("how_did_you_hear", "Word of Mouth: Jane") + + described_class.call(submission) + + expect(person.other_responses).to be_empty + end + + it "does not capture the organization-owned agency_type Other" do + answer("agency_type", "Other: Nonprofit collective") + + described_class.call(submission) + + expect(person.other_responses).to be_empty + end + + it "de-dupes repeat submissions of the same value per question" do + answer("additional_sectors", "Other: Equine therapy") + described_class.call(submission) + described_class.call(submission) + + expect(person.other_responses.count).to eq(1) + end + + it "records the source form answer for provenance" do + source = answer("additional_sectors", "Other: Equine therapy") + described_class.call(submission) + + expect(person.other_responses.first.source_form_answer).to eq(source) + end +end From e6b18791ec6e3f94faa2d748d3a202e57765ef6b Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 10:02:03 -0400 Subject: [PATCH 07/14] Capture only promotable "Other"s; curate from the person's chips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverse the "capture every question" scope: only questions whose "Other" can become a real tag are stored (sectors today; org-type once OrganizationType is a model). Every other "Other" stays searchable in the form answers rather than duplicated here. field_identifier is kept so switching a new question on later is a one-line change. Also let admins curate from the person: the profile and edit sector "Other" chips deep-link into the review queue (anchored to the row, with a back link to the person), where they can promote/keep/dismiss. Edit keeps its one-click × too. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 4 +-- app/controllers/other_responses_controller.rb | 1 + app/models/other_response.rb | 11 ++++++++ .../capture_from_submission.rb | 28 +++++++++++-------- app/views/other_responses/index.html.erb | 17 ++++++++--- app/views/people/_form.html.erb | 2 +- .../people/_other_sector_responses.html.erb | 19 ++++++++++--- app/views/people/show.html.erb | 2 +- spec/models/other_response_spec.rb | 6 ++++ spec/requests/other_responses_spec.rb | 19 +++++++++++++ spec/requests/people_other_responses_spec.rb | 11 ++++++-- .../capture_from_submission_spec.rb | 24 +++++++++------- 12 files changed, 107 insertions(+), 37 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b3ec64f1c..0b3912d09 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,7 +102,7 @@ This codebase (Rails 8.1) | `Story` | Editorial content with facilitators, primary/gallery assets | | `Resource` | Handouts, toolkits, templates with downloadable assets | | `Person` | Organization affiliates with contacts, addresses, sectors | -| `OtherResponse` | A free-text "Other" a person typed on any form question, captured at submission time (registration, scholarship, bulk payment). `field_identifier` records the question; `kind` is derived — `sector` (promotable into a `Sector`, shown on the profile) or `generic` (auxiliary data, admin-review only). Curated at `/other_responses`: `promote` (sectors only), `keep`, or `dismiss`. Org-type "Other" is excluded (owned by the org). | +| `OtherResponse` | A free-text "Other" a person typed on a **promotable** form question, captured at submission time (registration, scholarship, bulk payment). Only sectors are captured today (org-type will follow once `OrganizationType` is a model); every other "Other" stays searchable in the form answers. `field_identifier` records the question so a new one can be switched on later; `kind` is derived (`sector` today). Shown on the profile/edit as a chip (admins deep-link into the review queue from it), and curated at `/other_responses`: `promote`, `keep`, or `dismiss`. | | `Organization` | Groups with affiliations, addresses, logos via ActiveStorage | | `Grant` | Donated funds (polymorphic `donor`: Organization or Person) with eligibility criteria, tasks, deadlines; parent of `Scholarship`. Scholarship totals cannot exceed the grant amount | | `Scholarship` | Award to a `Person`; optionally drawn from a `Grant`, syncs to event registration `Allocation` | @@ -215,7 +215,7 @@ end ### Other responses -- `OtherResponses::CaptureFromSubmission` — Materializes a form submission's free-text "Other" answers as `OtherResponse` records. Runs over every answered field (uses `OtherOption.texts`, which keys strictly on the `Other:` prefix, so named specify options and the CE `Yes: N` box are ignored); excludes org-owned `agency_type`; de-dupes per person + question. Shared by the registration, scholarship, and bulk-payment submission paths +- `OtherResponses::CaptureFromSubmission` — Materializes a form submission's free-text "Other" answers as `OtherResponse` records, but only for **promotable** questions (sectors today; org-type later) — everything else is left searchable in the form answers. Uses `OtherOption.texts`, which keys strictly on the `Other:` prefix, so named specify options and the CE `Yes: N` box are ignored; de-dupes per person + question. Shared by the registration, scholarship, and bulk-payment submission paths ### Organizations diff --git a/app/controllers/other_responses_controller.rb b/app/controllers/other_responses_controller.rb index bd5d2a2bd..1917a6743 100644 --- a/app/controllers/other_responses_controller.rb +++ b/app/controllers/other_responses_controller.rb @@ -91,6 +91,7 @@ def build_group(rows) question_label: question_label_for(first), display_text: first.text, normalized_text: first.normalized_text, + anchor: first.review_anchor, count: rows.size, status_counts: rows.each_with_object(Hash.new(0)) { |r, h| h[r.status] += 1 } } diff --git a/app/models/other_response.rb b/app/models/other_response.rb index fd84c34ae..7b7a74502 100644 --- a/app/models/other_response.rb +++ b/app/models/other_response.rb @@ -55,6 +55,17 @@ def self.normalize(value) value.to_s.strip.downcase end + # Stable DOM id for a review-page group, so a person's chip can deep-link to + # its row. Matches how the review page buckets: sectors by kind, otherwise by + # question. Shared by the controller (row id) and the chip link. + def self.review_anchor(bucket, normalized_text) + "other-#{bucket}-#{normalized_text}".parameterize + end + + def review_anchor + self.class.review_anchor(promotable? ? kind : field_identifier, normalized_text) + end + def promotable? kind.in?(PROMOTABLE_KINDS) end diff --git a/app/services/other_responses/capture_from_submission.rb b/app/services/other_responses/capture_from_submission.rb index 4f9f111bf..8a66f3db3 100644 --- a/app/services/other_responses/capture_from_submission.rb +++ b/app/services/other_responses/capture_from_submission.rb @@ -2,20 +2,17 @@ module OtherResponses # Materializes the free-text "Other" answers on a form submission as # OtherResponse records so they can be curated (promoted/kept/dismissed). # - # Runs over every answered field — not just sectors — because any question can - # offer an "Other" option. OtherOption.texts keys strictly on the "Other:" - # prefix, so named specify options ("Word of Mouth: …") and the CE "Yes: 3" - # box are ignored. De-dupes on the person's normalized value per question, so - # re-submitting the same answer never creates a second row. + # Only questions whose "Other" can eventually become a real tag are captured + # (sectors today; organization type once OrganizationType is a model). Every + # other "Other" — and the org-owned agency_type — is left in the form answers, + # which stay searchable, rather than stored here. The record still carries + # `field_identifier`, so flipping a new question on later is a one-line change + # (add it to a promotable kind). OtherOption.texts keys strictly on the + # "Other:" prefix, so named specify options ("Word of Mouth: …") and the CE + # "Yes: 3" box are ignored. De-dupes per person + question. # # Shared by the registration, scholarship, and bulk-payment submission paths. class CaptureFromSubmission - # Fields whose "Other" belongs to the organization, not the person, so they - # must not land in the person's review queue. Organization type is synced - # onto the org (PublicRegistration#sync_agency_type) and will get its own - # promotable path when OrganizationType becomes a model. - EXCLUDED_FIELD_IDENTIFIERS = %w[agency_type].freeze - def self.call(submission) new(submission).call end @@ -27,7 +24,7 @@ def initialize(submission) def call answers.each do |answer| field_identifier = answer.form_field&.field_identifier - next if field_identifier.blank? || field_identifier.in?(EXCLUDED_FIELD_IDENTIFIERS) + next unless capturable?(field_identifier) OtherOption.texts(answer.submitted_answer).each do |text| capture(field_identifier, text, answer) @@ -37,6 +34,13 @@ def call private + # Capture only the questions whose "Other" is (or will be) promotable — the + # rest stay searchable in the form answers. + def capturable?(field_identifier) + field_identifier.present? && + OtherResponse.kind_for(field_identifier).in?(OtherResponse::PROMOTABLE_KINDS) + end + def answers @submission.form_answers.includes(:form_field) end diff --git a/app/views/other_responses/index.html.erb b/app/views/other_responses/index.html.erb index abbb4d00b..8079d218b 100644 --- a/app/views/other_responses/index.html.erb +++ b/app/views/other_responses/index.html.erb @@ -1,6 +1,15 @@ <% content_for(:page_bg_class, "admin-only bg-blue-100") %> -<%= link_to sectors_path, class: "inline-flex items-center gap-1.5 text-sm text-gray-500 hover:text-gray-700 px-2 py-1 mb-2" do %> - Sectors +<%# Eyebrow returns to wherever the admin came from — a person's profile/edit + when they clicked a chip (return_to + person_id), otherwise the Sectors list. %> +<% back_path, back_label = if params[:return_to] == "person_edit" && params[:person_id].present? + [ edit_person_path(params[:person_id]), "Back to person" ] + elsif params[:return_to] == "person_show" && params[:person_id].present? + [ person_path(params[:person_id]), "Back to person" ] + else + [ sectors_path, "Sectors" ] + end %> +<%= link_to back_path, class: "inline-flex items-center gap-1.5 text-sm text-gray-500 hover:text-gray-700 px-2 py-1 mb-2" do %> + <%= back_label %> <% end %>
@@ -19,7 +28,7 @@
<% [ [ "All", nil ], [ "Pending", "pending" ], [ "Kept", "kept" ] ].each do |label, value| %> <% active = @status_filter == value %> - <%= link_to label, other_responses_path(status: value), + <%= link_to label, other_responses_path(status: value, return_to: params[:return_to], person_id: params[:person_id]), class: "px-3 py-1 rounded-full #{active ? "bg-gray-800 text-white" : "bg-white text-gray-600 border border-gray-300 hover:bg-gray-100"}" %> <% end %>
@@ -37,7 +46,7 @@ <% @groups.each do |group| %> - + <%= group[:question_label] %> <%= group[:display_text] %> diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index fa8934913..9aa1e435c 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -160,7 +160,7 @@ .reject { |_, id| (@current_sector_ids || []).include?(id) }, show_admin_flags: true } }, class: "btn btn-secondary-outline" %> - <%= render "people/other_sector_responses", responses: @person.other_sector_responses, dismissable: true %> + <%= render "people/other_sector_responses", responses: @person.other_sector_responses, dismissable: true, curatable: true, return_to: "person_edit" %>
diff --git a/app/views/people/_other_sector_responses.html.erb b/app/views/people/_other_sector_responses.html.erb index 3a1e27c4d..09d8c9b5c 100644 --- a/app/views/people/_other_sector_responses.html.erb +++ b/app/views/people/_other_sector_responses.html.erb @@ -1,20 +1,31 @@ <%# Chips for a person's free-text "Other" sector responses (OtherResponse - records). Read-only on the profile; on the edit form (dismissable: true) - each chip carries an × that dismisses it (hides it from the profile/edit). + records). For admins (curatable: true) the chip links into the review queue, + anchored to its row, where it can be approved (promoted/kept) or dismissed — + passing return_to so the queue's back link returns here. On the edit form + (dismissable: true) each chip also carries an × for a one-click dismiss. Renders bare chips — the caller supplies the surrounding flex container. %> <% responses ||= [] %> <% dismissable = local_assigns.fetch(:dismissable, false) %> +<% curatable = local_assigns.fetch(:curatable, false) %> +<% return_to = local_assigns.fetch(:return_to, nil) %> <% responses.each do |response| %> - <%= response.text %> + <% if curatable %> + <%= link_to response.text, + other_responses_path(anchor: response.review_anchor, return_to: return_to, person_id: response.person_id), + class: "hover:underline", + title: "Review this response — approve or dismiss" %> + <% else %> + <%= response.text %> + <% end %> (other) <% if dismissable %> <%= link_to "×", other_response_path(response, other_response: { status: "dismissed" }, return_to: "person_edit"), data: { turbo_method: :patch }, class: "ml-2 -mr-1 px-1 leading-none text-gray-400 hover:text-red-600", - title: "Hide this response from the profile" %> + title: "Dismiss this response" %> <% end %> <% end %> diff --git a/app/views/people/show.html.erb b/app/views/people/show.html.erb index f98eb2132..c666bc51e 100644 --- a/app/views/people/show.html.erb +++ b/app/views/people/show.html.erb @@ -167,7 +167,7 @@ display_leader: true, is_leader: si.is_leader %> <% end %> - <%= render "people/other_sector_responses", responses: other_sectors %> + <%= render "people/other_sector_responses", responses: other_sectors, curatable: current_user&.super_user?, return_to: "person_show" %> <% else %>

None selected.

diff --git a/spec/models/other_response_spec.rb b/spec/models/other_response_spec.rb index ffdabe9a5..ad8a45fdd 100644 --- a/spec/models/other_response_spec.rb +++ b/spec/models/other_response_spec.rb @@ -69,6 +69,12 @@ end end + describe "#review_anchor" do + it "buckets sectors by kind, parameterized" do + expect(create(:other_response, text: "Equine Therapy").review_anchor).to eq("other-sector-equine-therapy") + end + end + describe "#dismiss! / #keep!" do it "transitions status" do response = create(:other_response) diff --git a/spec/requests/other_responses_spec.rb b/spec/requests/other_responses_spec.rb index d33f9d17d..74e126619 100644 --- a/spec/requests/other_responses_spec.rb +++ b/spec/requests/other_responses_spec.rb @@ -44,6 +44,25 @@ expect(response.body).to include("Kept value") expect(response.body).not_to include("Pending value") end + + it "anchors each group row for chip deep-links" do + sign_in admin + response_record = create(:other_response, text: "Equine therapy") + + get other_responses_path + + expect(response.body).to include(%(id="#{response_record.review_anchor}")) + end + + it "points the eyebrow back to the person when arrived from their page" do + sign_in admin + person = create(:person) + create(:other_response, person: person, text: "Equine therapy") + + get other_responses_path(return_to: "person_edit", person_id: person.id) + + expect(response.body).to include(edit_person_path(person)) + end end describe "POST /other_responses/curate (bulk)" do diff --git a/spec/requests/people_other_responses_spec.rb b/spec/requests/people_other_responses_spec.rb index c29d1a2ef..db77e1eb4 100644 --- a/spec/requests/people_other_responses_spec.rb +++ b/spec/requests/people_other_responses_spec.rb @@ -38,13 +38,18 @@ def answer(identifier, value) end describe "edit page" do - it "shows the Other sector in the sectors section with a dismiss control" do - create(:other_response, person: person, kind: "sector", text: "Music therapy") + it "shows the Other sector with a dismiss control and a link into the review queue" do + response_record = create(:other_response, person: person, text: "Music therapy") get edit_person_path(person) expect(response.body).to include("Music therapy") - expect(response.body).to include("Hide this response from the profile") + expect(response.body).to include("Dismiss this response") + # The chip deep-links into the review queue, anchored to its row, so the + # admin can approve (promote/keep) or dismiss it there. + expect(response.body).to include("#{other_responses_path}?") + expect(response.body).to include("return_to=person_edit") + expect(response.body).to include(response_record.review_anchor) end it "shows the Other workshop setting near the category checkboxes" do diff --git a/spec/services/other_responses/capture_from_submission_spec.rb b/spec/services/other_responses/capture_from_submission_spec.rb index cb3a48f91..a86207f86 100644 --- a/spec/services/other_responses/capture_from_submission_spec.rb +++ b/spec/services/other_responses/capture_from_submission_spec.rb @@ -10,22 +10,18 @@ def answer(identifier, value) create(:form_answer, form_submission: submission, form_field: field, submitted_answer: value) end - it "captures Other answers from any question, tagged with the field and kind" do + it "captures Other answers from promotable (sector) questions" do answer("additional_sectors", "5, Other: Equine therapy") - answer("how_did_you_hear", "Other: A friend") described_class.call(submission) - responses = person.other_responses.order(:field_identifier) - expect(responses.map { |r| [ r.field_identifier, r.text, r.kind ] }).to contain_exactly( - [ "additional_sectors", "Equine therapy", "sector" ], - [ "how_did_you_hear", "A friend", "generic" ] - ) + response = person.other_responses.sole + expect([ response.field_identifier, response.text, response.kind ]) + .to eq([ "additional_sectors", "Equine therapy", "sector" ]) end - it "ignores answers with no Other free text and named specify options" do - answer("additional_sectors", "5, 12") - answer("how_did_you_hear", "Word of Mouth: Jane") + it "leaves non-promotable questions' Other in the form answers, not captured" do + answer("how_did_you_hear", "Other: A friend") described_class.call(submission) @@ -40,6 +36,14 @@ def answer(identifier, value) expect(person.other_responses).to be_empty end + it "ignores sector answers with no Other free text" do + answer("additional_sectors", "5, 12") + + described_class.call(submission) + + expect(person.other_responses).to be_empty + end + it "de-dupes repeat submissions of the same value per question" do answer("additional_sectors", "Other: Equine therapy") described_class.call(submission) From 4069921a500b3fc0c453921b9ebe3ad00386ea02 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 10:42:14 -0400 Subject: [PATCH 08/14] Store organization-type "Other" too via a polymorphic owner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OtherResponse.owner is now polymorphic: a sector "Other" is owned by the Person (promotable, shown on their profile), an organization-type "Other" is owned by the Organization. Org-type is captured in sync_agency_type (where the org is known), stored now but not promotable — its promote button is hidden until OrganizationType is a model. The review queue groups by kind and labels the org-type group; keep/dismiss work for it, promote stays sector-only. Non-promotable "generic" questions are still not captured (searchable in form answers). field_identifier is retained so any question can be switched on later. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 4 +- app/controllers/other_responses_controller.rb | 29 ++++---- app/models/organization.rb | 1 + app/models/other_response.rb | 73 ++++++++++++------- app/models/person.rb | 2 +- .../public_registration.rb | 13 ++++ .../capture_from_submission.rb | 22 +++--- .../people/_other_sector_responses.html.erb | 2 +- .../20260705014639_create_other_responses.rb | 6 +- db/schema.rb | 8 +- spec/factories/other_responses.rb | 7 +- spec/models/other_response_spec.rb | 18 ++++- spec/models/person_spec.rb | 8 +- spec/requests/other_responses_spec.rb | 43 ++++++++--- spec/requests/people_other_responses_spec.rb | 6 +- .../public_registration_spec.rb | 14 ++++ 16 files changed, 169 insertions(+), 87 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0b3912d09..a38b65072 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,7 +102,7 @@ This codebase (Rails 8.1) | `Story` | Editorial content with facilitators, primary/gallery assets | | `Resource` | Handouts, toolkits, templates with downloadable assets | | `Person` | Organization affiliates with contacts, addresses, sectors | -| `OtherResponse` | A free-text "Other" a person typed on a **promotable** form question, captured at submission time (registration, scholarship, bulk payment). Only sectors are captured today (org-type will follow once `OrganizationType` is a model); every other "Other" stays searchable in the form answers. `field_identifier` records the question so a new one can be switched on later; `kind` is derived (`sector` today). Shown on the profile/edit as a chip (admins deep-link into the review queue from it), and curated at `/other_responses`: `promote`, `keep`, or `dismiss`. | +| `OtherResponse` | A free-text "Other" typed on a form question, captured at submission time (registration, scholarship, bulk payment). Polymorphic `owner`: a **sector** "Other" is owned by the `Person` (promotable into a `Sector`, shown on their profile/edit chip); an **organization_type** "Other" is owned by the `Organization` (stored now, not promotable until `OrganizationType` is a model). `generic` questions aren't captured — that stays searchable in the form answers. `field_identifier` records the question; `kind` is derived. Curated at `/other_responses` (grouped by kind/question): `promote` (sectors only), `keep`, `dismiss`. Admins deep-link there from a person's chip. | | `Organization` | Groups with affiliations, addresses, logos via ActiveStorage | | `Grant` | Donated funds (polymorphic `donor`: Organization or Person) with eligibility criteria, tasks, deadlines; parent of `Scholarship`. Scholarship totals cannot exceed the grant amount | | `Scholarship` | Award to a `Person`; optionally drawn from a `Grant`, syncs to event registration `Allocation` | @@ -215,7 +215,7 @@ end ### Other responses -- `OtherResponses::CaptureFromSubmission` — Materializes a form submission's free-text "Other" answers as `OtherResponse` records, but only for **promotable** questions (sectors today; org-type later) — everything else is left searchable in the form answers. Uses `OtherOption.texts`, which keys strictly on the `Other:` prefix, so named specify options and the CE `Yes: N` box are ignored; de-dupes per person + question. Shared by the registration, scholarship, and bulk-payment submission paths +- `OtherResponses::CaptureFromSubmission` — Materializes a form submission's **person-owned** "Other" answers (sectors) as `OtherResponse` records; org-type "Other" is captured separately in `PublicRegistration#sync_agency_type` (owned by the org). Uses `OtherOption.texts`, which keys strictly on the `Other:` prefix, so named specify options and the CE `Yes: N` box are ignored; de-dupes per owner + question. Shared by the registration, scholarship, and bulk-payment submission paths ### Organizations diff --git a/app/controllers/other_responses_controller.rb b/app/controllers/other_responses_controller.rb index 1917a6743..d1c5904cc 100644 --- a/app/controllers/other_responses_controller.rb +++ b/app/controllers/other_responses_controller.rb @@ -9,10 +9,10 @@ def index authorize! @status_filter = params[:status].presence_in(OtherResponse::VISIBLE_STATUSES) statuses = @status_filter ? [ @status_filter ] : OtherResponse::VISIBLE_STATUSES - responses = OtherResponse.where(status: statuses).includes(:person, :source_form_answer) + responses = OtherResponse.where(status: statuses).includes(:owner, :source_form_answer) @groups = responses - .group_by { |response| [ group_bucket(response), response.normalized_text ] } + .group_by { |response| [ response.group_key, response.normalized_text ] } .map { |_key, rows| build_group(rows) } .sort_by { |group| [ group[:promotable] ? 0 : 1, group[:question_label].downcase, -group[:count], group[:display_text].downcase ] } @@ -45,7 +45,7 @@ def update @other_response.update!(status: status) if OtherResponse::STATUSES.include?(status) if params[:return_to] == "person_edit" - redirect_to edit_person_path(@other_response.person), status: :see_other + redirect_to edit_person_path(@other_response.owner), status: :see_other else redirect_to other_responses_path, status: :see_other end @@ -61,8 +61,8 @@ def promote return redirect_to other_responses_path, alert: "Pick or name a sector to promote to." unless sector responses = group_scope.sectors.promotable_now - responses.includes(:person).find_each do |response| - response.person.tag_sectors(primary_ids: [], additional_ids: [ sector.id ]) + responses.includes(:owner).find_each do |response| + response.owner.tag_sectors(primary_ids: [], additional_ids: [ sector.id ]) response.update!(status: "promoted", promotable: sector) end @@ -76,12 +76,6 @@ def set_other_response @other_response = OtherResponse.find(params[:id]) end - # Group key: sector "Other"s share one bucket (kind), every other question is - # keyed by its field so distinct questions never merge. - def group_bucket(response) - response.promotable? ? response.kind : "field:#{response.field_identifier}" - end - def build_group(rows) first = rows.first { @@ -98,16 +92,19 @@ def build_group(rows) end def question_label_for(response) - return "Sectors" if response.promotable? - response.source_form_answer&.question_name_when_answered.presence || response.field_identifier.humanize + case response.kind + when "sector" then "Sectors" + when "organization_type" then "Organization type" + else response.source_form_answer&.question_name_when_answered.presence || response.field_identifier.humanize + end end # The set of responses a curate/promote action targets, matching how the group - # was bucketed: sectors by kind, everything else by field. + # was bucketed: captured kinds by kind, generic questions by field. def group_scope scope = OtherResponse.where(normalized_text: OtherResponse.normalize(params[:normalized_text])) - if params[:kind] == "sector" - scope.sectors + if params[:kind].present? && params[:kind] != "generic" + scope.where(kind: params[:kind]) else scope.where(field_identifier: params[:field_identifier]) end diff --git a/app/models/organization.rb b/app/models/organization.rb index fb579164c..007a9c03c 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -6,6 +6,7 @@ class Organization < ApplicationRecord belongs_to :windows_type, optional: true has_many :addresses, as: :addressable, dependent: :destroy has_many :bookmarks, as: :bookmarkable, dependent: :destroy + has_many :other_responses, as: :owner, dependent: :destroy has_many :affiliations, dependent: :restrict_with_error, inverse_of: :organization has_many :event_registration_organizations, dependent: :restrict_with_error has_many :event_registrations, through: :event_registration_organizations diff --git a/app/models/other_response.rb b/app/models/other_response.rb index 7b7a74502..a798af9cc 100644 --- a/app/models/other_response.rb +++ b/app/models/other_response.rb @@ -1,22 +1,33 @@ class OtherResponse < ApplicationRecord - # A free-text "Other" a person typed on any tag/option-backed form question, - # captured at submission time so a curator can promote / keep / dismiss it. - # `field_identifier` records which question it came from; `kind` is the coarse - # category that decides whether it can be promoted into a real tag. + # A free-text "Other" someone typed on a form question whose "Other" can (or + # will) become a real tag. Captured at submission time so a curator can + # promote / keep / dismiss it. The `owner` is polymorphic because the answer's + # subject differs by question: a sector "Other" is about the person, an + # organization-type "Other" is about their organization. # - # - "sector" → the additional-sectors question; promotable into a Sector, and - # shown on the person's profile beside the sector tags. - # - "generic" → any other question's "Other"; auxiliary data reviewed by admins - # (keep/dismiss only), never shown on a profile. + # - `sector` → owned by a Person; promotable into a `Sector`, shown + # on the person's profile beside the sector tags. + # - `organization_type` → owned by an Organization; stored now, but not yet + # promotable (its promote button is hidden until + # `OrganizationType` is a model). # - # Organization-type "Other" is deliberately NOT captured here — it belongs to - # the organization (see PublicRegistration#sync_agency_type) and will get its - # own promotable path once OrganizationType is a model. - KINDS = %w[sector generic].freeze + # Questions whose "Other" will never become a tag (`generic`) are not captured + # at all — that data stays searchable in the form answers. + KINDS = %w[sector organization_type generic].freeze - # Kinds that can be promoted into a real tag record. + # Kinds we materialize as records (the rest stay in the form answers). + CAPTURED_KINDS = %w[sector organization_type].freeze + + # Kinds captured against a Person, via their form submission. + PERSON_KINDS = %w[sector].freeze + + # Kinds that can be promoted into a real tag today. PROMOTABLE_KINDS = %w[sector].freeze + # The organization-type question. Its "Other" is org-owned (see + # PublicRegistration#sync_agency_type). + ORGANIZATION_TYPE_FIELD_IDENTIFIER = "agency_type" + # A response starts life as `pending` (awaiting a curator's decision) and is # then either promoted into a real tag, kept as auxiliary data, or dismissed. STATUSES = %w[pending kept promoted dismissed].freeze @@ -25,7 +36,7 @@ class OtherResponse < ApplicationRecord # in the review queue). VISIBLE_STATUSES = %w[pending kept].freeze - belongs_to :person + belongs_to :owner, polymorphic: true belongs_to :promotable, polymorphic: true, optional: true belongs_to :source_form_answer, class_name: "FormAnswer", optional: true @@ -36,40 +47,50 @@ class OtherResponse < ApplicationRecord validates :text, presence: true validates :kind, inclusion: { in: KINDS } validates :status, inclusion: { in: STATUSES } - validates :normalized_text, uniqueness: { scope: [ :person_id, :field_identifier ] } + validates :normalized_text, uniqueness: { scope: [ :owner_type, :owner_id, :field_identifier ] } scope :sectors, -> { where(kind: "sector") } scope :visible, -> { where(status: VISIBLE_STATUSES) } scope :pending, -> { where(status: "pending") } scope :promotable_now, -> { where.not(status: "dismissed") } - # The coarse category for a question. Sector fields promote into Sectors; - # everything else is generic auxiliary data. + # The coarse category (and thus owner + promotability) for a question. def self.kind_for(field_identifier) - field_identifier.to_s.in?(FormField::SECTOR_FIELD_IDENTIFIERS) ? "sector" : "generic" + identifier = field_identifier.to_s + if identifier.in?(FormField::SECTOR_FIELD_IDENTIFIERS) + "sector" + elsif identifier == ORGANIZATION_TYPE_FIELD_IDENTIFIER + "organization_type" + else + "generic" + end end # Case/whitespace-insensitive key used both for the unique index and for - # grouping the same typed value across many people on the review page. + # grouping the same typed value across owners on the review page. def self.normalize(value) value.to_s.strip.downcase end - # Stable DOM id for a review-page group, so a person's chip can deep-link to - # its row. Matches how the review page buckets: sectors by kind, otherwise by - # question. Shared by the controller (row id) and the chip link. + # Stable DOM id for a review-page group, so a chip can deep-link to its row. def self.review_anchor(bucket, normalized_text) "other-#{bucket}-#{normalized_text}".parameterize end - def review_anchor - self.class.review_anchor(promotable? ? kind : field_identifier, normalized_text) - end - def promotable? kind.in?(PROMOTABLE_KINDS) end + # How the review page buckets this response: captured kinds group by kind (all + # sector "Other"s together, all org-type together); generic groups by question. + def group_key + kind == "generic" ? field_identifier : kind + end + + def review_anchor + self.class.review_anchor(group_key, normalized_text) + end + def dismiss! update!(status: "dismissed") end diff --git a/app/models/person.rb b/app/models/person.rb index 9d5fe9609..0d2032be5 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -20,7 +20,7 @@ class Person < ApplicationRecord has_many :categorizable_items, inverse_of: :categorizable, as: :categorizable, dependent: :destroy has_many :notifications, as: :noticeable, dependent: :destroy has_many :sectorable_items, as: :sectorable, dependent: :destroy - has_many :other_responses, dependent: :destroy + has_many :other_responses, as: :owner, dependent: :destroy has_many :stories_as_spotlighted_facilitator, inverse_of: :spotlighted_facilitator, class_name: "Story", dependent: :restrict_with_error has_many :stories_as_author, inverse_of: :author, class_name: "Story", foreign_key: :author_id, diff --git a/app/services/event_registration_services/public_registration.rb b/app/services/event_registration_services/public_registration.rb index 53ca1d7d7..e58566b27 100644 --- a/app/services/event_registration_services/public_registration.rb +++ b/app/services/event_registration_services/public_registration.rb @@ -217,6 +217,19 @@ def sync_agency_type(organization) return if label.blank? other_text = FormField.other_option?(label) ? specified.strip.presence : nil organization.update!(agency_type: label, agency_type_other: other_text) + capture_organization_type_other(organization, other_text) + end + + # Materialize the org-type "Other" as an OtherResponse owned by the org, so it + # joins the curation queue alongside sector "Other"s. Not promotable yet (no + # OrganizationType model), but stored now so nothing is lost; de-duped per org. + def capture_organization_type_other(organization, text) + return if text.blank? + + organization.other_responses.find_or_create_by!( + field_identifier: OtherResponse::ORGANIZATION_TYPE_FIELD_IDENTIFIER, + normalized_text: OtherResponse.normalize(text) + ) { |response| response.text = text } end # Write value onto attribute when a non-blank value was submitted, overwriting diff --git a/app/services/other_responses/capture_from_submission.rb b/app/services/other_responses/capture_from_submission.rb index 8a66f3db3..26f4f480a 100644 --- a/app/services/other_responses/capture_from_submission.rb +++ b/app/services/other_responses/capture_from_submission.rb @@ -2,14 +2,14 @@ module OtherResponses # Materializes the free-text "Other" answers on a form submission as # OtherResponse records so they can be curated (promoted/kept/dismissed). # - # Only questions whose "Other" can eventually become a real tag are captured - # (sectors today; organization type once OrganizationType is a model). Every - # other "Other" — and the org-owned agency_type — is left in the form answers, - # which stay searchable, rather than stored here. The record still carries - # `field_identifier`, so flipping a new question on later is a one-line change - # (add it to a promotable kind). OtherOption.texts keys strictly on the - # "Other:" prefix, so named specify options ("Word of Mouth: …") and the CE - # "Yes: 3" box are ignored. De-dupes per person + question. + # Captures the person-owned "Other" answers (sectors today). Organization-type + # "Other" is owned by the org and captured separately, where the org is known + # (PublicRegistration#sync_agency_type). Questions whose "Other" will never + # become a tag are left in the form answers, which stay searchable, rather than + # stored here — but the record still carries `field_identifier`, so switching a + # new question on later is a one-line change. OtherOption.texts keys strictly + # on the "Other:" prefix, so named specify options ("Word of Mouth: …") and the + # CE "Yes: 3" box are ignored. De-dupes per person + question. # # Shared by the registration, scholarship, and bulk-payment submission paths. class CaptureFromSubmission @@ -34,11 +34,11 @@ def call private - # Capture only the questions whose "Other" is (or will be) promotable — the - # rest stay searchable in the form answers. + # Capture only the person-owned "Other" questions here — the rest stay + # searchable in the form answers (or, for org-type, are captured elsewhere). def capturable?(field_identifier) field_identifier.present? && - OtherResponse.kind_for(field_identifier).in?(OtherResponse::PROMOTABLE_KINDS) + OtherResponse.kind_for(field_identifier).in?(OtherResponse::PERSON_KINDS) end def answers diff --git a/app/views/people/_other_sector_responses.html.erb b/app/views/people/_other_sector_responses.html.erb index 09d8c9b5c..2ed933bc3 100644 --- a/app/views/people/_other_sector_responses.html.erb +++ b/app/views/people/_other_sector_responses.html.erb @@ -13,7 +13,7 @@ title="Other response entered during registration"> <% if curatable %> <%= link_to response.text, - other_responses_path(anchor: response.review_anchor, return_to: return_to, person_id: response.person_id), + other_responses_path(anchor: response.review_anchor, return_to: return_to, person_id: response.owner_id), class: "hover:underline", title: "Review this response — approve or dismiss" %> <% else %> diff --git a/db/migrate/20260705014639_create_other_responses.rb b/db/migrate/20260705014639_create_other_responses.rb index cf5d14d9f..1c6244b99 100644 --- a/db/migrate/20260705014639_create_other_responses.rb +++ b/db/migrate/20260705014639_create_other_responses.rb @@ -8,7 +8,7 @@ def up return if table_exists?(:other_responses) create_table :other_responses do |t| - t.references :person, null: false, foreign_key: true + t.references :owner, polymorphic: true, null: false t.string :field_identifier, null: false t.string :kind, null: false t.string :text, null: false @@ -19,8 +19,8 @@ def up t.timestamps end - add_index :other_responses, [ :person_id, :field_identifier, :normalized_text ], - unique: true, name: "index_other_responses_on_person_field_text" + add_index :other_responses, [ :owner_type, :owner_id, :field_identifier, :normalized_text ], + unique: true, name: "index_other_responses_on_owner_field_text" end def down diff --git a/db/schema.rb b/db/schema.rb index a0b5cdd47..a3721d9b8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -839,15 +839,16 @@ t.string "field_identifier", null: false t.string "kind", null: false t.string "normalized_text", null: false - t.bigint "person_id", null: false + t.bigint "owner_id", null: false + t.string "owner_type", null: false t.bigint "promotable_id" t.string "promotable_type" t.bigint "source_form_answer_id" t.string "status", default: "pending", null: false t.string "text", null: false t.datetime "updated_at", null: false - t.index ["person_id", "field_identifier", "normalized_text"], name: "index_other_responses_on_person_field_text", unique: true - t.index ["person_id"], name: "index_other_responses_on_person_id" + t.index ["owner_type", "owner_id", "field_identifier", "normalized_text"], name: "index_other_responses_on_owner_field_text", unique: true + t.index ["owner_type", "owner_id"], name: "index_other_responses_on_owner" t.index ["promotable_type", "promotable_id"], name: "index_other_responses_on_promotable" t.index ["source_form_answer_id"], name: "index_other_responses_on_source_form_answer_id" end @@ -1757,7 +1758,6 @@ add_foreign_key "organizations", "organization_statuses" add_foreign_key "organizations", "windows_types" add_foreign_key "other_responses", "form_answers", column: "source_form_answer_id" - add_foreign_key "other_responses", "people" add_foreign_key "pay_charges", "pay_customers", column: "customer_id" add_foreign_key "pay_charges", "pay_subscriptions", column: "subscription_id" add_foreign_key "pay_payment_methods", "pay_customers", column: "customer_id" diff --git a/spec/factories/other_responses.rb b/spec/factories/other_responses.rb index 8a20b089d..32d259e7e 100644 --- a/spec/factories/other_responses.rb +++ b/spec/factories/other_responses.rb @@ -1,11 +1,16 @@ FactoryBot.define do factory :other_response do - person + association :owner, factory: :person field_identifier { "additional_sectors" } sequence(:text) { |n| "Equine therapy #{n}" } status { "pending" } # kind is derived from field_identifier (sector fields → "sector"). + trait :organization_type do + association :owner, factory: :organization + field_identifier { "agency_type" } + end + trait :generic do field_identifier { "how_did_you_hear" } end diff --git a/spec/models/other_response_spec.rb b/spec/models/other_response_spec.rb index ad8a45fdd..3f2902fd1 100644 --- a/spec/models/other_response_spec.rb +++ b/spec/models/other_response_spec.rb @@ -16,16 +16,16 @@ it "is unique per person + field + normalized text" do person = create(:person) - create(:other_response, person: person, field_identifier: "additional_sectors", text: "Equine therapy") - dup = build(:other_response, person: person, field_identifier: "additional_sectors", text: " equine therapy ") + create(:other_response, owner: person, field_identifier: "additional_sectors", text: "Equine therapy") + dup = build(:other_response, owner: person, field_identifier: "additional_sectors", text: " equine therapy ") expect(dup).not_to be_valid end it "allows the same text on a different question" do person = create(:person) - create(:other_response, person: person, field_identifier: "additional_sectors", text: "Equine therapy") - other_question = build(:other_response, person: person, field_identifier: "how_did_you_hear", text: "Equine therapy") + create(:other_response, owner: person, field_identifier: "additional_sectors", text: "Equine therapy") + other_question = build(:other_response, owner: person, field_identifier: "how_did_you_hear", text: "Equine therapy") expect(other_question).to be_valid end @@ -41,6 +41,10 @@ expect(create(:other_response, field_identifier: "additional_sectors").kind).to eq("sector") end + it "is organization_type for the agency_type field" do + expect(create(:other_response, :organization_type).kind).to eq("organization_type") + end + it "is generic for any other field" do expect(create(:other_response, field_identifier: "how_did_you_hear").kind).to eq("generic") end @@ -66,6 +70,12 @@ it "only sector responses are promotable" do expect(create(:other_response).promotable?).to be(true) expect(create(:other_response, :generic).promotable?).to be(false) + expect(create(:other_response, :organization_type).promotable?).to be(false) + end + + it "an organization owns its organization-type response" do + response = create(:other_response, :organization_type) + expect(response.owner).to be_a(Organization) end end diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 5a231eeaa..2ba7b5675 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -422,16 +422,16 @@ def answer(identifier, value) describe "#other_sector_responses" do it "returns the person's visible sector OtherResponses" do - create(:other_response, person: person, kind: "sector", text: "Equine therapy") - create(:other_response, :kept, person: person, kind: "sector", text: "Music therapy") + create(:other_response, owner: person, kind: "sector", text: "Equine therapy") + create(:other_response, :kept, owner: person, kind: "sector", text: "Music therapy") expect(person.other_sector_responses.map(&:text)) .to contain_exactly("Equine therapy", "Music therapy") end it "omits dismissed and promoted responses" do - create(:other_response, :dismissed, person: person, kind: "sector", text: "Hidden") - create(:other_response, :promoted, person: person, kind: "sector", text: "Promoted") + create(:other_response, :dismissed, owner: person, kind: "sector", text: "Hidden") + create(:other_response, :promoted, owner: person, kind: "sector", text: "Promoted") expect(person.other_sector_responses).to be_empty end diff --git a/spec/requests/other_responses_spec.rb b/spec/requests/other_responses_spec.rb index 74e126619..7f753a5a2 100644 --- a/spec/requests/other_responses_spec.rb +++ b/spec/requests/other_responses_spec.rb @@ -13,9 +13,9 @@ sign_in admin alice = create(:person) bob = create(:person) - create(:other_response, person: alice, text: "Equine therapy") - create(:other_response, person: bob, text: "equine therapy") - create(:other_response, :dismissed, person: bob, text: "Hidden one") + create(:other_response, owner: alice, text: "Equine therapy") + create(:other_response, owner: bob, text: "equine therapy") + create(:other_response, :dismissed, owner: bob, text: "Hidden one") get other_responses_path @@ -34,6 +34,17 @@ expect(response.body).to include("How did you hear") end + it "shows organization-type responses labelled, with keep/dismiss but no promote" do + sign_in admin + create(:other_response, :organization_type, text: "Nonprofit collective") + + get other_responses_path + + expect(response.body).to include("Nonprofit collective") + expect(response.body).to include("Organization type") + expect(response.body).to include("Dismiss all") + end + it "filters to a single status" do sign_in admin create(:other_response, text: "Pending value") @@ -57,7 +68,7 @@ it "points the eyebrow back to the person when arrived from their page" do sign_in admin person = create(:person) - create(:other_response, person: person, text: "Equine therapy") + create(:other_response, owner: person, text: "Equine therapy") get other_responses_path(return_to: "person_edit", person_id: person.id) @@ -97,6 +108,16 @@ expect(sector.reload.status).to eq("pending") end + it "dismisses an organization-type group by kind" do + sign_in admin + org_response = create(:other_response, :organization_type, text: "Nonprofit collective") + + post curate_other_responses_path, + params: { kind: "organization_type", field_identifier: "agency_type", normalized_text: "nonprofit collective", status: "dismissed" } + + expect(org_response.reload.status).to eq("dismissed") + end + it "rejects an unsupported status" do sign_in admin response_record = create(:other_response, text: "Equine therapy") @@ -117,7 +138,7 @@ patch other_response_path(response_record), params: { other_response: { status: "dismissed" }, return_to: "person_edit" } - expect(response).to redirect_to(edit_person_path(response_record.person)) + expect(response).to redirect_to(edit_person_path(response_record.owner)) expect(response_record.reload.status).to eq("dismissed") end end @@ -133,22 +154,22 @@ it "tags every non-dismissed person and marks the responses promoted" do sign_in admin sector = create(:sector, name: "Equine Therapy") - kept = create(:other_response, person: create(:person), text: "Equine therapy") - dismissed = create(:other_response, :dismissed, person: create(:person), text: "Equine therapy") + kept = create(:other_response, owner: create(:person), text: "Equine therapy") + dismissed = create(:other_response, :dismissed, owner: create(:person), text: "Equine therapy") post promote_other_responses_path, params: { kind: "sector", normalized_text: "equine therapy", sector_id: sector.id } expect(kept.reload.status).to eq("promoted") - expect(kept.person.sectors).to include(sector) + expect(kept.owner.sectors).to include(sector) expect(dismissed.reload.status).to eq("dismissed") - expect(dismissed.person.sectors).not_to include(sector) + expect(dismissed.owner.sectors).not_to include(sector) end it "mints a new published sector when given a name" do sign_in admin person = create(:person) - create(:other_response, person: person, text: "Equine therapy") + create(:other_response, owner: person, text: "Equine therapy") expect { post promote_other_responses_path, @@ -169,7 +190,7 @@ params: { field_identifier: "how_did_you_hear", normalized_text: "a friend", sector_id: sector.id } expect(generic.reload.status).to eq("pending") - expect(generic.person.sectors).not_to include(sector) + expect(generic.owner.sectors).not_to include(sector) end end end diff --git a/spec/requests/people_other_responses_spec.rb b/spec/requests/people_other_responses_spec.rb index db77e1eb4..a0bae47b9 100644 --- a/spec/requests/people_other_responses_spec.rb +++ b/spec/requests/people_other_responses_spec.rb @@ -16,7 +16,7 @@ def answer(identifier, value) describe "profile page" do it "shows the Other sector as a free-text chip, not the primary sector" do person.update!(profile_show_sectors: true) - create(:other_response, person: person, kind: "sector", text: "Equine therapy") + create(:other_response, owner: person, kind: "sector", text: "Equine therapy") get person_path(person) @@ -29,7 +29,7 @@ def answer(identifier, value) it "hides a dismissed Other sector" do person.update!(profile_show_sectors: true) - create(:other_response, :dismissed, person: person, kind: "sector", text: "Equine therapy") + create(:other_response, :dismissed, owner: person, kind: "sector", text: "Equine therapy") get person_path(person) @@ -39,7 +39,7 @@ def answer(identifier, value) describe "edit page" do it "shows the Other sector with a dismiss control and a link into the review queue" do - response_record = create(:other_response, person: person, text: "Music therapy") + response_record = create(:other_response, owner: person, text: "Music therapy") get edit_person_path(person) diff --git a/spec/services/event_registration_services/public_registration_spec.rb b/spec/services/event_registration_services/public_registration_spec.rb index b5cf5164e..4a75db2ab 100644 --- a/spec/services/event_registration_services/public_registration_spec.rb +++ b/spec/services/event_registration_services/public_registration_spec.rb @@ -342,6 +342,20 @@ def register_with_agency_type(value) expect(answer.submitted_answer).to eq("Other: Equine therapy") end + it "captures the org-type 'Other' as an OtherResponse owned by the organization" do + register_with_agency_type("Other: Equine therapy") + + response = organization.other_responses.sole + expect([ response.text, response.kind, response.promotable? ]) + .to eq([ "Equine therapy", "organization_type", false ]) + end + + it "does not capture an OtherResponse for a non-'Other' classification" do + register_with_agency_type("501c3/nonprofit") + + expect(organization.other_responses).to be_empty + end + it "stores a non-'Other' classification with no agency_type_other" do register_with_agency_type("501c3/nonprofit") From 2175eeeb111be5a64913d238a895bdbdf5a54f9f Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 8 Jul 2026 10:59:13 -0400 Subject: [PATCH 09/14] Keep the review-queue actions on one row The promote form wrapped its button onto a second line and pushed Keep all / Dismiss all to a third. Put all actions in one flex row: the promote controls stay together (constrained input widths, nowrap), with Keep all / Dismiss all inline after them. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/other_responses/index.html.erb | 40 ++++++++++++------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/app/views/other_responses/index.html.erb b/app/views/other_responses/index.html.erb index 8079d218b..685d6cc75 100644 --- a/app/views/other_responses/index.html.erb +++ b/app/views/other_responses/index.html.erb @@ -56,31 +56,31 @@ <%= group[:count] %> - <% if group[:promotable] %> - <%= form_with url: promote_other_responses_path, method: :post, - class: "flex flex-wrap items-center gap-2" do %> - <%= hidden_field_tag :kind, group[:kind] %> - <%= hidden_field_tag :field_identifier, group[:field_identifier] %> - <%= hidden_field_tag :normalized_text, group[:normalized_text] %> - <%= select_tag :sector_id, - options_from_collection_for_select(@sectors, :id, :name), - include_blank: "Existing sector…", - class: "rounded-md border-gray-300 text-sm" %> - or - <%= text_field_tag :new_sector_name, group[:display_text], - placeholder: "New sector name", - class: "rounded-md border-gray-300 text-sm" %> - <%= submit_tag "Promote", class: "btn btn-primary-outline text-sm", - data: { turbo_confirm: "Promote “#{group[:display_text]}” for all #{group[:count]} people?" } %> +
+ <% if group[:promotable] %> + <%= form_with url: promote_other_responses_path, method: :post, + class: "flex items-center gap-2" do %> + <%= hidden_field_tag :kind, group[:kind] %> + <%= hidden_field_tag :field_identifier, group[:field_identifier] %> + <%= hidden_field_tag :normalized_text, group[:normalized_text] %> + <%= select_tag :sector_id, + options_from_collection_for_select(@sectors, :id, :name), + include_blank: "Existing sector…", + class: "w-40 rounded-md border-gray-300 text-sm" %> + or + <%= text_field_tag :new_sector_name, group[:display_text], + placeholder: "New sector name", + class: "w-36 rounded-md border-gray-300 text-sm" %> + <%= submit_tag "Promote", class: "btn btn-primary-outline text-sm whitespace-nowrap", + data: { turbo_confirm: "Promote “#{group[:display_text]}” for all #{group[:count]} people?" } %> + <% end %> <% end %> - <% end %> -
text-sm"> <%= button_to "Keep all", curate_other_responses_path(kind: group[:kind], field_identifier: group[:field_identifier], normalized_text: group[:normalized_text], status: "kept", return_status: @status_filter), - class: "text-gray-500 hover:text-gray-700" %> + class: "text-gray-500 hover:text-gray-700 whitespace-nowrap" %> <%= button_to "Dismiss all", curate_other_responses_path(kind: group[:kind], field_identifier: group[:field_identifier], normalized_text: group[:normalized_text], status: "dismissed", return_status: @status_filter), - class: "text-gray-500 hover:text-red-600", + class: "text-gray-500 hover:text-red-600 whitespace-nowrap", form: { data: { turbo_confirm: "Dismiss “#{group[:display_text]}” for all #{group[:count]} people?" } } %>
From 796aa97af73a32c752630430c7bd94a9d507c014 Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 14 Jul 2026 06:05:04 -0400 Subject: [PATCH 10/14] Never save the "Other" catch-all as a real sector tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Other" is the free-text catch-all (captured as an OtherResponse), not a selectable sector — but the person sector picker was the one selectable-sector list built without excluding_other, so it could be added and saved as the literal "Other" sector tag. - People picker now uses excluding_other, like every other selectable list. - tag_sectors (Person/Org) drops the Other sector id defensively. - Profile + edit drop a stray legacy "Other" tag from the displayed sectors. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/controllers/people_controller.rb | 5 ++++- app/models/concerns/sectors_taggable.rb | 5 ++++- app/views/people/_form.html.erb | 5 ++++- app/views/people/show.html.erb | 4 +++- spec/models/concerns/sectors_taggable_spec.rb | 9 +++++++++ 5 files changed, 24 insertions(+), 4 deletions(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index a314ef04a..192878248 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -296,7 +296,10 @@ def set_form_variables end @person.affiliations.build if @person.affiliations.empty? - @all_sectors = Sector.published.order(:name) + # "Other" is the free-text catch-all, not a real tag (see Sector), so it's + # never offered in the sector picker — a typed "Other" is captured as an + # OtherResponse, not saved as the literal "Other" sector. + @all_sectors = Sector.published.excluding_other.order(:name) @sectors_collection = @all_sectors.pluck(:name, :id) @current_sector_ids = @person.sectorable_items.map(&:sector_id) diff --git a/app/models/concerns/sectors_taggable.rb b/app/models/concerns/sectors_taggable.rb index 14b6bad2e..dde5e2ca7 100644 --- a/app/models/concerns/sectors_taggable.rb +++ b/app/models/concerns/sectors_taggable.rb @@ -53,7 +53,10 @@ def demote_unselected_primary_sectors(primary_ids) end def upsert_sector_items(sector_ids, is_primary:) - Sector.where(id: sector_ids).find_each do |sector| + # "Other" is the free-text catch-all, never a real tag — a typed "Other" is + # captured as an OtherResponse, so it must not become a SectorableItem even + # if its id slips into the submitted set. + Sector.where(id: sector_ids).excluding_other.find_each do |sector| item = sectorable_items.find_or_initialize_by(sector: sector) item.is_primary = is_primary item.save! diff --git a/app/views/people/_form.html.erb b/app/views/people/_form.html.erb index 9aa1e435c..948c079c1 100644 --- a/app/views/people/_form.html.erb +++ b/app/views/people/_form.html.erb @@ -147,7 +147,10 @@ data-primary-tag-primary-class="border-lime-500 bg-lime-200" data-primary-tag-default-class="border-gray-300 bg-white"> <% sectors_owner = f.object.respond_to?(:object) ? f.object.object : f.object %> - <%= f.simple_fields_for :sectorable_items, sectors_owner.sectorable_items_ordered do |sfi| %> + <%# "Other" is never a real sector tag (it's captured as an OtherResponse), + so drop any stray legacy tagging from the editable list. %> + <% editable_sector_items = sectors_owner.sectorable_items_ordered.reject { |item| item.sector&.name == Sector::OTHER_SECTOR_NAME } %> + <%= f.simple_fields_for :sectorable_items, editable_sector_items do |sfi| %> <%= render "shared/sectorable_item_fields", f: sfi, show_admin_flags: true %> <% end %> diff --git a/app/views/people/show.html.erb b/app/views/people/show.html.erb index c666bc51e..3eb046512 100644 --- a/app/views/people/show.html.erb +++ b/app/views/people/show.html.erb @@ -143,7 +143,9 @@
<% end %> - <% sectorable_items = @person.sectorable_items.includes(:sector).sort_by { |si| [ si.is_primary? ? 0 : 1, si.sector&.name.to_s.downcase ] } %> + <%# "Other" is never a real sector tag (captured as an OtherResponse), so a + stray legacy tagging is dropped from the displayed list. %> + <% sectorable_items = @person.sectorable_items.includes(:sector).reject { |si| si.sector&.name == Sector::OTHER_SECTOR_NAME }.sort_by { |si| [ si.is_primary? ? 0 : 1, si.sector&.name.to_s.downcase ] } %> <% other_sectors = @person.other_sector_responses %> <% show_sectors = @person.profile_show_sectors? %> <% primary_age_groups = @person.primary_age_groups.to_a %> diff --git a/spec/models/concerns/sectors_taggable_spec.rb b/spec/models/concerns/sectors_taggable_spec.rb index ec6e114af..0295083fe 100644 --- a/spec/models/concerns/sectors_taggable_spec.rb +++ b/spec/models/concerns/sectors_taggable_spec.rb @@ -18,6 +18,15 @@ expect(person.sectorable_items.find_by(sector: housing).is_primary).to be false end + it "never tags the 'Other' catch-all sector, even if its id is submitted" do + other = create(:sector, name: Sector::OTHER_SECTOR_NAME) + + person.tag_sectors(primary_ids: [ health.id ], additional_ids: [ education.id, other.id ]) + + expect(person.sectors).to include(health, education) + expect(person.sectors).not_to include(other) + end + it "treats a sector named in both lists as primary" do person.tag_sectors(primary_ids: [ health.id ], additional_ids: [ health.id, education.id ]) From 118aefe4b8bf583cac5ad6bbe07a081488e794a3 Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 14 Jul 2026 06:05:04 -0400 Subject: [PATCH 11/14] Report "Other" sectors on the event background page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pulling "Other" out of real sector tags left the background report with no "Other" signal at all. Restore it in two roles: a single aggregate "Other" slice in the All-sectors chart (how many chose it) and a separate detail card of the actual typed values (what they wrote), each drilling into its registrants. The primary chart gets no slice — "Other" is additional-only. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/services/event_dashboard.rb | 43 +++++++++++++++++++++++++++ app/views/events/background.html.erb | 13 ++++++++ spec/requests/events_spec.rb | 9 ++++++ spec/services/event_dashboard_spec.rb | 32 ++++++++++++++++++++ 4 files changed, 97 insertions(+) diff --git a/app/services/event_dashboard.rb b/app/services/event_dashboard.rb index 89c731de6..87b88e8c8 100644 --- a/app/services/event_dashboard.rb +++ b/app/services/event_dashboard.rb @@ -434,6 +434,37 @@ def additional_sector_count additional_sector_ids.size end + # Free-text "Other" sectors registrants typed, kept as OtherResponse records + # (never real sector tags). These back two things on the background report: a + # single aggregate "Other" bucket in the "All sectors" chart (#..._count / + # #..._registrant_ids), and a separate "Other responses" detail list of the + # actual typed values (#..._rows). Only visible (pending/kept) responses count + # — dismissed/promoted ones drop off. "Other" is an additional-sector answer + # only (the primary dropdown omits it), so it never touches the primary chart. + def other_sector_response_count + other_sector_response_registrant_ids.size + end + + # Distinct registrant ids with at least one visible other-sector response. + def other_sector_response_registrant_ids + @other_sector_response_registrant_ids ||= visible_other_sector_responses + .map(&:owner_id).uniq + end + + # The distinct typed values, each as [ text, registrant_count, registrant_ids ], + # for the "Other responses" detail card. Grouped by normalized text so the same + # answer typed with different casing/whitespace collapses into one row (the + # display text is the first spelling on file). Ordered by count desc, then text. + def other_sector_response_rows + @other_sector_response_rows ||= visible_other_sector_responses + .group_by(&:normalized_text) + .map do |_normalized, responses| + ids = responses.map(&:owner_id).uniq + [ responses.first.text, ids.size, ids ] + end + .sort_by { |text, count, _ids| [ -count, text.downcase ] } + end + # Primary age group(s) served, read from registrants' answers to the # registration form's "primary_age_group" question (each answer stores the # chosen AgeRange category ids, ", "-joined). Resolved to AgeRange categories, @@ -895,6 +926,18 @@ def additional_sector_ids end end + # Visible (pending/kept) sector "Other" responses owned by the registrants, + # loaded once and reused by the aggregate + detail methods above. Ordered by id + # so the display spelling picked per group is stable (first one on file). + def visible_other_sector_responses + @visible_other_sector_responses ||= OtherResponse + .sectors + .visible + .where(owner_type: "Person", owner_id: registrant_ids) + .order(:id) + .to_a + end + # [ person_id, sector_id ] pairs for every sector tag on the registrants. def registrant_sector_pairs @registrant_sector_pairs ||= SectorableItem diff --git a/app/views/events/background.html.erb b/app/views/events/background.html.erb index 42bbb8062..896cae5fd 100644 --- a/app/views/events/background.html.erb +++ b/app/views/events/background.html.erb @@ -2,6 +2,13 @@ <% primary_sector_data = @dashboard.primary_sectors.map { |s| [ s.name, @dashboard.primary_sector_counts.fetch(s.id, 0) ] }.sort_by { |_, count| -count } %> <% sector_data = @dashboard.sectors.map { |s| [ s.name, @dashboard.sector_counts.fetch(s.id, 0) ] }.sort_by { |_, count| -count } %> +<%# Free-text "Other" sectors ride along as one aggregate slice, pinned last (the + actual typed values get their own detail card below). The primary chart has no + such slice — "Other" is an additional-sector answer only. %> +<% sector_data += [ [ "Other", @dashboard.other_sector_response_count ] ] if @dashboard.other_sector_response_count.positive? %> +<%# Detail list of the actual typed values, each drilling into the registrants who typed it. %> +<% other_response_data = @dashboard.other_sector_response_rows.map { |text, count, _ids| [ text, count ] } %> +<% other_response_paths = @dashboard.other_sector_response_rows.to_h { |text, _count, ids| [ text, registrants_event_path(@event, registrant_ids: ids.join("-")) ] } %> <% age_group_data = @dashboard.age_groups.map { |c| [ c.name, @dashboard.age_group_counts.fetch(c.id, 0) ] }.sort_by { |_, count| -count } %> <% state_data = @dashboard.state_counts.sort_by { |_, count| -count } %> <% country_data = @dashboard.country_counts.sort_by { |_, count| -count } %> @@ -198,6 +205,7 @@ the matching registrant_ids set). %> <% primary_sector_paths = @dashboard.primary_sectors.to_h { |s| [ s.name, registrants_event_path(@event, registrant_ids: @dashboard.primary_sector_registrant_ids_by_sector.fetch(s.id, []).join("-")) ] } %> <% sector_paths = @dashboard.sectors.to_h { |s| [ s.name, registrants_event_path(@event, sector: s.id) ] } %> + <% sector_paths["Other"] = registrants_event_path(@event, registrant_ids: @dashboard.other_sector_response_registrant_ids.join("-")) if @dashboard.other_sector_response_count.positive? %> <% age_group_paths = @dashboard.age_groups.to_h { |c| [ c.name, registrants_event_path(@event, registrant_ids: @dashboard.age_group_registrant_ids_by_category.fetch(c.id, []).join("-")) ] } %> <% state_paths = @dashboard.states.index_with { |state| registrants_event_path(@event, state: state) } %> <% country_paths = @dashboard.country_registrant_ids_by_country.transform_values { |ids| registrants_event_path(@event, registrant_ids: ids.join("-")) } %> @@ -212,6 +220,11 @@ <% if sector_data.any? %> <%= render "breakdown_card", title: "All sectors", data: sector_data, chart: :pie, palette: palette, row_paths: sector_paths %> <% end %> + <%# The free-text values behind the "Other" sector slice — a list, since these + are un-curated answers a curator later promotes into real sectors. %> + <% if other_response_data.any? %> + <%= render "breakdown_card", title: "Other sector responses", data: other_response_data, chart: nil, palette: palette, row_paths: other_response_paths %> + <% end %> <%# States and countries use the addresses theme color (slate) for their map ramps. %> <% addresses_color = "#475569" %> <%# Age group and Countries are both short; stack them in one column so the pair diff --git a/spec/requests/events_spec.rb b/spec/requests/events_spec.rb index 13a03057b..933903540 100644 --- a/spec/requests/events_spec.rb +++ b/spec/requests/events_spec.rb @@ -1534,6 +1534,15 @@ def ce_chip_text expect(response.body).to include("Sexual Assault") end + it "shows the free-text \"Other\" sector responses as their own detail card" do + create(:other_response, owner: person, text: "Hospice care") + + get background_event_path(event) + + expect(response.body).to include("Other sector responses") + expect(response.body).to include("Hospice care") + end + it "labels the organizations count box and breaks it down by program status" do get background_event_path(event) diff --git a/spec/services/event_dashboard_spec.rb b/spec/services/event_dashboard_spec.rb index 9235ff7aa..fe4a3ae37 100644 --- a/spec/services/event_dashboard_spec.rb +++ b/spec/services/event_dashboard_spec.rb @@ -232,6 +232,38 @@ end end + describe "other sector responses" do + before do + # Free-text "Other" sectors registrants typed (kept as OtherResponse, never + # real tags). person1 and person2 both typed "Hospice" (different casing → + # one bucket); person2 also typed "Doula". A dismissed one and the cancelled + # registrant's response must be ignored. + create(:other_response, owner: person1, text: "Hospice") + create(:other_response, :kept, owner: person2, text: "hospice") + create(:other_response, owner: person2, text: "Doula") + create(:other_response, :dismissed, owner: person1, text: "Retired") + create(:other_response, owner: cancelled_person, text: "Hospice") + end + + it "counts distinct registrants with a visible other-sector response" do + expect(dashboard.other_sector_response_count).to eq(2) + end + + it "returns the registrant ids behind the other-sector responses" do + expect(dashboard.other_sector_response_registrant_ids).to contain_exactly(person1.id, person2.id) + end + + it "groups the typed values by normalized text, with per-value registrant counts and ids" do + rows = dashboard.other_sector_response_rows + expect(rows.map { |text, count, _ids| [ text, count ] }).to eq([ + [ "Hospice", 2 ], + [ "Doula", 1 ] + ]) + expect(rows.first.last).to contain_exactly(person1.id, person2.id) + expect(rows.last.last).to eq([ person2.id ]) + end + end + describe "sector primary/additional split (overlapping)" do before do # person1 names sector1 as primary; person2 names sector2 as primary via the From b9f39de1cec7f4f8a290f2d3bf12d0fc1ff0f3c7 Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 14 Jul 2026 06:35:23 -0400 Subject: [PATCH 12/14] Promote sector "Other"s through registration's tagging path Promotion tagged only the person; registration tags the person and their organizations. Extract that shared logic into SectorTagging.apply (person gets primary + additional, orgs get everything as additional-only) and route both registration and promotion through it. Promotion always passes additional-only, so a promoted sector now lands on the person and every org they're affiliated with, matching how a registration would have tagged it. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 4 +++ app/controllers/other_responses_controller.rb | 5 ++- .../public_registration.rb | 7 ++-- app/services/sector_tagging.rb | 21 ++++++++++++ spec/requests/other_responses_spec.rb | 16 ++++++++++ spec/services/sector_tagging_spec.rb | 32 +++++++++++++++++++ 6 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 app/services/sector_tagging.rb create mode 100644 spec/services/sector_tagging_spec.rb diff --git a/AGENTS.md b/AGENTS.md index a38b65072..522b73dba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -213,6 +213,10 @@ end - `AffiliationServices::CreateFromRegistration` — On registration / org linking, creates a "job affiliation" with the typed title (when present) plus a standing "Facilitator" affiliation, in one transaction. Skips the facilitator one only when the person already has an active-or-pending affiliation titled exactly "Facilitator" with that org (a current one or one dated to a future training); an ended facilitator affiliation gets a fresh second one. Dedupe is by title + org + dates, so a job title like "Lead Facilitator" still gets its own Facilitator affiliation. Accepts an optional `organization_address:` and sets it on every affiliation it creates (the registrant's typed agency address, upserted onto the org); when an affiliation already exists and is skipped, it backfills that address onto the existing one only if it has none (an admin-set address is never overwritten) +### Sectors + +- `SectorTagging` — `.apply(person:, organizations:, primary_ids:, additional_ids:)` tags a person's sectors (primary + additional) and mirrors them onto the given organizations as additional-only (orgs aggregate members' sectors and have no primary). Shared by registration (person's selections onto the org they registered with) and "Other" sector-response promotion (the promoted sector onto the person and their orgs, additional only) + ### Other responses - `OtherResponses::CaptureFromSubmission` — Materializes a form submission's **person-owned** "Other" answers (sectors) as `OtherResponse` records; org-type "Other" is captured separately in `PublicRegistration#sync_agency_type` (owned by the org). Uses `OtherOption.texts`, which keys strictly on the `Other:` prefix, so named specify options and the CE `Yes: N` box are ignored; de-dupes per owner + question. Shared by the registration, scholarship, and bulk-payment submission paths diff --git a/app/controllers/other_responses_controller.rb b/app/controllers/other_responses_controller.rb index d1c5904cc..1a350f96c 100644 --- a/app/controllers/other_responses_controller.rb +++ b/app/controllers/other_responses_controller.rb @@ -62,7 +62,10 @@ def promote responses = group_scope.sectors.promotable_now responses.includes(:owner).find_each do |response| - response.owner.tag_sectors(primary_ids: [], additional_ids: [ sector.id ]) + # Reuse registration's tagging so the sector lands on the person AND their + # organizations, always as an additional tag (never primary). + SectorTagging.apply(person: response.owner, organizations: response.owner.organizations, + additional_ids: [ sector.id ]) response.update!(status: "promoted", promotable: sector) end diff --git a/app/services/event_registration_services/public_registration.rb b/app/services/event_registration_services/public_registration.rb index e58566b27..a644da14b 100644 --- a/app/services/event_registration_services/public_registration.rb +++ b/app/services/event_registration_services/public_registration.rb @@ -372,11 +372,8 @@ def assign_tags(person, organization) additional_age_ids = collect_ids_from_checkboxes("additional_age_group") if primary_sector_ids.any? || additional_sector_ids.any? - person.tag_sectors(primary_ids: primary_sector_ids, additional_ids: additional_sector_ids) - # Organizations aggregate sectors across many people and have no single - # "primary", so union everyone's selections in as additional tags rather - # than churning the org's primary on each registration. - organization&.tag_sectors(primary_ids: [], additional_ids: primary_sector_ids + additional_sector_ids) + SectorTagging.apply(person: person, organizations: [ organization ], + primary_ids: primary_sector_ids, additional_ids: additional_sector_ids) end if primary_age_ids.any? || additional_age_ids.any? diff --git a/app/services/sector_tagging.rb b/app/services/sector_tagging.rb new file mode 100644 index 000000000..2ac4d7db0 --- /dev/null +++ b/app/services/sector_tagging.rb @@ -0,0 +1,21 @@ +module SectorTagging + # Tags a person's sectors and mirrors them onto the organizations they're + # tied to. Organizations aggregate sectors across many members and have no + # single "primary", so a person's primary + additional sectors are all unioned + # onto each org as additional tags rather than churning the org's primary. + # + # Shared by registration (a person's primary + additional selections onto the + # org they registered with) and "Other" sector-response promotion (an existing + # response's sector onto the person and their orgs — always additional only). + def self.apply(person:, organizations:, primary_ids: [], additional_ids: []) + primary_ids = Array(primary_ids) + additional_ids = Array(additional_ids) + return if primary_ids.empty? && additional_ids.empty? + + person.tag_sectors(primary_ids: primary_ids, additional_ids: additional_ids) + + Array(organizations).compact.each do |organization| + organization.tag_sectors(primary_ids: [], additional_ids: primary_ids + additional_ids) + end + end +end diff --git a/spec/requests/other_responses_spec.rb b/spec/requests/other_responses_spec.rb index 7f753a5a2..1d6394c58 100644 --- a/spec/requests/other_responses_spec.rb +++ b/spec/requests/other_responses_spec.rb @@ -166,6 +166,22 @@ expect(dismissed.owner.sectors).not_to include(sector) end + it "also tags the person's organizations (additional), like registration" do + sign_in admin + sector = create(:sector, name: "Equine Therapy") + person = create(:person) + organization = create(:organization) + create(:affiliation, person: person, organization: organization) + create(:other_response, owner: person, text: "Equine therapy") + + post promote_other_responses_path, + params: { kind: "sector", normalized_text: "equine therapy", sector_id: sector.id } + + expect(person.sectors).to include(sector) + expect(organization.sectors).to include(sector) + expect(organization.sectorable_items.find_by(sector: sector).is_primary).to be(false) + end + it "mints a new published sector when given a name" do sign_in admin person = create(:person) diff --git a/spec/services/sector_tagging_spec.rb b/spec/services/sector_tagging_spec.rb new file mode 100644 index 000000000..360a58ac2 --- /dev/null +++ b/spec/services/sector_tagging_spec.rb @@ -0,0 +1,32 @@ +require "rails_helper" + +RSpec.describe SectorTagging do + let(:person) { create(:person) } + let(:organization) { create(:organization) } + let!(:primary) { create(:sector, name: "Healthcare") } + let!(:additional) { create(:sector, name: "Education") } + + describe ".apply" do + it "tags the person's primary + additional, and the org additional-only" do + described_class.apply(person: person, organizations: [ organization ], + primary_ids: [ primary.id ], additional_ids: [ additional.id ]) + + expect(person.sectorable_items.find_by(sector: primary).is_primary).to be(true) + expect(person.sectorable_items.find_by(sector: additional).is_primary).to be(false) + # The org gets both, all as additional — organizations have no primary. + expect(organization.sectorable_items.pluck(:sector_id)).to contain_exactly(primary.id, additional.id) + expect(organization.sectorable_items.where(is_primary: true)).to be_empty + end + + it "is a no-op when no ids are given" do + expect { + described_class.apply(person: person, organizations: [ organization ]) + }.not_to change(SectorableItem, :count) + end + + it "tolerates a nil organization in the list" do + described_class.apply(person: person, organizations: [ nil ], additional_ids: [ additional.id ]) + expect(person.sectors).to include(additional) + end + end +end From 49c1c216aff6489ada6d8c0687c7f30635ce224a Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 14 Jul 2026 09:48:45 -0400 Subject: [PATCH 13/14] Derive promotion's org from the registration, not current affiliations The source_form_answer already ties a response to its submission and event, so the org can be found via the person's registration for that event rather than stored on the response or guessed from all current affiliations. Promotion now tags exactly the org(s) the person registered with when they typed the value. Co-Authored-By: Claude Opus 4.8 (1M context) --- AGENTS.md | 2 +- app/controllers/other_responses_controller.rb | 6 +++--- app/models/other_response.rb | 12 +++++++++++ spec/models/other_response_spec.rb | 21 +++++++++++++++++++ spec/requests/other_responses_spec.rb | 12 ++++++++--- 5 files changed, 46 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 522b73dba..4ee4ac50b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -215,7 +215,7 @@ end ### Sectors -- `SectorTagging` — `.apply(person:, organizations:, primary_ids:, additional_ids:)` tags a person's sectors (primary + additional) and mirrors them onto the given organizations as additional-only (orgs aggregate members' sectors and have no primary). Shared by registration (person's selections onto the org they registered with) and "Other" sector-response promotion (the promoted sector onto the person and their orgs, additional only) +- `SectorTagging` — `.apply(person:, organizations:, primary_ids:, additional_ids:)` tags a person's sectors (primary + additional) and mirrors them onto the given organizations as additional-only (orgs aggregate members' sectors and have no primary). Shared by registration (person's selections onto the org they registered with) and "Other" sector-response promotion (additional only). Promotion passes `OtherResponse#registration_organizations`, which derives the org from the response's `source_form_answer` → submission → the person's registration for that event — so it tags exactly the org registration would have, without storing one on the response ### Other responses diff --git a/app/controllers/other_responses_controller.rb b/app/controllers/other_responses_controller.rb index 1a350f96c..bed803d66 100644 --- a/app/controllers/other_responses_controller.rb +++ b/app/controllers/other_responses_controller.rb @@ -62,9 +62,9 @@ def promote responses = group_scope.sectors.promotable_now responses.includes(:owner).find_each do |response| - # Reuse registration's tagging so the sector lands on the person AND their - # organizations, always as an additional tag (never primary). - SectorTagging.apply(person: response.owner, organizations: response.owner.organizations, + # Reuse registration's tagging so the sector lands on the person AND the + # org(s) they registered with, always as an additional tag (never primary). + SectorTagging.apply(person: response.owner, organizations: response.registration_organizations, additional_ids: [ sector.id ]) response.update!(status: "promoted", promotable: sector) end diff --git a/app/models/other_response.rb b/app/models/other_response.rb index a798af9cc..7b0fe75ad 100644 --- a/app/models/other_response.rb +++ b/app/models/other_response.rb @@ -81,6 +81,18 @@ def promotable? kind.in?(PROMOTABLE_KINDS) end + # The organization(s) the person registered with when they typed this response, + # derived from the source form answer's submission + the person's registration + # for that event. Lets promotion mirror a sector onto exactly the orgs a + # registration would have — no need to store the org here. Empty when there's no + # registration context (no source answer, or a non-person owner). + def registration_organizations + event = source_form_answer&.form_submission&.event + return Organization.none unless event && owner.is_a?(Person) + + owner.event_registrations.find_by(event: event)&.organizations || Organization.none + end + # How the review page buckets this response: captured kinds group by kind (all # sector "Other"s together, all org-type together); generic groups by question. def group_key diff --git a/spec/models/other_response_spec.rb b/spec/models/other_response_spec.rb index 3f2902fd1..c805629d4 100644 --- a/spec/models/other_response_spec.rb +++ b/spec/models/other_response_spec.rb @@ -79,6 +79,27 @@ end end + describe "#registration_organizations" do + it "returns the org the person registered with, via the source answer's event" do + person = create(:person) + event = create(:event) + organization = create(:organization) + registration = create(:event_registration, registrant: person, event: event) + create(:event_registration_organization, event_registration: registration, organization: organization) + submission = create(:form_submission, person: person, event: event) + answer = create(:form_answer, form_submission: submission, + form_field: create(:form_field, field_identifier: "additional_sectors"), + submitted_answer: "Other: Equine therapy") + response = create(:other_response, owner: person, text: "Equine therapy", source_form_answer: answer) + + expect(response.registration_organizations).to contain_exactly(organization) + end + + it "is empty when there's no source form answer" do + expect(create(:other_response, source_form_answer: nil).registration_organizations).to be_empty + end + end + describe "#review_anchor" do it "buckets sectors by kind, parameterized" do expect(create(:other_response, text: "Equine Therapy").review_anchor).to eq("other-sector-equine-therapy") diff --git a/spec/requests/other_responses_spec.rb b/spec/requests/other_responses_spec.rb index 1d6394c58..03e74c4be 100644 --- a/spec/requests/other_responses_spec.rb +++ b/spec/requests/other_responses_spec.rb @@ -166,13 +166,19 @@ expect(dismissed.owner.sectors).not_to include(sector) end - it "also tags the person's organizations (additional), like registration" do + it "also tags the org the person registered with (derived via the response's submission)" do sign_in admin sector = create(:sector, name: "Equine Therapy") person = create(:person) + event = create(:event) organization = create(:organization) - create(:affiliation, person: person, organization: organization) - create(:other_response, owner: person, text: "Equine therapy") + registration = create(:event_registration, registrant: person, event: event) + create(:event_registration_organization, event_registration: registration, organization: organization) + submission = create(:form_submission, person: person, event: event) + answer = create(:form_answer, form_submission: submission, + form_field: create(:form_field, field_identifier: "additional_sectors"), + submitted_answer: "Other: Equine therapy") + create(:other_response, owner: person, text: "Equine therapy", source_form_answer: answer) post promote_other_responses_path, params: { kind: "sector", normalized_text: "equine therapy", sector_id: sector.id } From 42cf63d90f5f5aa3295064d0a50dd265822a2cf6 Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 14 Jul 2026 16:33:22 -0400 Subject: [PATCH 14/14] Clarify the two promote paths on the Other-responses review page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Action cell crammed a dropdown, "or", a text field, and one "Promote" button together, so it wasn't clear there are two distinct paths. Split them into two labeled actions — "New" (create a sector from the typed value → Create sector) and "Existing" (map it into an existing sector → Merge, the dedupe path) — separated from the Keep/Dismiss triage. Confirm dialogs and header copy now name the outcome of each. No controller change; each path posts the param the controller already handles. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/views/other_responses/index.html.erb | 45 +++++++++++++++++------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/app/views/other_responses/index.html.erb b/app/views/other_responses/index.html.erb index 685d6cc75..dda4b82ea 100644 --- a/app/views/other_responses/index.html.erb +++ b/app/views/other_responses/index.html.erb @@ -18,8 +18,10 @@

Review “Other” responses

- Free-text “Other” answers people typed on forms, grouped by question and value. - Promote a recurring sector into a real tag, or keep/dismiss any value. + Free-text “Other” answers people typed on forms, grouped by value. Turn a sector into a + real tag — create a new one or + merge it into an existing sector — or + keep / dismiss the value.

@@ -35,6 +37,7 @@
<% if @groups.any? %> +
@@ -56,25 +59,40 @@
<%= group[:count] %> -
- <% if group[:promotable] %> + <% if group[:promotable] %> + <%# Two distinct promote paths, each with its own verb: create a + brand-new sector from the typed value, or merge the value into + an existing sector (the dedupe path). %> +
<%= form_with url: promote_other_responses_path, method: :post, class: "flex items-center gap-2" do %> <%= hidden_field_tag :kind, group[:kind] %> <%= hidden_field_tag :field_identifier, group[:field_identifier] %> <%= hidden_field_tag :normalized_text, group[:normalized_text] %> + New + <%= text_field_tag :new_sector_name, group[:display_text], + "aria-label": "New sector name", + class: "flex-1 min-w-0 rounded-md border-gray-300 text-sm" %> + <%= submit_tag "Create sector", class: "btn btn-primary-outline text-sm whitespace-nowrap", + data: { turbo_confirm: "Create the sector “#{group[:display_text]}” and tag all #{group[:count]} people?" } %> + <% end %> + <%= form_with url: promote_other_responses_path, method: :post, + class: "flex items-center gap-2" do %> + <%= hidden_field_tag :kind, group[:kind] %> + <%= hidden_field_tag :field_identifier, group[:field_identifier] %> + <%= hidden_field_tag :normalized_text, group[:normalized_text] %> + Existing <%= select_tag :sector_id, options_from_collection_for_select(@sectors, :id, :name), - include_blank: "Existing sector…", - class: "w-40 rounded-md border-gray-300 text-sm" %> - or - <%= text_field_tag :new_sector_name, group[:display_text], - placeholder: "New sector name", - class: "w-36 rounded-md border-gray-300 text-sm" %> - <%= submit_tag "Promote", class: "btn btn-primary-outline text-sm whitespace-nowrap", - data: { turbo_confirm: "Promote “#{group[:display_text]}” for all #{group[:count]} people?" } %> + include_blank: "Select a sector…", + "aria-label": "Existing sector to merge into", + class: "flex-1 min-w-0 rounded-md border-gray-300 text-sm" %> + <%= submit_tag "Merge", class: "btn btn-secondary-outline text-sm whitespace-nowrap", + data: { turbo_confirm: "Map “#{group[:display_text]}” into the chosen sector for all #{group[:count]} people?" } %> <% end %> - <% end %> +
+ <% end %> +
<%= button_to "Keep all", curate_other_responses_path(kind: group[:kind], field_identifier: group[:field_identifier], normalized_text: group[:normalized_text], status: "kept", return_status: @status_filter), class: "text-gray-500 hover:text-gray-700 whitespace-nowrap" %> @@ -88,6 +106,7 @@ <% end %>
+
<% else %>

No “Other” responses to review.

<% end %>