Skip to content

fix: return empty when filtering menuItems by a location with no assigned items - #3043

Merged
jasonbahl merged 2 commits into
wp-graphql:developfrom
justlevine:fix/menu-items-location-filter
Feb 13, 2024
Merged

fix: return empty when filtering menuItems by a location with no assigned items#3043
jasonbahl merged 2 commits into
wp-graphql:developfrom
justlevine:fix/menu-items-location-filter

Conversation

@justlevine

@justlevine justlevine commented Feb 11, 2024

Copy link
Copy Markdown
Collaborator

What does this implement/fix? Explain your changes.

This PR fixes an issue in the MenuItemConnectionResolver logic, where querying a menuItem by a location with unset terms would return all menu items, instead of none.

Logic has been changed so the tax_query args are given a 'term_id' => [].

Tests have been added to cover:

  • A location with no menu items returns null.
  • A location with no menu items returns null as an admin.
  • A location with menu items, doesn't return items from a different location (either public or as an admin).

Does this close any currently open issues?

Fixes #3029

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

Any other comments?

Where has this been tested?

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

WordPress Version: 6.4.3

@qlty-cloud-legacy

Copy link
Copy Markdown

Code Climate has analyzed commit 670b1bf and detected 0 issues on this pull request.

View more on Code Climate.

@justlevine justlevine added type: bug Issue that causes incorrect or unexpected behavior 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 labels Feb 11, 2024
@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 84.275%. remained the same
when pulling 670b1bf on justlevine:fix/menu-items-location-filter
into afe6a52 on wp-graphql:develop.

@jasonbahl

Copy link
Copy Markdown
Collaborator

@justlevine I pulled this branch and querying for a list of menuItems when no menu has been assigned to a location still returns random menu items 🤔

CleanShot 2024-02-13 at 16 51 04

@jasonbahl

Copy link
Copy Markdown
Collaborator

@justlevine ah my bad. I was executing as an authenticated user who should see all menu items because they're not considered private. 🤦🏻‍♂️

@jasonbahl
jasonbahl merged commit 387b289 into wp-graphql:develop Feb 13, 2024
@justlevine

Copy link
Copy Markdown
Collaborator Author

@justlevine ah my bad. I was executing as an authenticated user who should see all menu items because they're not considered private. 🤦🏻‍♂️

If my fix worked correctly then even if the user is authenticated they should only receive the items they're actually filtering for, so it should still come back []. 🤔

@jasonbahl

Copy link
Copy Markdown
Collaborator

ok, ya, authenticated users still get the incorrect behavior then.

@jasonbahl

Copy link
Copy Markdown
Collaborator

@justlevine let me confirm that I didn't have anything else going on. . .like WPGraphQL Smart Cache object cache active or anything 👀

@jasonbahl

Copy link
Copy Markdown
Collaborator

@justlevine oooh. . .so this was related to WPGraphQL Smart Cache, but not really related to caching. . .

WPGraphQL Smart Cache declares WPGraphQL as a dev dependency and my local copy of WPGraphQL Smart Cache was overriding the checked out version of WPGraphQL I was testing 🤔

Interesting.

My hunch is that we don't actually need WPGraphQL defined as a dev dependency for WPGraphQL Smart Cache. That's wild though that the dev dependency was taking precedent over the actual active WPGraphQL plugin 🤔

Anyway, by de-activating WPGraphQL Smart Cache I was able to experience the intended behavior of no nodes resolving when filtering menus by location when that location has no menu assigned yet (even for admin user)

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: bug Issue that causes incorrect or unexpected behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Menu returning wrong items

3 participants

Sponsor
SponsoredKunjungi sekarang
Promo