From 562d396ea5c5fef5683fe1e37bb2b26690ac4b75 Mon Sep 17 00:00:00 2001
From: leogermani <leogermani@automattic.com>
Date: Wed, 23 Oct 2024 16:20:21 -0300
Subject: [PATCH 1/8] feat: display list remote name on settings page (#1672)

* feat: display list remote name on settings page

* feat: update list title if there is no local customization

* fix: fix the update_lists behavior and add tests
---
 .../class-newspack-newsletters-settings.php   |  2 +-
 includes/class-subscription-list.php          |  1 +
 includes/class-subscription-lists.php         | 25 ++++++++++-----
 tests/test-subscription-lists.php             | 32 ++++++++++++++++++-
 4 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/includes/class-newspack-newsletters-settings.php b/includes/class-newspack-newsletters-settings.php
index 968b3f778..23d89cf4f 100644
--- a/includes/class-newspack-newsletters-settings.php
+++ b/includes/class-newspack-newsletters-settings.php
@@ -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'] ); ?>
diff --git a/includes/class-subscription-list.php b/includes/class-subscription-list.php
index 79cc237ed..455f737c6 100644
--- a/includes/class-subscription-list.php
+++ b/includes/class-subscription-list.php
@@ -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(),
 		];
 	}
 }
diff --git a/includes/class-subscription-lists.php b/includes/class-subscription-lists.php
index b10d6ea7f..7c842103d 100644
--- a/includes/class-subscription-lists.php
+++ b/includes/class-subscription-lists.php
@@ -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 ) {
@@ -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 );
 			}
 
@@ -564,7 +574,6 @@ public static function update_lists( $lists ) {
 			}
 
 			$existing_ids[] = $stored_list->get_id();
-
 			$stored_list->update( $list );
 
 		}
diff --git a/tests/test-subscription-lists.php b/tests/test-subscription-lists.php
index 140b74ab7..8783a5e16 100644
--- a/tests/test-subscription-lists.php
+++ b/tests/test-subscription-lists.php
@@ -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() );
@@ -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
 	 *

From 49450d32c206435a60e0fbab36f179d75ee2bf46 Mon Sep 17 00:00:00 2001
From: Adam Cassis <adam.cassis@automattic.com>
Date: Wed, 23 Oct 2024 11:42:00 +0200
Subject: [PATCH 2/8] fix: preview & send buttons on WP 6.7

---
 src/components/send-button/index.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/components/send-button/index.js b/src/components/send-button/index.js
index 651368e53..4ff6bc0c8 100644
--- a/src/components/send-button/index.js
+++ b/src/components/send-button/index.js
@@ -298,7 +298,7 @@ export default compose( [
 		}
 
 		return (
-			<Fragment>
+			<div style={{ display: 'flex' }}>
 				<PreviewHTMLButton />
 				<Button
 					className="editor-post-publish-button"
@@ -366,7 +366,7 @@ export default compose( [
 						</div>
 					</Modal>
 				) }
-			</Fragment>
+			</div>
 		);
 	}
 );

From 7bde1190aa83e09e7b411db25c38884bddb92506 Mon Sep 17 00:00:00 2001
From: Derrick Koo <dkoo@users.noreply.github.com>
Date: Fri, 25 Oct 2024 10:39:34 -0600
Subject: [PATCH 3/8] fix: avoid race condition between post-save sync & test
 (#1679)

* fix: avoid race condition between post-save sync & test

* perf: no need to sync MC before test
---
 .../class-newspack-newsletters-mailchimp.php  | 12 ------
 src/editor/mjml/index.js                      | 28 ++++----------
 src/newsletter-editor/store.js                | 17 +++++++++
 src/newsletter-editor/testing/index.js        | 38 +++++++++++++------
 4 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php b/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
index 4a9604ef1..eef1b8406 100644
--- a/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
+++ b/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
@@ -958,18 +958,6 @@ public function test( $post_id, $emails ) {
 			);
 		}
 		try {
-
-			$sync_result = $this->sync( get_post( $post_id ) );
-			if ( ! $sync_result ) {
-				return new WP_Error(
-					'newspack_newsletters_mailchimp_error',
-					__( 'Unable to synchronize with Mailchimp.', 'newspack-newsletters' )
-				);
-			}
-			if ( is_wp_error( $sync_result ) ) {
-				return $sync_result;
-			}
-
 			$mc      = new Mailchimp( $this->api_key() );
 			$payload = [
 				'test_emails' => $emails,
diff --git a/src/editor/mjml/index.js b/src/editor/mjml/index.js
index 64faee7be..95d8d4ed9 100644
--- a/src/editor/mjml/index.js
+++ b/src/editor/mjml/index.js
@@ -5,31 +5,17 @@
  */
 import apiFetch from '@wordpress/api-fetch';
 import { useSelect, useDispatch } from '@wordpress/data';
-import { useEffect, useRef, useState } from '@wordpress/element';
-import { refreshEmailHtml } from '../../newsletter-editor/utils';
+import { useEffect } from '@wordpress/element';
+import { refreshEmailHtml, usePrevious } from '../../newsletter-editor/utils';
 
 /**
  * Internal dependencies
  */
-import { fetchNewsletterData, fetchSyncErrors, updateNewsletterDataError } from '../../newsletter-editor/store';
+import { fetchNewsletterData, fetchSyncErrors, updateNewsletterDataError, updateIsRefreshingHtml, useIsRefreshingHtml } from '../../newsletter-editor/store';
 import { getServiceProvider } from '../../service-providers';
 
-/**
- * Custom hook for fetching the prior value of a prop.
- *
- * @param {*} value The prop to track.
- * @return {*} The prior value of the prop.
- */
-const usePrevProp = value => {
-	const ref = useRef();
-	useEffect( () => {
-		ref.current = value;
-	}, [ value ] );
-	return ref.current;
-};
-
 function MJML() {
-	const [ isRefreshingHTML, setIsRefreshingHTML ] = useState( false );
+	const isRefreshingHTML = useIsRefreshingHtml();
 	const {
 		saveSucceeded,
 		isPublishing,
@@ -82,7 +68,7 @@ function MJML() {
 	}, [ isAutosaveLocked ] );
 
 	// After the post is successfully saved, refresh the email HTML.
-	const wasSaving = usePrevProp( isSaving );
+	const wasSaving = usePrevious( isSaving );
 	const { name: serviceProviderName } = getServiceProvider();
 	const { supported_esps: supportedESPs } = newspack_email_editor_data || [];
 	const isSupportedESP = serviceProviderName && 'manual' !== serviceProviderName && supportedESPs?.includes( serviceProviderName );
@@ -97,7 +83,7 @@ function MJML() {
 			! isSent &&
 			saveSucceeded
 		) {
-			setIsRefreshingHTML( true );
+			updateIsRefreshingHtml( true );
 			lockPostSaving( 'newspack-newsletters-refresh-html' );
 			refreshEmailHtml( postId, postTitle, postContent )
 				.then( refreshedHtml => {
@@ -125,7 +111,7 @@ function MJML() {
 				} )
 				.finally( () => {
 					unlockPostSaving( 'newspack-newsletters-refresh-html' );
-					setIsRefreshingHTML( false );
+					updateIsRefreshingHtml( false );
 				} );
 		}
 	}, [ isSaving, isAutosaving ] );
diff --git a/src/newsletter-editor/store.js b/src/newsletter-editor/store.js
index 4095a793d..53a6e81a6 100644
--- a/src/newsletter-editor/store.js
+++ b/src/newsletter-editor/store.js
@@ -28,7 +28,9 @@ export const STORE_NAMESPACE = 'newspack/newsletters';
 
 const DEFAULT_STATE = {
 	isRetrieving: false,
+	isRefreshingHtml: false,
 	newsletterData: {},
+	shouldSendTest: false,
 	error: null,
 };
 const createAction = type => payload => ( { type, payload } );
@@ -36,6 +38,8 @@ const reducer = ( state = DEFAULT_STATE, { type, payload = {} } ) => {
 	switch ( type ) {
 		case 'SET_IS_RETRIEVING':
 			return { ...state, isRetrieving: payload };
+		case 'SET_IS_REFRESHING_HTML':
+			return { ...state, isRefreshingHtml: payload };
 		case 'SET_DATA':
 			const updatedNewsletterData = { ...state.newsletterData, ...payload };
 			return { ...state, newsletterData: updatedNewsletterData };
@@ -49,12 +53,14 @@ const reducer = ( state = DEFAULT_STATE, { type, payload = {} } ) => {
 const actions = {
 	// Regular actions.
 	setIsRetrieving: createAction( 'SET_IS_RETRIEVING' ),
+	setIsRefreshingHtml: createAction( 'SET_IS_REFRESHING_HTML' ),
 	setData: createAction( 'SET_DATA' ),
 	setError: createAction( 'SET_ERROR' ),
 };
 
 const selectors = {
 	getIsRetrieving: state => state.isRetrieving,
+	getIsRefreshingHtml: state => state.isRefreshingHtml,
 	getData: state => state.newsletterData || {},
 	getError: state => state.error,
 };
@@ -74,6 +80,12 @@ export const useIsRetrieving = () =>
 		select( STORE_NAMESPACE ).getIsRetrieving()
 	);
 
+// Hook to use the refresh HTML status from any editor component.
+export const useIsRefreshingHtml = () =>
+	useSelect( select =>
+		select( STORE_NAMESPACE ).getIsRefreshingHtml()
+	);
+
 // Hook to use the newsletter data from any editor component.
 export const useNewsletterData = () =>
 	useSelect( select =>
@@ -86,10 +98,15 @@ export const useNewsletterDataError = () =>
 		select( STORE_NAMESPACE ).getError()
 	);
 
+
 // Dispatcher to update retrieval status in the store.
 export const updateIsRetrieving = isRetrieving =>
 	dispatch( STORE_NAMESPACE ).setIsRetrieving( isRetrieving );
 
+// Dispatcher to update refreshing HTML status in the store.
+export const updateIsRefreshingHtml = isRetrieving =>
+	dispatch( STORE_NAMESPACE ).setIsRefreshingHtml( isRetrieving );
+
 // Dispatcher to update newsletter data in the store.
 export const updateNewsletterData = data =>
 	dispatch( STORE_NAMESPACE ).setData( data );
diff --git a/src/newsletter-editor/testing/index.js b/src/newsletter-editor/testing/index.js
index b287f0548..2a480aafd 100644
--- a/src/newsletter-editor/testing/index.js
+++ b/src/newsletter-editor/testing/index.js
@@ -5,15 +5,15 @@ import apiFetch from '@wordpress/api-fetch';
 import { __ } from '@wordpress/i18n';
 import { withSelect, withDispatch } from '@wordpress/data';
 import { compose } from '@wordpress/compose';
-import { useState, Fragment } from '@wordpress/element';
+import { useEffect, useState, Fragment } from '@wordpress/element';
 import { Button, Spinner, TextControl } from '@wordpress/components';
-import { hasValidEmail } from '../utils';
+import { hasValidEmail , usePrevious } from '../utils';
 
 /**
  * Internal dependencies
  */
 import withApiHandler from '../../components/with-api-handler';
-import { useNewsletterData } from '../store';
+import { useIsRefreshingHtml, useNewsletterData } from '../store';
 import './style.scss';
 
 const serviceProvider =
@@ -43,16 +43,20 @@ export default compose( [
 		disabled,
 		inlineNotifications,
 	} ) => {
+		const isRefreshingHtml = useIsRefreshingHtml();
+		const wasRefreshingHtml = usePrevious( isRefreshingHtml );
+		const [ shouldSendTest, setShouldSendTest ] = useState( false );
 		const [ localInFlight, setLocalInFlight ] = useState( false );
 		const [ localMessage, setLocalMessage ] = useState( '' );
 		const { supports_multiple_test_recipients: supportsMultipleTestEmailRecipients } = useNewsletterData();
-		const sendTestEmail = async () => {
-			if ( inlineNotifications ) {
-				setLocalInFlight( true );
-			} else {
-				setInFlightForAsync();
+
+		useEffect( () => {
+			if ( wasRefreshingHtml && ! isRefreshingHtml && shouldSendTest ) {
+				sendTestEmail();
 			}
-			await savePost();
+		}, [ isRefreshingHtml ] );
+
+		const sendTestEmail = async () => {
 			const params = {
 				path: `/newspack-newsletters/v1/${ serviceProvider }/${ postId }/test`,
 				data: {
@@ -74,12 +78,24 @@ export default compose( [
 					} )
 					.finally( () => {
 						setLocalInFlight( false );
+						setShouldSendTest( false );
 					} );
 			} else {
-				apiFetchWithErrorHandling( params );
+				await apiFetchWithErrorHandling( params );
+				setShouldSendTest( false );
 			}
 		};
 
+		const triggerSave = async () => {
+			if ( inlineNotifications ) {
+				setLocalInFlight( true );
+			} else {
+				setInFlightForAsync();
+			}
+			await savePost();
+			setShouldSendTest( true );
+		};
+
 		return (
 			<Fragment>
 				<TextControl
@@ -96,7 +112,7 @@ export default compose( [
 				<div className="newspack-newsletters__testing-controls">
 					<Button
 						variant="secondary"
-						onClick={ sendTestEmail }
+						onClick={ triggerSave }
 						disabled={ disabled || localInFlight || inFlight || ! hasValidEmail( testEmail ) }
 					>
 						{ inFlight || localInFlight

From 44c1b12d54773a0badbaf462175ed42185e70839 Mon Sep 17 00:00:00 2001
From: Derrick Koo <dkoo@users.noreply.github.com>
Date: Tue, 29 Oct 2024 14:14:31 -0600
Subject: [PATCH 4/8] fix(mailchimp): avoid duplicate audiences in Mailchimp
 UIs (#1685)

* fix(mailchimp): avoid duplicate audiences in Mailchimp UIs

* fix: ensure sublists are still nested under their audiences
---
 .../mailchimp/class-newspack-newsletters-mailchimp.php   | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php b/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
index eef1b8406..55ff4cee1 100644
--- a/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
+++ b/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
@@ -618,14 +618,15 @@ public function retrieve( $post_id ) {
 	 * @return array|WP_Error List of subscription lists or error.
 	 */
 	public function get_lists( $audiences_only = false ) {
-		$lists = Newspack_Newsletters_Mailchimp_Cached_Data::get_lists();
-		if ( $audiences_only || is_wp_error( $lists ) ) {
-			return $lists;
+		$audiences = Newspack_Newsletters_Mailchimp_Cached_Data::get_lists();
+		if ( $audiences_only || is_wp_error( $audiences ) ) {
+			return $audiences;
 		}
+		$lists = [];
 
 		// In addition to Audiences, we also automatically fetch all groups and tags and offer them as Subscription Lists.
 		// Build the final list inside the loop so groups are added after the list they belong to and we can then represent the hierarchy in the UI.
-		foreach ( $lists as $list ) {
+		foreach ( $audiences as $list ) {
 
 			$lists[]        = $list;
 			$all_categories = Newspack_Newsletters_Mailchimp_Cached_Data::get_interest_categories( $list['id'] );

From 72897f206058af5c63afdd7fd2ab9946a9a0b923 Mon Sep 17 00:00:00 2001
From: Laurel <laurel.fulford@automattic.com>
Date: Thu, 31 Oct 2024 16:13:46 -0700
Subject: [PATCH 5/8] fix: move Preview, Send buttons to match Publish button
 location (#1689)

---
 src/newsletter-editor/editor/index.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/newsletter-editor/editor/index.js b/src/newsletter-editor/editor/index.js
index 248586ded..f84ec5fe0 100644
--- a/src/newsletter-editor/editor/index.js
+++ b/src/newsletter-editor/editor/index.js
@@ -102,7 +102,7 @@ const Editor = compose( [
 			const publishButton = document.getElementsByClassName(
 				'editor-post-publish-button__button'
 			)[ 0 ];
-			publishButton.parentNode.insertBefore( publishEl, publishButton );
+			publishButton.parentNode.insertBefore( publishEl, publishButton.nextSibling );
 		}, [] );
 
 		// Set color palette option.

From 4b42e3df26beef25d60673f2c6c59e1aa50e46ad Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Mon, 4 Nov 2024 12:58:41 +0000
Subject: [PATCH 6/8] chore(deps-dev): bump @wordpress/browserslist-config

Bumps [@wordpress/browserslist-config](https://github.com/WordPress/gutenberg/tree/HEAD/packages/browserslist-config) from 6.10.0 to 6.11.0.
- [Release notes](https://github.com/WordPress/gutenberg/releases)
- [Changelog](https://github.com/WordPress/gutenberg/blob/trunk/packages/browserslist-config/CHANGELOG.md)
- [Commits](https://github.com/WordPress/gutenberg/commits/@wordpress/browserslist-config@6.11.0/packages/browserslist-config)

---
updated-dependencies:
- dependency-name: "@wordpress/browserslist-config"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
---
 package-lock.json | 8 ++++----
 package.json      | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/package-lock.json b/package-lock.json
index 777f2061c..59be8ec7f 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -16,7 +16,7 @@
 			},
 			"devDependencies": {
 				"@rushstack/eslint-patch": "^1.10.4",
-				"@wordpress/browserslist-config": "^6.10.0",
+				"@wordpress/browserslist-config": "^6.11.0",
 				"eslint": "^8.57.0",
 				"lint-staged": "^15.2.10",
 				"newspack-scripts": "^5.5.2",
@@ -6197,9 +6197,9 @@
 			}
 		},
 		"node_modules/@wordpress/browserslist-config": {
-			"version": "6.10.0",
-			"resolved": "https://registry.npmjs.org/@wordpress/browserslist-config/-/browserslist-config-6.10.0.tgz",
-			"integrity": "sha512-X5BG4xWvr1Qq9S2x5ERCF7V4bpa24zbj8cWYbIJaGiCfi6vp6dFI1SbvuZPXfKyThyytTVYBvEIr6CSm6G8fuQ==",
+			"version": "6.11.0",
+			"resolved": "https://registry.npmjs.org/@wordpress/browserslist-config/-/browserslist-config-6.11.0.tgz",
+			"integrity": "sha512-wUDbJ3x7c8iMZLtwo+7VlWZ/vDc47PDW2eSAKW18RrQBSTdaNmWi4qiyFYi7Ye2XkyfUd2gp71MTJjZi6n/V2A==",
 			"dev": true,
 			"engines": {
 				"node": ">=18.12.0",
diff --git a/package.json b/package.json
index 0c6c9611d..dce92eb38 100644
--- a/package.json
+++ b/package.json
@@ -46,7 +46,7 @@
 	},
 	"devDependencies": {
 		"@rushstack/eslint-patch": "^1.10.4",
-		"@wordpress/browserslist-config": "^6.10.0",
+		"@wordpress/browserslist-config": "^6.11.0",
 		"eslint": "^8.57.0",
 		"lint-staged": "^15.2.10",
 		"newspack-scripts": "^5.5.2",

From f178b2355cd1198531972eee36939de70524eb93 Mon Sep 17 00:00:00 2001
From: leogermani <leogermani@automattic.com>
Date: Mon, 4 Nov 2024 18:19:05 -0300
Subject: [PATCH 7/8] fix: dont html encode ampersands in subject (#1686)

* fix: dont html encode amperstands in subject

* fix: decode HTML entities in both editor JS and sync payloads

---------

Co-authored-by: dkoo <derrick.koo@automattic.com>
Co-authored-by: Derrick Koo <dkoo@users.noreply.github.com>
---
 ...ass-newspack-newsletters-active-campaign.php |  2 +-
 ...ss-newspack-newsletters-campaign-monitor.php |  4 ++--
 ...ss-newspack-newsletters-constant-contact.php |  2 +-
 .../class-newspack-newsletters-mailchimp.php    |  2 +-
 src/newsletter-editor/sidebar/index.js          | 17 +++++++++++++++--
 5 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/includes/service-providers/active_campaign/class-newspack-newsletters-active-campaign.php b/includes/service-providers/active_campaign/class-newspack-newsletters-active-campaign.php
index 37292a36f..99d572bba 100644
--- a/includes/service-providers/active_campaign/class-newspack-newsletters-active-campaign.php
+++ b/includes/service-providers/active_campaign/class-newspack-newsletters-active-campaign.php
@@ -1050,7 +1050,7 @@ public function sync( $post ) {
 				'p[' . $send_list_id . ']' => 1,
 				'fromemail'                => $from_email,
 				'fromname'                 => $from_name,
-				'subject'                  => $post->post_title,
+				'subject'                  => html_entity_decode( $post->post_title ),
 			],
 			$message_data
 		);
diff --git a/includes/service-providers/campaign_monitor/class-newspack-newsletters-campaign-monitor.php b/includes/service-providers/campaign_monitor/class-newspack-newsletters-campaign-monitor.php
index 0d3ddbe38..5d09020ac 100644
--- a/includes/service-providers/campaign_monitor/class-newspack-newsletters-campaign-monitor.php
+++ b/includes/service-providers/campaign_monitor/class-newspack-newsletters-campaign-monitor.php
@@ -490,8 +490,8 @@ public function format_campaign_args( $post_id ) {
 		}
 
 		$args = [
-			'Subject'   => get_the_title( $post_id ),
-			'Name'      => get_the_title( $post_id ) . ' ' . gmdate( 'h:i:s A' ), // Name must be unique.
+			'Subject'   => html_entity_decode( get_the_title( $post_id ) ),
+			'Name'      => html_entity_decode( get_the_title( $post_id ) ) . ' ' . gmdate( 'h:i:s A' ), // Name must be unique.
 			'FromName'  => get_post_meta( $post_id, 'senderName', true ),
 			'FromEmail' => get_post_meta( $post_id, 'senderEmail', true ),
 			'ReplyTo'   => get_post_meta( $post_id, 'senderEmail', true ),
diff --git a/includes/service-providers/constant_contact/class-newspack-newsletters-constant-contact.php b/includes/service-providers/constant_contact/class-newspack-newsletters-constant-contact.php
index 08fb4c9c1..07f26b9fc 100644
--- a/includes/service-providers/constant_contact/class-newspack-newsletters-constant-contact.php
+++ b/includes/service-providers/constant_contact/class-newspack-newsletters-constant-contact.php
@@ -655,7 +655,7 @@ public function sender( $post_id, $from_name, $reply_to ) {
 			$activity = [
 				'format_type'      => 5,
 				'email_content'    => $content,
-				'subject'          => $post->post_title,
+				'subject'          => html_entity_decode( $post->post_title ),
 				'contact_list_ids' => $campaign->activity->contact_list_ids,
 				'from_name'        => $from_name,
 				'from_email'       => $reply_to,
diff --git a/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php b/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
index 55ff4cee1..e4393204e 100644
--- a/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
+++ b/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
@@ -1004,7 +1004,7 @@ public function get_sync_payload( $post ) {
 			'type'         => 'regular',
 			'content_type' => 'template',
 			'settings'     => [
-				'subject_line' => $post->post_title,
+				'subject_line' => html_entity_decode( $post->post_title ),
 				'title'        => $this->get_campaign_name( $post ),
 			],
 		];
diff --git a/src/newsletter-editor/sidebar/index.js b/src/newsletter-editor/sidebar/index.js
index c93a997d2..4421accd6 100644
--- a/src/newsletter-editor/sidebar/index.js
+++ b/src/newsletter-editor/sidebar/index.js
@@ -4,7 +4,7 @@
 import { __ } from '@wordpress/i18n';
 import { compose } from '@wordpress/compose';
 import { withSelect, withDispatch } from '@wordpress/data';
-import { useEffect } from '@wordpress/element';
+import { useEffect, useRef, useState } from '@wordpress/element';
 import { Button, Notice, Spinner, TextControl, TextareaControl } from '@wordpress/components';
 
 /**
@@ -40,11 +40,24 @@ const Sidebar = ( {
 	stringifiedCampaignDefaults,
 	postId,
 } ) => {
+	const [ plainTextTitle, setPlainTextTitle ] = useState( title );
 	const isRetrieving = useIsRetrieving();
 	const newsletterData = useNewsletterData();
 	const newsletterDataError = useNewsletterDataError();
 	const campaign = newsletterData?.campaign;
 	const updateMeta = ( toUpdate ) => editPost( { meta: toUpdate } );
+	const entityConverter = useRef( null );
+
+	// Create a temp textarea element that we can use to convert HTML entities like &amp; to unicode characters.
+	useEffect( () => {
+		if ( entityConverter.current ) {
+			entityConverter.current.innerHTML = title;
+			setPlainTextTitle( entityConverter.current.value );
+		} else {
+			entityConverter.current = document.createElement( 'textarea' );
+		}
+		return () => entityConverter?.current?.remove && entityConverter.current.remove(); // Clean up temp element from DOM on unmount.
+	}, [ title ] );
 
 	// Reconcile stored campaign data with data fetched from ESP.
 	useEffect( () => {
@@ -184,7 +197,7 @@ const Sidebar = ( {
 			<TextControl
 				label={ __( 'Subject', 'newspack-newsletters' ) }
 				className="newspack-newsletters__subject-textcontrol"
-				value={ title }
+				value={ plainTextTitle }
 				disabled={ inFlight }
 				onChange={ value => editPost( { title: value } ) }
 			/>

From d3f1f37a110d663cdd5fb31b8807f8e443eee3b5 Mon Sep 17 00:00:00 2001
From: Derrick Koo <dkoo@users.noreply.github.com>
Date: Tue, 5 Nov 2024 10:48:55 -0700
Subject: [PATCH 8/8] fix: avoid Redux usage and related errors in
 non-newsletter email editors (#1688)

---
 src/components/send-button/index.js |   3 +-
 src/editor/mjml/index.js            | 104 ++++++++++++++++++----------
 src/newsletter-editor/index.js      |   2 +-
 src/newsletter-editor/utils.js      |  30 --------
 4 files changed, 72 insertions(+), 67 deletions(-)

diff --git a/src/components/send-button/index.js b/src/components/send-button/index.js
index 4ff6bc0c8..e8df44af2 100644
--- a/src/components/send-button/index.js
+++ b/src/components/send-button/index.js
@@ -18,8 +18,9 @@ import { get } from 'lodash';
  * Internal dependencies
  */
 import { getServiceProvider } from '../../service-providers';
-import { refreshEmailHtml, validateNewsletter } from '../../newsletter-editor/utils';
+import { validateNewsletter } from '../../newsletter-editor/utils';
 import { useNewsletterData } from '../../newsletter-editor/store';
+import { refreshEmailHtml } from '../../editor/mjml';
 import './style.scss';
 
 function PreviewHTML() {
diff --git a/src/editor/mjml/index.js b/src/editor/mjml/index.js
index 95d8d4ed9..0defa8cfc 100644
--- a/src/editor/mjml/index.js
+++ b/src/editor/mjml/index.js
@@ -3,19 +3,52 @@
 /**
  * WordPress dependencies
  */
+import { __ } from '@wordpress/i18n';
 import apiFetch from '@wordpress/api-fetch';
 import { useSelect, useDispatch } from '@wordpress/data';
 import { useEffect } from '@wordpress/element';
-import { refreshEmailHtml, usePrevious } from '../../newsletter-editor/utils';
+import { usePrevious } from '../../newsletter-editor/utils';
 
 /**
  * Internal dependencies
  */
-import { fetchNewsletterData, fetchSyncErrors, updateNewsletterDataError, updateIsRefreshingHtml, useIsRefreshingHtml } from '../../newsletter-editor/store';
 import { getServiceProvider } from '../../service-providers';
+import {
+	fetchNewsletterData,
+	fetchSyncErrors,
+	updateIsRefreshingHtml,
+} from '../../newsletter-editor/store';
+
+/**
+ * External dependencies
+ */
+import mjml2html from 'mjml-browser';
+
+/**
+ * Refresh the email-compliant HTML for a post.
+ *
+ * @param {number} postId      The current post ID.
+ * @param {string} postTitle   The current post title.
+ * @param {string} postContent The current post content.
+ * @return {Promise<string>} The refreshed email HTML.
+ */
+export const refreshEmailHtml = async ( postId, postTitle, postContent ) => {
+	const mjml = await apiFetch( {
+		path: `/newspack-newsletters/v1/post-mjml`,
+		method: 'POST',
+		data: {
+			post_id: postId,
+			title: postTitle,
+			content: postContent,
+		},
+	} );
+
+	// Once received MJML markup, convert it to email-compliant HTML and save as post meta.
+	const { html } = mjml2html( mjml, { keepComments: false, minify: true } );
+	return html;
+};
 
 function MJML() {
-	const isRefreshingHTML = useIsRefreshingHtml();
 	const {
 		saveSucceeded,
 		isPublishing,
@@ -54,7 +87,7 @@ function MJML() {
 			isAutosaveLocked: isPostAutosavingLocked(),
 		};
 	} );
-
+	const { createNotice } = useDispatch( 'core/notices' );
 	const { lockPostAutosaving, lockPostSaving, unlockPostSaving, editPost } = useDispatch(
 		'core/editor'
 	);
@@ -79,42 +112,43 @@ function MJML() {
 			! isSaving &&
 			! isAutosaving &&
 			! isPublishing &&
-			! isRefreshingHTML &&
 			! isSent &&
 			saveSucceeded
 		) {
-			updateIsRefreshingHtml( true );
-			lockPostSaving( 'newspack-newsletters-refresh-html' );
-			refreshEmailHtml( postId, postTitle, postContent )
-				.then( refreshedHtml => {
-					updateMetaValue( newspack_email_editor_data.email_html_meta, refreshedHtml );
-					return apiFetch( {
-						data: { meta: { [ newspack_email_editor_data.email_html_meta ]: refreshedHtml } },
-						method: 'POST',
-						path: `/wp/v2/${ postType }/${ postId }`,
-					} );
-				} ).then( () => {
-					// Rehydrate ESP newsletter data after completing sync.
-					if ( isSupportedESP ) {
-						return fetchNewsletterData( postId );
-					}
-					return true;
-				} ).then ( () => {
-					// Check for sync errors after refreshing the HTML.
-					if ( isSupportedESP ) {
-						return fetchSyncErrors( postId );
-					}
-					return true;
-				} )
-				.catch( e => {
-					updateNewsletterDataError( e );
-				} )
-				.finally( () => {
-					unlockPostSaving( 'newspack-newsletters-refresh-html' );
-					updateIsRefreshingHtml( false );
-				} );
+			refreshHtml();
 		}
 	}, [ isSaving, isAutosaving ] );
+
+	const refreshHtml = async () => {
+		try {
+			lockPostSaving( 'newspack-newsletters-refresh-html' );
+			if ( isSupportedESP ) {
+				updateIsRefreshingHtml( true );
+			}
+			const refreshedHtml = await refreshEmailHtml( postId, postTitle, postContent );
+			updateMetaValue( newspack_email_editor_data.email_html_meta, refreshedHtml );
+
+			// Save the refreshed HTML to post meta.
+			await apiFetch( {
+				data: { meta: { [ newspack_email_editor_data.email_html_meta ]: refreshedHtml } },
+				method: 'POST',
+				path: `/wp/v2/${ postType }/${ postId }`,
+			} );
+
+			// Rehydrate ESP newsletter data after completing sync.
+			if ( isSupportedESP ) {
+				await fetchNewsletterData( postId );
+				await fetchSyncErrors( postId );
+				updateIsRefreshingHtml( false );
+			}
+			unlockPostSaving( 'newspack-newsletters-refresh-html' );
+		} catch ( e ) {
+			createNotice( 'error', e?.message || __( 'Error refreshing email HTML.', 'newspack-newsletters' ), {
+				id: 'newspack-newsletters-mjml-error',
+				isDismissible: true,
+			} );
+		}
+	}
 }
 
 export default MJML;
diff --git a/src/newsletter-editor/index.js b/src/newsletter-editor/index.js
index 1cd70f6b6..93866232e 100644
--- a/src/newsletter-editor/index.js
+++ b/src/newsletter-editor/index.js
@@ -90,7 +90,7 @@ function NewsletterEdit( { apiFetchWithErrorHandling, setInFlightForAsync, inFli
 	// Handle error messages from retrieve/sync requests with connected ESP.
 	useEffect( () => {
 		if ( newsletterDataError ) {
-			createNotice( 'error', newsletterDataError?.message || __( 'Error communicating with service provider.', 'newspack-newseltters' ), {
+			createNotice( 'error', newsletterDataError?.message || __( 'Error communicating with service provider.', 'newspack-newsletters' ), {
 				id: 'newspack-newsletters-newsletter-data-error',
 				isDismissible: true,
 			} );
diff --git a/src/newsletter-editor/utils.js b/src/newsletter-editor/utils.js
index db306da37..137dd4bd2 100644
--- a/src/newsletter-editor/utils.js
+++ b/src/newsletter-editor/utils.js
@@ -3,15 +3,9 @@
 /**
  * WordPress dependencies
  */
-import apiFetch from '@wordpress/api-fetch';
 import { useEffect, useRef } from '@wordpress/element';
 import { __ } from '@wordpress/i18n';
 
-/**
- * External dependencies
- */
-import mjml2html from 'mjml-browser';
-
 /**
  * Internal dependencies
  */
@@ -61,30 +55,6 @@ export const validateNewsletter = ( meta = {} ) => {
  */
 export const hasValidEmail = string => /\S+@\S+/.test( string );
 
-/**
- * Refresh the email-compliant HTML for a post.
- *
- * @param {number} postId      The current post ID.
- * @param {string} postTitle   The current post title.
- * @param {string} postContent The current post content.
- * @return {Promise<string>} The refreshed email HTML.
- */
-export const refreshEmailHtml = async ( postId, postTitle, postContent ) => {
-	const mjml = await apiFetch( {
-		path: `/newspack-newsletters/v1/post-mjml`,
-		method: 'POST',
-		data: {
-			post_id: postId,
-			title: postTitle,
-			content: postContent,
-		},
-	} );
-
-	// Once received MJML markup, convert it to email-compliant HTML and save as post meta.
-	const { html } = mjml2html( mjml, { keepComments: false, minify: true } );
-	return html;
-};
-
 /**
  * Custom hook to fetch a previous state or prop value.
  *