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)