From 774feb0823b770e1ca006102660a75ee6ce5ab7a Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 31 Aug 2026 17:04:33 -0400 Subject: [PATCH 1/4] Append a new form submission on re-registration instead of overwriting A returning registrant (same name + email) reuses their existing Person and single event registration, but re-submitting the form used to overwrite their prior FormSubmission in place. That silently lost their earlier answers. Each registration now records its own FormSubmission, so the full history is preserved, and the public confirmation view defaults to the most recent submission when none is named. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../events/public_registrations_controller.rb | 6 +- .../public_registration.rb | 12 +--- .../events/public_registrations_spec.rb | 15 +++++ .../public_registration_spec.rb | 66 ++++++++++++++----- 4 files changed, 68 insertions(+), 31 deletions(-) diff --git a/app/controllers/events/public_registrations_controller.rb b/app/controllers/events/public_registrations_controller.rb index 81c363342f..ca9832ed15 100644 --- a/app/controllers/events/public_registrations_controller.rb +++ b/app/controllers/events/public_registrations_controller.rb @@ -118,12 +118,14 @@ def show end # Scope submissions to this event so a person registered for multiple - # events that share the same form sees only the answers for this one. + # events that share the same form sees only the answers for this one. A + # returning registrant appends a new submission each time, so default to the + # most recent — their freshest answers — when none is named. submissions = @registration_form.form_submissions.where(person: person, event: @event) @form_submission = if params[:form_submission_id].present? submissions.find_by(id: params[:form_submission_id]) else - submissions.first + submissions.order(:created_at).last end unless @form_submission redirect_to event_path(@event), alert: "No registration form submission found." diff --git a/app/services/event_registration_services/public_registration.rb b/app/services/event_registration_services/public_registration.rb index 3e8050bf86..b7415b2bb4 100644 --- a/app/services/event_registration_services/public_registration.rb +++ b/app/services/event_registration_services/public_registration.rb @@ -107,7 +107,7 @@ def call send_notifications(existing) end organization_link = connect_organization(existing, organization) - submission = update_form_submission(person) + submission = create_form_submission(person) organization_link&.record_form_submission(submission) save_scholarship_submission(person) save_continuing_education_submission(person) @@ -494,16 +494,6 @@ def create_form_submission(person) submission end - def update_form_submission(person) - submission = FormSubmission.find_or_create_by!(person: person, form: @registration_form, role: "registration", event: @event) do |record| - record.event = @event - end - save_form_answers(submission) - OtherResponses::CaptureFromSubmission.call(submission) - Quotes::CaptureFromSubmission.call(submission) - submission - 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/spec/requests/events/public_registrations_spec.rb b/spec/requests/events/public_registrations_spec.rb index 4c7f89be12..f1439c017c 100644 --- a/spec/requests/events/public_registrations_spec.rb +++ b/spec/requests/events/public_registrations_spec.rb @@ -735,6 +735,21 @@ def identity_answers end end + # A returning registrant appends a new submission each time, so the default + # view (no form_submission_id) must land on their freshest answers. + it "defaults to the most recently created submission when the registrant re-registered" do + older = FormSubmission.find_by(person: person, form: form) + older.form_answers.create!(form_field: essay_field, submitted_answer: "my older reasons") + newer = create(:form_submission, person: person, form: form, event: event) + newer.form_answers.create!(form_field: essay_field, submitted_answer: "my newer reasons") + + get event_public_registration_path(event, person_id: person.id) + + expect(response).to have_http_status(:success) + expect(response.body).to include("my newer reasons") + expect(response.body).not_to include("my older reasons") + end + it "renders header and field-label HTML unescaped on the response page" do create(:form_field, form: form, answer_type: :group_header, name: "Your details") create(:form_field, form: form, answer_type: :free_form_input_one_line, name: "Name", required: false) diff --git a/spec/services/event_registration_services/public_registration_spec.rb b/spec/services/event_registration_services/public_registration_spec.rb index 98cefa9f97..fde82e850f 100644 --- a/spec/services/event_registration_services/public_registration_spec.rb +++ b/spec/services/event_registration_services/public_registration_spec.rb @@ -610,6 +610,29 @@ def register_with_organization_type(value) end end + describe "re-submitting the registration form with the same name and email" do + let(:params) { base_form_params(first_name: "Rae", last_name: "Fox", email: "rae@example.com") } + + it "reuses the same person rather than creating a duplicate" do + described_class.call(event: event, registration_form: form, form_params: params) + + expect { + described_class.call(event: event, registration_form: form, form_params: params) + }.not_to change(Person, :count) + + expect(Person.where(email: "rae@example.com").count).to eq(1) + end + + it "keeps a single event registration but appends a second form submission" do + described_class.call(event: event, registration_form: form, form_params: params) + described_class.call(event: event, registration_form: form, form_params: params) + + person = Person.find_by!(email: "rae@example.com") + expect(event.event_registrations.where(registrant: person).count).to eq(1) + expect(person.form_submissions.where(form: form, role: "registration", event: event).count).to eq(2) + end + end + describe "an answer longer than its database column" do # `city` (like the other mapped person/address columns) is a varchar(255). # A longer answer must surface as a form error, not an ActiveRecord::ValueTooLong @@ -1135,48 +1158,55 @@ def signed_id_for(fixture, content_type) expect(Person.find_by(email: "badid@example.com")).to be_nil end - # submitted_answer is only a cache of the attachment's name. Pinned across - # every transition so a second writer that skips FormAnswer#sync_uploaded_filename! - # fails here rather than silently desyncing the two. - it "keeps submitted_answer equal to the attached filename through upload, replace, and blank re-submit" do + # submitted_answer is only a cache of the attachment's name. Each re-submission + # is its own FormSubmission, so its answer's submitted_answer must equal its own + # attachment — a writer that skips FormAnswer#sync_uploaded_filename! fails here + # rather than silently desyncing the two. + it "keeps each submission's submitted_answer equal to its own attached filename" do params = base_form_params(first_name: "Sync", last_name: "Check", email: "sync@example.com") in_sync = lambda do |result| answer = result.form_submission.form_answers.find_by(form_field: upload_field) expect(answer.submitted_answer).to eq(answer.uploaded_file&.filename.to_s) - answer + result.form_submission end - uploaded = in_sync.call(described_class.call( + png = in_sync.call(described_class.call( event: event, registration_form: form, form_params: params.merge(upload_field.id.to_s => signed_id_for("sample.png", "image/png")) )) - expect(uploaded.submitted_answer).to eq("sample.png") - - replaced = in_sync.call(described_class.call( + pdf = in_sync.call(described_class.call( event: event, registration_form: form, form_params: params.merge(upload_field.id.to_s => signed_id_for("sample.pdf", "application/pdf")) )) - expect(replaced.submitted_answer).to eq("sample.pdf") - - blanked = in_sync.call(described_class.call( + blank = in_sync.call(described_class.call( event: event, registration_form: form, form_params: params.merge(upload_field.id.to_s => "") )) - expect(blanked.submitted_answer).to eq("sample.pdf") + + expect([ png, pdf, blank ].map(&:id).uniq.size).to eq(3) + expect(png.form_answers.find_by(form_field: upload_field).submitted_answer).to eq("sample.png") + expect(pdf.form_answers.find_by(form_field: upload_field).submitted_answer).to eq("sample.pdf") + expect(blank.form_answers.find_by(form_field: upload_field).submitted_answer).to eq("") end - it "keeps the file already on the answer when a re-submission leaves the field blank" do + it "leaves the earlier submission's file intact when a blank re-submission adds a fileless one" do params = base_form_params(first_name: "Re", last_name: "Sub", email: "resub@example.com").merge( upload_field.id.to_s => signed_id_for("sample.png", "image/png") ) - described_class.call(event: event, registration_form: form, form_params: params) + first = described_class.call(event: event, registration_form: form, form_params: params) result = described_class.call(event: event, registration_form: form, form_params: params.merge(upload_field.id.to_s => "")) expect(result.success?).to be true - answer = result.form_submission.form_answers.find_by(form_field: upload_field) - expect(answer.uploaded_file).to be_attached - expect(answer.submitted_answer).to eq("sample.png") + expect(result.form_submission).not_to eq(first.form_submission) + + original_answer = first.form_submission.form_answers.find_by(form_field: upload_field) + expect(original_answer.uploaded_file).to be_attached + expect(original_answer.submitted_answer).to eq("sample.png") + + blank_answer = result.form_submission.form_answers.find_by(form_field: upload_field) + expect(blank_answer.uploaded_file).to be_nil + expect(blank_answer.submitted_answer).to eq("") end end end From 6067cafacb4aab5ab8e530d41b4da1aa3332b344 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 31 Aug 2026 17:15:08 -0400 Subject: [PATCH 2/4] Prevent double-submit from creating duplicate people and registrations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A double-click on the public registration form fired two POSTs. Worst on paid events, which opt out of Turbo (turbo: false) and so had no submitter disabling at all. Each POST ran find_or_create_person independently, neither saw the other's uncommitted person, so both created a Person (and thus a second registration the registrant_id unique index couldn't catch) — two people, two registrations, same minute. Two layers of defense: - Client: attach the existing submit-once Stimulus controller to the form so the button locks after the first click, on both the Turbo (free) and non-Turbo (paid) variants. - Server: serialize concurrent submissions for the same identity behind a MySQL named lock (keyed on email + last name + event, namespaced by database so parallel test suites don't contend), so the second request waits and matches the first's committed person. Best-effort — a blank identity or an un-acquirable lock falls through rather than failing a real registration. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../public_registration.rb | 48 +++++++++++++++++++ .../events/public_registrations/new.html.erb | 2 +- .../events/public_registrations_spec.rb | 18 +++++++ .../public_registration_spec.rb | 20 ++++++++ 4 files changed, 87 insertions(+), 1 deletion(-) diff --git a/app/services/event_registration_services/public_registration.rb b/app/services/event_registration_services/public_registration.rb index b7415b2bb4..94512c87b9 100644 --- a/app/services/event_registration_services/public_registration.rb +++ b/app/services/event_registration_services/public_registration.rb @@ -2,6 +2,10 @@ module EventRegistrationServices class PublicRegistration Result = Struct.new(:success?, :event_registration, :form_submission, :errors, keyword_init: true) + # How long a concurrent submission waits for the in-flight one to finish before + # giving up on the identity lock and proceeding anyway (see #acquire_identity_lock). + LOCK_TIMEOUT_SECONDS = 10 + # Well-known field_identifier of the "magic" CE question seeded onto the # registration form. Answering it "Yes" creates a ContinuingEducationRegistration # (hours come from the event). Kept here so the seed, service, and specs agree. @@ -66,6 +70,7 @@ def initialize(event:, registration_form:, form_params:, scholarship_requested: end def call + lock_name = acquire_identity_lock ActiveRecord::Base.transaction do person = find_or_create_person sync_person_profile(person) @@ -138,10 +143,53 @@ def call else Result.new(success?: false, event_registration: nil, errors: [ e.message ]) end + ensure + release_identity_lock(lock_name) end private + # Serialize concurrent submissions for the same identity so a double-submit (two + # POSTs racing before either commits) can't slip past find_or_create_person and + # mint duplicate people and registrations. A MySQL named lock — no schema change, + # released the moment this submission commits — makes the second request wait, + # then match the first's now-committed person. The lock name is namespaced by + # database so parallel test suites (separate databases, one shared server) don't + # contend. Best-effort: a blank identity has nothing to dedupe, and a lock we + # couldn't get within the timeout falls through rather than failing a real + # registration. + def acquire_identity_lock + key = identity_lock_key + return if key.blank? + + connection = ActiveRecord::Base.connection + namespaced = "#{connection.current_database}|#{key}" + lock_name = "awbw-reg-#{Digest::SHA256.hexdigest(namespaced)[0, 40]}" + connection.select_value("SELECT GET_LOCK(#{connection.quote(lock_name)}, #{LOCK_TIMEOUT_SECONDS})") + lock_name + end + + def release_identity_lock(lock_name) + return if lock_name.blank? + + connection = ActiveRecord::Base.connection + connection.select_value("SELECT RELEASE_LOCK(#{connection.quote(lock_name)})") + end + + # The identity two concurrent submissions would collide on: a signed-in person is + # already resolved (no matching needed), so lock on them; an incognito submission + # locks on the strong, stable identifiers find_matching_person keys on — email and + # last name — for this event. Blank when we can't dedupe (missing email/last name). + def identity_lock_key + return "#{@event.id}|person|#{@person.id}" if @person + + last_name = field_value("last_name")&.strip&.downcase + email = field_value("primary_email")&.strip&.downcase + return if email.blank? || last_name.blank? + + "#{@event.id}|#{email}|#{last_name}" + end + # Turn a database "Data too long for column 'city'" failure into a friendly, # form-level message. We can't always map the column back to a single form # field (both the mailing and organization address write `city`), so we name the diff --git a/app/views/events/public_registrations/new.html.erb b/app/views/events/public_registrations/new.html.erb index a958acaee0..9f15c83d66 100644 --- a/app/views/events/public_registrations/new.html.erb +++ b/app/views/events/public_registrations/new.html.erb @@ -94,7 +94,7 @@ <% end %> - <%= form_with url: event_public_registration_path(@event), method: :post, local: true, class: "space-y-2", data: { turbo: !@event.cost_cents.to_i.positive? } do |f| %> + <%= form_with url: event_public_registration_path(@event), method: :post, local: true, class: "space-y-2", data: { turbo: !@event.cost_cents.to_i.positive?, controller: "submit-once", "submit-once-submitting-text-value": "Registering…" } do |f| %> <% if @scholarship %> diff --git a/spec/requests/events/public_registrations_spec.rb b/spec/requests/events/public_registrations_spec.rb index f1439c017c..4f960cc8b0 100644 --- a/spec/requests/events/public_registrations_spec.rb +++ b/spec/requests/events/public_registrations_spec.rb @@ -250,6 +250,24 @@ def post_with_scholarship(scholarship_answer) expect(response.body).to include("Minimum of 5 words.") end + # A double-click used to fire two POSTs and create duplicate people and + # registrations — worst on paid events, which opt out of Turbo (turbo: false) + # and so lose even Turbo's own submitter disabling. + it "guards the form against double-submit on a free event" do + get new_event_public_registration_path(event) + + expect(response.body).to include('data-controller="submit-once"') + end + + it "guards the form against double-submit on a paid event that opts out of Turbo" do + event.update!(cost_cents: 5_000) + + get new_event_public_registration_path(event) + + expect(response.body).to include('data-controller="submit-once"') + expect(response.body).to include('data-turbo="false"') + end + it "surfaces the CE deadlines on the continuing education section" do ce = FormBuilderService.new(name: "CE", sections: %i[continuing_education], role: "continuing_education").call event.event_forms.create!(form: ce, role: "continuing_education") diff --git a/spec/services/event_registration_services/public_registration_spec.rb b/spec/services/event_registration_services/public_registration_spec.rb index fde82e850f..44d4912a1a 100644 --- a/spec/services/event_registration_services/public_registration_spec.rb +++ b/spec/services/event_registration_services/public_registration_spec.rb @@ -631,6 +631,26 @@ def register_with_organization_type(value) expect(event.event_registrations.where(registrant: person).count).to eq(1) expect(person.form_submissions.where(form: form, role: "registration", event: event).count).to eq(2) end + + # Two POSTs racing before either commits used to each mint a person + registration. + # A MySQL named lock serializes them so the second waits and matches the first's + # person. A true concurrent race is not unit-testable without flaky threads, so we + # assert the guard is wired: the lock is taken and released around the flow. + it "serializes concurrent submissions behind a released identity lock" do + statements = [] + subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| + statements << ActiveSupport::Notifications::Event.new(*args).payload[:sql] + end + + begin + described_class.call(event: event, registration_form: form, form_params: params) + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) + end + + expect(statements).to include(a_string_matching(/GET_LOCK\('awbw-reg-/)) + expect(statements).to include(a_string_matching(/RELEASE_LOCK\('awbw-reg-/)) + end end describe "an answer longer than its database column" do From 1fea2352efb736c725c4f0646d9808ff0c038a82 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Mon, 31 Aug 2026 17:19:15 -0400 Subject: [PATCH 3/4] Add Features & tips entry for duplicate-registration handling Announce that repeat registration submissions no longer create duplicate people/registrations, framed around repeat submissions generally (not just a double-click) since it also covers registering again later. Co-Authored-By: Claude Opus 4.8 (1M context) --- config/features.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/config/features.yml b/config/features.yml index bf94271fa9..ed93a01108 100644 --- a/config/features.yml +++ b/config/features.yml @@ -39,6 +39,20 @@ action_path: /organizations pr_number: 2464 +- name: "Repeat registration submissions no longer create duplicates" + area: registration + display_status: user_facing + released_on: 2026-08-31 + summary: >- + If someone submits an event registration more than once — two submissions in a + row, or coming back to register again later — they now stay a single person with + one registration for that event, instead of appearing as duplicates on your + roster. Every submission is still saved, so no answers are lost. + pr_number: 2465 + pro_tips: + - "The registrant's confirmation page shows their most recent submission." + - "Each submission is kept in full, so you can see what changed between them." + - name: "Set who a workshop log is credited to" area: content display_status: admin_facing From 99930ee05b5b4cf23298548e8e88564f8a4c3a76 Mon Sep 17 00:00:00 2001 From: Justin Miller <16829344+jmilljr24@users.noreply.github.com> Date: Tue, 1 Sep 2026 10:19:05 -0400 Subject: [PATCH 4/4] remove lock --- .../public_registration.rb | 48 ------------------- .../public_registration_spec.rb | 20 -------- 2 files changed, 68 deletions(-) diff --git a/app/services/event_registration_services/public_registration.rb b/app/services/event_registration_services/public_registration.rb index 94512c87b9..b7415b2bb4 100644 --- a/app/services/event_registration_services/public_registration.rb +++ b/app/services/event_registration_services/public_registration.rb @@ -2,10 +2,6 @@ module EventRegistrationServices class PublicRegistration Result = Struct.new(:success?, :event_registration, :form_submission, :errors, keyword_init: true) - # How long a concurrent submission waits for the in-flight one to finish before - # giving up on the identity lock and proceeding anyway (see #acquire_identity_lock). - LOCK_TIMEOUT_SECONDS = 10 - # Well-known field_identifier of the "magic" CE question seeded onto the # registration form. Answering it "Yes" creates a ContinuingEducationRegistration # (hours come from the event). Kept here so the seed, service, and specs agree. @@ -70,7 +66,6 @@ def initialize(event:, registration_form:, form_params:, scholarship_requested: end def call - lock_name = acquire_identity_lock ActiveRecord::Base.transaction do person = find_or_create_person sync_person_profile(person) @@ -143,53 +138,10 @@ def call else Result.new(success?: false, event_registration: nil, errors: [ e.message ]) end - ensure - release_identity_lock(lock_name) end private - # Serialize concurrent submissions for the same identity so a double-submit (two - # POSTs racing before either commits) can't slip past find_or_create_person and - # mint duplicate people and registrations. A MySQL named lock — no schema change, - # released the moment this submission commits — makes the second request wait, - # then match the first's now-committed person. The lock name is namespaced by - # database so parallel test suites (separate databases, one shared server) don't - # contend. Best-effort: a blank identity has nothing to dedupe, and a lock we - # couldn't get within the timeout falls through rather than failing a real - # registration. - def acquire_identity_lock - key = identity_lock_key - return if key.blank? - - connection = ActiveRecord::Base.connection - namespaced = "#{connection.current_database}|#{key}" - lock_name = "awbw-reg-#{Digest::SHA256.hexdigest(namespaced)[0, 40]}" - connection.select_value("SELECT GET_LOCK(#{connection.quote(lock_name)}, #{LOCK_TIMEOUT_SECONDS})") - lock_name - end - - def release_identity_lock(lock_name) - return if lock_name.blank? - - connection = ActiveRecord::Base.connection - connection.select_value("SELECT RELEASE_LOCK(#{connection.quote(lock_name)})") - end - - # The identity two concurrent submissions would collide on: a signed-in person is - # already resolved (no matching needed), so lock on them; an incognito submission - # locks on the strong, stable identifiers find_matching_person keys on — email and - # last name — for this event. Blank when we can't dedupe (missing email/last name). - def identity_lock_key - return "#{@event.id}|person|#{@person.id}" if @person - - last_name = field_value("last_name")&.strip&.downcase - email = field_value("primary_email")&.strip&.downcase - return if email.blank? || last_name.blank? - - "#{@event.id}|#{email}|#{last_name}" - end - # Turn a database "Data too long for column 'city'" failure into a friendly, # form-level message. We can't always map the column back to a single form # field (both the mailing and organization address write `city`), so we name the diff --git a/spec/services/event_registration_services/public_registration_spec.rb b/spec/services/event_registration_services/public_registration_spec.rb index 44d4912a1a..fde82e850f 100644 --- a/spec/services/event_registration_services/public_registration_spec.rb +++ b/spec/services/event_registration_services/public_registration_spec.rb @@ -631,26 +631,6 @@ def register_with_organization_type(value) expect(event.event_registrations.where(registrant: person).count).to eq(1) expect(person.form_submissions.where(form: form, role: "registration", event: event).count).to eq(2) end - - # Two POSTs racing before either commits used to each mint a person + registration. - # A MySQL named lock serializes them so the second waits and matches the first's - # person. A true concurrent race is not unit-testable without flaky threads, so we - # assert the guard is wired: the lock is taken and released around the flow. - it "serializes concurrent submissions behind a released identity lock" do - statements = [] - subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| - statements << ActiveSupport::Notifications::Event.new(*args).payload[:sql] - end - - begin - described_class.call(event: event, registration_form: form, form_params: params) - ensure - ActiveSupport::Notifications.unsubscribe(subscriber) - end - - expect(statements).to include(a_string_matching(/GET_LOCK\('awbw-reg-/)) - expect(statements).to include(a_string_matching(/RELEASE_LOCK\('awbw-reg-/)) - end end describe "an answer longer than its database column" do