Skip to content

Fix: Connections missing nodes when before or after are empty.#2293

Merged
jasonbahl merged 11 commits intowp-graphql:developfrom
justlevine:fix/missing-nodes-on-empty-before-after
Apr 4, 2022
Merged

Fix: Connections missing nodes when before or after are empty.#2293
jasonbahl merged 11 commits intowp-graphql:developfrom
justlevine:fix/missing-nodes-on-empty-before-after

Conversation

@justlevine
Copy link
Copy Markdown
Collaborator

What does this implement/fix? Explain your changes.

This PR fixes certain connections missing nodes when the before or after where args are empty.

Issue stems from the fact that get_nodes() in several connection resolvers simply checked for isset( $this->args['{before|after'}]) before slicing the array.

Along those lines, I also got rid of the unnecessary isset() checks in a couple of places.

to test:

Run the following query and confirm withOffset and withoutOffset are the same:

{
  withOffset: taxonomies(first:2, before:""){
    nodes{
      name
    }
  }
  withoutOffset: taxonomies(first:2){
    nodes{
      name
    }
  }
}

taxonomies should be replaced with the other relevant connection types: contentTypes, registeredScripts, registeredStylesheets, plugins, taxonomies, themes, and roles.

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

Without PR (v1.7.2)
image

With PR:
image

Where has this been tested?

Operating System: Ubuntu 20.04 (wsl2) + Devilbox + PHP v8.0.15

WordPress Version: v5.9.1

@justlevine justlevine requested a review from jasonbahl March 11, 2022 00:46
@justlevine justlevine added type: bug Issue that causes incorrect or unexpected behavior needs: reviewer response This needs the attention of a codeowner or maintainer component: connections Relating to GraphQL Connections component: pagination Relating to pagination lang: php Pull requests that update PHP code labels Mar 11, 2022
@coveralls
Copy link
Copy Markdown

coveralls commented Mar 11, 2022

Coverage Status

Coverage increased (+0.1%) to 79.405% when pulling fb43c04 on justlevine:fix/missing-nodes-on-empty-before-after into 70dc88b on wp-graphql:develop.

@justlevine
Copy link
Copy Markdown
Collaborator Author

Added tests.

Note:

  • I was sloppy with the commit history. I refactored some tests to use WPGraphQLTestCase, fixed UserConnectionPaginationTest::testPaginateForwardAndBackward(), and added barebones missing pagination tests for contentTypes, registeredScripts, registeredStylesheets, and taxonomies, in addition to testing for an empty before/after string.
  • There's some commented out code regarding last args, as those tests will only work once fix: refactor AbstractConnectionResolver::get_nodes() to prevent children double slicing #2294 is committed.

@justlevine
Copy link
Copy Markdown
Collaborator Author

Worthwhile to note that I didnt try 'backfilling' missing connection tests in the new test files I created, only added those relevant to the changes in this PR.

jasonbahl and others added 4 commits April 1, 2022 14:48
…ing-nodes-on-empty-before-after

# Conflicts:
#	src/Data/Connection/ContentTypeConnectionResolver.php
#	src/Data/Connection/EnqueuedScriptsConnectionResolver.php
#	src/Data/Connection/EnqueuedStylesheetConnectionResolver.php
#	src/Data/Connection/PluginConnectionResolver.php
#	src/Data/Connection/TaxonomyConnectionResolver.php
#	src/Data/Connection/ThemeConnectionResolver.php
#	src/Data/Connection/UserRoleConnectionResolver.php
- updating tests to properly test the last argument
- one broken test remaining inUSerConnectionPaginationTest.php
@qlty-cloud-legacy
Copy link
Copy Markdown

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

View more on Code Climate.

@jasonbahl jasonbahl merged commit 2ef1304 into wp-graphql:develop Apr 4, 2022
@justlevine justlevine deleted the fix/missing-nodes-on-empty-before-after branch April 4, 2022 17:29
@jasonbahl jasonbahl mentioned this pull request Apr 6, 2022
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 component: pagination Relating to pagination lang: php Pull requests that update PHP code needs: reviewer response This needs the attention of a codeowner or maintainer type: bug Issue that causes incorrect or unexpected behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants