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

Hotfix for publishing pages #9161

Merged
merged 1 commit into from Aug 20, 2018

Conversation

Projects
None yet
4 participants
@nosolosw
Member

nosolosw commented Aug 20, 2018

Additional fix for #9054

@nosolosw nosolosw requested review from tofumatt and youknowriad Aug 20, 2018

@nosolosw nosolosw self-assigned this Aug 20, 2018

@nosolosw nosolosw added the [Type] Bug label Aug 20, 2018

@tofumatt

Troubling that our new page-publishing test didn't catch this one, that's worth looking into 😢

@nosolosw

This comment has been minimized.

Show comment
Hide comment
@nosolosw

nosolosw Aug 20, 2018

Member

Yeah :( I'll do that today. Need to dig into how taxonomies are retrieved and modify that test case.

Member

nosolosw commented Aug 20, 2018

Yeah :( I'll do that today. Need to dig into how taxonomies are retrieved and modify that test case.

@nosolosw nosolosw merged commit ec1fd21 into master Aug 20, 2018

2 checks passed

codecov/project 50.83% remains the same compared to e8f6bb0
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nosolosw nosolosw deleted the fix/maybe-tags-panel branch Aug 20, 2018

@nosolosw nosolosw removed the request for review from youknowriad Aug 20, 2018

@@ -60,7 +60,7 @@ export default compose(
withSelect( ( select ) => {
const postType = select( 'core/editor' ).getCurrentPostType();
const tagsTaxonomy = select( 'core' ).getTaxonomy( 'post_tag' );
const tags = select( 'core/editor' ).getEditedPostAttribute( tagsTaxonomy.rest_base );
const tags = tagsTaxonomy && select( 'core/editor' ).getEditedPostAttribute( tagsTaxonomy.rest_base );
return {
areTagsFetched: tagsTaxonomy !== undefined,
isPostTypeSupported: tagsTaxonomy && tagsTaxonomy.types.some( ( type ) => type === postType ),

This comment has been minimized.

@aduth

aduth Aug 20, 2018

Member

FWIW on the line below this one we're passing hasTags, which reads as though it's a boolean value, but following the && and its behavior of using the lefthand value, we're likely passing null or undefined when there are no tags, not false.

Making a point of it here only because this tends to be overlooked in applying the && pattern for conditions, added here.

@aduth

aduth Aug 20, 2018

Member

FWIW on the line below this one we're passing hasTags, which reads as though it's a boolean value, but following the && and its behavior of using the lefthand value, we're likely passing null or undefined when there are no tags, not false.

Making a point of it here only because this tends to be overlooked in applying the && pattern for conditions, added here.

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Aug 22, 2018

Member

@nosolosw Can we make sure merged pull requests are assigned to milestones, please?

Member

danielbachhuber commented Aug 22, 2018

@nosolosw Can we make sure merged pull requests are assigned to milestones, please?

@tofumatt tofumatt added this to the 3.7 milestone Aug 22, 2018

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