From 02cfe0879c5051628f72d5aa3351ce77639c528b Mon Sep 17 00:00:00 2001 From: maebeale Date: Sun, 7 Jun 2026 17:32:12 -0400 Subject: [PATCH 1/3] Guard event registration deletion against orphaning financial or attendance records Hiding the Delete button alone leaves the DELETE route open to admins/owners, so deletability is enforced in the model, view, and controller. Co-Authored-By: Claude Opus 4.8 --- .../event_registrations_controller.rb | 4 +- app/models/event_registration.rb | 12 +++++ app/views/event_registrations/_form.html.erb | 2 +- spec/models/event_registration_spec.rb | 46 +++++++++++++++++++ spec/requests/event_registrations_spec.rb | 11 +++++ 5 files changed, 73 insertions(+), 2 deletions(-) diff --git a/app/controllers/event_registrations_controller.rb b/app/controllers/event_registrations_controller.rb index 8bba54a91c..fa8013af25 100644 --- a/app/controllers/event_registrations_controller.rb +++ b/app/controllers/event_registrations_controller.rb @@ -273,7 +273,9 @@ def unlink_organization def destroy authorize! @event_registration event = @event_registration.event - if @event_registration.destroy + if !@event_registration.deletable? + flash[:alert] = "This registration can't be deleted because it has payments, scholarships, or attendance on record." + elsif @event_registration.destroy flash[:notice] = "Registration deleted." else flash[:alert] = @event_registration.errors.full_messages.to_sentence diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index 63e0ca6ebb..0c522c6e83 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -195,6 +195,18 @@ def active? status.in?(ACTIVE_STATUSES) end + def attended? + status.in?(%w[ attended incomplete_attendance ]) + end + + # Safe to delete only when removing the record would not orphan financial + # data or erase attendance history. Allocations (payments and scholarships) + # have no dependent: :destroy, so a registration with any allocation must be + # kept, as must one with attendance on record. + def deletable? + !allocations.exists? && !attended? + end + def checked_in? # checked_in_at.present? end diff --git a/app/views/event_registrations/_form.html.erb b/app/views/event_registrations/_form.html.erb index 13a1091ad2..da55f3db5b 100644 --- a/app/views/event_registrations/_form.html.erb +++ b/app/views/event_registrations/_form.html.erb @@ -432,7 +432,7 @@ <%# ---- Footer actions ---- %>
- <% if allowed_to?(:destroy?, f.object) %> + <% if allowed_to?(:destroy?, f.object) && f.object.deletable? %> <%= link_to event_registration_path(@event_registration, return_to: params[:return_to].presence), class: "btn btn-danger-outline", data: { turbo: true, turbo_method: :delete, turbo_confirm: "Are you sure you want to delete?" } do %> diff --git a/spec/models/event_registration_spec.rb b/spec/models/event_registration_spec.rb index 9fdc1ef1b8..8e65673192 100644 --- a/spec/models/event_registration_spec.rb +++ b/spec/models/event_registration_spec.rb @@ -50,6 +50,52 @@ end end + describe "#attended?" do + it "returns true for attended status" do + expect(create(:event_registration, status: "attended")).to be_attended + end + + it "returns true for incomplete_attendance status" do + expect(create(:event_registration, status: "incomplete_attendance")).to be_attended + end + + it "returns false for registered, cancelled, and no_show statuses" do + expect(create(:event_registration, status: "registered")).not_to be_attended + expect(create(:event_registration, status: "cancelled")).not_to be_attended + expect(create(:event_registration, status: "no_show")).not_to be_attended + end + end + + describe "#deletable?" do + it "returns true for a plain registration with no allocations or attendance" do + reg = create(:event_registration, status: "registered") + expect(reg).to be_deletable + end + + it "returns false when the registration has a payment allocation" do + reg = create(:event_registration, status: "registered") + payment = create(:payment, person: reg.registrant, amount_cents: 1000, amount_cents_remaining: nil) + create(:allocation, source: payment, allocatable: reg, amount: 1000) + expect(reg).not_to be_deletable + end + + it "returns false when the registration has a scholarship allocation" do + reg = create(:event_registration, status: "registered") + scholarship = create(:scholarship, recipient: reg.registrant, amount_cents: 1000) + create(:allocation, source: scholarship, allocatable: reg, amount: 1000) + expect(reg).not_to be_deletable + end + + it "returns false when the registration has attendance on record" do + expect(create(:event_registration, status: "attended")).not_to be_deletable + expect(create(:event_registration, status: "incomplete_attendance")).not_to be_deletable + end + + it "returns true for a cancelled registration with no allocations" do + expect(create(:event_registration, status: "cancelled")).to be_deletable + end + end + describe ".registrant_ids" do it "returns registrations for the registrants in a hyphenated id list" do person_a = create(:person) diff --git a/spec/requests/event_registrations_spec.rb b/spec/requests/event_registrations_spec.rb index cbbf84ab23..bb8ae52abc 100644 --- a/spec/requests/event_registrations_spec.rb +++ b/spec/requests/event_registrations_spec.rb @@ -328,6 +328,17 @@ def unrequest(registration) delete event_registration_path(existing_registration) }.to change(EventRegistration, :count).by(-1) end + + it "refuses to delete a registration with payments on record" do + payment = create(:payment, person: regular_user.person, amount_cents: 1000, amount_cents_remaining: nil) + create(:allocation, source: payment, allocatable: existing_registration, amount: 1000) + + expect { + delete event_registration_path(existing_registration) + }.not_to change(EventRegistration, :count) + + expect(flash[:alert]).to include("can't be deleted") + end end describe "organization linking" do From d19ee4cc332aea43cde6d6354901c8f2687cb0ee Mon Sep 17 00:00:00 2001 From: maebeale Date: Sun, 7 Jun 2026 17:35:34 -0400 Subject: [PATCH 2/3] Apply deletability guard to public self-service de-register path Main's registration-suite added a public De-register button that hard-deletes the registration; without the same guard a paid registrant could orphan their allocation. Hide the button and refuse the destroy when not deletable. Co-Authored-By: Claude Opus 4.8 --- .../events/registrations_controller.rb | 9 +++++++++ app/views/events/_registration_button.html.erb | 12 +++++++----- spec/requests/events/registrations_spec.rb | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/app/controllers/events/registrations_controller.rb b/app/controllers/events/registrations_controller.rb index 1d62daecf0..a60fa45320 100644 --- a/app/controllers/events/registrations_controller.rb +++ b/app/controllers/events/registrations_controller.rb @@ -125,6 +125,15 @@ def destroy authorize! @event_registration + unless @event_registration.deletable? + alert = "You can't de-register because there are payments or attendance on record. Cancel your registration instead." + respond_to do |format| + format.turbo_stream { flash.now[:alert] = alert } + format.html { redirect_to @event, alert: alert } + end + return + end + if @event_registration.destroy success = "You are no longer registered." respond_to do |format| diff --git a/app/views/events/_registration_button.html.erb b/app/views/events/_registration_button.html.erb index 1fe235fe6f..f3c1636eca 100644 --- a/app/views/events/_registration_button.html.erb +++ b/app/views/events/_registration_button.html.erb @@ -7,11 +7,13 @@ No event <% elsif Current.user.nil? %> Login to register - <% elsif event.actively_registered?(Current.user.person) %> - <%= link_to "De-register", - event_registrant_registration_path(event_id: event), - data: { turbo_method: :delete, turbo_confirm: "Are you sure?"}, - class: "btn btn-secondary-outline text-sm" %> + <% elsif (registration = event.active_registration_for(Current.user.person)) %> + <% if registration.deletable? %> + <%= link_to "De-register", + event_registrant_registration_path(event_id: event), + data: { turbo_method: :delete, turbo_confirm: "Are you sure?"}, + class: "btn btn-secondary-outline text-sm" %> + <% end %> Registered <% elsif event.ended? %> Event ended diff --git a/spec/requests/events/registrations_spec.rb b/spec/requests/events/registrations_spec.rb index a425630c06..12b07e58ea 100644 --- a/spec/requests/events/registrations_spec.rb +++ b/spec/requests/events/registrations_spec.rb @@ -586,6 +586,24 @@ end end + context "when the registration has payments on record" do + let!(:registration) { create(:event_registration, event: event, registrant: user.person) } + + before do + payment = create(:payment, person: user.person, amount_cents: 1000, amount_cents_remaining: nil) + create(:allocation, source: payment, allocatable: registration, amount: 1000) + end + + it "refuses to destroy and returns an alert" do + expect { + delete event_registrant_registration_path(event_id: event.id), + headers: turbo_headers + }.not_to change(EventRegistration, :count) + + expect(flash.now[:alert]).to include("Cancel your registration instead") + end + end + context "when destroy fails" do let!(:registration) { create(:event_registration, event: event, registrant: user.person) } From 796ac982bc62da23375e87bfb94034aa517b6a54 Mon Sep 17 00:00:00 2001 From: maebeale Date: Mon, 22 Jun 2026 08:27:05 -0400 Subject: [PATCH 3/3] Reconcile deletability with main's attended? after rebase Main added its own attended? (status == "attended", used by certificate and joinable logic). Drop the duplicate this branch introduced and inline the attendance check in deletable? so it still blocks both attended and incomplete_attendance without clobbering main's method. Co-Authored-By: Claude Opus 4.8 --- app/models/event_registration.rb | 8 ++------ spec/models/event_registration_spec.rb | 16 ---------------- 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/app/models/event_registration.rb b/app/models/event_registration.rb index 0c522c6e83..029ee37fbf 100644 --- a/app/models/event_registration.rb +++ b/app/models/event_registration.rb @@ -195,16 +195,12 @@ def active? status.in?(ACTIVE_STATUSES) end - def attended? - status.in?(%w[ attended incomplete_attendance ]) - end - # Safe to delete only when removing the record would not orphan financial # data or erase attendance history. Allocations (payments and scholarships) # have no dependent: :destroy, so a registration with any allocation must be - # kept, as must one with attendance on record. + # kept, as must one with attendance (attended or incomplete) on record. def deletable? - !allocations.exists? && !attended? + !allocations.exists? && !status.in?(%w[ attended incomplete_attendance ]) end def checked_in? diff --git a/spec/models/event_registration_spec.rb b/spec/models/event_registration_spec.rb index 8e65673192..1b92cc6724 100644 --- a/spec/models/event_registration_spec.rb +++ b/spec/models/event_registration_spec.rb @@ -50,22 +50,6 @@ end end - describe "#attended?" do - it "returns true for attended status" do - expect(create(:event_registration, status: "attended")).to be_attended - end - - it "returns true for incomplete_attendance status" do - expect(create(:event_registration, status: "incomplete_attendance")).to be_attended - end - - it "returns false for registered, cancelled, and no_show statuses" do - expect(create(:event_registration, status: "registered")).not_to be_attended - expect(create(:event_registration, status: "cancelled")).not_to be_attended - expect(create(:event_registration, status: "no_show")).not_to be_attended - end - end - describe "#deletable?" do it "returns true for a plain registration with no allocations or attendance" do reg = create(:event_registration, status: "registered")