From 6b6b33a0fcb515e4bfebae0b9695dcffa534507e Mon Sep 17 00:00:00 2001 From: maebeale Date: Mon, 22 Jun 2026 07:30:01 -0400 Subject: [PATCH] Save logged_out_only answers for signed-in registrants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A signed-in registrant's identity/contact fields are hidden from the registration form once we have them on file, so those answers were never recorded on the submission — leaving an incomplete snapshot that made the saved registration look like the registrant skipped questions they didn't actually need to answer. Record the known values onto the submission via the new PersonKnownFields PORO, and align the hide rule with the bulk-payment form: hide every logged_out_only field for signed-in users regardless of what's on file, with a temporary exception for the Organization Information fields (kept visible until we can backfill them from the person's affiliations). Co-Authored-By: Claude Opus 4.8 --- AGENTS.md | 3 +- .../events/public_registrations_controller.rb | 82 ++++++++----------- .../public_registration.rb | 26 ++++++ app/services/person_known_fields.rb | 78 ++++++++++++++++++ .../events/public_registrations_spec.rb | 30 +++++++ .../public_registration_spec.rb | 75 +++++++++++++++++ spec/services/person_known_fields_spec.rb | 66 +++++++++++++++ ...ublic_registration_form_submission_spec.rb | 26 ++++-- 8 files changed, 333 insertions(+), 53 deletions(-) create mode 100644 app/services/person_known_fields.rb create mode 100644 spec/services/person_known_fields_spec.rb diff --git a/AGENTS.md b/AGENTS.md index df764e7b9f..c92e9f69ba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -49,7 +49,7 @@ This codebase (Rails 8.1) | Directory | Purpose | Count | |---|---|---| | `app/models/` | ActiveRecord models | ~78 files | -| `app/services/` | Service objects and POROs (e.g. `MoneyFormatter` for currency display) | ~28 files | +| `app/services/` | Service objects and POROs (e.g. `MoneyFormatter` for currency display) | ~29 files | | `app/jobs/` | SolidQueue background jobs | 3 files | | `app/models/concerns/` | Shared model modules | 15 concerns | @@ -188,6 +188,7 @@ end - `PersonFromUserService` — Create Person from User account - `BulkInviteService` — Bulk send welcome instructions and reset created_at for users - `FormBuilderService` — Builds configurable forms from composable sections with per-field visibility +- `PersonKnownFields` — Maps a person's on-file identity/contact data to the registration form's `logged_out_only` field identifiers; `PublicRegistration` uses it to backfill those (hidden) answers onto a signed-in registrant's submission - `ModelDeduper` — Deduplication logic - `RichTextMigrator` — Rich text migration utility - `DisplayImagePresenter` — Image display logic diff --git a/app/controllers/events/public_registrations_controller.rb b/app/controllers/events/public_registrations_controller.rb index e44b1e3244..3450c56a47 100644 --- a/app/controllers/events/public_registrations_controller.rb +++ b/app/controllers/events/public_registrations_controller.rb @@ -210,29 +210,7 @@ def visible_form_fields person = current_user&.person if person - # Always hide logged_out_only fields for logged-in users with known data - known_identifiers = person_known_identifiers(person) - if known_identifiers.any? - known_ids = @form.form_fields - .where(visibility: :logged_out_only, field_identifier: known_identifiers) - .ids - scope = scope.where.not(id: known_ids) if known_ids.any? - end - - # Hide logged_out_only headers when all their non-header fields are hidden - logged_out_sections = @form.form_fields.where(visibility: :logged_out_only) - .where.not(answer_type: :group_header) - .pluck(:section).uniq.compact - logged_out_sections.each do |sect| - section_field_ids = @form.form_fields.where(section: sect, visibility: :logged_out_only) - .where.not(answer_type: :group_header).ids - if section_field_ids.any? && known_identifiers.any? && (section_field_ids - scope.where(id: section_field_ids).ids).any? - remaining = scope.where(id: section_field_ids).ids - if remaining.empty? - scope = scope.where.not(section: sect, answer_type: :group_header, visibility: :logged_out_only) - end - end - end + scope = hide_logged_out_only_fields(scope) if @form.hide_answered_form_questions? answered_field_ids = [] @@ -294,31 +272,43 @@ def visible_form_fields scope.reorder(position: :asc) end - def person_known_identifiers(person) - keys = [] - keys << "first_name" if person.first_name.present? - keys << "last_name" if person.last_name.present? - keys << "primary_email" << "confirm_email" if person.email.present? - keys << "primary_email_type" if person.email_type.present? - keys << "nickname" if person.legal_first_name.present? || person.first_name.present? - keys << "pronouns" if person.pronouns.present? - keys << "secondary_email" if person.email_2.present? - keys << "secondary_email_type" if person.email_2_type.present? - - if person.addresses.exists? - address = person.addresses.find_by(primary: true) || person.addresses.first - keys << "mailing_street" if address.street_address.present? - keys << "mailing_address_type" if address.address_type.present? - keys << "mailing_city" if address.city.present? - keys << "mailing_state" if address.state.present? - keys << "mailing_zip" if address.zip_code.present? - end - - if person.contact_methods.where(kind: :phone).exists? - keys << "phone" << "phone_type" + # `logged_out_only` fields are asked only of logged-out registrants — a + # signed-in registrant manages this data on their profile, and whatever we + # already have is backfilled onto the submission by PersonKnownFields. So hide + # the whole set for signed-in users regardless of what's on file, matching the + # bulk-payment form's rule. + # + # Temporary exception: the Organization Information fields (`agency_*`) stay + # visible until we backfill them from the person's affiliations. We keep their + # section header too, and drop every other logged_out_only header once nothing + # under it survives (walking in position order so headers that share a DB + # `section` with the org fields are handled individually). + ORG_FIELD_PREFIX = "agency_".freeze + + def hide_logged_out_only_fields(scope) + ordered = @form.form_fields.where(visibility: :logged_out_only).reorder(position: :asc).to_a + return scope if ordered.empty? + + hidden_ids = [] + pending_header = nil + group_kept = false + + drop_empty_header = -> { hidden_ids << pending_header.id if pending_header && !group_kept } + + ordered.each do |field| + if field.group_header? + drop_empty_header.call + pending_header = field + group_kept = false + elsif field.field_identifier.to_s.start_with?(ORG_FIELD_PREFIX) + group_kept = true + else + hidden_ids << field.id + end end + drop_empty_header.call - keys + hidden_ids.any? ? scope.where.not(id: hidden_ids) : scope end def ensure_registerable diff --git a/app/services/event_registration_services/public_registration.rb b/app/services/event_registration_services/public_registration.rb index ff7c3e3e82..f29ecb8e4f 100644 --- a/app/services/event_registration_services/public_registration.rb +++ b/app/services/event_registration_services/public_registration.rb @@ -353,6 +353,7 @@ def invoice_requested? def create_form_submission(person) submission = FormSubmission.create!(person: person, form: @form, event: @event) save_form_answers(submission) + save_known_field_answers(submission) submission end @@ -361,6 +362,7 @@ def update_form_submission(person) record.event = @event end save_form_answers(submission) + save_known_field_answers(submission) submission end @@ -381,6 +383,30 @@ def save_form_answers(submission) end end + # A signed-in registrant's `logged_out_only` fields are hidden from the form + # (see PublicRegistrationsController#hide_logged_out_only_fields) and so never + # reach @form_params. Record whatever we already have on file onto the + # submission anyway, so it's a complete snapshot rather than silently dropping + # the hidden answers. Only fires for signed-in registrants (@person); + # logged-out registrants are shown — and submit — every field. + def save_known_field_answers(submission) + return unless @person + + PersonKnownFields.call(@person).each do |identifier, value| + next if identifier == "confirm_email" || value.blank? + + field = @form.form_fields.find_by(visibility: :logged_out_only, field_identifier: identifier) + next unless field + next if submission.form_answers.exists?(form_field: field) + + submission.form_answers.create!( + form_field: field, + submitted_answer: value.to_s, + question_name_when_answered: field.name + ) + end + end + # Persist the answers to the separate scholarship form (when one is asked and a # scholarship was requested) as its own role: "scholarship" submission tied to # the event, mirroring how the registration submission is saved above. diff --git a/app/services/person_known_fields.rb b/app/services/person_known_fields.rb new file mode 100644 index 0000000000..578f495194 --- /dev/null +++ b/app/services/person_known_fields.rb @@ -0,0 +1,78 @@ +# Maps a person's already-on-file identity and contact data onto the registration +# form's `logged_out_only` field identifiers. +# +# Used by EventRegistrationServices::PublicRegistration to backfill a signed-in +# registrant's answers: those `logged_out_only` fields are hidden from the form +# (see PublicRegistrationsController#hide_logged_out_only_fields) and so never +# reach the submitted params, but whatever we already have on file should still +# be recorded on the submission so it's a complete snapshot rather than silently +# omitting them. +# +# A "nickname" value is only carried when a real preferred name exists (otherwise +# it maps to the legal first name, which belongs to the "first_name" field); the +# backfill skips blank values either way. +class PersonKnownFields + def self.call(person) + new(person).call + end + + def initialize(person) + @person = person + end + + def call + return {} unless @person + + fields = {} + fields["first_name"] = legal_first_name if @person.first_name.present? + fields["last_name"] = @person.last_name if @person.last_name.present? + if @person.email.present? + fields["primary_email"] = @person.email + fields["confirm_email"] = @person.email + end + fields["primary_email_type"] = humanize_type(@person.email_type) if @person.email_type.present? + # The "Preferred Nickname" field maps to first_name, but only when a real + # nickname exists (resolve_names then stores the legal name on legal_first_name). + fields["nickname"] = @person.first_name if @person.legal_first_name.present? + fields["pronouns"] = @person.pronouns if @person.pronouns.present? + fields["secondary_email"] = @person.email_2 if @person.email_2.present? + fields["secondary_email_type"] = humanize_type(@person.email_2_type) if @person.email_2_type.present? + add_address(fields) + add_phone(fields) + fields + end + + private + + # The "First Name" field collects the legal first name; resolve_names stores it + # on legal_first_name when a nickname was given, otherwise on first_name. + def legal_first_name + @person.legal_first_name.presence || @person.first_name + end + + def add_address(fields) + return unless @person.addresses.exists? + + address = @person.addresses.find_by(primary: true) || @person.addresses.first + fields["mailing_street"] = address.street_address if address.street_address.present? + fields["mailing_address_type"] = humanize_type(address.address_type) if address.address_type.present? + fields["mailing_city"] = address.city if address.city.present? + fields["mailing_state"] = address.state if address.state.present? + fields["mailing_zip"] = address.zip_code if address.zip_code.present? + end + + def add_phone(fields) + phones = @person.contact_methods.where(kind: :phone) + return unless phones.exists? + + phone = phones.find_by(primary: true) || phones.first + fields["phone"] = phone.value + fields["phone_type"] = humanize_type(phone.contact_type) + end + + # The *_type selector options are capitalized labels ("Personal" / "Work"), + # while the underlying columns store them lowercased. + def humanize_type(value) + value.to_s.capitalize.presence + end +end diff --git a/spec/requests/events/public_registrations_spec.rb b/spec/requests/events/public_registrations_spec.rb index 47b25b5f9a..fe263746da 100644 --- a/spec/requests/events/public_registrations_spec.rb +++ b/spec/requests/events/public_registrations_spec.rb @@ -633,4 +633,34 @@ def post_with_scholarship(scholarship_answer) end end end + + describe "GET new hides logged_out_only fields for a signed-in registrant" do + let(:form) do + FormBuilderService.new( + name: "Registration", + sections: %i[person_identifier person_contact_info] + ).call + end + let(:user) { create(:user, :with_person) } + + it "hides logged_out_only fields but keeps the organization section visible" do + sign_in user + + get new_event_public_registration_path(event) + + expect(response.body).not_to include("Preferred Nickname") + expect(response.body).not_to include("Secondary Email") + expect(response.body).not_to include("Mailing Address") + expect(response.body).to include("Organization Information") + expect(response.body).to include("Organization Name") + end + + it "shows the logged_out_only fields to a logged-out registrant" do + get new_event_public_registration_path(event) + + expect(response.body).to include("Preferred Nickname") + expect(response.body).to include("Secondary Email") + expect(response.body).to include("Organization Name") + end + end end diff --git a/spec/services/event_registration_services/public_registration_spec.rb b/spec/services/event_registration_services/public_registration_spec.rb index 7f9eb316a2..542a3416b8 100644 --- a/spec/services/event_registration_services/public_registration_spec.rb +++ b/spec/services/event_registration_services/public_registration_spec.rb @@ -479,4 +479,79 @@ def register_with_additional_forms(selections) expect(FormSubmission.where(person: person, role: "scholarship")).to be_empty end end + + describe "backfilling on-file fields a signed-in registrant didn't fill in" do + let(:person) do + create(:person, + first_name: "Jordy", legal_first_name: "Jordan", last_name: "Rivera", + pronouns: "they/them", + email: "jordan@example.com", email_type: "personal", + email_2: "jordan.work@example.com", email_2_type: "work") + end + + before do + person.addresses.create!( + street_address: "1 Main St", city: "Austin", state: "TX", zip_code: "78701", + address_type: "personal", locality: "Unknown", primary: true + ) + person.contact_methods.create!(kind: :phone, value: "555-867-5309", contact_type: "work", primary: true) + end + + def answers_by_identifier(submission) + submission.form_answers + .includes(:form_field) + .index_by { |answer| answer.form_field.field_identifier } + end + + it "records the hidden logged_out_only fields from the person's on-file data" do + # A signed-in registrant whose identity/contact data is on file sees those + # fields hidden, so the submitted params carry none of them. + result = described_class.call(event: event, form: form, person: person, form_params: {}) + + expect(result.success?).to be true + answers = answers_by_identifier(result.form_submission) + + expect(answers["first_name"].submitted_answer).to eq("Jordan") + expect(answers["nickname"].submitted_answer).to eq("Jordy") + expect(answers["last_name"].submitted_answer).to eq("Rivera") + expect(answers["pronouns"].submitted_answer).to eq("they/them") + expect(answers["primary_email"].submitted_answer).to eq("jordan@example.com") + expect(answers["primary_email_type"].submitted_answer).to eq("Personal") + expect(answers["secondary_email"].submitted_answer).to eq("jordan.work@example.com") + expect(answers["secondary_email_type"].submitted_answer).to eq("Work") + expect(answers["mailing_street"].submitted_answer).to eq("1 Main St") + expect(answers["mailing_city"].submitted_answer).to eq("Austin") + expect(answers["mailing_state"].submitted_answer).to eq("TX") + expect(answers["mailing_zip"].submitted_answer).to eq("78701") + expect(answers["mailing_address_type"].submitted_answer).to eq("Personal") + expect(answers["phone"].submitted_answer).to eq("555-867-5309") + expect(answers["phone_type"].submitted_answer).to eq("Work") + end + + it "never records a confirm_email answer" do + result = described_class.call(event: event, form: form, person: person, form_params: {}) + + expect(answers_by_identifier(result.form_submission)).not_to have_key("confirm_email") + end + + it "does not overwrite an answer the registrant actually submitted" do + result = described_class.call( + event: event, form: form, person: person, + form_params: { field_id("pronouns") => "she/her" } + ) + + expect(answers_by_identifier(result.form_submission)["pronouns"].submitted_answer).to eq("she/her") + end + + it "does not backfill when no one is signed in" do + result = described_class.call( + event: event, form: form, + form_params: base_form_params(first_name: "Pat", last_name: "Lee", email: "pat@example.com") + ) + + answers = answers_by_identifier(result.form_submission) + expect(answers).not_to have_key("pronouns") + expect(answers).not_to have_key("phone") + end + end end diff --git a/spec/services/person_known_fields_spec.rb b/spec/services/person_known_fields_spec.rb new file mode 100644 index 0000000000..06a92b9f22 --- /dev/null +++ b/spec/services/person_known_fields_spec.rb @@ -0,0 +1,66 @@ +require "rails_helper" + +RSpec.describe PersonKnownFields do + it "returns an empty hash when there is no person" do + expect(described_class.call(nil)).to eq({}) + end + + it "maps the registrant's on-file identity and contact data to field identifiers" do + person = create(:person, + first_name: "Jordy", legal_first_name: "Jordan", last_name: "Rivera", + pronouns: "they/them", + email: "jordan@example.com", email_type: "personal", + email_2: "jordan.work@example.com", email_2_type: "work") + person.addresses.create!( + street_address: "1 Main St", city: "Austin", state: "TX", zip_code: "78701", + address_type: "personal", locality: "Unknown", primary: true + ) + person.contact_methods.create!(kind: :phone, value: "555-867-5309", contact_type: "work", primary: true) + + expect(described_class.call(person)).to eq( + "first_name" => "Jordan", + "nickname" => "Jordy", + "last_name" => "Rivera", + "primary_email" => "jordan@example.com", + "confirm_email" => "jordan@example.com", + "primary_email_type" => "Personal", + "pronouns" => "they/them", + "secondary_email" => "jordan.work@example.com", + "secondary_email_type" => "Work", + "mailing_street" => "1 Main St", + "mailing_address_type" => "Personal", + "mailing_city" => "Austin", + "mailing_state" => "TX", + "mailing_zip" => "78701", + "phone" => "555-867-5309", + "phone_type" => "Work" + ) + end + + it "uses first_name as the legal name and carries no nickname when none was given" do + person = create(:person, first_name: "Sam", legal_first_name: nil, last_name: "Doe", email: "sam@example.com") + + result = described_class.call(person) + + expect(result["first_name"]).to eq("Sam") + expect(result).not_to have_key("nickname") + end + + it "omits fields the person has no data for" do + person = create(:person, first_name: "Sam", last_name: "Doe", email: "sam@example.com") + + result = described_class.call(person) + + expect(result).not_to have_key("secondary_email") + expect(result).not_to have_key("mailing_city") + expect(result).not_to have_key("phone") + end + + it "prefers the primary address" do + person = create(:person, first_name: "Sam", last_name: "Doe", email: "sam@example.com") + create(:address, addressable: person, city: "Old Town", primary: false) + create(:address, addressable: person, city: "New Town", primary: true) + + expect(described_class.call(person)["mailing_city"]).to eq("New Town") + end +end diff --git a/spec/system/public_registration_form_submission_spec.rb b/spec/system/public_registration_form_submission_spec.rb index ac7c04f389..216104101c 100644 --- a/spec/system/public_registration_form_submission_spec.rb +++ b/spec/system/public_registration_form_submission_spec.rb @@ -37,8 +37,10 @@ let(:user) { create(:user) } # A returning registrant whose name, email, primary address, and phone are on - # file — so the logged_out_only identity / mailing / phone fields are hidden when - # signed in, leaving the always-ask + answers-on-file questions to fill. + # file. When signed in, every logged_out_only field is hidden — the identity / + # mailing / phone ones get backfilled from this data, while background fields we + # don't have are simply never asked — leaving the always-ask + answers-on-file + # questions to fill. let(:logged_in_person) do person = create(:person, user: user, first_name: "Dana", last_name: "Lee", email: "dana.lee@example.com", email_type: "personal") @@ -121,20 +123,22 @@ expect(person.additional_age_groups).to include(age_teens) end - it "logged in: reuses the signed-in person, hides known fields, saves the rest" do + it "logged in: reuses the signed-in person, hides logged_out_only fields, backfills known answers" do logged_in_person sign_in user visit new_event_public_registration_path(event) - # Known identity / mailing / phone questions are hidden for a returning person. + # Every logged_out_only question is hidden for a signed-in registrant: the + # identity / mailing / phone fields we have on file, and background fields + # like racial / ethnic identity that we don't (but still never ask). expect(page).not_to have_selector("##{pr_dom_id(reg_field('first_name'))}") expect(page).not_to have_selector("##{pr_dom_id(reg_field('mailing_city'))}") + expect(page).not_to have_selector("##{pr_dom_id(reg_field('racial_ethnic_identity'))}") select_pr reg_field("primary_sector_single"), "Education" check_pr_box reg_field("additional_sectors"), sector_dv.id.to_s select_pr reg_field("primary_age_group"), "Teens (13-17)" - choose_pr_radio reg_field("racial_ethnic_identity"), "Asian" choose_pr_radio reg_field("referral_source"), "Social Media" choose_pr_radio reg_field("payment_method"), "Check" check_pr_box reg_field("communication_consent"), "Yes" @@ -150,11 +154,21 @@ "primary_sector_single" => sector_education.id.to_s, "additional_sectors" => sector_dv.id.to_s, "primary_age_group" => age_teens.id.to_s, - "racial_ethnic_identity" => "Asian", "referral_source" => "Social Media", "payment_method" => "Check", "communication_consent" => "Yes" ) + # The hidden identity / mailing / phone answers are still recorded on the + # submission, backfilled from the signed-in person's profile. + expect(answers).to include( + "first_name" => "Dana", + "last_name" => "Lee", + "primary_email" => "dana.lee@example.com", + "mailing_city" => "Oakland", + "phone" => "555-000-1111" + ) + # The background field we never ask has no answer. + expect(answers).not_to have_key("racial_ethnic_identity") logged_in_person.reload expect(logged_in_person.primary_age_groups).to include(age_teens) expect(logged_in_person.sectorable_items.map(&:sector)).to include(sector_education, sector_dv)