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(ras): sync class #3362

Merged
merged 87 commits into from
Sep 6, 2024
Merged

feat(ras): sync class #3362

merged 87 commits into from
Sep 6, 2024

Conversation

miguelpeixe
Copy link
Member

@miguelpeixe miguelpeixe commented Aug 22, 2024

All Submissions:

Changes proposed in this Pull Request:

Following #3359 and #3360, this PR continues to work on centralizing how we perform reader sync. Sync data is currently handled in a few different places:

The Newspack\Newspack_Newsletters class has been deprecated and its metadata handling moved to Newspack\Reader_Activation\Sync\Metadata. A few notable changes in its behavior:

  • The normalize_contact_data is no longer applied as a filter to newspack_newsletters_contact_data. It's now called inside Newspack\Reader_Activation\ESP_Sync::sync(), right after the newspack_esp_sync_contact filter and before the Newspack_Newsletters_Contacts::upsert() call:

/**
* Filters the contact data before normalizing and syncing to the ESP.
*
* @param array $contact The contact data to sync.
* @param string $context The context of the sync.
*/
$contact = \apply_filters( 'newspack_esp_sync_contact', $contact, $context );
$contact = Sync\Metadata::normalize_contact_data( $contact );
$result = \Newspack_Newsletters_Contacts::upsert( $contact, $master_list_id, $context );
return \is_wp_error( $result ) ? $result : true;

From Newspack\WooCommerce_Connection, the methods should_sync_order(), get_order_metadata(), get_contact_from_customer(), and get_contact_from_order() moved to a new Newspack\Reader_Activation\Sync\WooCommerce class.

The metadata generation on get_order_metadata() and get_contact_from_customer() no longer formats the keys with the prefixes from Metadata. Instead, it uses the raw keys to be later transformed via normalize_contact_data. This is also the case in other places where metadata is populated, like the Newspack\Data_Events\Connectors\ESP_Connector and the Teams for Memberships integration.

How to test the changes in this Pull Request:

All sync methods should be tested. First, repeat the testing steps from #3359 and #3360.

Finally, we should also test the UTM integration:

  1. In a fresh session, visit your site with UTM parameters: ?utm_source=website&utm_medium=homepage&utm_campaign=header
  2. Donate and confirm the sync goes through including the UTM parameters

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?

