From f27402f21a6fd7a52e32227683acae8f76a0fe1b Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Fri, 10 Jul 2026 08:27:52 +0200 Subject: [PATCH 1/4] refactor: move SponsorsSearch from app/models to app/queries SponsorsSearch is a query object (ActiveModel with #call), not a domain model. Place it in the correct architectural layer. Move both the source file and its spec to mirror the new location. Rails 8 autoloads all app/ subdirectories, so no require path updates are needed. --- app/{models => queries}/sponsors_search.rb | 0 spec/{models => queries}/sponsors_search_spec.rb | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename app/{models => queries}/sponsors_search.rb (100%) rename spec/{models => queries}/sponsors_search_spec.rb (100%) diff --git a/app/models/sponsors_search.rb b/app/queries/sponsors_search.rb similarity index 100% rename from app/models/sponsors_search.rb rename to app/queries/sponsors_search.rb diff --git a/spec/models/sponsors_search_spec.rb b/spec/queries/sponsors_search_spec.rb similarity index 100% rename from spec/models/sponsors_search_spec.rb rename to spec/queries/sponsors_search_spec.rb From 673c5016f5edd36583b4eb8b18400e93dd915719 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Fri, 10 Jul 2026 08:32:38 +0200 Subject: [PATCH 2/4] test: adopt described_class and deterministic params in SponsorsSearch spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the shared let(:search_params) that generated random values via Faker with inline literal hashes. This makes the test deterministic and removes a source of test flake. Also adopt described_class.new(...) call style instead of the local variable invocation pattern. No test semantics changed — same assertions, same message-expectation approach. --- spec/queries/sponsors_search_spec.rb | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/spec/queries/sponsors_search_spec.rb b/spec/queries/sponsors_search_spec.rb index 5c79259f2..a0e1e609f 100644 --- a/spec/queries/sponsors_search_spec.rb +++ b/spec/queries/sponsors_search_spec.rb @@ -1,13 +1,10 @@ RSpec.describe SponsorsSearch do - let(:search_params) { { name: Faker::Name.name, chapter: Faker::Name.name } } + describe 'initialization' do + it 'configures its properties from the param hash' do + search = described_class.new(name: 'Acme', chapter: 'London') - describe 'initialization params' do - it 'configures its properties using the param hash' do - params = { name: Faker::Name.name, chapter: Faker::Name.name } - - sponsors_search = described_class.new(search_params) - - expect(sponsors_search.name).to eq(search_params[:name]) + expect(search.name).to eq('Acme') + expect(search.chapter).to eq('London') end end @@ -15,13 +12,13 @@ it 'searches by_name if a name is specified' do expect(Sponsor).to receive(:by_name) - described_class.new(search_params).call + described_class.new(name: 'Acme', chapter: 'London').call end it 'returns all sponsors when no filter is specified' do expect(Sponsor).to_not receive(:by_name) - described_class.new({ name: nil, chapter: nil}).call + described_class.new(name: nil, chapter: nil).call end end end From 2a6b73c1cc74073a1e8342a98d305dc6d3e5280e Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Fri, 10 Jul 2026 08:32:56 +0200 Subject: [PATCH 3/4] test: replace message expectations with real-data assertions The previous tests used expect(Sponsor).to receive(:by_name) which verifies that a method was *called*, not that the right records were *returned*. A caller passing the wrong args to by_name, or a by_name scope with a bug, would still pass. Drop message expectations entirely. Seed the database with specific sponsors, invoke #call, and assert on the actual returned records. This tests the contract (what comes back) rather than implementation (what gets called). --- spec/queries/sponsors_search_spec.rb | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/spec/queries/sponsors_search_spec.rb b/spec/queries/sponsors_search_spec.rb index a0e1e609f..f502760f9 100644 --- a/spec/queries/sponsors_search_spec.rb +++ b/spec/queries/sponsors_search_spec.rb @@ -8,17 +8,31 @@ end end - describe 'when search is called' do - it 'searches by_name if a name is specified' do - expect(Sponsor).to receive(:by_name) + describe '#call' do + it 'returns all sponsors when no filter is specified' do + Fabricate.times(3, :sponsor) + + results = described_class.new(name: nil, chapter: nil).call - described_class.new(name: 'Acme', chapter: 'London').call + expect(results.size).to eq(3) + expect(results).to all(be_a(Sponsor)) end - it 'returns all sponsors when no filter is specified' do - expect(Sponsor).to_not receive(:by_name) + it 'filters by name' do + matching = Fabricate(:sponsor, name: 'Zebra Technologies') + Fabricate(:sponsor, name: 'Apple Inc') + + results = described_class.new(name: 'Zebra', chapter: nil).call + + expect(results).to contain_exactly(matching) + end + + it 'returns an empty relation when nothing matches' do + Fabricate(:sponsor, name: 'Apple Inc') + + results = described_class.new(name: 'Nonexistent', chapter: nil).call - described_class.new(name: nil, chapter: nil).call + expect(results).to be_empty end end end From 19bff26b1da91e2abb2ccbd2bf7f3c04d4a1f960 Mon Sep 17 00:00:00 2001 From: Morgan Roderick Date: Fri, 10 Jul 2026 08:33:18 +0200 Subject: [PATCH 4/4] test: add edge-case coverage for SponsorsSearch query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The core filter tests (name, no-filter, empty) were covered in the previous commit. These three additions fill the gaps: - nil params in initialization: ensures the object handles blank form submissions without errors - case-insensitive name filter: the SQL uses LOWER() — this tests that lowercase and uppercase input both match - chapter filter: exercises the by_chapter join path with a workshop that has no auto-created sponsor (workshop_no_sponsor) to avoid false passes from the workshop fabricator cascade - combined name + chapter filter: the query object chains scopes — this verifies that the AND combination works correctly --- spec/queries/sponsors_search_spec.rb | 40 ++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/spec/queries/sponsors_search_spec.rb b/spec/queries/sponsors_search_spec.rb index f502760f9..5a202c1ca 100644 --- a/spec/queries/sponsors_search_spec.rb +++ b/spec/queries/sponsors_search_spec.rb @@ -6,6 +6,13 @@ expect(search.name).to eq('Acme') expect(search.chapter).to eq('London') end + + it 'handles nil params' do + search = described_class.new(name: nil, chapter: nil) + + expect(search.name).to be_nil + expect(search.chapter).to be_nil + end end describe '#call' do @@ -27,6 +34,39 @@ expect(results).to contain_exactly(matching) end + it 'is case insensitive when filtering by name' do + matching = Fabricate(:sponsor, name: 'Zebra Technologies') + + results = described_class.new(name: 'zebra', chapter: nil).call + expect(results).to contain_exactly(matching) + + results = described_class.new(name: 'ZEBRA', chapter: nil).call + expect(results).to contain_exactly(matching) + end + + it 'filters by chapter' do + chapter = Fabricate(:chapter) + matching = Fabricate(:sponsor) + Fabricate(:workshop_sponsor, workshop: Fabricate(:workshop_no_sponsor, chapter: chapter), sponsor: matching) + Fabricate(:sponsor) + + results = described_class.new(name: nil, chapter: chapter.id.to_s).call + + expect(results).to contain_exactly(matching) + end + + it 'filters by name and chapter combined' do + chapter = Fabricate(:chapter) + matching = Fabricate(:sponsor, name: 'Zebra Technologies') + Fabricate(:workshop_sponsor, workshop: Fabricate(:workshop_no_sponsor, chapter: chapter), sponsor: matching) + Fabricate(:sponsor, name: 'Zebra Technologies') + Fabricate(:sponsor, name: 'Apple Inc') + + results = described_class.new(name: 'Zebra', chapter: chapter.id.to_s).call + + expect(results).to contain_exactly(matching) + end + it 'returns an empty relation when nothing matches' do Fabricate(:sponsor, name: 'Apple Inc')