refactor: move SponsorsSearch from models to queries layer#2697
Merged
Conversation
mroderick
force-pushed
the
feature/move-sponsors-search
branch
from
992b276 to
7dee8dd
Compare
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.
mroderick
force-pushed
the
feature/move-sponsors-search
branch
from
7dee8dd to
9ce1b5a
Compare
…h spec 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.
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).
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
mroderick
force-pushed
the
feature/move-sponsors-search
branch
from
9ce1b5a to
19bff26
Compare
KimberleyCook
approved these changes
Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Move
SponsorsSearchfromapp/models/(domain layer) toapp/queries/(application layer), where query objects belong in layered architecture.Why
As identified in the layered-architecture analysis,
SponsorsSearchis a query object — it wraps a search/filter operation usingActiveModel::Modeland#call. It has no domain identity, no persistence, and no business invariants. The domain layer should not host query objects.Changes
app/models/sponsors_search.rb→app/queries/sponsors_search.rb— no code changes, just a rename (Rails 8 autoloads allapp/subdirectories, so no require path updates needed)spec/models/sponsors_search_spec.rb→spec/queries/sponsors_search_spec.rb— test follows the fileVerification
SponsorsSearchresolves correctly via autoloading from new location