Comment on lines -368 to -380
// Parse full name into first + last for MC, which stores these as separate merge fields.
if ( 'mailchimp' === \get_option( 'newspack_newsletters_service_provider', false ) ) {
if ( isset( $contact['name'] ) ) {
if ( ! isset( $contact['metadata'] ) ) {
$contact['metadata'] = [];
}
$name_fragments = explode( ' ', $contact['name'], 2 );
$contact['metadata']['First Name'] = $name_fragments[0];
if ( isset( $name_fragments[1] ) ) {
$contact['metadata']['Last Name'] = $name_fragments[1];
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This should now live here: Automattic/newspack-newsletters#1628

Comment on lines -2067 to 2062
/**
* Automatically force deactivate the ESP sync on staging sites to prevent polluting the data in ESPs.
*
* @param mixed $value Setting value.
* @param string $setting Setting name.
* @return mixed Possibly modified $value.
*/
public static function disable_esp_sync_on_staging_sites( $value, $setting ) {
if ( 'sync_esp' !== $setting ) {
return $value;
}

if ( defined( 'NEWSPACK_FORCE_ALLOW_ESP_SYNC' ) && NEWSPACK_FORCE_ALLOW_ESP_SYNC ) {
return $value;
}

$site_url = strtolower( \untrailingslashit( \get_site_url() ) );
if ( false !== stripos( $site_url, '.newspackstaging.com' ) ) {
return false;
}

// Neither WCS_Staging::is_duplicate_site() nor is_plugin_active() are initialized early enough for all situations.
// So we need to re-create the logic from both.
if ( in_array( 'woocommerce-subscriptions/woocommerce-subscriptions.php', (array) \get_option( 'active_plugins', [] ), true ) ) {
$subscriptions_site_url = \get_option( 'wc_subscriptions_siteurl', false );
if ( $subscriptions_site_url ) {
$cleaned_subscriptions_site_url = strtolower( untrailingslashit( str_ireplace( '_[wc_subscriptions_siteurl]_', '', $subscriptions_site_url ) ) );
if ( $cleaned_subscriptions_site_url !== $site_url ) {
return false;
}
}
}

return $value;
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing this as we should always rely on Newspack\Reader_Activation\ESP_Sync::can_esp_sync().

Comment on lines -68 to +88
'NP_Payment Page' => $payment_page_url,
'NP_Membership Status' => 'customer',
'NP_Product Name' => '',
'NP_Last Payment Amount' => '$' . $order_data['total'],
'NP_Last Payment Date' => $today,
'NP_Payment UTM: source' => 'test_source',
'NP_Payment UTM: medium' => '',
'NP_Payment UTM: campaign' => 'test_campaign',
'NP_Payment UTM: term' => 'test_term',
'NP_Payment UTM: content' => 'test_content',
'NP_Current Subscription Start Date' => '',
'NP_Current Subscription End Date' => '',
'NP_Billing Cycle' => '',
'NP_Recurring Payment' => '',
'NP_Next Payment Date' => '',
'NP_Total Paid' => '$' . ( self::USER_DATA['meta_input']['wc_total_spent'] + $order_data['total'] ),
'NP_Account' => self::$user_id,
'NP_Registration Date' => $today,
'NP_Membership Plan' => '',
'NP_Current Membership Start Date' => '',
'NP_Current Membership End Date' => '',
'payment_page' => $payment_page_url,
'membership_status' => 'customer',
'product_name' => '',
'last_payment_amount' => '$' . $order_data['total'],
'last_payment_date' => $today,
'payment_page_utm_source' => 'test_source',
'payment_page_utm_medium' => '',
'payment_page_utm_campaign' => 'test_campaign',
'payment_page_utm_term' => 'test_term',
'payment_page_utm_content' => 'test_content',
'sub_start_date' => '',
'sub_end_date' => '',
'billing_cycle' => '',
'recurring_payment' => '',
'next_payment_date' => '',
'total_paid' => '$' . ( self::USER_DATA['meta_input']['wc_total_spent'] + $order_data['total'] ),
'account' => self::$user_id,
'registration_date' => $today,
'membership_plan' => '',
'membership_start_date' => '',
'membership_end_date' => '',
Copy link
Member Author

Choose a reason for hiding this comment

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

We are now sending raw keys to be later transformed before syncing.

@miguelpeixe miguelpeixe self-assigned this Sep 4, 2024
@miguelpeixe miguelpeixe added the [Status] Needs Review The issue or pull request needs to be reviewed label Sep 4, 2024
@miguelpeixe miguelpeixe marked this pull request as ready for review September 4, 2024 18:27
@miguelpeixe miguelpeixe requested a review from a team as a code owner September 4, 2024 18:27
Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

@miguelpeixe I love the consolidation and reorg of all of these various sync-related features. I don't have any blocking feedback, just a couple of minor nits. I'll let @leogermani handle the follow-up since he reviewed first.

includes/reader-activation/sync/class-metadata.php Outdated Show resolved Hide resolved
@miguelpeixe
Copy link
Member Author

@dkoo @leogermani is there anything left to revise here?

@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 Sep 6, 2024
@miguelpeixe miguelpeixe merged commit 88acbee into trunk Sep 6, 2024
16 checks passed
@miguelpeixe miguelpeixe deleted the feat/ras-sync-class branch September 6, 2024 14:09
matticbot pushed a commit that referenced this pull request Sep 20, 2024
# [5.4.0-alpha.6](v5.4.0-alpha.5...v5.4.0-alpha.6) (2024-09-20)

### Bug Fixes

* change the current product criteria for sync ([#3416](#3416)) ([28a84bc](28a84bc))
* **esp-sync:** sync Connected Account field ([#3414](#3414)) ([61c02bc](61c02bc))
* hide My Account links if not relevant ([#3394](#3394)) ([0d039d8](0d039d8))
* **phpcs:** specify path in custom ruleset ref ([#3384](#3384)) ([b143e74](b143e74))
* prevent PHP notice while checking my-account page ([#3435](#3435)) ([146a26f](146a26f))
* **ras-sync:** deprecate redundant Signup_Page meta field ([#3439](#3439)) ([61d6de8](61d6de8))
* **reader-registration-block:** fix initial newsletter checkbox state ([1890efe](1890efe))
* replace `newspack_image_credits_placeholder` default value ([#3433](#3433)) ([c754fc2](c754fc2))
* **sync:** method name for `membership_saved` handler ([#3399](#3399)) ([2c0bf26](2c0bf26))
* **sync:** place esp sync admin features behind a constant ([#3438](#3438)) ([20a0970](20a0970))
* **sync:** remove localized number format ([#3434](#3434)) ([2243a5d](2243a5d))

### Features

* add a new action to when a ras setting is updated ([#3357](#3357)) ([35d3492](35d3492))
* **ga4:** detect gate interaction blocks ([#3408](#3408)) ([e14913c](e14913c))
* media kit page handling ([#3358](#3358)) ([4454850](4454850))
* **ras:** sync class ([#3362](#3362)) ([88acbee](88acbee))
* **ras:** unify ESP connector strategy for data events ([#3360](#3360)) ([7080864](7080864))
* **reader-activation:** ESP-related tweaks ([#3381](#3381)) ([ac68b67](ac68b67))
* remove Woo Membersip sync fields ([#3411](#3411)) ([28052e8](28052e8))
* **sync:** add ESP sync notice to RAS wizard ([#3400](#3400)) ([f9acd56](f9acd56))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 5.4.0-alpha.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Oct 3, 2024
# [5.5.0-alpha.1](v5.4.0...v5.5.0-alpha.1) (2024-10-03)

### Bug Fixes

* change the current product criteria for sync ([#3416](#3416)) ([28a84bc](28a84bc))
* **esp-meta:** handle state of the 'Woo Team' meta ([#3352](#3352)) ([ba5ea1e](ba5ea1e))
* **esp-sync:** sync Connected Account field ([#3414](#3414)) ([61c02bc](61c02bc))
* **esp-wc-metadata:** last payment date & amount handling ([#3363](#3363)) ([d1abbfe](d1abbfe))
* **guest-author:** enqueue the guest author admin script selectively ([0bf37af](0bf37af))
* hide My Account links if not relevant ([#3394](#3394)) ([0d039d8](0d039d8))
* make email template fetching deterministic ([#3341](#3341)) ([ace91aa](ace91aa))
* **phpcs:** specify path in custom ruleset ref ([#3384](#3384)) ([b143e74](b143e74))
* prevent PHP notice while checking my-account page ([#3435](#3435)) ([146a26f](146a26f))
* **ras-sync:** deprecate redundant Signup_Page meta field ([#3439](#3439)) ([61d6de8](61d6de8))
* **reader-registration-block:** fix initial newsletter checkbox state ([1890efe](1890efe))
* replace `newspack_image_credits_placeholder` default value ([#3433](#3433)) ([c754fc2](c754fc2))
* **sync:** method name for `membership_saved` handler ([#3399](#3399)) ([2c0bf26](2c0bf26))
* **sync:** place esp sync admin features behind a constant ([#3438](#3438)) ([20a0970](20a0970))
* **sync:** remove localized number format ([#3434](#3434)) ([2243a5d](2243a5d))
* wizards - update type check conditional for `custom_logo` ([#3442](#3442)) ([125f756](125f756))
* woocommerce connection tests ([#3372](#3372)) ([5cea128](5cea128))
* **woocommerce-connection:** handle explicit UTM meta fields meta ([#3371](#3371)) ([bf9e997](bf9e997))

### Features

* add a new action to when a ras setting is updated ([#3357](#3357)) ([35d3492](35d3492))
* **esp-sync:** sync membership data regardless of subscription ([#3353](#3353)) ([9f7d1de](9f7d1de))
* **ga4:** detect gate interaction blocks ([#3408](#3408)) ([e14913c](e14913c))
* **ga:** disable tracking for editors regardless of RA status ([81323c3](81323c3))
* media kit page handling ([#3358](#3358)) ([4454850](4454850))
* **memberships:** add memberships-related body classes ([b56b9d8](b56b9d8))
* **ras:** esp sync tools ([#3359](#3359)) ([d7dd754](d7dd754))
* **ras:** helper method for ESP master list ([#3355](#3355)) ([ec56d5b](ec56d5b))
* **ras:** sync class ([#3362](#3362)) ([88acbee](88acbee))
* **ras:** unify ESP connector strategy for data events ([#3360](#3360)) ([7080864](7080864))
* **reader-activation:** ESP-related tweaks ([#3381](#3381)) ([ac68b67](ac68b67))
* remove Woo Membersip sync fields ([#3411](#3411)) ([28052e8](28052e8))
* **sync:** add ESP sync notice to RAS wizard ([#3400](#3400)) ([f9acd56](f9acd56))
@matticbot
Copy link
Contributor

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Oct 8, 2024
# [5.5.0](v5.4.1...v5.5.0) (2024-10-08)

### Bug Fixes

* cancelled subscriptions sync ([#3466](#3466)) ([b605a7f](b605a7f))
* change the current product criteria for sync ([#3416](#3416)) ([28a84bc](28a84bc))
* **esp-meta:** handle state of the 'Woo Team' meta ([#3352](#3352)) ([ba5ea1e](ba5ea1e))
* **esp-sync:** sync Connected Account field ([#3414](#3414)) ([61c02bc](61c02bc))
* **esp-wc-metadata:** last payment date & amount handling ([#3363](#3363)) ([d1abbfe](d1abbfe))
* **guest-author:** enqueue the guest author admin script selectively ([0bf37af](0bf37af))
* hide My Account links if not relevant ([#3394](#3394)) ([0d039d8](0d039d8))
* make email template fetching deterministic ([#3341](#3341)) ([ace91aa](ace91aa))
* **phpcs:** specify path in custom ruleset ref ([#3384](#3384)) ([b143e74](b143e74))
* prevent PHP notice while checking my-account page ([#3435](#3435)) ([146a26f](146a26f))
* **ras-sync:** deprecate redundant Signup_Page meta field ([#3439](#3439)) ([61d6de8](61d6de8))
* **reader-registration-block:** fix initial newsletter checkbox state ([1890efe](1890efe))
* replace `newspack_image_credits_placeholder` default value ([#3433](#3433)) ([c754fc2](c754fc2))
* **sync:** method name for `membership_saved` handler ([#3399](#3399)) ([2c0bf26](2c0bf26))
* **sync:** place esp sync admin features behind a constant ([#3438](#3438)) ([20a0970](20a0970))
* **sync:** remove localized number format ([#3434](#3434)) ([2243a5d](2243a5d))
* wizards - update type check conditional for `custom_logo` ([#3442](#3442)) ([125f756](125f756))
* woocommerce connection tests ([#3372](#3372)) ([5cea128](5cea128))
* **woocommerce-connection:** handle explicit UTM meta fields meta ([#3371](#3371)) ([bf9e997](bf9e997))

### Features

* add a new action to when a ras setting is updated ([#3357](#3357)) ([35d3492](35d3492))
* **esp-sync:** sync membership data regardless of subscription ([#3353](#3353)) ([9f7d1de](9f7d1de))
* **ga4:** detect gate interaction blocks ([#3408](#3408)) ([e14913c](e14913c))
* **ga:** disable tracking for editors regardless of RA status ([81323c3](81323c3))
* media kit page handling ([#3358](#3358)) ([4454850](4454850))
* **memberships:** add memberships-related body classes ([b56b9d8](b56b9d8))
* **ras:** esp sync tools ([#3359](#3359)) ([d7dd754](d7dd754))
* **ras:** helper method for ESP master list ([#3355](#3355)) ([ec56d5b](ec56d5b))
* **ras:** sync class ([#3362](#3362)) ([88acbee](88acbee))
* **ras:** unify ESP connector strategy for data events ([#3360](#3360)) ([7080864](7080864))
* **reader-activation:** ESP-related tweaks ([#3381](#3381)) ([ac68b67](ac68b67))
* remove Woo Membersip sync fields ([#3411](#3411)) ([28052e8](28052e8))
* **sync:** add ESP sync notice to RAS wizard ([#3400](#3400)) ([f9acd56](f9acd56))
@adekbadek adekbadek mentioned this pull request Oct 14, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants