diff --git a/app/controllers/event_registrations_controller.rb b/app/controllers/event_registrations_controller.rb index 4a750e9ebd..0106bf832d 100644 --- a/app/controllers/event_registrations_controller.rb +++ b/app/controllers/event_registrations_controller.rb @@ -685,8 +685,7 @@ def csv_export(registrations) end end - # Each registration-form submission with the org name and position the registrant - # entered on it: [{ submission:, org_name:, position: }], oldest first. Memoized: + # Each registration-form submission's FormSubmission#org_entry, oldest first. Memoized: # the linking actions ask for these entries half a dozen times per request # (profile, address, position, then again to build the notice). def registration_submission_entries(registration) @@ -698,39 +697,11 @@ def build_registration_submission_entries(registration) form = registration.event.registration_form return [] unless form - organization_identifiers = %w[ - organization_name organization_position organization_website organization_type - organization_street organization_city organization_state organization_zip organization_country - ] - field_ids = form.form_fields - .where(field_identifier: organization_identifiers) - .pluck(:field_identifier, :id).to_h - entries = registration.registrant.form_submissions .where(form: form) .order(:created_at) - .includes(:form_answers) - .map do |submission| - answers = submission.form_answers.index_by(&:form_field_id) - answer = ->(identifier) do - id = field_ids[identifier] - id && answers[id]&.submitted_answer - end - { - submission: submission, - org_name: answer.call("organization_name"), - position: answer.call("organization_position"), - website: answer.call("organization_website"), - organization_type: answer.call("organization_type"), - address: { - street_address: answer.call("organization_street"), - city: answer.call("organization_city"), - state: answer.call("organization_state"), - zip_code: answer.call("organization_zip"), - country: answer.call("organization_country") - } - } - end + .includes(form_answers: :form_field) + .map(&:org_entry) # Attach the matching DB org (if any) for each submitted name, so the view can # show its city/state next to the answer. Batched to avoid a query per entry. @@ -813,12 +784,7 @@ def profile_diff_for(organization, linked_count: nil) entry = submission_entry_for(@event_registration, organization, linked_count: linked_count) return [] unless entry - OrganizationServices::ProfileDiff.call( - organization: organization, - website: entry[:website], - organization_type: entry[:organization_type], - address: entry[:address] || {} - ) + OrganizationServices::ProfileDiff.for_entry(organization: organization, entry: entry) end # Link the org to the registration, fill what it's missing from the submission diff --git a/app/controllers/form_submissions_controller.rb b/app/controllers/form_submissions_controller.rb index 159c8422a2..61a1530669 100644 --- a/app/controllers/form_submissions_controller.rb +++ b/app/controllers/form_submissions_controller.rb @@ -154,13 +154,7 @@ def registration_linked_organization end def profile_diff_for(organization) - entry = submission_org_entry - OrganizationServices::ProfileDiff.call( - organization: organization, - website: entry[:website], - organization_type: entry[:organization_type], - address: entry[:address] - ) + OrganizationServices::ProfileDiff.for_entry(organization: organization, entry: submission_org_entry) end # Fill the org's blank profile/address fields from the submission, create the diff --git a/app/models/form_submission.rb b/app/models/form_submission.rb index 78cc037b3b..e305c32a59 100644 --- a/app/models/form_submission.rb +++ b/app/models/form_submission.rb @@ -131,22 +131,30 @@ def self.org_name_answers # OrganizationServices::LinkSubmittedOrganization reads them. Shared by the # admin linking editor and the auto-processing a public submission runs. def org_entry - answers = answers_by_identifier + self.class.org_entry_from(answers_by_identifier).merge(submission: self) + end + + ORG_ENTRY_FIELDS = { + org_name: FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER, + position: FormField::ORGANIZATION_POSITION_FIELD_IDENTIFIER, + website: "organization_website", + organization_type: "organization_type" + }.freeze + ORG_ADDRESS_FIELDS = { + street_address: "organization_street", + city: "organization_city", + state: "organization_state", + zip_code: "organization_zip", + country: "organization_country" + }.freeze + ORG_ENTRY_FIELD_IDENTIFIERS = (ORG_ENTRY_FIELDS.values + ORG_ADDRESS_FIELDS.values).freeze + + # The same org entry from a bare field_identifier => answer hash, for callers + # holding answers that aren't saved on a submission (the spreadsheet importer, + # a public form mid-submit). + def self.org_entry_from(answers) read = ->(identifier) { answers[identifier].presence } - { - submission: self, - org_name: read.call(FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER), - position: read.call(FormField::ORGANIZATION_POSITION_FIELD_IDENTIFIER), - website: read.call("organization_website"), - organization_type: read.call("organization_type"), - address: { - street_address: read.call("organization_street"), - city: read.call("organization_city"), - state: read.call("organization_state"), - zip_code: read.call("organization_zip"), - country: read.call("organization_country") - } - } + ORG_ENTRY_FIELDS.transform_values(&read).merge(address: ORG_ADDRESS_FIELDS.transform_values(&read)) end # The registration-org row a submission is pinned to — the link public diff --git a/app/services/event_registration_importer.rb b/app/services/event_registration_importer.rb index e6e20aeacf..b61fd9f537 100644 --- a/app/services/event_registration_importer.rb +++ b/app/services/event_registration_importer.rb @@ -849,16 +849,8 @@ def capture_organization(person, registration, submission, organization, answers # The org answers fill-blanks won't apply, from the comparison the link-organization # flow already uses. def organization_declines(organization, answers) - OrganizationServices::ProfileDiff.call( - organization: organization, - website: answers["organization_website"], - organization_type: answers["organization_type"], - address: { - street_address: answers["organization_street"], city: answers["organization_city"], - state: answers["organization_state"], zip_code: answers["organization_zip"], - country: answers["organization_country"] - } - ).map { |discrepancy| [ discrepancy.label, discrepancy.submitted, discrepancy.saved ] } + OrganizationServices::ProfileDiff.for_entry(organization: organization, entry: FormSubmission.org_entry_from(answers)) + .map { |discrepancy| [ discrepancy.label, discrepancy.submitted, discrepancy.saved ] } end def enroll_ce(registration, values) diff --git a/app/services/organization_services/capture_from_submission.rb b/app/services/organization_services/capture_from_submission.rb index abe0303e52..e5cc5f9bc0 100644 --- a/app/services/organization_services/capture_from_submission.rb +++ b/app/services/organization_services/capture_from_submission.rb @@ -51,26 +51,18 @@ def call profile_changes = SyncProfile.call( organization: organization, - website: field_value("organization_website"), - organization_type: field_value("organization_type"), + website: entry[:website], + organization_type: entry[:organization_type], overwrite: @overwrite, submission: @submission ).changes - address_result = UpsertAddress.call( - organization: organization, - street_address: field_value("organization_street"), - city: field_value("organization_city"), - state: field_value("organization_state"), - zip_code: field_value("organization_zip"), - country: field_value("organization_country"), - overwrite: @overwrite - ) + address_result = UpsertAddress.call(organization: organization, overwrite: @overwrite, **entry[:address]) AffiliationServices::CreateFromRegistration.call( person: @person, organization: organization, - job_title: field_value(FormField::ORGANIZATION_POSITION_FIELD_IDENTIFIER), + job_title: entry[:position], training_date: @training_date, organization_address: address_result.address, facilitator_training: @facilitator_training, @@ -85,18 +77,23 @@ def call def find_organization return @organization if @organization - name = field_value(FormField::ORGANIZATION_NAME_FIELD_IDENTIFIER)&.strip + name = entry[:org_name]&.strip return nil if name.blank? Organization.find_by(name: name) end - def field_value(key) - return @answers[key] if @answers + def entry + @entry ||= FormSubmission.org_entry_from(org_answers) + end + + def org_answers + identifiers = FormSubmission::ORG_ENTRY_FIELD_IDENTIFIERS + return @answers.slice(*identifiers) if @answers - field = @form.form_fields.find_by(field_identifier: key) - return nil unless field - @form_params[field.id.to_s] + @form.form_fields.where(field_identifier: identifiers) + .pluck(:field_identifier, :id) + .to_h { |identifier, id| [ identifier, @form_params[id.to_s] ] } end end end diff --git a/app/services/organization_services/link_submitted_organization.rb b/app/services/organization_services/link_submitted_organization.rb index b082901f9e..502b111f99 100644 --- a/app/services/organization_services/link_submitted_organization.rb +++ b/app/services/organization_services/link_submitted_organization.rb @@ -118,12 +118,7 @@ def upsert_address def conflicts return [] unless @entry - OrganizationServices::ProfileDiff.call( - organization: @organization, - website: @entry[:website], - organization_type: @entry[:organization_type], - address: submitted_address - ) + ProfileDiff.for_entry(organization: @organization, entry: @entry) end def submitted_address diff --git a/app/services/organization_services/profile_diff.rb b/app/services/organization_services/profile_diff.rb index be9a36c262..7a07bee2c1 100644 --- a/app/services/organization_services/profile_diff.rb +++ b/app/services/organization_services/profile_diff.rb @@ -20,6 +20,11 @@ def self.call(organization:, website: nil, organization_type: nil, address: {}) new(organization:, website:, organization_type:, address:).call end + # Compares the website, type, and address of a FormSubmission#org_entry-shaped hash. + def self.for_entry(organization:, entry:) + call(organization:, website: entry[:website], organization_type: entry[:organization_type], address: entry[:address]) + end + def initialize(organization:, website: nil, organization_type: nil, address: {}) @organization = organization @website = website diff --git a/spec/models/form_submission_spec.rb b/spec/models/form_submission_spec.rb index 9e1266f6d7..487d9ca2e1 100644 --- a/spec/models/form_submission_spec.rb +++ b/spec/models/form_submission_spec.rb @@ -111,6 +111,28 @@ end end + describe ".org_entry_from" do + it "shapes org answers by identifier, blanks as nil" do + entry = described_class.org_entry_from( + "organization_name" => "Acme", "organization_position" => "", "organization_city" => "Denver", "organization_state" => "CO" + ) + + expect(entry).to include(org_name: "Acme", position: nil, website: nil) + expect(entry[:address]).to include(city: "Denver", state: "CO", street_address: nil) + end + end + + describe "#org_entry" do + it "builds the entry from the submission's answers and carries the submission" do + form = create(:form) + field = create(:form_field, form: form, field_identifier: "organization_name", name: "Organization") + submission = create(:form_submission, form: form) + submission.form_answers.create!(form_field: field, submitted_answer: "Acme") + + expect(submission.org_entry).to include(submission: submission, org_name: "Acme") + end + end + describe "index filter scopes" do def submission_with_org_answer(org_name, person: create(:person, user: nil)) submission = create(:form_submission, person: person) diff --git a/spec/services/organization_services/profile_diff_spec.rb b/spec/services/organization_services/profile_diff_spec.rb index f8943a6286..52df0b36f6 100644 --- a/spec/services/organization_services/profile_diff_spec.rb +++ b/spec/services/organization_services/profile_diff_spec.rb @@ -61,6 +61,15 @@ expect(diff.map(&:field)).to contain_exactly(:website_url, :organization_type) end + it "reads website, type, and address from an org entry" do + organization.update!(organization_type: "For-profit", website_url: "https://acme.org") + entry = FormSubmission.org_entry_from("organization_website" => "https://other.org", "organization_type" => "Government agency") + + diff = described_class.for_entry(organization: organization, entry: entry) + + expect(diff.map(&:field)).to contain_exactly(:website_url, :organization_type) + end + describe "address" do it "flags street, ZIP and country that differ from the org's same-city address" do create(:address, addressable: organization, street_address: "5 Oak Ave", city: "Austin", state: "TX", zip_code: "78701", country: "USA") diff --git a/spec/services/smart_form_fields_spec.rb b/spec/services/smart_form_fields_spec.rb index 58d8229125..427132d420 100644 --- a/spec/services/smart_form_fields_spec.rb +++ b/spec/services/smart_form_fields_spec.rb @@ -48,11 +48,12 @@ app/services/organization_services/capture_from_submission.rb app/services/public_form_submission.rb ].map { |path| Rails.root.join(path).read }.join("\n") - read_identifiers = sources.scan(/field_value\("([a-z0-9_]+)"\)/).flatten.uniq + scanned = sources.scan(/field_value\("([a-z0-9_]+)"\)/).flatten.uniq + read_identifiers = (scanned + FormSubmission::ORG_ENTRY_FIELD_IDENTIFIERS).uniq documented = described_class.identifiers + described_class::ANSWER_ONLY_IDENTIFIERS # Guards the guard: a regex that stopped matching would pass vacuously. - expect(read_identifiers.size).to be > 20 + expect(scanned.size).to be > 15 expect(read_identifiers - documented).to be_empty end end