Skip to content

fix: or_* resource/proposal filters escaped the AND-joined filters - #70

Merged
phoebus-84 merged 1 commit into
masterfrom
fix/or-filter-scoping
Sep 8, 2026
Merged

fix: or_* resource/proposal filters escaped the AND-joined filters#70
phoebus-84 merged 1 commit into
masterfrom
fix/or-filter-scoping

Conversation

@phoebus-84

Copy link
Copy Markdown
Collaborator

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 be conformsTo = [design] AND (name ILIKE '%solar%' OR note ILIKE '%solar%'). Zenflows.VF.Person.Query already does this correctly (folds the OR into one where).

EconomicResource.Query and Proposal.Query instead applied each or_* field with Ecto's or_where/3. or_where appends a top-level OR, which has lower SQL precedence than the AND-joined where clauses. Actual generated SQL for {conformsTo, notCustodian, classifiedAs, orName, orNote}:

WHERE ((classified_as @> $1) AND (conforms_to_id = ANY($2)) AND (NOT (custodian_id = ANY($3))))
   OR (name ILIKE $4)
   OR (note ILIKE $5)

Because AND binds tighter than OR, the orName/orNote conditions are top-level alternatives to the whole AND-group. A single orName therefore returns every economic resource in the database whose name matches the term — any type, any classification, any custodian — and conformsTo / notCustodian are silently bypassed whenever any or_* field is present. This is the "search returns too much data" symptom on the interfacer-gui catalogue / search pages.

Proposal.Query has the identical defect on orPrimaryIntentsResourceInventoriedAs*.

The fix

Fold the or_* filters into one dynamic/2 expression and apply it with a single where/3, matching Person.Query. Generated SQL after:

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 (the latter backs count/1 and count_distinct_primary_accountable/1) now share one apply_filters/2 helper, so the result set and the counts can't drift.
  • Proposal.Query gets the same treatment for the shared primary_intents_resource_inventoried_as join; or_status (HAVING axis, mutually exclusive with status/not_status via exist_nand, so its or_having is unreachable) is left untouched.
  • No public filter field changes. Same param names and shapes — only zenflows' interpretation is corrected. @dyne/interfacer-client needs no change (it only carries generated GraphQL types + operation documents).

Tests

New test/vf/economic_resource/filter_scoping.test.exs (4 tests) and test/vf/proposal/filter_scoping.test.exs (2 tests). They fail on master (4 of the 6 assertions catch the escape) and pass with this change.

mix test test/vf/economic_resource/ test/vf/proposal/ test/vf/person/
73 tests, 3 failures, 10 skipped

The 3 remaining failures are pre-existing on master, unrelated to filtering: two Person.Type admin-key mutation tests (need a running Restroom/zenroom) and EconomicResource.Domain "classifications/1" (a distinct unnest quirk).

mix credo on the changed files: no issues. (The repo has no .formatter.exs and mix format is not in CI — master's query.ex also fails mix format --check-formatted — so the hand-maintained hard-tab style is kept.)

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
phoebus-84 merged commit 893489d into master Sep 8, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant