Skip to content

Commit

Permalink
chore: refactor upsert_contact and update_contact flows (#1620)
Browse files Browse the repository at this point in the history 
* chore: refactor upsert_contact and update_contact flows

* fix: fix subscription list method

* chore: rename variable

* test: update tests

* chore: rename form id to public id

* test: refactor tests organization

* test: adding abstract ESP tests class and first esp test

* test: add some specific mailchimp tests

* test: more mailchimp tests

* test: add esp common tests for constant contact

* chore: remove unnecessary unsets

* fix: correct usage of WP_Error

* chore: rename method for better readability

* chore: better error handling

* chore: rename test file

* fix: remove statuses from the metadata payload
  • Loading branch information
@leogermani
leogermani authored Aug 27, 2024
1 parent b47f7b7 commit 4f206ff
Show file tree
Hide file tree
Showing 26 changed files with 1,916 additions and 734 deletions.
32 changes: 14 additions & 18 deletions includes/class-newspack-newsletters-contacts.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

defined( 'ABSPATH' ) || exit;

use Newspack\Newsletters\Subscription_List;

/**
* Class Newspack_Newsletters_Contacts
*/
Expand Down Expand Up @@ -172,16 +174,19 @@ public static function upsert( $contact, $lists = false, $context = 'Unknown', $

$errors = new WP_Error();
$result = [];
try {
if ( method_exists( $provider, 'add_contact_with_groups_and_tags' ) ) {
$result = $provider->add_contact_with_groups_and_tags( $contact, $lists );
} elseif ( empty( $lists ) ) {
$result = $provider->add_contact( $contact );
} else {
foreach ( $lists as $list_id ) {
$result = $provider->add_contact( $contact, $list_id );
}

$lists_objects = [];
foreach ( $lists as $list_id ) {
$list_obj = Subscription_List::from_public_id( $list_id );
if ( ! $list_obj ) {
$errors->add( 'newspack_newsletters_invalid_list', 'Invalid list ID: ' . $list_id );
continue;
}
$lists_objects[] = $list_obj;
}

try {
$result = $provider->upsert_contact( $contact, $lists_objects );
} catch ( \Exception $e ) {
$errors->add( 'newspack_newsletters_subscription_add_contact', $e->getMessage() );
}
Expand All @@ -190,15 +195,6 @@ public static function upsert( $contact, $lists = false, $context = 'Unknown', $
$errors->add( $result->get_error_code(), $result->get_error_message() );
}

// Handle local lists feature.
foreach ( $lists as $list_id ) {
try {
$provider->add_contact_handling_local_list( $contact, $list_id );
} catch ( \Exception $e ) {
$errors->add( 'newspack_newsletters_subscription_handling_local_list', $e->getMessage() );
}
}

/**
* Fires after a contact is upserted.
*
Expand Down
2 changes: 1 addition & 1 deletion includes/class-newspack-newsletters-subscription.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ public static function get_lists_config() {
if ( ! $list->is_active() ) {
continue;
}
$active_lists[ $list->get_form_id() ] = $list->to_array();
$active_lists[ $list->get_public_id() ] = $list->to_array();
}

return $active_lists;
Expand Down
54 changes: 28 additions & 26 deletions includes/class-subscription-list.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
*/
class Subscription_List {

use \Newspack_Newsletters_Mailchimp_Subscription_List_Trait;

/**
* Hold the WP_Post object associated with this List.
*
Expand All @@ -33,9 +35,9 @@ class Subscription_List {
const META_KEY = 'newspack_nl_provider_settings';

/**
* The prefix used to build the form iD
* The prefix used to build the Public iD
*/
const FORM_ID_PREFIX = 'newspack-';
const PUBLIC_ID_PREFIX = 'newspack-';

/**
* The post meta key used to mark a list as local
Expand Down Expand Up @@ -63,30 +65,30 @@ class Subscription_List {
const SUBSCRIBER_COUNT_META = '_subscriber_count';

/**
* Checks if a string $id is in the format of a local Subscription List Form ID
* Checks if a string $id is in the format of a local Subscription List Public ID
*
* @see self::get_form_id
* @see self::get_public_id
* @param string $id The ID to be checked.
* @return boolean
*/
public static function is_local_form_id( $id ) {
return (bool) self::get_id_from_local_form_id( $id );
public static function is_local_public_id( $id ) {
return (bool) self::get_id_from_local_public_id( $id );
}

/**
* Extracts the numeric ID from a properly formatted Form ID
* Extracts the numeric ID from a properly formatted Public ID
*
* @see self::get_form_id
* @param string $form_id The Form id.
* @see self::get_public_id
* @param string $public_id The Public id.
* @return ?int The ID on success, NULL on failure
*/
public static function get_id_from_local_form_id( $form_id ) {
if ( ! is_string( $form_id ) ) {
public static function get_id_from_local_public_id( $public_id ) {
if ( ! is_string( $public_id ) ) {
return;
}
$search = preg_match(
'/^' . self::FORM_ID_PREFIX . '([0-9]+)$/',
$form_id,
'/^' . self::PUBLIC_ID_PREFIX . '([0-9]+)$/',
$public_id,
$matches
);
if ( $search && ! empty( $matches[1] ) ) {
Expand All @@ -95,22 +97,22 @@ public static function get_id_from_local_form_id( $form_id ) {
}

/**
* Gets a Subscription List object by its form_id
* Gets a Subscription List object by its public_id
*
* @param string $form_id The list's form ID.
* @param string $public_id The list's public ID.
* @return ?Subscription_List
*/
public static function from_form_id( $form_id ) {
$form_id = (string) $form_id;
if ( self::is_local_form_id( $form_id ) ) {
$post_id = self::get_id_from_local_form_id( $form_id );
public static function from_public_id( $public_id ) {
$public_id = (string) $public_id;
if ( self::is_local_public_id( $public_id ) ) {
$post_id = self::get_id_from_local_public_id( $public_id );
try {
return new self( $post_id );
} catch ( \InvalidArgumentException $e ) {
return;
}
}
return self::from_remote_id( $form_id );
return self::from_remote_id( $public_id );
}

/**
Expand Down Expand Up @@ -166,15 +168,15 @@ public function get_id() {
}

/**
* Returns the Form ID for this List
* Returns the Public ID for this List
*
* Form ID is the ID that will represent this list in all UI forms across the application
* Public ID is the ID that will represent this list in all UI forms across the application
*
* @return string
*/
public function get_form_id() {
public function get_public_id() {
if ( $this->is_local() ) {
return self::FORM_ID_PREFIX . $this->get_id();
return self::PUBLIC_ID_PREFIX . $this->get_id();
}
return $this->get_remote_id();
}
Expand Down Expand Up @@ -225,7 +227,7 @@ public function set_type( $type ) {
public function get_type_label() {
$provider = Newspack_Newsletters::get_service_provider();
if ( ! empty( $provider ) ) {
return $this->is_local() ? $provider::label( 'local_list_explanation', $this->get_form_id() ) : $provider::label( 'list_explanation', $this->get_form_id() );
return $this->is_local() ? $provider::label( 'local_list_explanation', $this->get_public_id() ) : $provider::label( 'list_explanation', $this->get_public_id() );
}
return '';
}
Expand Down Expand Up @@ -537,7 +539,7 @@ public function update( $fields ) {
*/
public function to_array() {
return [
'id' => $this->get_form_id(),
'id' => $this->get_public_id(),
'db_id' => $this->get_id(),
'title' => $this->get_title(),
'name' => $this->get_title(),
Expand Down
8 changes: 4 additions & 4 deletions includes/class-subscription-lists.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ public static function get_or_create_remote_list( $list ) {

$subscriber_count = ! empty( $list['subscriber_count'] ) ? (int) $list['subscriber_count'] : 0;
$subscriber_count = ! empty( $list['member_count'] ) ? (int) $list['member_count'] : 0; // Tags have member_count instead of subscriber_count.
$saved_list = Subscription_List::from_form_id( $list['id'] );
$saved_list = Subscription_List::from_public_id( $list['id'] );
if ( $saved_list ) {
// Update remote name, in case it's changed in the ESP.
$saved_list->set_remote_name( $list['title'] );
Expand Down Expand Up @@ -551,9 +551,9 @@ public static function update_lists( $lists ) {
$existing_ids = [];

foreach ( $lists as $list ) {
if ( Subscription_List::is_local_form_id( $list['id'] ) ) {
if ( Subscription_List::is_local_public_id( $list['id'] ) ) {
// Local lists will be fetched here.
$stored_list = Subscription_List::from_form_id( $list['id'] );
$stored_list = Subscription_List::from_public_id( $list['id'] );
} else {
// Remote lists will be either fetched or created here.
$stored_list = self::get_or_create_remote_list( $list );
Expand Down Expand Up @@ -667,7 +667,7 @@ public static function migrate_lists() {

foreach ( $lists as $list_id => $list ) {

if ( Subscription_List::is_local_form_id( $list_id ) ) {
if ( Subscription_List::is_local_public_id( $list_id ) ) {
continue;
}

Expand Down
6 changes: 3 additions & 3 deletions includes/plugins/class-woocommerce-memberships.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public static function filter_lists( $lists ) {
$lists = array_filter(
$lists,
function ( $list ) {
$list_object = Subscription_List::from_form_id( $list );
$list_object = Subscription_List::from_public_id( $list );
if ( ! $list_object ) {
return false;
}
Expand Down Expand Up @@ -195,7 +195,7 @@ public static function remove_user_from_lists( $user_membership ) {
foreach ( $object_ids as $object_id ) {
try {
$subscription_list = new Subscription_List( $object_id );
$lists_to_remove[] = $subscription_list->get_form_id();
$lists_to_remove[] = $subscription_list->get_public_id();
} catch ( \InvalidArgumentException $e ) {
continue;
}
Expand Down Expand Up @@ -302,7 +302,7 @@ public static function add_user_to_lists( $plan, $args ) {
foreach ( $object_ids as $object_id ) {
try {
$subscription_list = new Subscription_List( $object_id );
$list_id = $subscription_list->get_form_id();
$list_id = $subscription_list->get_public_id();

$lists_to_add[] = $list_id;
} catch ( \InvalidArgumentException $e ) {
Expand Down
73 changes: 54 additions & 19 deletions includes/service-providers/class-newspack-newsletters-service-provider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,37 +468,72 @@ public static function label( $key, $context = '' ) {
return $labels[ $key ] ?? '';
}

/**
* Upserts a contact to the ESP using the provider specific methods.
*
* Note: Mailchimp overrides this method.
*
* @param array $contact {
* Contact data.
*
* @type string $email Contact email address.
* @type string $name Contact name. Optional.
* @type string[] $metadata Contact additional metadata. Optional.
* }
* @param Subscription_List[] $lists The lists.
* @return array|WP_Error Contact data if it was added, or error otherwise.
*/
public function upsert_contact( $contact, $lists ) {

if ( empty( $lists ) ) {
return $provider->add_contact( $contact );
}

foreach ( $lists as $list ) {
if ( $list->is_local() ) {
$result = $this->add_contact_to_local_list( $contact, $list );
} else {
$result = $this->add_contact( $contact, $list->get_public_id() );
}
if ( is_wp_error( $result ) ) {
return $result;
}
}

// on success, return the last result.
return $result;
}

/**
* Handle adding to local lists.
* If the $list_id is a local list, a tag will be added to the contact.
*
* @param array $contact {
* Contact data.
* @param array $contact {
* Contact data.
*
* @type string $email Contact email address.
* @type string $name Contact name. Optional.
* @type string[] $metadata Contact additional metadata. Optional.
* }
* @param string $list_id List to add the contact to.
* @param Subscription_List $list The list object.
*
* @return true|WP_Error True or error.
*/
public function add_contact_handling_local_list( $contact, $list_id ) {
protected function add_contact_to_local_list( $contact, Subscription_List $list ) {
if ( ! static::$support_local_lists ) {
return true;
}
if ( Subscription_List::is_local_form_id( $list_id ) ) {
try {
$list = Subscription_List::from_form_id( $list_id );
if ( ! $list->is_configured_for_provider( $this->service ) ) {
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 $list_id not found" );
}

if ( ! $list->is_local() ) {
return new WP_Error( 'newspack_newsletters_list_not_local', "List {$list->get_public_id()} is not a local list" );
}

if ( ! $list->is_configured_for_provider( $this->service ) ) {
return new WP_Error( 'newspack_newsletters_list_not_configured_for_provider', "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'] );
}

/**
Expand Down Expand Up @@ -545,11 +580,11 @@ public function update_contact_lists_handling_local( $email, $lists_to_add = [],
* @param string $action The action to be performed. add or remove.
* @return array|WP_Error The remaining lists that were not handled by this method, because they are not local lists.
*/
public function update_contact_local_lists( $email, $lists = [], $action = 'add' ) {
protected function update_contact_local_lists( $email, $lists = [], $action = 'add' ) {
foreach ( $lists as $key => $list_id ) {
if ( Subscription_List::is_local_form_id( $list_id ) ) {
if ( Subscription_List::is_local_public_id( $list_id ) ) {
try {
$list = Subscription_List::from_form_id( $list_id );
$list = Subscription_List::from_public_id( $list_id );

if ( ! $list->is_configured_for_provider( $this->service ) ) {
return new WP_Error( 'List not properly configured for the provider' );
Expand Down Expand Up @@ -591,7 +626,7 @@ public function get_contact_local_lists( $email ) {
}
$list_settings = $list->get_provider_settings( $this->service );
if ( in_array( $list_settings['tag_id'], $tags, false ) ) { // phpcs:ignore WordPress.PHP.StrictInArray.FoundNonStrictFalse
$ids[] = $list->get_form_id();
$ids[] = $list->get_public_id();
}
}
return $ids;
Expand Down
Loading

0 comments on commit 4f206ff

Please sign in to comment.