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(nrh): fixes for ESP contact syncing when Reader Revenue platform is NRH #3779

Open
wants to merge 6 commits into
base: release
Choose a base branch
from

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Feb 24, 2025

All Submissions:

Changes proposed in this Pull Request:

It's possible to enable Reader Activation contact syncing while also using NRH instead of WooCommerce as a reader revenue platform, but this is currently broken.

Along with Automattic/newspack-newsletters#1763, handles ESP contact syncing when using NRH as a reader revenue platform, and/or WooCommerce Subscriptions is not available. WooCommerce itself is still a soft requirement for ESP syncing when using NRH, as we rely on its WC_Customer class for fetching basic user meta fields.

This is an alternative to #1648 and brings back contact data filtering as proposed in #3415. I don't think we can get around using this filter, because certain reader registration and newsletters signup flows bypass the ESP sync classes (e.g. Newsletter Subscription Form block, Registration block).

Closes 1200550061930446/1209469667970736.

How to test the changes in this Pull Request:

  1. On a site with Reader Activation syncing active and connected to an ESP, set your reader revenue platform to NRH and deactivate the WooCommerce Subscriptions plugin.
  2. On release, as a reader, sign up for newsletters via the Newsletter Subscription Form and Reader Registration blocks on a page that contains UTM parameters in the URL (e.g. ?utm_medium=test&utm_campaign=NRH&utm_content=NRH%20content. Observe that the contact sync to the connected ESP is logged, but without most of the RAS contact metadata fields. Also observe a fatal error in PHP: Fatal error: Uncaught Error: Call to undefined function wcs_get_users_subscriptions()
  3. Check out this branch and fix(subscribe): optionally include metadata on subscribe newspack-newsletters#1763. As an admin, navigate to Newspack > Reader Activation > Advanced Settings > Metadata field settings. Confirm that only basic metadata fields are available (as payment fields assume that the reader revenue platform is WooCommerce):
Screenshot 2025-02-24 at 12 16 23 PM
  1. Repeat signing up for newsletters with UTM params as in step 2, but this time confirm that all metadata fields are synced to the ESP without the fatal error.
  2. Follow testing instructions for fix: dont send metadata on subscribe newspack-newsletters#1648 and ensure that GA4 data is unaffected.

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 the [Status] Needs Review The issue or pull request needs to be reviewed label Feb 24, 2025
@dkoo dkoo self-assigned this Feb 24, 2025
@dkoo dkoo requested a review from a team as a code owner February 24, 2025 19:52
Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

Works accordingly to the testing instructions, but there's an ongoing discussion on the other PR (Automattic/newspack-newsletters#1763 (review)), which should be resolved before this is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Needs Review The issue or pull request needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants