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

feat: enable public newsletter posts #272

Merged
merged 20 commits into from
Aug 18, 2020
Merged

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Jul 20, 2020

All Submissions:

Changes proposed in this Pull Request:

Allows editors to mark sent/published newsletters as publicly viewable and creates an archive page for newsletter posts. If a published newsletter is publicly viewable, its single page is visible to all users. If not, it's visible as a preview only to logged-in WP admin users (drafts are also visible to logged-in admin users). Also hijacks the main query on the Newsletter archive pages and filters out any posts that are NOT marked as publicly viewable.

Further work required?

  • Posts Inserter block isn't rendering anything on the front-end—need to fix that before we merge fixed in 91b596c

  • Currently, newsletter ads are not rendered in the page view. I'm not sure this is necessary, but feel free to change my mind.

  • There might be some further design/front-end work needed on the theme side if we want the archive and single templates for newsletter posts to differ from regular articles (there is probably no need for a byline, or the sidebar widgets, for example). (cc @thomasguillot and/or @laurelfulford)

  • Maybe disable some interactions with other plugins—e.g. Jetpack Related Posts (this example done in this PR)

Closes #171.
Closes #214.

How to test the changes in this Pull Request:

  1. Check out/build this branch, and flush permalinks. Go to Newsletters > All Newsletters in WP admin.

  2. For any newsletters that have been sent, observe a new "Preview" link when hovering over the item in the post list:

Screen Shot 2020-08-12 at 10 05 55 AM

  1. Click the Preview link. Observe that the newsletter is viewable as a single post page while logged into WP admin.

Screen Shot 2020-08-12 at 10 54 14 AM

  1. Open a new Incognito window and attempt to visit the same newsletter post. Observe that the page is a 404 while not logged in.

Screen Shot 2020-08-12 at 10 57 58 AM

  1. Go to https://[YOUR SITE URL]/newsletter/ to view the newsletter archive page. Right now it should be empty while both logged in and not logged in (only posts explicitly marked as publicly viewable should appear on the archive page).

  2. Edit the newsletter post. Observe a new toggle option in the Document settings sidebar, disabled by default:

Screen Shot 2020-08-12 at 10 07 43 AM

  1. Enable the setting. Observe that the previously disabled "Sent" button is now enabled and says "Update." Click "Update" to save the new setting.

  2. Go back to the "All Newsletters" post list. Hover over the post and observe that there's now a "View" link instead of "Preview." Also observe that the "Sent" notice should now have an additional "Published as a page" note attached.

Screen Shot 2020-08-12 at 11 13 08 AM

  1. Refresh the archive page—now observe that your public post is listed while both logged in and not logged in.

  2. Visit the newsletter post's single page. Observe that the public post is now visible while both logged in and not logged in.

  3. Set up at least one campaign in the Newspack Campaigns plugin (can be either overlay or inline). Observe that the campaign will never render in the newsletter post's single page under any circumstances.

  4. Edit the newsletter post again and disable the "Make newsletter page public?" setting, then save. Observe that the page is no longer visible in the archive or as a single page to non-logged-in users.

  5. As a sanity check, create a new newsletter but DON'T send it. DO enable the "Make newsletter page public?" setting.

  • Observe that the "Send" button says "Send and Publish" when the setting is enabled.
  • Click "Save Draft," then confirm that the draft is visible to logged-in users only.
  1. As another sanity check, ensure that sending newsletters with both Mailchimp and Constant Contact still works as before.

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?

