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

Add arg filters for menu items #719

Merged
merged 2 commits into from Mar 13, 2019

Conversation

Projects
None yet
2 participants
@epeli
Copy link
Contributor

commented Mar 12, 2019

Adds two filters for menu items connection resolvers

A graphql_menu_item_connection_args which can be used to filter the args passed to the menuItems query before the $query_args is generated.

A graphql_menu_item_connection_query_args which can be used to modify the $query_args once the wpgraphql logic is executed.

Rationale

In the wp-graphql-polylang were aiming to add a language where arg to all translatable items and with menus I was not able to find any way to modify the menu query based on the input args.

Eg. we want to be able to write

{
  menuItems(where: {language: EN, location: TOP_MENU}) {
    nodes {
      label
    }
  }
}

Polylang handles menu translations by dynamically generating translated versions of the menu locations. Ex. TOP_MENU to TOP_MENU___en, TOP_MENU___fi etc. so we need to convert the location where arg using the language field.

Here's how we would use graphql_menu_item_connection_args to implement this:

https://github.com/valu-digital/wp-graphql-polylang/blob/507f095a87d86177a7dc41d35a54ee4efd019540/src/MenuItem.php#L26-L56


Currently we don't have a use case for graphql_menu_item_connection_query_args but I added it just for consistency because equivalents exists for posts, terms.

@epeli epeli changed the title Add arg filters Add arg filters for menu items Mar 12, 2019

@codecov

This comment has been minimized.

Copy link

commented Mar 12, 2019

Codecov Report

Merging #719 into develop will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #719      +/-   ##
===========================================
+ Coverage    60.14%   60.15%   +0.01%     
===========================================
  Files          110      110              
  Lines         6744     6746       +2     
===========================================
+ Hits          4056     4058       +2     
  Misses        2688     2688
Impacted Files Coverage Δ
src/Data/MenuItemConnectionResolver.php 78.94% <100%> (+0.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1caf66...4a50406. Read the comment docs.

@jasonbahl

This comment has been minimized.

Copy link
Collaborator

commented Mar 13, 2019

@epeli I think this is needed. I'm holding off a second on merging because I have some ideas on how we can centralize this a bit for all connections to benefit.

@jasonbahl

This comment has been minimized.

Copy link
Collaborator

commented Mar 13, 2019

@epeli I'll merge this and get it out so that when I work on my refactor it's in front of my face as something I need to account for. I think it's needed to be able to hook in here regardless.

@jasonbahl jasonbahl merged commit b359595 into wp-graphql:develop Mar 13, 2019

3 checks passed

codecov/patch 100% of diff hit (target 60.14%)
Details
codecov/project 60.15% (+0.01%) compared to b1caf66
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jasonbahl jasonbahl added this to the 0.2.3 milestone Mar 18, 2019

This was referenced Mar 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.