From accbeaec48efc03f1df85c8724dca742a651ec7b Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:11:41 +0000 Subject: [PATCH 1/4] fix: n+1 in upcoming_events_for_user --- app/queries/dashboard_query.rb | 2 ++ spec/queries/dashboard_query_spec.rb | 49 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/app/queries/dashboard_query.rb b/app/queries/dashboard_query.rb index 8c9f33537..598f538b0 100644 --- a/app/queries/dashboard_query.rb +++ b/app/queries/dashboard_query.rb @@ -24,6 +24,7 @@ def self.upcoming_events def self.upcoming_events_for_user(member) chapter_workshops = Workshop.upcoming .eager_load(:chapter, :sponsors, :organisers, :permissions) + .preload(workshop_host: :sponsor) .where(chapter: member.chapters) .to_a @@ -31,6 +32,7 @@ def self.upcoming_events_for_user(member) .joins(:workshop) .merge(Workshop.upcoming) .eager_load(workshop: %i[chapter sponsors organisers permissions]) + .preload(workshop: { workshop_host: :sponsor }) .map(&:workshop) all_events(chapter_workshops + accepted_workshops) diff --git a/spec/queries/dashboard_query_spec.rb b/spec/queries/dashboard_query_spec.rb index bdc24fab2..0bcf5d09e 100644 --- a/spec/queries/dashboard_query_spec.rb +++ b/spec/queries/dashboard_query_spec.rb @@ -139,6 +139,55 @@ def read_venue(event) expect(result.values.flatten).to include(workshop) end + it 'preloads workshop hosts without adding per-workshop queries' do + chapter = Fabricate(:chapter) + member = Fabricate(:member, groups: [Fabricate(:students, chapter:)]) + host = Fabricate(:sponsor) + extra_sponsor = Fabricate(:sponsor) + organiser = Fabricate(:member) + + 3.times do + workshop = Fabricate(:workshop_no_sponsor, chapter:) + Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor) + Fabricate(:workshop_sponsor, workshop:, sponsor: host, host: true) + organiser.add_role(:organiser, workshop) + Fabricate(:attending_workshop_invitation, member:, workshop:) + end + + result = nil + queries = count_queries { result = described_class.upcoming_events_for_user(member) } + + expect(queries).to be_positive + expect(result.values.flatten.map { |event| event.host if event.is_a?(Workshop) }).to all(eq(host)) + end + + it 'resolves the host for accepted workshops with multiple sponsors' do + chapter = Fabricate(:chapter) + member = Fabricate(:member, groups: [Fabricate(:students, chapter:)]) + host = Fabricate(:sponsor) + extra_sponsor = Fabricate(:sponsor) + workshop = Fabricate(:workshop_no_sponsor, chapter:) + Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor) + Fabricate(:workshop_sponsor, workshop:, sponsor: host, host: true) + Fabricate(:attending_workshop_invitation, member:, workshop:) + + result = described_class.upcoming_events_for_user(member) + + expect(result.values.flatten.find { |event| event == workshop }.host).to eq(host) + end + + it 'resolves host to nil for accepted workshops with only non-host sponsors' do + chapter = Fabricate(:chapter) + member = Fabricate(:member, groups: [Fabricate(:students, chapter:)]) + workshop = Fabricate(:workshop_no_sponsor, chapter:) + Fabricate(:workshop_sponsor, workshop:, sponsor: Fabricate(:sponsor), host: false) + Fabricate(:attending_workshop_invitation, member:, workshop:) + + result = described_class.upcoming_events_for_user(member) + + expect(result.values.flatten.find { |event| event == workshop }.host).to be_nil + end + it 'returns an empty hash when the member has no upcoming events' do member = Fabricate(:member) From aca77d5e688358add0322849091023da6dad7cc0 Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:28:53 +0000 Subject: [PATCH 2/4] fix: regression coverage for host resolution and query growth --- spec/queries/dashboard_query_spec.rb | 63 ++++++++++++++++++---------- 1 file changed, 41 insertions(+), 22 deletions(-) diff --git a/spec/queries/dashboard_query_spec.rb b/spec/queries/dashboard_query_spec.rb index 0bcf5d09e..7ba8c7054 100644 --- a/spec/queries/dashboard_query_spec.rb +++ b/spec/queries/dashboard_query_spec.rb @@ -139,53 +139,72 @@ def read_venue(event) expect(result.values.flatten).to include(workshop) end - it 'preloads workshop hosts without adding per-workshop queries' do - chapter = Fabricate(:chapter) - member = Fabricate(:member, groups: [Fabricate(:students, chapter:)]) + it 'preloads workshop hosts for accepted invitations without growing query count with workshop count' do + member = Fabricate(:member) host = Fabricate(:sponsor) extra_sponsor = Fabricate(:sponsor) - organiser = Fabricate(:member) - 3.times do - workshop = Fabricate(:workshop_no_sponsor, chapter:) - Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor) - Fabricate(:workshop_sponsor, workshop:, sponsor: host, host: true) - organiser.add_role(:organiser, workshop) - Fabricate(:attending_workshop_invitation, member:, workshop:) + build_invited_workshops = lambda do |count| + count.times do + workshop = Fabricate(:workshop_no_sponsor) + Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor) + Fabricate(:workshop_sponsor, workshop:, sponsor: host, host: true) + Fabricate(:attending_workshop_invitation, member:, workshop:) + end + end + + build_invited_workshops.call(1) + expect(member.chapters).to be_empty + result_one = nil + one_workshop_queries = count_queries do + result_one = described_class.upcoming_events_for_user(member) + workshops = result_one.values.flatten.select { |event| event.is_a?(Workshop) } + expect(workshops.map(&:host)).to all(eq(host)) end - result = nil - queries = count_queries { result = described_class.upcoming_events_for_user(member) } + expect(result_one.values.flatten.select { |event| event.is_a?(Workshop) }.count).to eq(1) + expect(one_workshop_queries).to be_positive + + build_invited_workshops.call(3) + result_four = nil + four_workshop_queries = count_queries do + result_four = described_class.upcoming_events_for_user(member) + workshops = result_four.values.flatten.select { |event| event.is_a?(Workshop) } + expect(workshops.map(&:host)).to all(eq(host)) + end - expect(queries).to be_positive - expect(result.values.flatten.map { |event| event.host if event.is_a?(Workshop) }).to all(eq(host)) + expect(result_four.values.flatten.select { |event| event.is_a?(Workshop) }.count).to eq(4) + expect(four_workshop_queries).to be_positive + expect(four_workshop_queries).to eq(one_workshop_queries) end it 'resolves the host for accepted workshops with multiple sponsors' do - chapter = Fabricate(:chapter) - member = Fabricate(:member, groups: [Fabricate(:students, chapter:)]) + member = Fabricate(:member) host = Fabricate(:sponsor) extra_sponsor = Fabricate(:sponsor) - workshop = Fabricate(:workshop_no_sponsor, chapter:) + workshop = Fabricate(:workshop_no_sponsor) Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor) Fabricate(:workshop_sponsor, workshop:, sponsor: host, host: true) Fabricate(:attending_workshop_invitation, member:, workshop:) result = described_class.upcoming_events_for_user(member) + loaded = result.values.flatten.find { |event| event == workshop } - expect(result.values.flatten.find { |event| event == workshop }.host).to eq(host) + expect(loaded).to eq(workshop) + expect(loaded.host).to eq(host) end it 'resolves host to nil for accepted workshops with only non-host sponsors' do - chapter = Fabricate(:chapter) - member = Fabricate(:member, groups: [Fabricate(:students, chapter:)]) - workshop = Fabricate(:workshop_no_sponsor, chapter:) + member = Fabricate(:member) + workshop = Fabricate(:workshop_no_sponsor) Fabricate(:workshop_sponsor, workshop:, sponsor: Fabricate(:sponsor), host: false) Fabricate(:attending_workshop_invitation, member:, workshop:) result = described_class.upcoming_events_for_user(member) + loaded = result.values.flatten.find { |event| event == workshop } - expect(result.values.flatten.find { |event| event == workshop }.host).to be_nil + expect(loaded).to eq(workshop) + expect(loaded.host).to be_nil end it 'returns an empty hash when the member has no upcoming events' do From be7a0492d177df1f2601fabfb5d4d6144e7e9570 Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:41:42 +0000 Subject: [PATCH 3/4] refactor: extract method and test complexity into helper methods --- app/queries/dashboard_query.rb | 34 ++++++++++-------- spec/queries/dashboard_query_spec.rb | 53 ++++++++++++++-------------- 2 files changed, 46 insertions(+), 41 deletions(-) diff --git a/app/queries/dashboard_query.rb b/app/queries/dashboard_query.rb index 598f538b0..00e0b69a8 100644 --- a/app/queries/dashboard_query.rb +++ b/app/queries/dashboard_query.rb @@ -22,20 +22,7 @@ def self.upcoming_events end def self.upcoming_events_for_user(member) - chapter_workshops = Workshop.upcoming - .eager_load(:chapter, :sponsors, :organisers, :permissions) - .preload(workshop_host: :sponsor) - .where(chapter: member.chapters) - .to_a - - accepted_workshops = member.workshop_invitations.accepted - .joins(:workshop) - .merge(Workshop.upcoming) - .eager_load(workshop: %i[chapter sponsors organisers permissions]) - .preload(workshop: { workshop_host: :sponsor }) - .map(&:workshop) - - all_events(chapter_workshops + accepted_workshops) + all_events(chapter_workshops_for(member) + accepted_workshops_for(member)) .sort_by(&:date_and_time) .group_by(&:date) end @@ -65,4 +52,23 @@ def self.upcoming_workshops .to_a end private_class_method :upcoming_workshops + + def self.chapter_workshops_for(member) + Workshop.upcoming + .eager_load(:chapter, :sponsors, :organisers, :permissions) + .preload(workshop_host: :sponsor) + .where(chapter: member.chapters) + .to_a + end + private_class_method :chapter_workshops_for + + def self.accepted_workshops_for(member) + member.workshop_invitations.accepted + .joins(:workshop) + .merge(Workshop.upcoming) + .eager_load(workshop: %i[chapter sponsors organisers permissions]) + .preload(workshop: { workshop_host: :sponsor }) + .map(&:workshop) + end + private_class_method :accepted_workshops_for end diff --git a/spec/queries/dashboard_query_spec.rb b/spec/queries/dashboard_query_spec.rb index 7ba8c7054..c21afdd4a 100644 --- a/spec/queries/dashboard_query_spec.rb +++ b/spec/queries/dashboard_query_spec.rb @@ -128,6 +128,26 @@ def read_venue(event) end describe '.upcoming_events_for_user' do + def create_invited_workshops(member:, count:, host:, extra_sponsor:) + count.times do + workshop = Fabricate(:workshop_no_sponsor) + Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor) + Fabricate(:workshop_sponsor, workshop:, sponsor: host, host: true) + Fabricate(:attending_workshop_invitation, member:, workshop:) + end + end + + def load_user_events_and_count_host_queries(member:, host:) + result = nil + queries = count_queries do + result = described_class.upcoming_events_for_user(member) + workshops = result.values.flatten.select { |event| event.is_a?(Workshop) } + expect(workshops.map(&:host)).to all(eq(host)) + end + + [result, queries] + end + it 'returns events for the member chapters and accepted workshops' do chapter = Fabricate(:chapter) member = Fabricate(:member, groups: [Fabricate(:students, chapter:)]) @@ -143,37 +163,16 @@ def read_venue(event) member = Fabricate(:member) host = Fabricate(:sponsor) extra_sponsor = Fabricate(:sponsor) - - build_invited_workshops = lambda do |count| - count.times do - workshop = Fabricate(:workshop_no_sponsor) - Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor) - Fabricate(:workshop_sponsor, workshop:, sponsor: host, host: true) - Fabricate(:attending_workshop_invitation, member:, workshop:) - end - end - - build_invited_workshops.call(1) expect(member.chapters).to be_empty - result_one = nil - one_workshop_queries = count_queries do - result_one = described_class.upcoming_events_for_user(member) - workshops = result_one.values.flatten.select { |event| event.is_a?(Workshop) } - expect(workshops.map(&:host)).to all(eq(host)) - end - expect(result_one.values.flatten.select { |event| event.is_a?(Workshop) }.count).to eq(1) + create_invited_workshops(member:, count: 1, host:, extra_sponsor:) + result_one, one_workshop_queries = load_user_events_and_count_host_queries(member:, host:) + expect(result_one.values.flatten.count { |event| event.is_a?(Workshop) }).to eq(1) expect(one_workshop_queries).to be_positive - build_invited_workshops.call(3) - result_four = nil - four_workshop_queries = count_queries do - result_four = described_class.upcoming_events_for_user(member) - workshops = result_four.values.flatten.select { |event| event.is_a?(Workshop) } - expect(workshops.map(&:host)).to all(eq(host)) - end - - expect(result_four.values.flatten.select { |event| event.is_a?(Workshop) }.count).to eq(4) + create_invited_workshops(member:, count: 3, host:, extra_sponsor:) + result_four, four_workshop_queries = load_user_events_and_count_host_queries(member:, host:) + expect(result_four.values.flatten.count { |event| event.is_a?(Workshop) }).to eq(4) expect(four_workshop_queries).to be_positive expect(four_workshop_queries).to eq(one_workshop_queries) end From 67496241b4d4c19483559199d0513d309d485b6e Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Tue, 3 Nov 2026 10:26:50 +0000 Subject: [PATCH 4/4] test: add coverage for a member with a chapter, remove duplicate test and reduce query count test's strictness --- spec/queries/dashboard_query_spec.rb | 46 ++++++++++++++++++---------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/spec/queries/dashboard_query_spec.rb b/spec/queries/dashboard_query_spec.rb index c21afdd4a..ce9d2eecf 100644 --- a/spec/queries/dashboard_query_spec.rb +++ b/spec/queries/dashboard_query_spec.rb @@ -137,6 +137,14 @@ def create_invited_workshops(member:, count:, host:, extra_sponsor:) end end + def create_chapter_workshops(chapter:, count:, host:, extra_sponsor:) + count.times do + workshop = Fabricate(:workshop_no_sponsor, chapter:) + Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor) + Fabricate(:workshop_sponsor, workshop:, sponsor: host, host: true) + end + end + def load_user_events_and_count_host_queries(member:, host:) result = nil queries = count_queries do @@ -159,6 +167,26 @@ def load_user_events_and_count_host_queries(member:, host:) expect(result.values.flatten).to include(workshop) end + it 'preloads workshop hosts for chapter workshops without growing query count' do + chapter = Fabricate(:chapter) + member = Fabricate(:member, groups: [Fabricate(:students, chapter:)]) + host = Fabricate(:sponsor) + extra_sponsor = Fabricate(:sponsor) + + create_chapter_workshops(chapter:, count: 1, host:, extra_sponsor:) + result_one = described_class.upcoming_events_for_user(member) + one_query_count = count_queries { described_class.upcoming_events_for_user(member) } + + create_chapter_workshops(chapter:, count: 3, host:, extra_sponsor:) + result_four = nil + four_query_count = count_queries { result_four = described_class.upcoming_events_for_user(member) } + + expect(result_four.values.flatten.count { |event| event.is_a?(Workshop) }).to eq(4) + expect(result_four.values.flatten.select { |event| event.is_a?(Workshop) }.map(&:host)).to all(eq(host)) + expect(four_query_count).to be <= one_query_count + 2 + expect(result_one.values.flatten.select { |event| event.is_a?(Workshop) }.map(&:host)).to all(eq(host)) + end + it 'preloads workshop hosts for accepted invitations without growing query count with workshop count' do member = Fabricate(:member) host = Fabricate(:sponsor) @@ -174,23 +202,7 @@ def load_user_events_and_count_host_queries(member:, host:) result_four, four_workshop_queries = load_user_events_and_count_host_queries(member:, host:) expect(result_four.values.flatten.count { |event| event.is_a?(Workshop) }).to eq(4) expect(four_workshop_queries).to be_positive - expect(four_workshop_queries).to eq(one_workshop_queries) - end - - it 'resolves the host for accepted workshops with multiple sponsors' do - member = Fabricate(:member) - host = Fabricate(:sponsor) - extra_sponsor = Fabricate(:sponsor) - workshop = Fabricate(:workshop_no_sponsor) - Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor) - Fabricate(:workshop_sponsor, workshop:, sponsor: host, host: true) - Fabricate(:attending_workshop_invitation, member:, workshop:) - - result = described_class.upcoming_events_for_user(member) - loaded = result.values.flatten.find { |event| event == workshop } - - expect(loaded).to eq(workshop) - expect(loaded.host).to eq(host) + expect(four_workshop_queries).to be <= one_workshop_queries + 2 end it 'resolves host to nil for accepted workshops with only non-host sponsors' do