From 1d449832f240cac52ea62c574990b20674653eb0 Mon Sep 17 00:00:00 2001 From: maebeale Date: Tue, 3 Mar 2026 09:23:13 -0500 Subject: [PATCH 1/5] Restrict ended events to registered users and grey out cards Ended events were visible to all users regardless of registration status. Now only admins and actively registered users can see them in the index and via direct URL. Ended event cards render with reduced opacity and muted styling. Co-Authored-By: Claude Opus 4.6 --- app/policies/event_policy.rb | 35 ++++++++++++++++---- app/views/events/_card.html.erb | 8 +++-- spec/factories/events.rb | 6 ++++ spec/policies/event_policy_spec.rb | 52 +++++++++++++++++++++++++----- 4 files changed, 84 insertions(+), 17 deletions(-) diff --git a/app/policies/event_policy.rb b/app/policies/event_policy.rb index 21e860c7e..49f00aed3 100644 --- a/app/policies/event_policy.rb +++ b/app/policies/event_policy.rb @@ -8,7 +8,13 @@ def index? end def show? - admin? || record.publicly_visible? || (authenticated? && record.published?) + return true if admin? + + if record.ended? + authenticated? && record.published? && record.actively_registered?(user.person) + else + record.publicly_visible? || (authenticated? && record.published?) + end end def register? @@ -39,15 +45,30 @@ def owner? relation_scope do |relation| next relation if admin? - if authenticated? # logged in users can see events they are registered for even if registration is closed - relation.left_outer_joins(:registrants) - .published - .where("registration_close_date IS NULL OR registration_close_date >= ? OR people.id = ?", Time.current, user.person_id) - .distinct + + if authenticated? + active_statuses = EventRegistration::ACTIVE_STATUSES.map { |s| relation.connection.quote(s) }.join(", ") + relation + .joins( + "LEFT OUTER JOIN event_registrations + ON event_registrations.event_id = events.id + AND event_registrations.status IN (#{active_statuses}) + LEFT OUTER JOIN people + ON people.id = event_registrations.registrant_id" + ) + .published + .where( + "(events.end_date >= :now AND (events.registration_close_date IS NULL OR events.registration_close_date >= :now)) + OR people.id = :person_id", + now: Time.current, + person_id: user.person_id + ) + .distinct else relation.publicly_visible .published - .where("registration_close_date IS NULL OR registration_close_date >= ?", Time.current) + .where("events.end_date >= ?", Time.current) + .where("events.registration_close_date IS NULL OR events.registration_close_date >= ?", Time.current) end end end diff --git a/app/views/events/_card.html.erb b/app/views/events/_card.html.erb index 7057f2cb9..e84731775 100644 --- a/app/views/events/_card.html.erb +++ b/app/views/events/_card.html.erb @@ -1,8 +1,12 @@ <% event = event.decorate %> +<% ended = event.ended? %> <%= tag.div id: dom_id(event, :card), - class: "relative bg-white border border-gray-200 rounded-2xl shadow-sm hover:shadow-md - transition p-4 flex flex-col gap-4 h-full" do %> + class: class_names( + "relative rounded-2xl shadow-sm transition p-4 flex flex-col gap-4 h-full", + "bg-gray-100 border border-gray-300 opacity-60": ended, + "bg-white border border-gray-200 hover:shadow-md": !ended + ) do %>
<%= render "bookmarks/editable_bookmark_icon", resource: event.object %> diff --git a/spec/factories/events.rb b/spec/factories/events.rb index 8bf9f2e77..24ee68031 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -29,6 +29,12 @@ publicly_featured { true } end + trait :ended do + start_date { 14.days.ago } + end_date { 12.days.ago } + registration_close_date { 13.days.ago } + end + trait :registration_closed do registration_close_date { 13.days.ago } end diff --git a/spec/policies/event_policy_spec.rb b/spec/policies/event_policy_spec.rb index 598ff26aa..001b4df66 100644 --- a/spec/policies/event_policy_spec.rb +++ b/spec/policies/event_policy_spec.rb @@ -6,6 +6,7 @@ let(:published_event) { build_stubbed :event, :published } let(:public_event) { build_stubbed :event, publicly_visible: true } let(:unpublished_event) { build_stubbed :event, :unpublished } + let(:ended_event) { build_stubbed :event, :published, :ended } let(:open_registration_event) { build_stubbed :event, registration_close_date: 1.day.from_now } let(:closed_registration_event) { build_stubbed :event, registration_close_date: 1.day.ago } @@ -54,6 +55,36 @@ def policy_for(record: nil, user:) end end + context "when event has ended" do + context "with admin user" do + subject { policy_for(record: ended_event, user: admin_user) } + + it { is_expected.to be_allowed_to(:show?) } + end + + context "with registered user" do + subject { policy_for(record: ended_event, user: regular_user) } + + before { allow(ended_event).to receive(:actively_registered?).with(regular_user.person).and_return(true) } + + it { is_expected.to be_allowed_to(:show?) } + end + + context "with unregistered user" do + subject { policy_for(record: ended_event, user: regular_user) } + + before { allow(ended_event).to receive(:actively_registered?).with(regular_user.person).and_return(false) } + + it { is_expected.not_to be_allowed_to(:show?) } + end + + context "with no user" do + subject { policy_for(record: ended_event, user: nil) } + + it { is_expected.not_to be_allowed_to(:show?) } + end + end + context "when event is not visible" do context "with admin user" do subject { policy_for(record: unpublished_event, user: admin_user) } @@ -175,22 +206,27 @@ def policy_for(record: nil, user:) context "with regular user" do let(:policy) { policy_for(record: Event, user: regular_user) } - it "returns only visible events with open registration" do + it "returns only visible events with open registration or active registrations" do scope = policy.apply_scope(Event.all, type: :active_record_relation) - expect(scope.to_sql).to include('`events`.`published` = TRUE') - expect(scope.to_sql).to include('registration_close_date IS NULL OR registration_close_date >=') - expect(scope.to_sql).to include('LEFT OUTER JOIN `event_registrations`') + sql = scope.to_sql + expect(sql).to include('`events`.`published` = TRUE') + expect(sql).to include("events.end_date >=") + expect(sql).to include("events.registration_close_date IS NULL OR events.registration_close_date >=") + expect(sql).to include("LEFT OUTER JOIN event_registrations") + expect(sql).to include("event_registrations.status IN") end end context "with no user" do let(:policy) { policy_for(record: Event, user: nil) } - it "returns only visible events with open registration" do + it "excludes ended events and events with closed registration" do scope = policy.apply_scope(Event.all, type: :active_record_relation) - expect(scope.to_sql).to include('`events`.`published` = TRUE') - expect(scope.to_sql).to include('registration_close_date IS NULL OR registration_close_date >=') - expect(scope.to_sql).not_to include('LEFT OUTER JOIN `registrants`') + sql = scope.to_sql + expect(sql).to include('`events`.`published` = TRUE') + expect(sql).to include("events.end_date >=") + expect(sql).to include("events.registration_close_date IS NULL OR events.registration_close_date >=") + expect(sql).not_to include("LEFT OUTER JOIN") end end end From 3da9d9b0865afa4d0926471eadcaebf287d0ae65 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 4 Mar 2026 08:46:34 -0500 Subject: [PATCH 2/5] Fix CI: brakeman SQL injection warning and test failures - Hardcode active status values in policy join to avoid brakeman string interpolation warning - Register user for ended event in system spec so show? allows access - Update timezone test dates from 2025 to 2031 (now past, causing ended event policy to block access) Co-Authored-By: Claude Opus 4.6 --- app/policies/event_policy.rb | 3 +-- spec/requests/events_spec.rb | 6 +++--- spec/system/events_show_spec.rb | 1 + 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/policies/event_policy.rb b/app/policies/event_policy.rb index 49f00aed3..7b1f5d2a7 100644 --- a/app/policies/event_policy.rb +++ b/app/policies/event_policy.rb @@ -47,12 +47,11 @@ def owner? next relation if admin? if authenticated? - active_statuses = EventRegistration::ACTIVE_STATUSES.map { |s| relation.connection.quote(s) }.join(", ") relation .joins( "LEFT OUTER JOIN event_registrations ON event_registrations.event_id = events.id - AND event_registrations.status IN (#{active_statuses}) + AND event_registrations.status IN ('registered', 'attended', 'incomplete_attendance') LEFT OUTER JOIN people ON people.id = event_registrations.registrant_id" ) diff --git a/spec/requests/events_spec.rb b/spec/requests/events_spec.rb index eac574b37..f607e6e13 100644 --- a/spec/requests/events_spec.rb +++ b/spec/requests/events_spec.rb @@ -30,9 +30,9 @@ end context "when user time_zone is set" do - # 19:00 UTC = 12:00 noon PT = 15:00 (3 pm) ET (June 15, 2025 with DST) - let(:utc_start) { Time.utc(2025, 6, 15, 19, 0, 0) } - let(:utc_end) { Time.utc(2025, 6, 15, 20, 0, 0) } + # 19:00 UTC = 12:00 noon PT = 15:00 (3 pm) ET (June 15, 2031 with DST) + let(:utc_start) { Time.utc(2031, 6, 15, 19, 0, 0) } + let(:utc_end) { Time.utc(2031, 6, 15, 20, 0, 0) } let!(:event_with_fixed_times) do create(:event, :published, start_date: utc_start, diff --git a/spec/system/events_show_spec.rb b/spec/system/events_show_spec.rb index 5451c4342..ca88463bd 100644 --- a/spec/system/events_show_spec.rb +++ b/spec/system/events_show_spec.rb @@ -171,6 +171,7 @@ context "event ended" do before do event.update!(end_date: 1.day.ago) + create(:event_registration, event: event, registrant: user.person, status: "registered") end it "shows 'Event ended' and hides registration buttons" do From 21da8466830e90f7c4cc53172ce34242b75dce3a Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 4 Mar 2026 08:53:22 -0500 Subject: [PATCH 3/5] Improve ended event display for registered users - Card: show "Event ended" label alongside "View registration" button for registered users on ended events - Registration section: restructure conditionals so ended events show "Event ended" above "View Registration" without also showing "Registration closed" - Hide calendar links on ended events since the dates have passed Co-Authored-By: Claude Opus 4.6 --- app/views/events/_card.html.erb | 3 +++ app/views/events/_registration_section.html.erb | 16 +++++++--------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/app/views/events/_card.html.erb b/app/views/events/_card.html.erb index e84731775..b0edbe501 100644 --- a/app/views/events/_card.html.erb +++ b/app/views/events/_card.html.erb @@ -54,6 +54,9 @@ <% end %> <% if registered %> + <% if ended %> + Event ended + <% end %> <% registration = event.active_registration_for(current_user.person) %> <% if registration&.slug.present? %> <%= link_to "View registration", registration_ticket_path(registration.slug), diff --git a/app/views/events/_registration_section.html.erb b/app/views/events/_registration_section.html.erb index 9238b97bf..90dbb78f1 100644 --- a/app/views/events/_registration_section.html.erb +++ b/app/views/events/_registration_section.html.erb @@ -5,21 +5,19 @@ <% slug_registered = slug_registration&.active? %> <% slug_cancelled = slug_registration.present? && slug_registration.status == "cancelled" %> <%= tag.div id: dom_id(event.object, "registration_section_#{instance}"), class: "registration-section flex flex-col items-center gap-4 mb-6" do %> - -
+
<% if event.ended? %> - Event ended + Event ended <% if allowed_to?(:manage?, event) %> <%= button_to "Register", event_registrant_registration_path(event_id: event), class: "admin-only bg-blue-100 btn btn-primary-outline" %> <% end %> - <% elsif registered %> - <% elsif slug_cancelled && event.registerable? %> - <%= button_to "Register again", registration_reactivate_path(slug_registration.slug), - class: "text-sm text-gray-500 hover:text-blue-600 underline bg-transparent border-0 cursor-pointer p-0" %> <% elsif !registered && !slug_registered %> - <% if !event.published? %> + <% if slug_cancelled && event.registerable? %> + <%= button_to "Register again", registration_reactivate_path(slug_registration.slug), + class: "text-sm text-gray-500 hover:text-blue-600 underline bg-transparent border-0 cursor-pointer p-0" %> + <% elsif !event.published? %> Not published <% elsif event.registerable? %> <% if user_signed_in? %> @@ -50,7 +48,7 @@ <% end %>
- <% if registered || slug_registered %> + <% if (registered || slug_registered) && !event.ended? %>

