Skip to content

Conversation

@justlevine
Copy link
Collaborator

What does this implement/fix? Explain your changes.

This PR refactors how AbstractConnectionResolver calculates the query amount in the following ways:

  • Refactor ::get_query_amount() to only calculate the query amount 1x in the lifecycle, by checking if AbstractConnectionResolver::$query_amount is set.
  • Source the default $max_query_amount from ::max_query_amount(), allowing child classes to overload the max amount without using the graphql_connection_max_query_amount filter. Child classes using that filter have been updated to set ::max_query_amount() instead.
  • Move the graphql_connection_amount_requested filter into ::get_query_amount() to improve DX. The filter will only be applied once due to the aformentioned isset().
  • Add a graphql_connection_default_query_amount filter, for changing the default query amount when no first or last argument is passed to the query.
  • DRYout ::get_amount_requested().
  • Replace direct calls to ::$query_amount with ::get_query_amount().
  • Fixed logic in several ::get_query_args() child methods to rely on ::get_query_amount() and avoid unnecessary min()/max()ing.

There are no breaking changes in this release.

Does this close any currently open issues?

Part of #2749

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

Any other comments?

  • The reason this is not a breaking change is that get_query_amount() is called both in ::get_args() and in the constructor, so there is no possibility of ::$query_amount being null in the child class. Since the types and method signatures, overloaded classes will work as they do currently, making the enhancements in this PR essentially opt-in (i.e. devs can refactor at their leisure).
  • This PR is based on chore: prepare ConnectionResolver classes for v2 backport #3082 , which should be merged first.

Where has this been tested?

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

WordPress Version: 6.4.3

@justlevine justlevine added type: enhancement Improvements to existing functionality status: in review Awaiting review before merging or closing needs: reviewer response This needs the attention of a codeowner or maintainer component: connections Relating to GraphQL Connections scope: api Issues related to access functions, actions, and filters labels Mar 30, 2024
@justlevine justlevine requested a review from jasonbahl March 30, 2024 16:11
@jasonbahl jasonbahl merged commit 0dd3143 into chore/connection-resolvers-cleanup Apr 22, 2024
This was referenced Apr 22, 2024
@justlevine justlevine deleted the dev/cr2-backport/refactor-get_query_amount branch April 23, 2024 20:01
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 needs: reviewer response This needs the attention of a codeowner or maintainer scope: api Issues related to access functions, actions, and filters 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.

3 participants