Skip to content

Commit

Permalink
Merge pull request #1574 from Automattic/add/logging
Browse files Browse the repository at this point in the history
Add/logging
  • Loading branch information
leogermani authored Aug 2, 2024
2 parents 5d36537 + 234868d commit ec0cbe0
Show file tree
Hide file tree
Showing 9 changed files with 275 additions and 39 deletions.
104 changes: 90 additions & 14 deletions includes/class-newspack-newsletters-contacts.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,11 @@ class Newspack_Newsletters_Contacts {
* }
* @param string[]|false $lists Array of list IDs to subscribe the contact to. If empty or false, contact will be created but not subscribed to any lists.
* @param bool $async Whether to add the contact asynchronously. Default is false.
* @param string $context Context of the update for logging purposes.
*
* @return array|WP_Error|true Contact data if it was added, or error otherwise. True if async.
*/
public static function upsert( $contact, $lists = false, $async = false ) {
public static function upsert( $contact, $lists = false, $async = false, $context = 'Unknown' ) {
if ( ! is_array( $lists ) && false !== $lists ) {
$lists = [ $lists ];
}
Expand All @@ -59,7 +60,7 @@ public static function upsert( $contact, $lists = false, $async = false ) {
}

if ( defined( 'NEWSPACK_NEWSLETTERS_ASYNC_SUBSCRIPTION_ENABLED' ) && NEWSPACK_NEWSLETTERS_ASYNC_SUBSCRIPTION_ENABLED && true === $async ) {
Newspack_Newsletters_Subscription::add_subscription_intent( $contact, $lists );
Newspack_Newsletters_Subscription::add_subscription_intent( $contact, $lists, $context );
return true;
}

Expand Down Expand Up @@ -113,17 +114,18 @@ public static function upsert( $contact, $lists = false, $async = false ) {
*/
$lists = apply_filters( 'newspack_newsletters_contact_lists', $lists, $contact, $provider->service );

return self::add_to_esp( $contact, $lists, $is_updating );
return self::add_to_esp( $contact, $lists, $is_updating, $context );
}

/**
* Permanently delete a user subscription.
*
* @param int $user_id User ID.
* @param int $user_id User ID.
* @param string $context Context of the update for logging purposes.
*
* @return bool|WP_Error Whether the contact was deleted or error.
*/
public static function delete( $user_id ) {
public static function delete( $user_id, $context = 'Unknown' ) {
$user = get_user_by( 'id', $user_id );
if ( ! $user ) {
return new WP_Error( 'newspack_newsletters_invalid_user', __( 'Invalid user.' ) );
Expand All @@ -139,7 +141,24 @@ public static function delete( $user_id ) {
if ( ! method_exists( $provider, 'delete_contact' ) ) {
return new WP_Error( 'newspack_newsletters_invalid_provider_method', __( 'Provider does not support deleting user subscriptions.' ) );
}
return $provider->delete_contact( $user->user_email );
$result = $provider->delete_contact( $user->user_email );

do_action(
'newspack_log',
'newspack_esp_sync_delete_contact',
$context,
[
'type' => is_wp_error( $result ) ? 'error' : 'debug',
'data' => [
'provider' => $provider->service,
'errors' => is_wp_error( $result ) ? $result->get_error_message() : [],
],
'user_email' => $user->user_email,
'file' => 'newspack_esp_sync',
]
);

return $result;
}

/**
Expand All @@ -150,10 +169,11 @@ public static function delete( $user_id ) {
*
* @param string $email Contact email address.
* @param string[] $lists Array of list IDs to subscribe the contact to.
* @param string $context Context of the update for logging purposes.
*
* @return bool|WP_Error Whether the contact was updated or error.
*/
public static function update_lists( $email, $lists = [] ) {
public static function update_lists( $email, $lists = [], $context = 'Unknown' ) {
if ( ! Newspack_Newsletters_Subscription::has_subscription_management() ) {
return new WP_Error( 'newspack_newsletters_not_supported', __( 'Not supported for this provider', 'newspack-newsletters' ) );
}
Expand All @@ -175,7 +195,26 @@ public static function update_lists( $email, $lists = [] ) {
return false;
}

$result = $provider->update_contact_lists_handling_local( $email, $lists_to_add, $lists_to_remove );
return self::add_and_remove_lists( $email, $lists_to_add, $lists_to_remove, $context );
}

/**
* Add and remove a contact from lists.
*
* @param string $email Contact email address.
* @param string[] $lists_to_add Array of list IDs to subscribe the contact to.
* @param string[] $lists_to_remove Array of list IDs to remove the contact from.
* @param string $context Context of the update for logging purposes.
*
* @return bool|WP_Error Whether the contact was updated or error.
*/
public static function add_and_remove_lists( $email, $lists_to_add = [], $lists_to_remove = [], $context = 'Unknown' ) {
if ( ! Newspack_Newsletters_Subscription::has_subscription_management() ) {
return new WP_Error( 'newspack_newsletters_not_supported', __( 'Not supported for this provider', 'newspack-newsletters' ) );
}
$provider = Newspack_Newsletters::get_service_provider();

$result = $provider->update_contact_lists_handling_local( $email, $lists_to_add, $lists_to_remove, $context );

/**
* Fires after a contact's lists are updated.
Expand All @@ -185,8 +224,26 @@ public static function update_lists( $email, $lists = [] ) {
* @param string[] $lists_to_add Array of list IDs to subscribe the contact to.
* @param string[] $lists_to_remove Array of list IDs to remove the contact from.
* @param bool|WP_Error $result True if the contact was updated or error if failed.
* @param string $context Context of the update for logging purposes.
*/
do_action( 'newspack_newsletters_update_contact_lists', $provider->service, $email, $lists_to_add, $lists_to_remove, $result );
do_action( 'newspack_newsletters_update_contact_lists', $provider->service, $email, $lists_to_add, $lists_to_remove, $result, $context );

do_action(
'newspack_log',
'newspack_esp_sync_update_lists',
$context,
[
'type' => is_wp_error( $result ) ? 'error' : 'debug',
'data' => [
'provider' => $provider->service,
'lists_to_add' => $lists_to_add,
'lists_to_remove' => $lists_to_remove,
'errors' => is_wp_error( $result ) ? $result->get_error_messages() : [],
],
'user_email' => $email,
'file' => 'newspack_esp_sync',
]
);

return $result;
}
Expand All @@ -195,13 +252,14 @@ public static function update_lists( $email, $lists = [] ) {
* Internal method to add a contact to lists. Should be called by the
* `add_contact` method or `handle_async_subscribe` for the async strategy.
*
* @param array $contact Contact information.
* @param array $lists Array of list IDs to subscribe the contact to.
* @param bool $is_updating Whether the contact is being updated. If false, the contact is being created.
* @param array $contact Contact information.
* @param array $lists Array of list IDs to subscribe the contact to.
* @param bool $is_updating Whether the contact is being updated. If false, the contact is being created.
* @param string $context Context of the update for logging purposes.
*
* @return array|WP_Error Contact data if it was added, or error otherwise.
*/
private static function add_to_esp( $contact, $lists = [], $is_updating = false ) {
private static function add_to_esp( $contact, $lists = [], $is_updating = false, $context = 'Unknown' ) {
$provider = Newspack_Newsletters::get_service_provider();
$errors = new WP_Error();
$result = [];
Expand Down Expand Up @@ -247,8 +305,26 @@ private static function add_to_esp( $contact, $lists = [], $is_updating = false
* @param string[]|false $lists Array of list IDs to subscribe the contact to.
* @param array|WP_Error $result Array with data if the contact was added or error if failed.
* @param bool $is_updating Whether the contact is being updated. If false, the contact is being created.
* @param string $context Context of the update for logging purposes.
*/
do_action( 'newspack_newsletters_add_contact', $provider->service, $contact, $lists, $result, $is_updating );
do_action( 'newspack_newsletters_add_contact', $provider->service, $contact, $lists, $result, $is_updating, $context );

do_action(
'newspack_log',
'newspack_esp_sync_upsert_contact',
$context,
[
'type' => $errors->has_errors() ? 'error' : 'debug',
'data' => [
'provider' => $provider->service,
'lists' => $lists,
'contact' => $contact,
'errors' => $errors->get_error_messages(),
],
'user_email' => $contact['email'],
'file' => 'newspack_esp_sync',
]
);

if ( $errors->has_errors() ) {
return $errors;
Expand Down
23 changes: 14 additions & 9 deletions includes/class-newspack-newsletters-subscription.php
Original file line number Diff line number Diff line change
Expand Up @@ -357,18 +357,19 @@ public static function get_contact_data( $email_address, $return_details = false
*/
public static function add_contact( $contact, $lists = false, $async = false ) {
_deprecated_function( __METHOD__, '2.21', 'Newspack_Newsletters_Contacts::upsert' );
return Newspack_Newsletters_Contacts::upsert( $contact, $lists, $async );
return Newspack_Newsletters_Contacts::upsert( $contact, $lists, $async, 'deprecated' );
}

/**
* Register a subscription intent and dispatches a async request to process it.
*
* @param array $contact Contact information.
* @param array $lists Array of list IDs to subscribe the contact to.
* @param array $contact Contact information.
* @param array $lists Array of list IDs to subscribe the contact to.
* @param string $context Context of the update for logging purposes.
*
* @return int|WP_Error Subscription intent ID or error.
*/
public static function add_subscription_intent( $contact, $lists ) {
public static function add_subscription_intent( $contact, $lists, $context = '' ) {
$intent_id = \wp_insert_post(
[
'post_type' => self::SUBSCRIPTION_INTENT_CPT,
Expand All @@ -377,6 +378,7 @@ public static function add_subscription_intent( $contact, $lists ) {
'contact' => $contact,
'lists' => $lists,
'errors' => [],
'context' => $context,
],
]
);
Expand Down Expand Up @@ -422,6 +424,7 @@ private static function get_subscription_intent( $intent_id_or_post ) {
'contact' => get_post_meta( $intent->ID, 'contact', true ),
'lists' => get_post_meta( $intent->ID, 'lists', true ),
'errors' => get_post_meta( $intent->ID, 'errors', true ),
'context' => get_post_meta( $intent->ID, 'context', true ),
];
}

Expand Down Expand Up @@ -473,7 +476,8 @@ public static function process_subscription_intents( $intent_id = null ) {
$contact = $intent['contact'];
$email = $contact['email'];
$lists = $intent['lists'];
$result = Newspack_Newsletters_Contacts::upsert( $contact, $lists, false );
$context = $intent['context'];
$result = Newspack_Newsletters_Contacts::upsert( $contact, $lists, false, $context . ' (ASYNC)' );

$user = get_user_by( 'email', $email );
if ( \is_wp_error( $result ) ) {
Expand Down Expand Up @@ -576,13 +580,14 @@ public static function newspack_registered_reader( $email, $authenticate, $user_

// Adding is actually upserting, so no need to check if the hook is called for an existing user.
try {
self::add_contact(
Newspack_Newsletters_Contacts::upsert(
[
'email' => $email,
'metadata' => $metadata,
],
$lists,
true // Async.
true, // Async.
'Reader registration hook on Newsletters plugin'
);
} catch ( \Exception $e ) {
// Avoid breaking the registration process.
Expand All @@ -603,7 +608,7 @@ public static function newspack_registered_reader( $email, $authenticate, $user_
*/
private static function update_contact_lists( $email, $lists = [] ) {
_deprecated_function( __METHOD__, '2.21', 'Newspack_Newsletters_Contacts::update_lists' );
return Newspack_Newsletters_Contacts::update_lists( $email, $lists );
return Newspack_Newsletters_Contacts::update_lists( $email, $lists, 'deprecated' );
}

/**
Expand Down Expand Up @@ -1001,7 +1006,7 @@ public static function process_subscription_update() {
} else {
$email = get_userdata( get_current_user_id() )->user_email;
$lists = isset( $_POST['lists'] ) ? array_map( 'sanitize_text_field', $_POST['lists'] ) : [];
$result = Newspack_Newsletters_Contacts::update_lists( $email, $lists );
$result = Newspack_Newsletters_Contacts::update_lists( $email, $lists, 'User updated their subscriptions on My Account page' );
if ( is_wp_error( $result ) ) {
wc_add_notice( $result->get_error_message(), 'error' );
} elseif ( false === $result ) {
Expand Down
12 changes: 4 additions & 8 deletions includes/plugins/class-woocommerce-memberships.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Newspack\Newsletters\Subscription_List;
use Newspack\Newsletters\Subscription_Lists;
use Newspack_Newsletters;
use Newspack_Newsletters_Contacts;
use Newspack_Newsletters_Logger;

defined( 'ABSPATH' ) || exit;
Expand Down Expand Up @@ -206,8 +207,6 @@ public static function remove_user_from_lists( $user_membership ) {
return;
}

$provider = Newspack_Newsletters::get_service_provider();

/**
* Check if the user is already in one of the lists. If they are, store it.
*
Expand All @@ -225,10 +224,8 @@ public static function remove_user_from_lists( $user_membership ) {

self::update_user_lists_on_deactivation( $user->ID, $user_membership->get_id(), $existing_lists );

if ( ! empty( $provider ) ) {
$provider->update_contact_lists_handling_local( $user_email, [], $lists_to_remove );
Newspack_Newsletters_Logger::log( 'Reader ' . $user_email . ' removed from the following lists: ' . implode( ', ', $lists_to_remove ) );
}
Newspack_Newsletters_Contacts::add_and_remove_lists( $user_email, [], $lists_to_remove, 'Removing user from lists tied to Memberships being marked as inactive' );
Newspack_Newsletters_Logger::log( 'Reader ' . $user_email . ' removed from the following lists: ' . implode( ', ', $lists_to_remove ) );
}

/**
Expand Down Expand Up @@ -336,8 +333,7 @@ public static function add_user_to_lists( $plan, $args ) {
return;
}

$provider = Newspack_Newsletters::get_service_provider();
$result = $provider->update_contact_lists_handling_local( $user_email, $lists_to_add );
$result = Newspack_Newsletters_Contacts::add_and_remove_lists( $user_email, $lists_to_add, [], 'Adding user to lists tied to Memberships being marked as active' );

if ( is_wp_error( $result ) ) {
Newspack_Newsletters_Logger::log( 'An error occured while updating lists for ' . $user_email . ': ' . $result->get_error_message() );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,12 +491,12 @@ public function add_contact_handling_local_list( $contact, $list_id ) {
try {
$list = Subscription_List::from_form_id( $list_id );
if ( ! $list->is_configured_for_provider( $this->service ) ) {
return new WP_Error( 'List not properly configured for the provider' );
return new WP_Error( "List $list_id not properly configured for the provider" );
}
$list_settings = $list->get_provider_settings( $this->service );
return $this->add_esp_local_list_to_contact( $contact['email'], $list_settings['tag_id'], $list_settings['list'] );
} catch ( \InvalidArgumentException $e ) {
return new WP_Error( 'List not found' );
return new WP_Error( "List $list_id not found" );
}
}
}
Expand All @@ -509,15 +509,16 @@ public function add_contact_handling_local_list( $contact, $list_id ) {
* @param string $email Contact email address.
* @param string[] $lists_to_add Array of list IDs to subscribe the contact to.
* @param string[] $lists_to_remove Array of list IDs to remove the contact from.
* @param string $context The context in which the update is being performed. For logging purposes.
*
* @return true|WP_Error True if the contact was updated or error.
*/
public function update_contact_lists_handling_local( $email, $lists_to_add = [], $lists_to_remove = [] ) {
public function update_contact_lists_handling_local( $email, $lists_to_add = [], $lists_to_remove = [], $context = 'Unknown' ) {
$contact = $this->get_contact_data( $email );
if ( is_wp_error( $contact ) ) {
// Create contact.
// Use Newspack_Newsletters_Subscription::add_contact to trigger hooks and call add_contact_handling_local_list if needed.
$result = Newspack_Newsletters_Subscription::add_contact( [ 'email' => $email ], $lists_to_add );
// Use Newspack_Newsletters_Contacts::upsert to trigger hooks and call add_contact_handling_local_list if needed.
$result = Newspack_Newsletters_Contacts::upsert( [ 'email' => $email ], $lists_to_add, false, $context );
if ( is_wp_error( $result ) ) {
return $result;
}
Expand Down
Loading

0 comments on commit ec0cbe0

Please sign in to comment.