Skip to content

Conversation

@justlevine
Copy link
Collaborator

What does this implement/fix? Explain your changes.

This PR adds a debug message to the GraphQL response when the requested amount (first, last) is greater than the graphql_connection_max_query_amount filter value set on the server.

Why

This addition provides developers with an important indication that they're not actually getting the full results requested, which is particularly important to new users coming from traditional WP-PHP for whom setting WP_Query( [ 'posts_per_page' => -1 ] ) is discouraged, but commonplace.

A debug message was chosen over throwing a GraphQLError, both to preserve back-compat, and because IMO this is the ideal GraphQL pattern: a too-large first isnt a schema error, it's something that gets filtered on the backend (ostensibly even based on user role).

More importantly, it keeps the frontend code reusable and server-agnostic. E.g. a pagination pattern uses the last returned item to fetch the next page of results no matter what the server-limit is, whereas if a GraphQLError was thrown, users would need to manually update their frontend code and eschew component libraries / framework starters).

Does this close any currently open issues?

Closes #3012

Any relevant logs, error output, GraphiQL screenshots, etc?

image

Any other comments?

Where has this been tested?

Operating System: Ubuntu 20.04 (wsl2 + devilbox + php 8.1.15)

WordPress Version: 6.4.2

@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit 471ca6a and detected 0 issues on this pull request.

View more on Code Climate.

@justlevine justlevine changed the title dev: output GRAPHQL_DEBUG message if requested amount is larger than connection limit feat: output GRAPHQL_DEBUG message if requested amount is larger than connection limit Jan 6, 2024
@justlevine
Copy link
Collaborator Author

(relabeled the PR as a feat because of the linter)

@justlevine justlevine requested a review from jasonbahl January 6, 2024 06:47
@justlevine justlevine added type: enhancement Improvements to existing functionality status: in review Awaiting review before merging or closing component: connections Relating to GraphQL Connections labels Jan 6, 2024
@coveralls
Copy link

Coverage Status

coverage: 84.798% (+0.006%) from 84.792%
when pulling 471ca6a on justlevine:dev/max-query-amount-debug
into 683352d on wp-graphql:develop.

@jasonbahl jasonbahl assigned jasonbahl and justlevine and unassigned jasonbahl Jan 8, 2024
@jasonbahl jasonbahl merged commit 84b640b into wp-graphql:develop Jan 8, 2024
@justlevine justlevine deleted the dev/max-query-amount-debug branch January 8, 2024 15:35
This was referenced Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: connections Relating to GraphQL Connections status: in review Awaiting review before merging or closing type: enhancement Improvements to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No error thrown when query value first exceeds max query amount

3 participants