feat: add pagination integration tests, fix backward pagination bug#1694
Conversation
Add generic testDomainPagination helper that tests all 6 ordering permutations (3 orderBy × 2 dir) with forward and backward relay cursor iteration for Query.domains, Domain.subdomains, Account.domains, and Registry.domains. Fix cursorFilter using effectiveDesc (which includes the Relay inverted XOR) instead of the raw user orderDir for comparison operator selection. This caused backward pagination (last/before) to select rows after the cursor instead of before it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
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 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 |
Greptile SummaryFixed critical backward pagination bug where Key Changes:
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 80c55d4 |
There was a problem hiding this comment.
Pull request overview
Adds a reusable integration test suite to validate Relay-style forward/backward pagination across multiple domain connection fields, and updates the findDomains keyset cursor filtering logic intended to fix a backward-pagination (last/before) bug.
Changes:
- Introduce generic
testDomainPagination()helper plus shared pagination GraphQL queries/fragments. - Expand integration tests for
Query.domains,Domain.subdomains,Account.domains, andRegistry.domainsto cover all order permutations and both pagination directions. - Modify
findDomainscursor filtering and resolver wiring related to effective ordering during inverted (backward) pagination.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/ensapi/src/test/integration/test-domain-pagination.ts | New shared integration test helper to iterate forward/backward through domain connections and assert ordering. |
| apps/ensapi/src/test/integration/graphql-utils.ts | Extends GraphQLConnection test type to include cursor + pageInfo. |
| apps/ensapi/src/test/integration/domain-pagination-queries.ts | New shared GraphQL queries/fragments for paginated domain connections. |
| apps/ensapi/src/graphql-api/schema/query.integration.test.ts | Adds Query.domains pagination integration coverage using the new helper. |
| apps/ensapi/src/graphql-api/schema/domain.integration.test.ts | Adds Domain.subdomains pagination integration coverage using the new helper. |
| apps/ensapi/src/graphql-api/schema/account.integration.test.ts | Adds Account.domains pagination integration coverage using the new helper. |
| apps/ensapi/src/graphql-api/schema/registry.integration.test.ts | Adds Registry.domains pagination integration coverage using the new helper. |
| apps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver.ts | Updates resolver cursor filtering calls as part of the backward pagination fix. |
| apps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver-helpers.ts | Updates cursorFilter comparison-operator selection logic for keyset pagination. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver-helpers.ts
Show resolved
Hide resolved
…-integration-tests
…-integration-tests
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver-helpers.ts
Show resolved
Hide resolved
lightwalker-eth
left a comment
There was a problem hiding this comment.
@shrugs Also looks great 👍
ec46900
into
feat/graphql-api-integration-test
closes #1622
Summary
Query.domains,Domain.subdomains,Account.domains,Registry.domains) covering all 6 ordering permutations × forward/backward paginationcursorFilter— removed theeffectiveDescparameter and derived direction comparison directly fromqueryOrderDir, which was incorrect wheninvertedflipped the sorttestDomainPaginationtest harness into reusable test utilitiesWhy
effectiveDescbeing passed through from the caller (which accounted forinverted) whilecursorFilteronly needs the declaredqueryOrderDirTesting