Skip to content

feat: allow global/database IDs in MenuItem connection where args ID inputs#2523

Merged
jasonbahl merged 4 commits intowp-graphql:developfrom
justlevine:feat/allow-global-db-ids-in-menuItem-where-args
Sep 19, 2022
Merged

feat: allow global/database IDs in MenuItem connection where args ID inputs#2523
jasonbahl merged 4 commits intowp-graphql:developfrom
justlevine:feat/allow-global-db-ids-in-menuItem-where-args

Conversation

@justlevine
Copy link
Copy Markdown
Collaborator

What does this implement/fix? Explain your changes.

This PR allows Comment connection where args with a type ID to accept either global IDs or database IDs.

This PR replaces #2340 , and requires #2521 to be merged as a prerequisite.

Does this close any currently open issues?

Part of #998

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

(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)

Any other comments?

The testMenuItemsQueryWithChildItemsUsingDatabaseParentId was removed, as it is covered by testMenuItemsOrder(). Other tests were renamed to match their 'where' arg directly.

Where has this been tested?

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

WordPress Version: 6.0.2

@justlevine justlevine added type: enhancement Improvements to existing functionality status: in review Awaiting review before merging or closing component: connections Relating to GraphQL Connections object type: menu Relating to the Menu or MenuItem Type labels Sep 17, 2022
@justlevine justlevine changed the title feat:allow global/database IDs in MenuItem connection where args ID inputs feat: allow global/database IDs in MenuItem connection where args ID inputs Sep 17, 2022
@coveralls
Copy link
Copy Markdown

coveralls commented Sep 17, 2022

Coverage Status

Coverage increased (+0.01%) to 81.813% when pulling 108f48a on justlevine:feat/allow-global-db-ids-in-menuItem-where-args into a2098ea on wp-graphql:develop.

@qlty-cloud-legacy
Copy link
Copy Markdown

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

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@jasonbahl jasonbahl merged commit b91867c into wp-graphql:develop Sep 19, 2022
@justlevine justlevine deleted the feat/allow-global-db-ids-in-menuItem-where-args branch September 19, 2022 16:52
@jasonbahl jasonbahl mentioned this pull request Sep 19, 2022
@jignam
Copy link
Copy Markdown

jignam commented Sep 21, 2022

Hi There,

We are using Graph QL plugin and it was working fine. After today's update, to fetch only parent menu code is not working.

To fetch parent menu only, we were using ParentID=0 in where condition and it was working fine but after today's update, it is returning all menu instead of parent menu only. Also, there is no new parameter for parent menu only in where condition. I want to first fetch all parent menu then child of parent. I am using currently below query. Can you please advise if any code changes needed in below query? Or it will resolve with new updates.

query MyQuery {
menus(where: {location: MENU_1}) {
nodes {
name
slug
menuItems(where: {parentId: "0"}, first: 100) {
nodes {
label
cssClasses
menuItemId
path
childItems(first: 100) {
nodes {
label
cssClasses
path
menuItemId
}
}
}
}
}
}
}

@jasonbahl
Copy link
Copy Markdown
Collaborator

Thanks for the report. Will check it out and get it patched!

@justlevine
Copy link
Copy Markdown
Collaborator Author

justlevine commented Sep 21, 2022

@jasonbahl
Copy link
Copy Markdown
Collaborator

I have a PR in the works for this.

@jignam
Copy link
Copy Markdown

jignam commented Sep 22, 2022

Thanks for resolving issue, it is working fine in new updates.

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 object type: menu Relating to the Menu or MenuItem Type 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.

4 participants