Skip to content
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.

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

fix: [PHPStan] $block_attributes is always set #90

Merged
merged 3 commits into from
May 22, 2023

Conversation

justlevine
Copy link
Contributor

This PR fixes an issue in Block::get_block_attributes_field(), where the method was incorrectly checking if the paramater $block_attributes was passed to the method ( isset() ) instead of if the value passed was null.

The param signature has also been fixed and moved into PHP, to prevent errors like this from going unnoticed in the future.

Caught by PHPStan level 1.

@justlevine justlevine requested a review from a team as a code owner May 19, 2023 02:41
@changeset-bot
Copy link

changeset-bot bot commented May 19, 2023

🦋 Changeset detected

Latest commit: 99ac818

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@wpengine/wp-graphql-content-blocks Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

$graphql_type = null;

if ( ! isset( $attribute_config['type'] ) ) {
return $block_attribute_fields;
Copy link
Contributor Author

@justlevine justlevine May 19, 2023

Choose a reason for hiding this comment

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

Should this really be returning? My limited understanding of the code is that we want to continue.

Beyond the scope of this PR, but I didnt know a better way to flag it.

Copy link
Contributor

Choose a reason for hiding this comment

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

we want to continue

I think you're right.

mindctrl
mindctrl previously approved these changes May 22, 2023
@mindctrl mindctrl requested a review from a team May 22, 2023 14:01
Copy link
Member

@josephfusco josephfusco left a comment

Choose a reason for hiding this comment

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

Thank you @justlevine!

@mindctrl mindctrl merged commit b075a98 into wpengine:main May 22, 2023
@justlevine justlevine deleted the fix/block-attributes-always-set branch May 22, 2023 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants