From 89db5e2552a16777283890fac8f7f5def52dadd3 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Thu, 1 Oct 2026 13:12:23 +0200 Subject: [PATCH 1/2] Fix wrong sponsor shown as workshop venue on events listing Eager-loading workshop_host together with :sponsors routes both through the workshop_sponsors table in one join; the host scope then binds to the wrong table alias, so workshop_host comes back nil or points at a non-host sponsor and event cards show the wrong venue. Preload the host instead, mirroring Listable.most_recent. Sponsors and organiser avatars on event cards now render in name order; previously the order followed join-row order. --- app/controllers/events_controller.rb | 6 ++- app/presenters/event_presenter.rb | 2 +- app/presenters/workshop_presenter.rb | 9 ++-- spec/controllers/events_controller_spec.rb | 48 ++++++++++++++++++++++ 4 files changed, 59 insertions(+), 6 deletions(-) diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index fcb24aec2..babe5c4d4 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -175,9 +175,13 @@ def load_events(rows) (hash[row['event_type']] ||= []) << row['id'].to_i end + # workshop_host must be preloaded: eager_loading it alongside :sponsors + # routes both through the workshop_sponsors table in one join, and the + # host scope then binds to the wrong table alias — the host comes back + # nil or points at a non-host sponsor. Same pattern as Listable.most_recent. workshops = Workshop.eager_load(:sponsors, :organisers, - workshop_host: :sponsor, chapter: { permissions: :members }) + .preload(workshop_host: :sponsor) .where(id: grouped['Workshop']) .to_a.index_by(&:id) meetings = Meeting.eager_load(:venue, :organisers).where(id: grouped['Meeting']) diff --git a/app/presenters/event_presenter.rb b/app/presenters/event_presenter.rb index b00f500b1..fbf96cb8a 100644 --- a/app/presenters/event_presenter.rb +++ b/app/presenters/event_presenter.rb @@ -30,7 +30,7 @@ def short_description end def organisers - @organisers ||= model.organisers.to_a + @organisers ||= model.organisers.to_a.sort_by(&:name) end def month diff --git a/app/presenters/workshop_presenter.rb b/app/presenters/workshop_presenter.rb index e26252a4d..660a25d15 100644 --- a/app/presenters/workshop_presenter.rb +++ b/app/presenters/workshop_presenter.rb @@ -31,11 +31,12 @@ def venue @venue ||= model.host end + def sponsors + model.sponsors.sort_by(&:name) + end + def organisers - @organisers ||= begin - orgs = model.organisers.to_a - orgs.any? ? orgs : chapter_organisers - end + @organisers ||= (model.organisers.to_a.presence || chapter_organisers).sort_by(&:name) end def attendees_checklist diff --git a/spec/controllers/events_controller_spec.rb b/spec/controllers/events_controller_spec.rb index 306fa616b..32f6b05da 100644 --- a/spec/controllers/events_controller_spec.rb +++ b/spec/controllers/events_controller_spec.rb @@ -244,4 +244,52 @@ expect(response).to have_http_status(:ok) end end + + describe '#load_events' do + let(:chapter) { Fabricate(:chapter) } + let(:host_sponsor) { Fabricate(:sponsor, name: 'Zulu GmbH') } + let(:extra_sponsor) { Fabricate(:sponsor, name: 'Alpha AG') } + + def rows_for(workshops) + workshops.map { |w| { 'event_type' => 'Workshop', 'id' => w.id } } + end + + it 'loads the host sponsor as workshop_host for every workshop' do + workshops = Fabricate.times(6, :workshop_no_sponsor, chapter:) + workshops.each do |workshop| + Fabricate(:workshop_sponsor, workshop:, sponsor: host_sponsor, host: true) + Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor, host: false) + end + + loaded = controller.send(:load_events, rows_for(workshops)) + + expect(loaded.map { |e| e.workshop_host&.sponsor&.id }).to all(eq(host_sponsor.id)) + end + + it 'lists all sponsors of a workshop' do + workshop = Fabricate(:workshop_no_sponsor, chapter:) + Fabricate(:workshop_sponsor, workshop:, sponsor: host_sponsor, host: true) + Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor, host: false) + + loaded = controller.send(:load_events, rows_for([workshop])) + + expect(loaded.first.sponsors).to contain_exactly(host_sponsor, extra_sponsor) + end + + it 'renders sponsors and organisers in name order' do + workshop = Fabricate(:workshop_no_sponsor, chapter:) + Fabricate(:workshop_sponsor, workshop:, sponsor: host_sponsor, host: true) + Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor, host: false) + anton = Fabricate(:member, name: 'Anton') + zora = Fabricate(:member, name: 'Zora') + anton.add_role :organiser, workshop + zora.add_role :organiser, workshop + + loaded = controller.send(:load_events, rows_for([workshop])) + presenter = EventPresenter.decorate(loaded.first) + + expect(presenter.sponsors.map(&:name)).to eq(['Alpha AG', 'Zulu GmbH']) + expect(presenter.organisers.map(&:name)).to eq(%w[Anton Zora]) + end + end end From 579c59830d9a4635c1bb4e7dcf3e5b8fca5c67f9 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Thu, 1 Oct 2026 14:33:54 +0200 Subject: [PATCH 2/2] Apply review findings: nil-safe name sorting, card cache version bump Sorting sponsors and organisers by name compared nil against String when a member or sponsor has no name (member names are only required for active members), which would raise while rendering a card. Sort with a nil-safe key instead. Bump the event card fragment cache version to v3: previously cached cards rendered the wrong venue and must not be served after this fix deploys. --- app/components/event_card_component.html.erb | 2 +- app/presenters/event_presenter.rb | 2 +- app/presenters/workshop_presenter.rb | 4 ++-- spec/controllers/events_controller_spec.rb | 13 +++++++++++++ 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/components/event_card_component.html.erb b/app/components/event_card_component.html.erb index 915a6efad..d8251e631 100644 --- a/app/components/event_card_component.html.erb +++ b/app/components/event_card_component.html.erb @@ -1,4 +1,4 @@ -<% cache [@event.cache_key_with_version, I18n.locale, :v2] do %> +<% cache [@event.cache_key_with_version, I18n.locale, :v3] do %>
diff --git a/app/presenters/event_presenter.rb b/app/presenters/event_presenter.rb index fbf96cb8a..d2614ea52 100644 --- a/app/presenters/event_presenter.rb +++ b/app/presenters/event_presenter.rb @@ -30,7 +30,7 @@ def short_description end def organisers - @organisers ||= model.organisers.to_a.sort_by(&:name) + @organisers ||= model.organisers.to_a.sort_by { |organiser| organiser.name.to_s } end def month diff --git a/app/presenters/workshop_presenter.rb b/app/presenters/workshop_presenter.rb index 660a25d15..84148ef7b 100644 --- a/app/presenters/workshop_presenter.rb +++ b/app/presenters/workshop_presenter.rb @@ -32,11 +32,11 @@ def venue end def sponsors - model.sponsors.sort_by(&:name) + model.sponsors.sort_by { |sponsor| sponsor.name.to_s } end def organisers - @organisers ||= (model.organisers.to_a.presence || chapter_organisers).sort_by(&:name) + @organisers ||= (model.organisers.to_a.presence || chapter_organisers).sort_by { |organiser| organiser.name.to_s } end def attendees_checklist diff --git a/spec/controllers/events_controller_spec.rb b/spec/controllers/events_controller_spec.rb index 32f6b05da..6b0a6b029 100644 --- a/spec/controllers/events_controller_spec.rb +++ b/spec/controllers/events_controller_spec.rb @@ -291,5 +291,18 @@ def rows_for(workshops) expect(presenter.sponsors.map(&:name)).to eq(['Alpha AG', 'Zulu GmbH']) expect(presenter.organisers.map(&:name)).to eq(%w[Anton Zora]) end + + it 'sorts organisers with blank names without raising' do + workshop = Fabricate(:workshop_no_sponsor, chapter:) + anton = Fabricate(:member, name: 'Anton') + anton.update_column(:name, nil) + anton.add_role :organiser, workshop + Fabricate(:member, name: 'Zora').add_role :organiser, workshop + + loaded = controller.send(:load_events, rows_for([workshop])) + presenter = EventPresenter.decorate(loaded.first) + + expect(presenter.organisers.map(&:name)).to eq([nil, 'Zora']) + end end end