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.

Sign up for GitHub

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

Jump to bottom

fix: output `list:$type` keys for Root fields that return a listOf Nodes #2861

Merged

jasonbahl merged 3 commits into wp-graphql:develop from jasonbahl:fix/#2860-missing-list-type-keys

Jul 24, 2023

Collaborator

jasonbahl commented Jul 18, 2023 •

edited

What does this implement/fix? Explain your changes.

This fixes a regression to the 1.14.5 release where list:$type keys are no longer being output for RootQuery fields that return a listOf Nodes.

Does this close any currently open issues?

closes #2860

related: wp-graphql/wp-graphql-smart-cache#228

Any other comments?

Given the following snippet:

add_action( 'graphql_register_types', function() {

	register_graphql_field( 'RootQuery', 'testListOfPosts', [
		'type' => [
			'non_null' => [
				'list_of' => [
					'non_null' => 'Post'
				],
			],
		],
		'resolve' => function() {
			$posts = new \WP_Query([ 'posts_per_page' => 10 ]);
			return array_map( function( $post ) { return new Post( $post ); }, $posts->posts );
		}
	]);

});

I could query for testListOfPosts:

Before

We do not see list:post in the keys

After

We DO see list:post in the keys


          - Adds failing tests

4b4ad4a

jasonbahl changed the title ~~fix: output list:$type keys for Root fields that return a listOf Nodes~~ fix: output list:$type keys for Root fields that return a listOf Nodes


          - Update QueryAnalyzer to track list types a bit differently

cf800f9

codeclimate bot reviewed

View reviewed changes

src/Utils/QueryAnalyzer.php

+              			return $type;
+              		}
+              		return null;

codeclimate bot Jul 18, 2023

Avoid too many return statements within this method.

src/Utils/QueryAnalyzer.php

+              				$to_type = $field_def->config['toType'] ?? null;
+              				if ( empty( $to_type ) ) {
+              					return null;

codeclimate bot Jul 18, 2023

Avoid too many return statements within this method.

src/Utils/QueryAnalyzer.php

+              					return null;
+              				}
+              				return $to_type;

codeclimate bot Jul 18, 2023

Avoid too many return statements within this method.

src/Utils/QueryAnalyzer.php

+              				return null;
+              			}
+              			return $type;

codeclimate bot Jul 18, 2023

Avoid too many return statements within this method.

src/Utils/QueryAnalyzer.php

+              			}
+              			if ( ! $is_list_type ) {
+              				return null;

codeclimate bot Jul 18, 2023

Avoid too many return statements within this method.

coveralls commented Jul 18, 2023 •

edited

coverage: 85.049% (+0.02%) from 85.034% when pulling 18da2f0 on jasonbahl:fix/#2860-missing-list-type-keys into e3940e3 on wp-graphql:develop.

jasonbahl self-assigned this


          - phpcs fixes

18da2f0

codeclimate bot reviewed

View reviewed changes

src/Utils/QueryAnalyzer.php

+              	 *
+              	 * @return  \GraphQL\Type\Definition\Type|String|null
+              	 */
+              	public function get_wrapped_field_type( Type $type, FieldDefinition $field_def, $parent_type, bool $is_list_type = false ) {

codeclimate bot Jul 19, 2023

Function get_wrapped_field_type has a Cognitive Complexity of 20 (exceeds 5 allowed). Consider refactoring.

codeclimate bot commented Jul 19, 2023

Code Climate has analyzed commit 18da2f0 and detected 6 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	6

View more on Code Climate.

jasonbahl mentioned this pull request

ci: tests failing after wpgraphql v1.14.5 release wp-graphql/wp-graphql-smart-cache#235

Merged

markkelnar self-requested a review

July 24, 2023 19:01

markkelnar approved these changes

View reviewed changes

jasonbahl merged commit a71b261 into wp-graphql:develop

32 of 33 checks passed

jasonbahl mentioned this pull request

release: v1.14.8 #2864

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment