Fix/post statuses in query #1114

adekbadek · 2022-04-19T09:54:56Z

All Submissions:

Have you followed the Newspack Contributing guideline?
Does your code follow the WordPress' coding standards and VIP Go coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Closes #38

A question – is checking for edit_others_posts capability a safe-enough check?

For design review – the label which marks non-published posts:

How to test the changes in this Pull Request:

Insert Homepage Posts and Carousel blocks on a page
Ensure you have some recent draft and scheduled posts
For both, observe a new panel in block settings – "Post Statuses". Set different values for this attribute, and observe the post list is updated – non-published posts should be displayed with a label
Publish and visit the page as the logged-in editor user – observe the posts are shown as in the editor
Visit the page as a non-logged-in user – observe only published posts are displayed

Other information:

Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
Have you successfully ran tests with your changes locally?

keepPlaceholderOnFocus is not used, and makes Reader validation throw an error https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/rich-text/README.md

registerGenericStore is being deprecated

…posts Closes #38

dkoo

This works well as described, but I'm uncertain about the idea of introducing a block attribute to show or not show preview posts.

Would it be enough and easier to understand if we simply showed all draft, scheduled, and published posts with labels for each non-published status in the block editor? On the front-end side, we could show these non-published posts if the user is logged in, has the edit_others_posts capability, and there's a preview=true param on the URL (meaning the post is visited from the "Preview" functionality in the editor).

Or, to make it even simpler, just always show all of the draft, scheduled and published posts to logged-in users with the right capabilities. In this case if an editor really wants to see a post "as it would appear to readers" they can view it in an incognito window.

At the very least, I would advocate for a simpler interface: instead of being able to select Publish, Draft, and/or Scheduled, just make it a single toggle (off by default) to "Show draft and scheduled posts" with the same explanation instead. Right now I can't really think of a good use case for not showing published posts since these will always be shown to readers, right?

A question – is checking for edit_others_posts capability a safe-enough check?

If we want to further lock it down we could add a nonce verification, but I think checking the login state plus edit_others_posts is probably enough.

adekbadek · 2022-04-20T08:42:03Z

Thank you for your input, but I think it's based on assumptions about publishing workflows that we cannot make.

Would it be enough and easier to understand if we simply showed all draft, scheduled, and published posts with labels for each non-published status in the block editor?

to make it even simpler, just always show all of the draft, scheduled and published posts to logged-in users

I imagine a WP user might:

Have a ton of insignificant drafts, and a few scheduled posts. They want to see how the block would look like after the scheduled posts are published.
Have a few drafts, which are planned for publication this week, and a couple of posts scheduled further into the future. They want to see the block as it would appear at the end of the week (drafts only).
Not use scheduling and have a ton of insignificant drafts, which they don't ever want to see in the block.

This PR would support all these workflows, while the proposition in the comment would impair all of these, assuming drafts and scheduled posts are always few and desired when previewing.

I'm all for simplification, but this is an non-core feature, and one which which uses established CMS concepts, so I don't think it's too complicated.

Right now I can't really think of a good use case for not showing published posts since these will always be shown to readers, right?

That's right – how about removing the "published" status from the UI, so user can only select what they want to see in addition to the published posts/

dkoo · 2022-04-20T15:47:29Z

Thank you for your input, but I think it's based on assumptions about publishing workflows that we cannot make.

That's true—I think this is a feature which might benefit from some user feedback across sites of various sizes. I can certainly imagine the scenarios you describe but can't really say how many sites will actually encounter them.

That's right – how about removing the "published" status from the UI, so user can only select what they want to see in addition to the published posts

I'm okay with this approach for now if you just want to get the feature out there.

adekbadek · 2022-05-13T12:42:59Z

After an internal discussion:

changed labels to specific posts status instead of generic "preview"
removed "published" option from the checkbox-selection

dkoo

A minor style issue with Post Carousels, otherwise looking 👍

dkoo · 2022-05-13T16:26:23Z

src/blocks/homepage-articles/view.scss

@@ -758,3 +758,17 @@ amp-script .wpnbha.has-more-button.is-error {
 	}
 }
 /* stylelint-enable */
