feat(ensapi): operator-based filter inputs for events and permissions#2126
feat(ensapi): operator-based filter inputs for events and permissions#2126shrugs wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: 35a1978 The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the Omnigraph GraphQL API to use operator-based filter input objects for events and permissions, aligning multiple fields with the existing @oneOf filter approach and adding a timestamp range filter with zod refinements. It rewires resolver SQL where-building to support { eq | in } set-membership filters and { gt/gte/lt/lte } timestamp ranges, regenerates the SDL, and updates integration tests accordingly.
Changes:
- Introduces per-field
@oneOfset filters (eq/in) for event selector/from/sender and domain-permissions user filtering, plus a refined timestamp range filter. - Refactors
find-events-resolverto use sharedsetFilterCondition/rangeFilterConditionhelpers (including empty-in: []short-circuit semantics). - Updates GraphQL schema wiring (imports, new inputs) and integration tests; adds a Changeset entry.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/enssdk/src/omnigraph/generated/schema.graphql | Regenerated SDL reflecting new operator-based filter input shapes and argument rename(s). |
| apps/ensapi/src/omnigraph-api/schema/resolver.ts | Switches EventsWhereInput import to the new event-inputs module. |
| apps/ensapi/src/omnigraph-api/schema/permissions.ts | Switches EventsWhereInput import to the new event-inputs module. |
| apps/ensapi/src/omnigraph-api/schema/permissions.integration.test.ts | Updates event-filter queries to new shapes; adds Domain.permissions user in test. |
| apps/ensapi/src/omnigraph-api/schema/event.ts | Removes event-related input definitions (leaving EventRef). |
| apps/ensapi/src/omnigraph-api/schema/event-inputs.ts | Adds new event filter inputs and zod refinements; defines EventsWhereInput / AccountEventsWhereInput. |
| apps/ensapi/src/omnigraph-api/schema/domain.ts | Updates Domain.permissions resolver to support `{ eq |
| apps/ensapi/src/omnigraph-api/schema/domain.integration.test.ts | Updates event-filter queries to the new where shapes. |
| apps/ensapi/src/omnigraph-api/schema/domain-inputs.ts | Adds DomainPermissionsUserFilter and updates DomainPermissionsWhereInput.user to use it. |
| apps/ensapi/src/omnigraph-api/schema/account.ts | Updates Account.events sender filter shape; renames Account.permissions arg to where and adds AccountPermissionsWhereInput. |
| apps/ensapi/src/omnigraph-api/schema/account.integration.test.ts | Updates Account.events filter tests to new operator-based input shapes. |
| apps/ensapi/src/omnigraph-api/lib/find-events/find-events-resolver.ts | Refactors event WHERE building to generic helpers; implements empty-in short-circuit and range filters. |
| apps/ensapi/src/omnigraph-api/lib/find-domains/layers/filter-by-name-in.ts | Minor comment adjustment; preserves empty-inArray([]) short-circuit semantics. |
| .changeset/events-filters-oneof.md | Declares a minor release note describing the breaking Omnigraph filter shape changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return lower < upper; | ||
| }, | ||
| { message: "Lower bound must be less than upper bound." }, |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| return lower < upper; | ||
| }, | ||
| { message: "Lower bound must be less than upper bound." }, |
There was a problem hiding this comment.
| return lower < upper; | |
| }, | |
| { message: "Lower bound must be less than upper bound." }, | |
| return lower <= upper; | |
| }, | |
| { message: "Lower bound must be less than or equal to upper bound." }, |
EventsTimestampFilter validation incorrectly rejects equal bounds (e.g., gte: 100, lte: 100) which should match events at exactly that timestamp
Greptile SummaryThis PR extends the operator-based
Confidence Score: 3/5Safe to merge after fixing the timestamp equal-bound validation; the rest of the filter logic and schema reshaping is correct. The apps/ensapi/src/omnigraph-api/schema/event-inputs.ts — the Zod inversion refine at the bottom of EventsTimestampFilter. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
GQL["GraphQL Query"] --> WHERE["EventsWhereInput / AccountEventsWhereInput / DomainPermissionsWhereInput"]
WHERE --> SEL["EventsSelectorFilter @oneOf { eq, in }"]
WHERE --> TS["EventsTimestampFilter { gt?, gte?, lt?, lte? } + Zod refines"]
WHERE --> FROM["EventsFromFilter @oneOf { eq, in }"]
WHERE --> SEND["EventsSenderFilter @oneOf { eq, in }"]
SEL --> SFC["setFilterCondition() in ?? [eq] inArray / [] false"]
FROM --> SFC
SEND --> SFC
TS --> RFC["rangeFilterCondition() and(gt?, gte?, lt?, lte?)"]
SFC --> AND["and(...conditions)"]
RFC --> AND
AND --> DRIZZLE["Drizzle ORM query"]
DRIZZLE --> PG[(PostgreSQL)]
Reviews (1): Last reviewed commit: "fix: schema" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| [ | ||
| (data) => { | ||
| const lower = data.gt ?? data.gte; | ||
| const upper = data.lt ?? data.lte; | ||
| if (lower == null || upper == null) return true; | ||
| return lower < upper; | ||
| }, | ||
| { message: "Lower bound must be less than upper bound." }, | ||
| ], |
There was a problem hiding this comment.
<, so supplying equal lower and upper bounds — e.g. { gte: T, lte: T } to pin-point a specific timestamp — will fail validation with "Lower bound must be less than upper bound." Equal bounds are a valid, semantically meaningful input (match events at exactly timestamp T) and should be allowed. The comparison should be <=.
| [ | |
| (data) => { | |
| const lower = data.gt ?? data.gte; | |
| const upper = data.lt ?? data.lte; | |
| if (lower == null || upper == null) return true; | |
| return lower < upper; | |
| }, | |
| { message: "Lower bound must be less than upper bound." }, | |
| ], | |
| [ | |
| (data) => { | |
| const lower = data.gt ?? data.gte; | |
| const upper = data.lt ?? data.lte; | |
| if (lower == null || upper == null) return true; | |
| return lower <= upper; | |
| }, | |
| { message: "Lower bound must be less than or equal to upper bound." }, | |
| ], |
| export function filterByNameIn(base: BaseDomainSet, names: InterpretedName[]) { | ||
| // Drizzle footgun: `inArray(col, [])` generates `col in ()`, a Postgres syntax error. | ||
| // Short-circuit to an explicit empty result. | ||
| // NOTE: avoid inArray([]) runtime error by short-circuit to an explicit empty result |
There was a problem hiding this comment.
inArray(col, []) generates col in (), which is a Postgres syntax error, not just a generic "runtime error". The precise diagnosis helps future maintainers understand why the short-circuit is necessary.
| // NOTE: avoid inArray([]) runtime error by short-circuit to an explicit empty result | |
| // NOTE: Drizzle footgun — `inArray(col, [])` generates `col in ()`, a Postgres syntax error. | |
| // Short-circuit to an explicit empty result instead. |
| function setFilterCondition<T>(column: AnyColumn, filter?: SetFilter<T> | null): SQL | undefined { | ||
| if (!filter) return undefined; | ||
| const values = filter.in ?? [filter.eq]; | ||
| // NOTE: avoid inArray([]) runtime error by short-circuit to `false` | ||
| if (values.length === 0) return sql`false`; | ||
| return inArray(column, values); | ||
| } |
There was a problem hiding this comment.
setFilterCondition passes [undefined] to inArray when neither eq nor in is set
filter.in ?? [filter.eq] produces [undefined] when both fields are absent (e.g. a programmatic caller passes {} without @oneOf enforcement). Drizzle's inArray with [undefined] does not short-circuit via the values.length === 0 guard (length is 1) and will generate SQL col IN (null) or a type coercion error. The @oneOf GraphQL directive prevents this path for schema consumers, but the internal EventsWhere interface accepts the same type and is composed directly in account.ts via { ...args.where, sender: { eq: parent.id } }. Adding a filter.eq != null guard or a stricter internal type would close the gap.
Reviewer Focus
find-events-resolver.tsset/range helpers + emptyin: []→ no matches (mirrorsfilter-by-name-in.ts).EventsTimestampFilterzod refines (≥1 bound;gt/gtemutex;lt/ltemutex; inverted-range rejected) — not expressible via@oneOf.Problem & Motivation
extends the
@oneOfoperator pattern fromDomain.nameto the rest of Omnigraph's filter args where multiple operators have a real use case today. tight scope; expand on demand.What Changed
@oneOf { eq, in }filters (in: max 10):EventsSelectorFilter,EventsFromFilter,EventsSenderFilter,DomainPermissionsUserFilter.EventsTimestampFilter { gt?, gte?, lt?, lte? }.EventsWhereInput/AccountEventsWhereInput/DomainPermissionsWhereInputreshaped to use them.Account.permissions(in: AccountIdInput)→Account.permissions(where: { contract }).event-inputs.ts.Design
set-membership =
@oneOf { eq, in }; range = flat (bounds combine, can't be@oneOf). per-field-named inputs (no shared generics). emptyin: []matches nothing rather than 400 — matchesDomainsNameFilterprecedent + search-page consumer DX.Self-Review
minLength: 1onin:that would have forced consumers to special-case empty UI state.Domain.permissionsuserScope to mirrorsetFilterCondition'sin ?? [eq]pattern.event-inputs.tsfor symmetry withdomain-inputs.ts.Cross-Codebase Alignment
grepped
selector_in,timestamp_gte/_lte,EventsWhereInput,AccountEventsWhereInput,DomainPermissionsWhereInput,permissions(in:. no live references inapps/ensadmin,packages/ensnode-sdk,packages/ensnode-react,docs/ensnode.io,examples/. subgraph API intentionally untouched (compat).Downstream Impact
breaking Omnigraph schema. SDL regenerated. no in-repo consumers affected. changeset added (minor, ensapi only).
Testing
typecheck clean, lint clean, 98 unit + 295 integration pass.
gap: timestamp cross-field refines not asserted by integration tests (zod-layer behavior).
Scope Reductions
no
eq/infor timestamp, no negation, no version-filter extension, no shared generic filter types. open on demand.Risk
pre-1.0 API. blast radius: ensapi GraphQL clients. rollback = revert.
named owner: @shrugs.
Pre-Review Checklist