fix: bugs from Newsletters editor data refactor

includes/service-providers/active_campaign/class-newspack-newsletters-active-campaign.php

@@ -626,7 +626,7 @@ public function get_send_lists( $args = [], $to_array = false ) {
 						'provider'    => $this->service,
 						'type'        => 'sublist',
 						'id'          => $segment['id'],
-						'parent'      => $args['parent'] ?? null,
+						'parent_id'   => $args['parent_id'] ?? null,
 						'name'        => $segment_name,
 						'entity_type' => 'segment',
 						'count'       => $segment['subscriber_count'] ?? null,
@@ -790,19 +790,24 @@ public function retrieve( $post_id, $skip_sync = false ) {
 			$campaign_id     = get_post_meta( $post_id, 'ac_campaign_id', true );
 			$send_list_id    = get_post_meta( $post_id, 'send_list_id', true );
 			$send_sublist_id = get_post_meta( $post_id, 'send_sublist_id', true );
+			$newsletter_data = [
+				'campaign'                          => true, // Satisfy the JS API.
+				'campaign_id'                       => $campaign_id,
+				'supports_multiple_test_recipients' => true,
+			];
 
 			// Handle legacy send-to meta.
 			if ( ! $send_list_id ) {
 				$legacy_list_id = get_post_meta( $post_id, 'ac_list_id', true );
 				if ( $legacy_list_id ) {
-					$newsletter_data['list_id'] = $legacy_list_id;
+					$newsletter_data['send_list_id'] = $legacy_list_id;
 					$send_list_id               = $legacy_list_id;
 				}
 			}
 			if ( ! $send_sublist_id ) {
 				$legacy_sublist_id = get_post_meta( $post_id, 'ac_segment_id', true );
 				if ( $legacy_sublist_id ) {
-					$newsletter_data['sublist_id'] = $legacy_sublist_id;
+					$newsletter_data['send_sublist_id'] = $legacy_sublist_id;
 					$send_sublist_id               = $legacy_sublist_id;
 				}
 			}
@@ -816,6 +821,7 @@ public function retrieve( $post_id, $skip_sync = false ) {
 			if ( is_wp_error( $send_lists ) ) {
 				throw new Exception( wp_kses_post( $send_lists->get_error_message() ) );
 			}
+			$newsletter_data['lists'] = $send_lists;
 			$send_sublists = $send_list_id || $send_sublist_id ?
 				$this->get_send_lists(
 					[
@@ -829,13 +835,7 @@ public function retrieve( $post_id, $skip_sync = false ) {
 			if ( is_wp_error( $send_sublists ) ) {
 				throw new Exception( wp_kses_post( $send_sublists->get_error_message() ) );
 			}
-			$newsletter_data = [
-				'campaign'                          => true, // Satisfy the JS API.
-				'campaign_id'                       => $campaign_id,
-				'supports_multiple_test_recipients' => true,
-				'lists'                             => $send_lists,
-				'sublists'                          => $send_sublists,
-			];
+			$newsletter_data['sublists'] = $send_sublists;
 
 			if ( $campaign_id ) {
 				$newsletter_data['link'] = sprintf(

includes/service-providers/campaign_monitor/class-newspack-newsletters-campaign-monitor.php

@@ -221,9 +221,9 @@ public function get_segments() {
 		$segments = array_map(
 			function ( $item ) {
 				return [
-					'id'     => $item->SegmentID, // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase
-					'name'   => $item->Title, // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase
-					'parent' => $item->ListID, // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase
+					'id'        => $item->SegmentID, // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase
+					'name'      => $item->Title, // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase
+					'parent_id' => $item->ListID, // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase
 				];
 			},
 			$segments->response
@@ -400,7 +400,7 @@ public function retrieve( $post_id ) {
 			if ( ! $send_list_id ) {
 				$legacy_list_id = get_post_meta( $post_id, 'cm_list_id', true ) ?? get_post_meta( $post_id, 'cm_segment_id', true );
 				if ( $legacy_list_id ) {
-					$newsletter_data['list_id'] = $legacy_list_id;
+					$newsletter_data['send_list_id'] = $legacy_list_id;
 				}
 			}
 

includes/service-providers/class-newspack-newsletters-service-provider.php

@@ -62,6 +62,7 @@ public function __construct() {
 			add_action( 'rest_api_init', [ $this->controller, 'register_routes' ] );
 		}
 		add_action( 'pre_post_update', [ $this, 'pre_post_update' ], 10, 2 );
+		add_action( 'save_post', [ $this, 'save_post' ], 10, 2 );
 		add_action( 'transition_post_status', [ $this, 'transition_post_status' ], 10, 3 );
 		add_action( 'updated_post_meta', [ $this, 'updated_post_meta' ], 10, 4 );
 		add_action( 'wp_insert_post', [ $this, 'insert_post' ], 10, 3 );
@@ -144,6 +145,22 @@ public function pre_post_update( $post_id, $data ) {
 		}
 	}
 
+	/**
+	 * Delete layout defaults meta after saving the post.
+	 * We don't want layout defaults overwriting saved values unless the layout has just been set.
+	 *
+	 * @param int $post_id The ID of the post being saved.
+	 * @return void
+	 */
+	public function save_post( $post_id ) {
+		$post_type = get_post_type( $post_id );
+		if ( Newspack_Newsletters::NEWSPACK_NEWSLETTERS_CPT !== $post_type ) {
+			return;
+		}
+
+		delete_post_meta( $post_id, 'stringifiedCampaignDefaults' );
+	}
+
 	/**
 	 * Handle post status transition for scheduled newsletters.
 	 *

includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp-cached-data.php

@@ -586,7 +586,7 @@ public static function fetch_segment( $segment_id, $list_id ) {
 		}
 		$response = ( self::get_mc_instance() )->validate(
 			$mc->get(
-				"lists/$list_id/segment/$segment_id",
+				"lists/$list_id/segments/$segment_id",
 				[
 					'fields' => 'id,name,member_count,type,options,list_id',
 				],

includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php

@@ -560,11 +560,11 @@ public function retrieve( $post_id ) {
 				$newsletter_data['senderEmail'] = $campaign_info['senderEmail']; // If campaign has different sender info set, update ours.
 			}
 			if ( $list_id && $list_id !== $send_list_id ) {
-				$newsletter_data['list_id'] = $list_id; // If campaign has a different list selected, update ours.
-				$send_list_id               = $list_id;
+				$newsletter_data['send_list_id'] = $list_id; // If campaign has a different list selected, update ours.
+				$send_list_id                    = $list_id;
 
 				if ( ! empty( $campaign_info['sublist_id'] ) && $campaign_info['sublist_id'] !== $send_sublist_id ) {
-						$newsletter_data['sublist_id'] = $campaign_info['sublist_id']; // If campaign has a different sublist selected, update ours.
+						$newsletter_data['send_sublist_id'] = $campaign_info['sublist_id']; // If campaign has a different sublist selected, update ours.
 						$send_sublist_id = $campaign_info['sublist_id'];
 				}
 			}
@@ -725,7 +725,7 @@ public function get_send_lists( $args = [], $to_array = false ) {
 									'id'          => $interest['id'],
 									'name'        => $interest['name'],
 									'entity_type' => $entity_type,
-									'parent'      => $interest['list_id'],
+									'parent_id'   => $interest['list_id'],
 									'count'       => $interest['subscriber_count'],
 								];
 								if ( $admin_url && $audience['web_id'] ) {
@@ -749,7 +749,7 @@ public function get_send_lists( $args = [], $to_array = false ) {
 						'id'          => $tag['id'],
 						'name'        => $tag['name'],
 						'entity_type' => $entity_type,
-						'parent'      => $tag['list_id'],
+						'parent_id'   => $tag['list_id'],
 						'count'       => $tag['member_count'],
 					];
 					if ( $admin_url && $audience['web_id'] ) {
@@ -770,7 +770,7 @@ public function get_send_lists( $args = [], $to_array = false ) {
 						'id'          => $segment['id'],
 						'name'        => $segment['name'],
 						'entity_type' => $entity_type,
-						'parent'      => $segment['list_id'],
+						'parent_id'   => $segment['list_id'],
 						'count'       => $segment['member_count'],
 					];
 					if ( $admin_url && $audience['web_id'] ) {
@@ -1044,12 +1044,12 @@ public function get_sync_payload( $post ) {
 				$sublist = $this->get_send_lists(
 					[
 						'ids'       => [ $send_sublist_id ],
-						'limit'     => 1,
+						'limit'     => 1000,
 						'parent_id' => $send_list_id,
 						'type'      => 'sublist',
 					]
 				);
-				if ( ! empty( $sublist[0]->get_entity_type() ) ) {
+				if ( ! empty( $sublist ) && ! empty( $sublist[0]->get_entity_type() ) ) {
 					$sublist_type = $sublist[0]->get_entity_type();
 					switch ( $sublist_type ) {
 						case 'group':

src/newsletter-editor/sidebar/autocomplete.js

@@ -35,8 +35,8 @@ const Autocomplete = ( {
 						{ selectedInfo?.hasOwnProperty( 'count' )
 							? ' • ' +
 							sprintf(
-									// Translators: If available, show a contact count alongside the selected item's type. %d is the number of contacts in the item.
-									_n( '%d contact', '%d contacts', selectedInfo.count, 'newspack-newsletters' ),
+									// Translators: If available, show a contact count alongside the selected item's type. %s is the number of contacts in the item.
+									_n( '%s contact', '%s contacts', selectedInfo.count, 'newspack-newsletters' ),
 									selectedInfo.count.toLocaleString()
 							)
 							: '' }

src/newsletter-editor/sidebar/index.js

@@ -19,7 +19,7 @@ import Sender from './sender';
 import SendTo from './send-to';
 import { getServiceProvider } from '../../service-providers';
 import withApiHandler from '../../components/with-api-handler';
-import { fetchNewsletterData, useIsRetrieving, useNewsletterData, useNewsletterDataError } from '../store';
+import { fetchNewsletterData, updateNewsletterData, useIsRetrieving, useNewsletterData, useNewsletterDataError } from '../store';
 import { isSupportedESP } from '../utils';
 import './style.scss';
 
@@ -49,40 +49,56 @@ const Sidebar = ( {
 	// Reconcile stored campaign data with data fetched from ESP.
 	useEffect( () => {
 		const updatedMeta = {};
+		const updatedNewsletterData = { ...newsletterData };
+
 		if ( newsletterData?.senderEmail ) {
 			updatedMeta.senderEmail = newsletterData.senderEmail;
+			delete updatedNewsletterData.senderEmail;
 		}
 		if ( newsletterData?.senderName ) {
 			updatedMeta.senderName = newsletterData.senderName;
+			delete updatedNewsletterData.senderName;
 		}
 		if ( newsletterData?.send_list_id ) {
 			updatedMeta.send_list_id = newsletterData.send_list_id;
+			delete updatedNewsletterData.send_list_id;
 		}
 		if ( newsletterData?.send_sublist_id ) {
 			updatedMeta.send_sublist_id = newsletterData.send_sublist_id;
+			delete updatedNewsletterData.send_sublist_id;
 		}
 		if ( Object.keys( updatedMeta ).length ) {
 			updateMeta( updatedMeta );
 		}
-	}, [ newsletterData ] );
+		if ( Object.keys( updatedNewsletterData ).length ) {
+			updateNewsletterData( updatedNewsletterData );
+		}
+	}, [
+		newsletterData?.senderEmail,
+		newsletterData?.senderName,
+		newsletterData?.send_list_id,
+		newsletterData?.send_sublist_id
+	] );
 
 	useEffect( () => {
-		const campaignDefaults = 'string' === typeof stringifiedCampaignDefaults ? JSON.parse( stringifiedCampaignDefaults ) : stringifiedCampaignDefaults;
-		const updatedMeta = {};
-		if ( campaignDefaults?.senderEmail ) {
-			updatedMeta.senderEmail = campaignDefaults.senderEmail;
-		}
-		if ( campaignDefaults?.senderName ) {
-			updatedMeta.senderName = campaignDefaults.senderName;
-		}
-		if ( campaignDefaults?.send_list_id ) {
-			updatedMeta.send_list_id = campaignDefaults.send_list_id;
-		}
-		if ( campaignDefaults?.send_sublist_id ) {
-			updatedMeta.send_sublist_id = campaignDefaults.send_sublist_id;
-		}
-		if ( Object.keys( updatedMeta ).length ) {
-			updateMeta( updatedMeta );
+		if ( stringifiedCampaignDefaults ) {
+			const campaignDefaults = 'string' === typeof stringifiedCampaignDefaults ? JSON.parse( stringifiedCampaignDefaults ) : stringifiedCampaignDefaults;
+			const updatedMeta = {};
+			if ( campaignDefaults?.senderEmail ) {
+				updatedMeta.senderEmail = campaignDefaults.senderEmail;
+			}
+			if ( campaignDefaults?.senderName ) {
+				updatedMeta.senderName = campaignDefaults.senderName;
+			}
+			if ( campaignDefaults?.send_list_id ) {
+				updatedMeta.send_list_id = campaignDefaults.send_list_id;
+			}
+			if ( campaignDefaults?.send_sublist_id ) {
+				updatedMeta.send_sublist_id = campaignDefaults.send_sublist_id;
+			}
+			if ( Object.keys( updatedMeta ).length ) {
+				updateMeta( updatedMeta );
+			}
 		}
 	}, [ stringifiedCampaignDefaults ] );
 

src/newsletter-editor/sidebar/send-to.js

@@ -13,6 +13,7 @@ import { useEffect, useState } from '@wordpress/element';
  */
 import Autocomplete from './autocomplete';
 import { fetchSendLists, useNewsletterData } from '../store';
+import { usePrevious } from '../utils';
 
 // The container for list + sublist autocomplete fields.
 const SendTo = () => {
@@ -34,8 +35,9 @@ const SendTo = () => {
 	const { labels } = newspack_newsletters_data || {};
 	const listLabel = labels?.list || __( 'list', 'newspack-newsletters' );
 	const sublistLabel = labels?.sublist || __( 'sublist', 'newspack-newsletters' );
-	const selectedList = lists.find( item => item.id === listId );
-	const selectedSublist = sublists?.find( item => item.id === sublistId );
+	const selectedList = listId ? lists.find( item => item.id.toString() === listId.toString() ) : null;
+	const selectedSublist = sublistId ? sublists?.find( item => item.id.toString() === sublistId.toString() ) : null;
+	const prevListId = usePrevious( listId );
 
 	// Cancel any queued fetches on unmount.
 	useEffect( () => {
@@ -55,9 +57,10 @@ const SendTo = () => {
 			fetchSendLists( { ids: [ sublistId ], type: 'sublist', parent_id: listId } );
 		}
 
-		// Prefetch sublist info when selecting a new list ID.
-		if ( listId && ! sublistId && newsletterData?.sublists && 1 >= newsletterData.sublists.length ) {
-			fetchSendLists( { type: 'sublist', parent_id: listId } );
+		// If selecting a new list entirely.
+		if ( listId && prevListId && listId !== prevListId ) {
+			fetchSendLists( { type: 'sublist', parent_id: listId }, true );
+			updateMeta( { send_sublist_id: null } );
 		}
 	}, [ newsletterData, listId, sublistId ] );
 
@@ -118,7 +121,7 @@ const SendTo = () => {
 				</Notice>
 			) }
 			{
-				( newsletterData?.fetched_list || newsletterData?.fetched_sublist ) && (
+				( newsletterData?.send_list_id || newsletterData?.send_sublist_id ) && (
 					<Notice status="success" isDismissible={ false }>
 						{ __( 'Updated send-to info fetched from ESP.', 'newspack-newsletters' ) }
 					</Notice>

src/newsletter-editor/store.js

@@ -150,7 +150,7 @@ export const fetchSyncErrors = async postId => {
 }
 
 // Dispatcher to fetch send lists and sublists from the connected ESP and update the newsletterData in store.
-export const fetchSendLists = debounce( async ( opts ) => {
+export const fetchSendLists = debounce( async ( opts, replace = false ) => {
 	updateNewsletterDataError( null );
 	try {
 		const { name } = getServiceProvider();
@@ -190,7 +190,7 @@ export const fetchSendLists = debounce( async ( opts ) => {
 		}
 
 		const updatedNewsletterData = { ...newsletterData };
-		const updatedSendLists = [ ...sendLists ];
+		const updatedSendLists = replace ? [] : [ ...sendLists ];
 
 		// If no existing items found, fetch from the ESP.
 		const isRetrieving = coreSelect( STORE_NAMESPACE ).getIsRetrieving();

src/newsletter-editor/utils.js

@@ -4,6 +4,7 @@
  * WordPress dependencies
  */
 import apiFetch from '@wordpress/api-fetch';
+import { useEffect, useRef } from '@wordpress/element';
 import { __ } from '@wordpress/i18n';
 
 /**
@@ -84,3 +85,16 @@ export const refreshEmailHtml = async ( postId, postTitle, postContent ) => {
 	return html;
 };
 
+/**
+ * Custom hook to fetch a previous state or prop value.
+ *
+ * @param {string} value of the prop or state to fetch.
+ * @return {*} The previous value of the prop or state.
+ */
+export const usePrevious = value => {
+	const ref = useRef();
+	useEffect( () => {
+		ref.current = value;
+	},[ value ] );
+	return ref.current;
+}