Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: output GRAPHQL_DEBUG message if requested amount is larger than connection limit #3013

Merged

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

Copy link

codeclimate bot commented Jan 6, 2024

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 added Type: Enhancement New feature or request Status: In Review Needs to be reviewed by a project maintainer before merge Component: Connections Issues related to 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
27 of 29 checks passed
@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 Issues related to connections Status: In Review Needs to be reviewed by a project maintainer before merge Type: Enhancement New feature or request
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