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: add graphql_pre_resolve_menu_item_connected_node filter #3044

Conversation

justlevine
Copy link
Collaborator

What does this implement/fix? Explain your changes.

This PR adds the graphql_pre_resolve_menu_item_connected_node filter to the MenuItem.connectedNode resolver, allowing one to short-circuit the resolver logic and use a custom AbstractConnectionResolver->get_connection() as the result.

Additionally, the graphql_resolve_menu_item filter on the deprecated MenuItem.connectedObject resolver has been deprecated.

This change brings parity between connectedNode and connectedObject, and will allow developers still relying on the latter to finally migrate.

Tests have been added both to cover the new filter, and to cover resolving TermObjects (since that was missing).

Does this close any currently open issues?

Fixes #2484

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

Any other comments?

  • Theres a @todo tag that will need to be replaced on release and will need to be replaced manually (its not in the usual @since @todo format.

Where has this been tested?

Operating System: Ubuntu 20.04 (wsl2 + devilbox + php8.1.14)

WordPress Version: 6.4.3

@@ -180,14 +201,19 @@ public static function register_type() {
*
* @since 0.0.30
*/
return apply_filters(
return apply_filters_deprecated(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid too many return statements within this method.

Copy link

codeclimate bot commented Feb 11, 2024

Code Climate has analyzed commit e65fbc8 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@justlevine justlevine added Type: Enhancement New feature or request Status: In Review Needs to be reviewed by a project maintainer before merge scope: API Issues related to access functions, actions, and filters labels Feb 11, 2024
@coveralls
Copy link

Coverage Status

coverage: 84.332% (+0.06%) from 84.275%
when pulling e65fbc8 on justlevine:dev/graphql_pre_resolve_menu_item_connected_node
into afe6a52 on wp-graphql:develop.

@jasonbahl jasonbahl merged commit e8262a9 into wp-graphql:develop Feb 14, 2024
22 of 23 checks passed
@justlevine justlevine deleted the dev/graphql_pre_resolve_menu_item_connected_node branch February 14, 2024 00:36
@jasonbahl jasonbahl mentioned this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: API Issues related to access functions, actions, and filters 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.

MenuItem doesn't apply graphql_resolve_menu_item filter in connectedNode like it does for connectedObject
3 participants