dkoo added 3 commits July 20, 2020 17:20
Adds a meta option to newsletter posts that allows specific newsletters to be publicly viewable on the site front-end. Currently uses the standard post template.
@dkoo dkoo mentioned this pull request Jul 20, 2020
@dkoo dkoo requested review from adekbadek and jeffersonrabb and removed request for adekbadek August 12, 2020 16:13
dkoo added 3 commits August 12, 2020 10:51
Previously, if a non-logged-in user tried to view a non-public newsletter post, the page would display as a 404 but the document title would still match the post title. This fixes that.
Moves the title filter above the template set, otherwise it won't work.
@dkoo dkoo marked this pull request as ready for review August 12, 2020 16:59
@dkoo dkoo changed the title Allow public newsletter posts feat: enable public newsletter posts Aug 12, 2020
dkoo added 2 commits August 12, 2020 11:30
Adds a check for publish status in the Constant Contact class before sending the newsletter, to match Mailchimp (so that we don't try to send a newsletter again if it's been published already and we're just updating the post). Also restores an error message in the Mailchimp class that got lost.
@dkoo
Copy link
Contributor Author

dkoo commented Aug 12, 2020

@adekbadek I just realized that the Post Inserter block isn't rendering anything on the front-end. Before I go down the rabbit hole of trying to make it work, is there something off the top of your head we can change to fix this?

@dkoo dkoo changed the title feat: enable public newsletter posts [WIP] feat: enable public newsletter posts Aug 12, 2020
@adekbadek
Copy link
Member

adekbadek commented Aug 13, 2020

I just realized that the Post Inserter block isn't rendering anything on the front-end. Before I go down the rabbit hole of trying to make it work, is there something off the top of your head we can change to fix this?

@dkoo, it can be rendered as in the email renderer. The block saves resulting markup in a innerBlocksToInsert attribute. Other solution would be to duplicate the block rendering logic in PHP, as most blocks out there do.

dkoo added 2 commits August 13, 2020 12:23
Adds a server-side render callback for the Posts Inserter so that it can show content on the front-end. Also updates the checkbox for the "disable ads" attribute to be a ToggleControl, to match the other toggleable UI elements from the plugin.
@dkoo
Copy link
Contributor Author

dkoo commented Aug 13, 2020

@dkoo, it can be rendered as in the email renderer. The block saves resulting markup in a innerBlocksToInsert attribute. Other solution would be to duplicate the block rendering logic in PHP, as most blocks out there do.

Thanks for the hint! I ended up using that attribute to just render the inner blocks via server-side callback. Added in 91b596c.

@dkoo dkoo changed the title [WIP] feat: enable public newsletter posts feat: enable public newsletter posts Aug 13, 2020
dkoo added 2 commits August 14, 2020 10:25
- use lockPostAutosaving instead of dequeueing autosave script to disable autosave
- change language of "published as a page" to "published as a post"
@dkoo
Copy link
Contributor Author

dkoo commented Aug 14, 2020

The post publish button is set to "Update" after being briefly set to "Send". On master, this button is set to "Send" and stays that way. It's content does not change after marking the Newsletter as public in the sidebar.

Addressed this in c15f2e6. The problem was that I was relying on WP core's isEditedPostDirty selector, which supposedly checks whether the editor is in a dirty state. But because we're setting some meta based on async ops from the service providers, isEditedPostDirty always returns true. Now, I only check whether the is_public meta value has changed from the post's saved state before changing the button state.

This means that if you edit other post attributes or post content, the "Update" button won't be enabled, but it's still possible to make those edits, change the "public" setting, and then save all changes. Kind of an edge case since I don't see much of a use case for editing newsletter content after it has been sent, but I could also add a comparison of post content vs. edited post content if you think that's warranted.

@dkoo dkoo requested a review from adekbadek August 14, 2020 18:16
@adekbadek
Copy link
Member

This means that if you edit other post attributes or post content [of a sent newsletter], the "Update" button won't be enabled […] Kind of an edge case

I agree it's an edge case, I'm fine with it staying that way

@github-actions github-actions bot added the [Status] Approved Ready to merge label Aug 17, 2020
Copy link
Contributor

@thomasguillot thomasguillot left a comment

Choose a reason for hiding this comment

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

It works fairly well but I noticed a bug when I change the public status of a newsletter where the newsletter single view is still available even when setting has been changed to don't publish.

Kapture 2020-08-17 at 14 41 30

@dkoo
Copy link
Contributor Author

dkoo commented Aug 17, 2020

I noticed a bug when I change the public status of a newsletter where the newsletter single view is still available even when setting has been changed to don't publish.

@thomasguillot, are you logged in as an admin user? If that's the case then newsletter posts will always be visible as a preview. Try viewing the same post in a new incognito window and see if the page still renders?

Adds a checkbox option that lets editors decide whether Jetpack Related Posts should be enabled on single newsletter posts. Only shown if Related Posts module is active.
@thomasguillot
Copy link
Contributor

I noticed a bug when I change the public status of a newsletter where the newsletter single view is still available even when setting has been changed to don't publish.

@thomasguillot, are you logged in as an admin user? If that's the case then newsletter posts will always be visible as a preview. Try viewing the same post in a new incognito window and see if the page still renders?

Oh ok yeah it work as expected then. But why will it always be visible as a preview if I specifically un-toggled the option?

@dkoo
Copy link
Contributor Author

dkoo commented Aug 17, 2020

But why will it always be visible as a preview if I specifically un-toggled the option?

It's the only way I could think of that would allow editors to preview a single newsletter post without making it public. It more or less works the same way as a draft post, but since the newsletter is already "published" (a.k.a. sent) regardless of whether the post is marked as public, we can't use the WP publish status in the same way as we would use it in a regular post.

Copy link
Contributor

@thomasguillot thomasguillot left a comment

Choose a reason for hiding this comment

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

Thanks @dkoo -- makes sense 😊

@dkoo dkoo merged commit ef90bf9 into master Aug 18, 2020
@dkoo dkoo deleted the add/allow-public-newsletter-posts branch August 18, 2020 15:26
matticbot pushed a commit that referenced this pull request Aug 20, 2020
# [1.9.0-alpha.1](v1.8.1...v1.9.0-alpha.1) (2020-08-20)

### Bug Fixes

* button custom colors ([#301](#301)) ([8ebcf5d](8ebcf5d))
* first time set up flow ([c16d6bf](c16d6bf))
* pre-send info if no list set ([#297](#297)) ([f575d94](f575d94))

### Features

* enable public newsletter posts ([#272](#272)) ([ef90bf9](ef90bf9))
@matticbot
Copy link
Contributor

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Aug 20, 2020
# [1.9.0](v1.8.1...v1.9.0) (2020-08-20)

### Bug Fixes

* button custom colors ([#301](#301)) ([8ebcf5d](8ebcf5d))
* first time set up flow ([c16d6bf](c16d6bf))
* pre-send info if no list set ([#297](#297)) ([f575d94](f575d94))

### Features

* enable public newsletter posts ([#272](#272)) ([ef90bf9](ef90bf9))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.9.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Archive page for newsletters Convert Newsletter To Post
4 participants