fix: or_* resource/proposal filters escaped the AND-joined filters - #70
Merged
Conversation
The `or_*` filter fields on `economicResources` (`orName`, `orNote`,
`orConformsTo`, …) and on `proposals`
(`orPrimaryIntentsResourceInventoriedAsName`, …) are meant to form one
OR group that is ANDed with the non-`or_` filters — e.g.
`conformsTo AND (name ILIKE q OR note ILIKE q)`. `Person.Query` already
does this correctly by folding the OR into a single `where`.
`EconomicResource.Query` and `Proposal.Query` instead applied each
`or_*` field with Ecto's `or_where/3`, which appends a *top-level* `OR`
with lower precedence than the `AND`-joined `where` clauses. The
generated SQL was:
WHERE ((classified_as @> $1) AND (conforms_to_id = ANY($2))
AND (NOT (custodian_id = ANY($3))))
OR (name ILIKE $4)
OR (note ILIKE $5)
so a single `orName` returned every resource in the table whose name
matched, regardless of type, classification or custodian — and
`notCustodian` / `conformsTo` were silently bypassed whenever any
`or_*` field was present.
Fix: fold the `or_*` filters into one `dynamic/2` expression and apply
it with a single `where/3`, matching `Person.Query`. Now:
WHERE (classified_as @> $1) AND (conforms_to_id = ANY($2))
AND (NOT (custodian_id = ANY($3)))
AND ((name ILIKE $4) OR (note ILIKE $5))
`EconomicResource.Query.all/1` and `.filtered_query/1` (used by the
count aggregates) now share one `apply_filters/2` helper so results and
counts cannot drift. No public filter field changes — only the
interpretation is corrected.
Adds `test/vf/{economic_resource,proposal}/filter_scoping.test.exs`,
which fail on the previous implementation and pass with this change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
phoebus-84
had a problem deploying
to
test_trace
September 8, 2026 17:11 — with
GitHub Actions
Failure
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.
The bug
The
or_*filter fields are meant to form a single OR group that is ANDed with the non-or_filters — e.g. searching designs for "solar" should beconformsTo = [design] AND (name ILIKE '%solar%' OR note ILIKE '%solar%').Zenflows.VF.Person.Queryalready does this correctly (folds the OR into onewhere).EconomicResource.QueryandProposal.Queryinstead applied eachor_*field with Ecto'sor_where/3.or_whereappends a top-levelOR, which has lower SQL precedence than theAND-joinedwhereclauses. Actual generated SQL for{conformsTo, notCustodian, classifiedAs, orName, orNote}:Because
ANDbinds tighter thanOR, theorName/orNoteconditions are top-level alternatives to the whole AND-group. A singleorNametherefore returns every economic resource in the database whose name matches the term — any type, any classification, any custodian — andconformsTo/notCustodianare silently bypassed whenever anyor_*field is present. This is the "search returns too much data" symptom on the interfacer-gui catalogue / search pages.Proposal.Queryhas the identical defect onorPrimaryIntentsResourceInventoriedAs*.The fix
Fold the
or_*filters into onedynamic/2expression and apply it with a singlewhere/3, matchingPerson.Query. Generated SQL after:EconomicResource.Query.all/1and.filtered_query/1(the latter backscount/1andcount_distinct_primary_accountable/1) now share oneapply_filters/2helper, so the result set and the counts can't drift.Proposal.Querygets the same treatment for the sharedprimary_intents_resource_inventoried_asjoin;or_status(HAVING axis, mutually exclusive withstatus/not_statusviaexist_nand, so itsor_havingis unreachable) is left untouched.@dyne/interfacer-clientneeds no change (it only carries generated GraphQL types + operation documents).Tests
New
test/vf/economic_resource/filter_scoping.test.exs(4 tests) andtest/vf/proposal/filter_scoping.test.exs(2 tests). They fail onmaster(4 of the 6 assertions catch the escape) and pass with this change.The 3 remaining failures are pre-existing on
master, unrelated to filtering: twoPerson.Typeadmin-key mutation tests (need a running Restroom/zenroom) andEconomicResource.Domain "classifications/1"(adistinct unnestquirk).mix credoon the changed files: no issues. (The repo has no.formatter.exsandmix formatis not in CI —master'squery.exalso failsmix format --check-formatted— so the hand-maintained hard-tab style is kept.)