Skip to content

chore: refactor codebase using WPGraphQL::get_allowed_{type}( 'objects ')#2356

Merged
jasonbahl merged 3 commits intowp-graphql:developfrom
justlevine:chore/cleanup-type-rehydration
Apr 29, 2022
Merged

chore: refactor codebase using WPGraphQL::get_allowed_{type}( 'objects ')#2356
jasonbahl merged 3 commits intowp-graphql:developfrom
justlevine:chore/cleanup-type-rehydration

Conversation

@justlevine
Copy link
Copy Markdown
Collaborator

@justlevine justlevine commented Apr 29, 2022

What does this implement/fix? Explain your changes.

This PR builds on #2353, by replacing the previous hydration pattern that used allowed post type / taxonomy names to get their objects, with the updated `WPGraphQL::get_allowed_{type}( 'object' ) method.

e.g this:

$allowed_post_types = \WPGraphQL::get_allowed_post_types();

if ( ! empty( $allowed_posts_types ) && is_array( $allowed_post_types ) {
  foreach( $allowed_post_types as $post_type ) {
    $post_type_object = get_post_type_object( $post_type );

    if ( $post_type_object instanceof \WP_Post ) {
      // do stuff with $post_type_object.
    }
  }
}

becomes this :

$allowed_post_types = \WPGraphQL::get_allowed_post_types( 'objects' );

foreach( $allowed_post_types as $post_type_object ) {
  // do stuff with $post_type_object
}

This pattern is more readable, easier to test and maintain, and will probably improve some codeclimate scores too.

Does this close any currently open issues?

no

Any other comments?

  • For consistency (and to better help code scanning), I also renamed taxonomy object variables to $tax_object in the few places they were using a different variation.

Where has this been tested?

Operating System: Ubuntu 20.04 (wsl2 + devilbox + PHP 8.0.15)

WordPress Version: 5.9.3

@justlevine justlevine added status: in review Awaiting review before merging or closing scope: code quality Refactoring, linting, and enforcing coding standards labels Apr 29, 2022
@justlevine
Copy link
Copy Markdown
Collaborator Author

justlevine commented Apr 29, 2022

All those codeclimate smells are on preexisting logic thats unchanged in this PR. Marking them as resolved. Meanwhile:

image

}

// Register a connection from each post type that
foreach ( $allowed_post_types as $post_type_object ) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unclear why this foreach loop is separate from earlier one on L85.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, so the one on line 85 is within the foreach ( $allowed_taxonomies as $tax_object ) { loop.

So, this is getting the intersection of allowed post types for each allowed taxonomy so we can create connections:

  • from TaxonomyA -> PostTypeA, etc

This foreach, on line 176 is top level post types, not nested within another foreach.

I'm open to re-thinking how this was done if you have ideas to dry it up or whatever.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 29, 2022

Coverage Status

Coverage decreased (-0.07%) to 79.275% when pulling d85ee38 on justlevine:chore/cleanup-type-rehydration into 4bc1cfa on wp-graphql:develop.

@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit d85ee38 and detected 7 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 5
Duplication 2

View more on Code Climate.

Copy link
Copy Markdown
Collaborator

@jasonbahl jasonbahl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! 👏🏻

@jasonbahl jasonbahl merged commit f228e73 into wp-graphql:develop Apr 29, 2022
@justlevine justlevine deleted the chore/cleanup-type-rehydration branch April 29, 2022 15:27
@jasonbahl jasonbahl mentioned this pull request May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: code quality Refactoring, linting, and enforcing coding standards status: in review Awaiting review before merging or closing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants