Editor: Use post.utils for Status Testing

[timmyc]

While working in post-editor.jsx today I noticed a few places where we are still manually testing post.status instead of leveraging lib/posts/utils. This branch is simply some trash pickup for the locations where the post status was being manually tested. Additionally it adds a new method for testing to see if a post's status is private and some tests for isPublished().

To Test

• Open up a new post in the editor
• Privately publish a post and ensure the proper messaging is displayed in the confirmation notice, and in Ground Control
• Hard refresh the page to ensure the post was indeed published privately
• Make an update to the privately published post, verify the messaging is correct
• Create a new public post, follow the steps above to confirm messaging is correct

[dmsnell]

@timmyc I think this might need some updating following some work that went on with the Notices. They received some love recently.

Set post as private
Note this surprised me because although I hadn't published my post yet, I had to publish it in order to select private visibility. Perhaps this is simply how WordPress works, but I could not find a way to make it a private draft, or go back to a normal public draft after I had selected private.

Notice says that the post was privately published.

Update private post

Notice doesn't mention private blog - just that it was updated.

It does also show a JavaScript error in the prop to the notice.

Publish a public post

Update a public post

This was the same as for the private post, so I am not copying the screenshots here. The error message came back.

I think that if the notice code gets updated this will run smoothly.

[timmyc]

@dmsnell thanks for the review - I had forgotten about this PR over the holidays :)

Note this surprised me because although I hadn't published my post yet, I had to publish it in order to select private visibility. Perhaps this is simply how WordPress works

Indeed this is an odd flow, but it is how WordPress works. When a post status is set to private it is essentially published at that point. There are a few old PR's with discussion around this, and the current flow that I'd be happy to point you towards if you'd like to read up on that.

As for warning messages, in 7cb3cc1 I added textOnly to the translate calls, but still the props.text was being passed in as an Array instead of an object or a string which was resulting in the warning from components/notice. As such I added array to the list of valid prop types for text. This remedies the warning and the notices are showing properly still.

[dmsnell]

As for warning messages, in 7cb3cc1 I added textOnly to the translate calls, but still the props.text was being passed in as an Array instead of an object or a string which was resulting in the warning from components/notice. As such I added array to the list of valid prop types for text. This remedies the warning and the notices are showing properly still.

why are they being passed as an array and is it the right solution to just accept the arrays?

[timmyc]

the output of this.translate is producing the array in this particular scenario. I thought that by adding in textOnly: true as an option to the translate call would prevent this from happening but I was wrong.

The text propType already allows an object so it seems fair to allow an array of strings or jsx elements too.

[dmsnell]

would you be willing to humor me and paste the output of one of those translate calls here? I'm simply not grasping what the array elements would be

[timmyc]

Surely. So the array here was a result of publishing a post, console.loging this.state.notice.text in renderNotice. At this point, the text is the output of the this.translate call here and results in an array:

First and last elements are strings, in position 1 we have a react element for the site name / link.

[dmsnell]

thanks @timmyc that helps a bunch. I didn't realize it handled such translation strings like that. my previous thoughts were that it passed a React element.

there is one way I would recommend supporting our code here, though I admit the likelihood of messing up here is probably pretty slim. we should basically have either a single or an array of either text or React node

text: PropTypes.oneOfType( [
    PropTypes.oneOfType( [ PropTypes.string, PropTypes.node ] ),
    PropTypes.arrayOf( PropTypes.oneOfType( [ PropTypes.string, PropTypes.node ] ) )
] ).isRequired

so this should cover all our cases. I would be interested at least to learn if it passes all the tests and works in the code. the thing I like here is that it gives slightly more hinting to the developers what should be expected, whereas [string, object, array] covers just about every possible parameter

[timmyc]

@dmsnell updated in 4ea1a42

[dmsnell]

Thanks @timmyc - next ❓ are those textOnly properties helpful/useful/necessary?

[timmyc]

Backed out in e3c9268 - but did discover some places it is needed. I'll tackle that in a different branch though since this one has meandered a bit.

[nylen]

LGTM 👍