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(ras): do not require Woo plugins if using NRH #3614

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Dec 10, 2024

All Submissions:

Changes proposed in this Pull Request:

Reader Activation features support both Newspack (WooCommerce) and NRH reader revenue platforms, but currently the UI requires WooCommerce plugins to be active even if using NRH. This fixes prerequisites by only requiring Woo plugins if the reader revenue platform is set to "Newspack".

Also fixes a feature that was implemented in #2482 but incorrectly. The message shown for plugins that can't be installed programmatically from a public URL should differ for hosted Newspack sites compared to externally-hosted sites.

How to test the changes in this Pull Request:

  1. Deactivate WooCommerce and WooCommerce Subscriptions plugins.
  2. Set your reader revenue platform to NRH and set an Organization ID string and a Donor Landing Page.
  3. Visit Engagement > Reader Activation. On trunk, observe that you see a notification that the Woo plugins are required instead of the Reader Activation settings UI:
Screenshot 2024-12-10 at 4 26 50 PM
  1. Check out this branch and refresh. Now confirm that you can see the Reader Activation UI:
Screenshot 2024-12-10 at 4 28 23 PM
  1. Set your reader revenue platform to Newspack, then revisit Engagement > Reader Activation (without reactivating the plugins) and confirm that you see the required plugins notification again.
  2. Delete the WooCommerce Subscriptions plugin from your site and refresh. Confirm that the message shown in the plugin notification says Plugin must be installed manually if your site is not a managed Newspack site:
Screenshot 2024-12-10 at 4 57 03 PM
  1. Connect your site to a manager instance and refresh. Confirm that the message now says Contact Newspack support to install:
Screenshot 2024-12-10 at 4 51 40 PM

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 dkoo added [Status] Needs Review The issue or pull request needs to be reviewed News Revenue Hub labels Dec 10, 2024
@dkoo dkoo self-assigned this Dec 10, 2024
@dkoo dkoo requested a review from a team as a code owner December 10, 2024 23:29
Copy link
Contributor

@leogermani leogermani left a comment

Choose a reason for hiding this comment

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

Everything works as described!

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Dec 12, 2024
@leogermani leogermani merged commit 363a834 into trunk Dec 12, 2024
8 checks passed
@leogermani leogermani deleted the fix/reader-revenue-prerequisites branch December 12, 2024 19:30
Copy link

Hey @dkoo, good job getting this PR merged! 🎉

Now, the needs-changelog label has been added to it.

Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label.

If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label.

Thank you! ❤️

matticbot pushed a commit that referenced this pull request Dec 12, 2024
# [5.10.0-alpha.1](v5.9.1...v5.10.0-alpha.1) (2024-12-12)

### Bug Fixes

* duplicate orders save on cron ([#3604](#3604)) ([ec69167](ec69167))
* **ras-acc:** re-add recaptcha to the WooCommerce checkout ([#3605](#3605)) ([f42a75b](f42a75b))
* **ras:** do not require Woo plugins if using NRH ([#3614](#3614)) ([363a834](363a834))
* **wcs:** remove subscriptions expiration feature flag ([#3618](#3618)) ([7c175d9](7c175d9))
* **wcs:** update subscription expiration feature ([#3613](#3613)) ([ebf6e6d](ebf6e6d))
* **wcs:** update subscriptions expiration cli behavior ([#3617](#3617)) ([07e768c](07e768c))

### Features

* **subscriptions:** add cancellation reason metadata ([#3568](#3568)) ([de83e02](de83e02))
* **wc:** duplicate orders admin notice ([#3555](#3555)) ([cb764e3](cb764e3))
* **wcs:** add expired subscription cli tool ([#3593](#3593)) ([5d39398](5d39398))
* **webhooks:** filter request priority ([#3587](#3587)) ([1928a6a](1928a6a))
* **woocommerce-subscriptions:** add url redirect for wc subscription renewals ([#3525](#3525)) ([5b14aeb](5b14aeb))

### Reverts

* Revert "feat: command to initialize cron job to slowly backfill CAP term data (#3425)" (#3620) ([c9a9d45](c9a9d45)), closes [#3425](#3425) [#3620](#3620)
@matticbot
Copy link
Contributor

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

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
Labels
News Revenue Hub released on @alpha [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants