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.

Sign up for GitHub

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: display list remote name on settings page #1672

Merged
merged 3 commits into from
Oct 23, 2024
Merged

feat: display list remote name on settings page #1672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
49b8b58
feat: display list remote name on settings page
leogermani Oct 15, 2024
ef6008b
feat: update list title if there is no local customization
leogermani Oct 17, 2024
8d0a772
fix: fix the update_lists behavior and add tests
leogermani Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion includes/class-newspack-newsletters-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ private static function render_lists_table() {
/>
</td>
<td class="name">
<label for="<?php echo esc_attr( $checkbox_id ); ?>"><?php echo esc_html( $list['name'] ); ?></strong>
<label for="<?php echo esc_attr( $checkbox_id ); ?>"><?php echo esc_html( $list['remote_name'] ); ?></strong>
<br/>
<small>
<?php echo esc_html( $list['type_label'] ); ?>
Expand Down
1 change: 1 addition & 0 deletions includes/class-subscription-list.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ public function to_array() {
'type_label' => $this->get_type_label(),
'edit_link' => $this->get_edit_link(),
'active' => $this->is_active(),
'remote_name' => $this->get_remote_name(),
];
}
}
25 changes: 17 additions & 8 deletions includes/class-subscription-lists.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,8 +466,18 @@ public static function get_or_create_remote_list( $list ) {
$subscriber_count = ! empty( $list['member_count'] ) ? (int) $list['member_count'] : 0; // Tags have member_count instead of subscriber_count.
$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'] );

$has_customized_title = $saved_list->get_title() !== $saved_list->get_remote_name();

if ( $list['title'] !== $saved_list->get_remote_name() ) {
// The remote name has changed, let's update it locally.
$saved_list->set_remote_name( $list['title'] );

// Only update the title if it was not customized by the user.
if ( ! $has_customized_title ) {
$saved_list->update( [ 'title' => $list['title'] ] );
}
}

// Update subscriber count, if available.
if ( 0 > $subscriber_count ) {
Expand Down Expand Up @@ -551,11 +561,11 @@ public static function update_lists( $lists ) {
$existing_ids = [];

foreach ( $lists as $list ) {
if ( Subscription_List::is_local_public_id( $list['id'] ) ) {
// Local lists will be fetched here.
$stored_list = Subscription_List::from_public_id( $list['id'] );
} else {
// Remote lists will be either fetched or created here.

$stored_list = Subscription_List::from_public_id( $list['id'] );

// If a remote list was not found, create one.
if ( ! $stored_list instanceof Subscription_List && ! Subscription_List::is_local_public_id( $list['id'] ) ) {
$stored_list = self::get_or_create_remote_list( $list );
}

Expand All @@ -564,7 +574,6 @@ public static function update_lists( $lists ) {
}

$existing_ids[] = $stored_list->get_id();

$stored_list->update( $list );

}
Expand Down
32 changes: 31 additions & 1 deletion tests/test-subscription-lists.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public function test_get_or_create_remote_list() {
// existing.
$existing = [
'id' => 'xyz-' . self::$posts['remote_mailchimp'],
'title' => 'Remote mailchimp new title',
'title' => 'Test List 5',
];
$list = Subscription_Lists::get_or_create_remote_list( $existing );
$this->assertSame( self::$posts['remote_mailchimp'], $list->get_id() );
Expand All @@ -152,6 +152,36 @@ public function test_get_or_create_remote_list() {
$this->assertSame( $count + 1, $count_after_new );
}

/**
* Test update title on fetch from ESP
*/
public function test_update_title_when_fetching_from_esp() {

$existing = [
'id' => 'xyz-' . self::$posts['remote_mailchimp'],
'title' => 'Remote mailchimp new title',
];
$list = Subscription_Lists::get_or_create_remote_list( $existing );

$this->assertSame( self::$posts['remote_mailchimp'], $list->get_id() );
$this->assertSame( 'Remote mailchimp new title', $list->get_title() );
$this->assertSame( 'Remote mailchimp new title', $list->get_remote_name() );

// Now we edit the local title.
$list->update( [ 'title' => 'Customized title' ] );

$existing = [
'id' => 'xyz-' . self::$posts['remote_mailchimp'],
'title' => 'Remote mailchimp super new title',
];

$list = Subscription_Lists::get_or_create_remote_list( $existing );

$this->assertSame( self::$posts['remote_mailchimp'], $list->get_id() );
$this->assertSame( 'Customized title', $list->get_title(), 'The local name should not be updated if it was customized' );
$this->assertSame( 'Remote mailchimp super new title', $list->get_remote_name(), 'The remote name should always be updated' );
}

/**
* Test update_lists method
*
Expand Down