+
+// Also used by the Carousel block.
+.newspack-preview-label {


These styles aren't enqueued if a post only contains a Carousel block and not the Homepage Posts block.

Thanks for catching that! Fixed in b9bf681

dkoo

Looking good now! 👍

# [1.51.0-alpha.1](v1.50.0...v1.51.0-alpha.1) (2022-05-19) ### Features * **homepage-posts:** query by post statuses ([#1114](#1114)) ([7ec20a3](7ec20a3)), closes [#38](#38)

matticbot · 2022-05-19T18:55:56Z

🎉 This PR is included in version 1.51.0-alpha.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

# [1.51.0](v1.50.1...v1.51.0) (2022-05-30) ### Features * **homepage-posts:** query by post statuses ([#1114](#1114)) ([7ec20a3](7ec20a3)), closes [#38](#38)

matticbot · 2022-05-30T18:47:40Z

🎉 This PR is included in version 1.51.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

adekbadek added 4 commits April 19, 2022 11:33

refactor: PostTypesPanel component

bfb6261

chore: remove unused prop

821f501

keepPlaceholderOnFocus is not used, and makes Reader validation throw an error https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/rich-text/README.md

chore: update store registration API use

88d5c45

registerGenericStore is being deprecated

feat(homepage-posts, carousel): enable previewing with non-published …

cd3304c

…posts Closes #38

adekbadek added [Status] Needs Review [Status] Needs Design Review labels Apr 19, 2022

adekbadek requested a review from a team as a code owner April 19, 2022 09:54

adekbadek requested review from thomasguillot and laurelfulford April 19, 2022 09:55

dkoo requested changes Apr 19, 2022

View reviewed changes

adekbadek added 4 commits May 13, 2022 14:29

feat: improvements

b0007d0

Merge master into fix/post-statuses-in-query

e2bc89b

feat: label text

80b3e77

chore: remove comment

a392e6a

adekbadek requested a review from dkoo May 13, 2022 12:43

dkoo requested changes May 13, 2022

View reviewed changes

fix: shared SCSS code

b9bf681

adekbadek requested a review from dkoo May 16, 2022 10:05

dkoo approved these changes May 16, 2022

View reviewed changes

github-actions bot added [Status] Approved and removed [Status] Needs Review labels May 16, 2022

adekbadek merged commit 7ec20a3 into master May 17, 2022

adekbadek deleted the fix/post-statuses-in-query branch May 17, 2022 15:54

matticbot added the released on @alpha label May 19, 2022

matticbot pushed a commit that referenced this pull request May 30, 2022

chore(release): 1.51.0 [skip ci]

aabf857

# [1.51.0](v1.50.1...v1.51.0) (2022-05-30) ### Features * **homepage-posts:** query by post statuses ([#1114](#1114)) ([7ec20a3](7ec20a3)), closes [#38](#38)

matticbot added the released label May 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/post statuses in query #1114

Fix/post statuses in query #1114

adekbadek commented Apr 19, 2022

dkoo left a comment

adekbadek commented Apr 20, 2022

dkoo commented Apr 20, 2022

adekbadek commented May 13, 2022

dkoo left a comment

dkoo May 13, 2022

adekbadek May 16, 2022 •

edited

Loading

dkoo left a comment

matticbot commented May 19, 2022

matticbot commented May 30, 2022

Fix/post statuses in query #1114

Fix/post statuses in query #1114

Conversation

adekbadek commented Apr 19, 2022

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

dkoo left a comment

Choose a reason for hiding this comment

adekbadek commented Apr 20, 2022

dkoo commented Apr 20, 2022

adekbadek commented May 13, 2022

dkoo left a comment

Choose a reason for hiding this comment

dkoo May 13, 2022

Choose a reason for hiding this comment

adekbadek May 16, 2022 • edited Loading

Choose a reason for hiding this comment

dkoo left a comment

Choose a reason for hiding this comment

matticbot commented May 19, 2022

matticbot commented May 30, 2022

adekbadek May 16, 2022 •

edited

Loading