From eca785c63f3f02e6c85d264bcd6bf73cb67855c6 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 4 Mar 2026 08:55:32 -0500 Subject: [PATCH 4/5] Add tests for ended event display and access control System specs: - Registered user sees both "Event ended" and "View Registration" - Calendar links hidden on ended events - Unregistered user blocked from ended event show page - Admin can view ended events Request specs: - Admin can view ended event (200) - Registered user can view ended event (200) - Unregistered user redirected from ended event - Unauthenticated user redirected to sign in Co-Authored-By: Claude Opus 4.6 --- spec/requests/events_spec.rb | 31 +++++++++++++++++++++++++++++++ spec/system/events_show_spec.rb | 31 +++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/spec/requests/events_spec.rb b/spec/requests/events_spec.rb index f607e6e13..c7ec7362b 100644 --- a/spec/requests/events_spec.rb +++ b/spec/requests/events_spec.rb @@ -63,6 +63,37 @@ end end + describe "GET /show" do + context "when event has ended" do + let(:ended_event) { create(:event, :published, :ended) } + + it "allows admin to view" do + sign_in admin + get event_path(ended_event) + expect(response).to have_http_status(:ok) + end + + it "allows registered user to view" do + user_with_person = create(:user, :with_person) + sign_in user_with_person + create(:event_registration, event: ended_event, registrant: user_with_person.person, status: "registered") + get event_path(ended_event) + expect(response).to have_http_status(:ok) + end + + it "redirects unregistered user" do + sign_in user + get event_path(ended_event) + expect(response).to redirect_to(root_path) + end + + it "redirects unauthenticated user" do + get event_path(ended_event) + expect(response).to redirect_to(new_user_session_path) + end + end + end + describe "GET /new" do context "as admin" do it "renders successfully" do diff --git a/spec/system/events_show_spec.rb b/spec/system/events_show_spec.rb index ca88463bd..a34a0f4b4 100644 --- a/spec/system/events_show_spec.rb +++ b/spec/system/events_show_spec.rb @@ -182,6 +182,37 @@ expect(page).not_to have_button("Register") expect(page).not_to have_button("De-register") end + + it "shows 'View Registration' for registered user" do + sign_in(user) + visit event_path(event) + + expect(page).to have_text("Event ended") + expect(page).to have_text("View Registration") + end + + it "hides calendar links" do + sign_in(user) + visit event_path(event) + + expect(page).not_to have_text("Add to Your Calendar") + end + + it "blocks unregistered users" do + other_user = create(:user, :with_person) + sign_in(other_user) + visit event_path(event) + + expect(page).to have_current_path(root_path) + end + + it "allows admin to view" do + sign_in(admin) + visit event_path(event) + + expect(page).to have_text("Event ended") + expect(page).to have_text("My Event") + end end context "guest with reg slug param" do From f81d8d64205d3c8052f04710344e2d4c3a616598 Mon Sep 17 00:00:00 2001 From: maebeale Date: Wed, 4 Mar 2026 09:05:21 -0500 Subject: [PATCH 5/5] Fix unauthenticated ended event test to expect root_path redirect ActionPolicy::Unauthorized rescues redirect to root_path, not sign_in, since the event is publicly visible and guests can reach the controller. Co-Authored-By: Claude Opus 4.6 --- spec/requests/events_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/events_spec.rb b/spec/requests/events_spec.rb index c7ec7362b..8bb317d04 100644 --- a/spec/requests/events_spec.rb +++ b/spec/requests/events_spec.rb @@ -89,7 +89,7 @@ it "redirects unauthenticated user" do get event_path(ended_event) - expect(response).to redirect_to(new_user_session_path) + expect(response).to redirect_to(root_path) end end end