feat(target): filter through declared association paths - #15
Merged
Conversation
- datable/equatable/rangeable targets may walk associations with an
explicit nested hash — `equatable account_name: { account: :name }`,
multi-level `{ account: { bank: :name } }` — never inferred from
names
- Filterable::Target joins the declared path and merges the condition
on the target model, adding DISTINCT when the path crosses a
collection; the never-raise contract holds: unknown or polymorphic
associations and ambiguous multi-key hashes narrow nothing
- sortable refuses association targets (DISTINCT + ORDER BY pitfall);
scopable/togglable guard against hash-valued declarations that used
to raise TypeError on respond_to?
- declarations validator resolves paths with the same Target helpers
(no drift): unresolvable path, unknown column on the target model,
ambiguous target and sortable-through-association are all reported
📊 Code Quality Report
🔍 16 new reek smells introduced — review below. 🔍 16 new reek smells introduced by this PR
✅ 2 reek smells resolved by this PRThe exact list isn't shown — these smells are gone, that's what matters. 🔍 All 59 reek smells on this PR
📊 Flog complexity breakdown📊 Flay duplication breakdown |
🛡️ Documentation Quality: 100.0% (0.0%)
✅ Documentation quality above threshold. Changed files
|
🛡️ Coverage Report: 100.0% (0.0%)
✅ All changes are tested. Changed files
|
🛡️ RuboCop: 0 offenses (0)
✅ No RuboCop offenses. |
- a polymorphic association — delegated_type included — is crossed by
naming the concrete type as the second segment:
`equatable subject: { entryable: { message: :subject } }`; there is
no ambiguity since an association name can never follow a
polymorphic reflection
- the join is built in arel (no SQL strings) and guarded by the
polymorphic *_type column via polymorphic_name, so two types sharing
ids can never leak into each other — proven by an id-collision spec
- the hop must open the path and target a column directly on the
concrete type; unknown or non-model types narrow nothing and the
validator reports them through the shared Target resolution
- specs cover the real delegated_type macro and a bare polymorphic
belongs_to, so support does not depend on the sugar
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
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.
Summary
Closes the last item of the gem-landscape backlog — association filtering, including the
delegated_typecase.datable/equatable/rangeabledeclarations may target a column through associations with an explicit nested hash:equatable account_name: { account: :name }, multi-level{ account: { bank: :name } }, and polymorphic hop{ entryable: { message: :subject } }. The path is declared, never inferred from names (the toschas anti-pattern flagged in the review).Changes
Filterable::Target: resolves declared targets — own column, or association path viareflect_on_association; filtering joins the declared path and merges the condition on the target model (filterameter's clean pattern), addingDISTINCTwhen the path crosses a collection; INNER JOIN semantics, rows without the association drop outbelongs_to—delegated_typeincluded — crossed by naming the concrete type as the second segment; arel join (no SQL strings) guarded by the*_typecolumn viapolymorphic_name, proven safe by an id-collision spec; the hop must open the path and target a column directly on the concrete type; specs cover both the realdelegated_typemacro and a bare polymorphicbelongs_tosortablerefuses association targets (theDISTINCT+ORDER BYselect-list pitfall on Postgres);scopable/togglablenow guard against hash-valued declarations that previously raisedTypeErroronrespond_to?Targethelpers — no runtime/validator drift — and reports: unresolvable path (unknown association, polymorphic without type, unknown concrete type), unknown column on the target model, ambiguous target, sortable-through-associationdelegated_type :attachableand a bare polymorphicbelongs_to :sourceto cover every branch (147 examples, 100% coverage)How to Test
bundle exec rakeMovementDetail.filterable(filters: { account_name: 'Main' })→INNER JOIN accounts ... WHERE accounts.name = 'Main';filters: { attached_bank_name: 'X' }→ jointure arel + gardeattachable_type = 'Bank';Account.filterable(filters: { detail_reference: 'DUP' })→ une seule ligne malgré deux détails (DISTINCT) ; déclarerequatable x: { nope: :col }→ no-op au runtime, erreur dansfilterable_declarations.errors