fix: fetching and display of sync errors after saving (#1670)

* fix: fetching and display of sync errors after saving * refactor: avoid nested promise handlers * chore: not sure if needed, but make sure promises are resolved
Automattic · Oct 18, 2024 · cdf2e24 · cdf2e24
1 parent 0ae5cc5
commit cdf2e24
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 20 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
@@ -1066,7 +1066,7 @@ public function sync( $post ) {
 		// Retrieve and store campaign data.
 		$data = $this->retrieve( $post->ID, true );
 		if ( is_wp_error( $data ) ) {
-			set_transient( $transient_name, __( 'Error syncing with ESP. ', 'newspack-newsletters' ) . $data->get_error_message(), 45 );
+			set_transient( $transient_name, __( 'ActiveCampaign sync error: ', 'newspack-newsletters' ) . $data->get_error_message(), 45 );
 			return $data;
 		} else {
 			$data = array_merge( $data, $sync_data );

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
@@ -884,7 +884,7 @@ public function sync( $post ) {
 
 			return $campaign_result;
 		} catch ( Exception $e ) {
-			set_transient( $transient_name, __( 'Error syncing with ESP. ', 'newspack-newsletters' ) . $e->getMessage(), 45 );
+			set_transient( $transient_name, 'Constant Contact campaign sync error: ' . wp_specialchars_decode( $e->getMessage(), ENT_QUOTES ), 45 );
 			return new WP_Error( 'newspack_newsletters_constant_contact_error', $e->getMessage() );
 		}
 	}

diff --git a/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php b/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
@@ -13,6 +13,8 @@
 use Newspack\Newsletters\Send_Lists;
 use Newspack\Newsletters\Send_List;
 
+use function cli\err;
+
 /**
  * Main Newspack Newsletters Class.
  */
@@ -1146,13 +1148,11 @@ public function sync( $post ) {
 
 			if ( $mc_campaign_id ) {
 				$campaign_result = $this->validate(
-					$mc->patch( "campaigns/$mc_campaign_id", $payload ),
-					__( 'Error updating existing campaign draft.', 'newspack_newsletters' )
+					$mc->patch( "campaigns/$mc_campaign_id", $payload )
 				);
 			} else {
 				$campaign_result = $this->validate(
-					$mc->post( 'campaigns', $payload ),
-					__( 'Error creating campaign.', 'newspack_newsletters' )
+					$mc->post( 'campaigns', $payload )
 				);
 				$mc_campaign_id  = $campaign_result['id'];
 				update_post_meta( $post->ID, 'mc_campaign_id', $mc_campaign_id );
@@ -1177,7 +1177,7 @@ public function sync( $post ) {
 				'content_result'  => $content_result,
 			];
 		} catch ( Exception $e ) {
-			set_transient( $transient_name, 'Mailchimp: ' . $e->getMessage(), 45 );
+			set_transient( $transient_name, 'Mailchimp campaign sync error: ' . wp_specialchars_decode( $e->getMessage(), ENT_QUOTES ), 45 );
 			return new WP_Error( 'newspack_newsletters_mailchimp_error', $e->getMessage() );
 		}
 	}

diff --git a/src/editor/mjml/index.js b/src/editor/mjml/index.js
@@ -11,7 +11,7 @@ import { refreshEmailHtml } from '../../newsletter-editor/utils';
 /**
  * Internal dependencies
  */
-import { fetchNewsletterData, fetchSyncErrors } from '../../newsletter-editor/store';
+import { fetchNewsletterData, fetchSyncErrors, updateNewsletterDataError } from '../../newsletter-editor/store';
 import { getServiceProvider } from '../../service-providers';
 
 /**
@@ -83,6 +83,10 @@ function MJML() {
 
 	// After the post is successfully saved, refresh the email HTML.
 	const wasSaving = usePrevProp( isSaving );
+	const { name: serviceProviderName } = getServiceProvider();
+	const { supported_esps: supportedESPs } = newspack_email_editor_data || [];
+	const isSupportedESP = serviceProviderName && 'manual' !== serviceProviderName && supportedESPs?.includes( serviceProviderName );
+
 	useEffect( () => {
 		if (
 			wasSaving &&
@@ -103,24 +107,25 @@ function MJML() {
 						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 => {
-					console.warn( e ); // eslint-disable-line no-console
+					updateNewsletterDataError( e );
 				} )
 				.finally( () => {
 					unlockPostSaving( 'newspack-newsletters-refresh-html' );
 					setIsRefreshingHTML( false );
-
-					const { name: serviceProviderName } = getServiceProvider();
-					const { supported_esps: supportedESPs } = newspack_email_editor_data || [];
-					const isSupportedESP = serviceProviderName && 'manual' !== serviceProviderName && supportedESPs?.includes( serviceProviderName );
-					if ( isSupportedESP ) {
-						// Rehydrate ESP newsletter data after completing sync.
-						fetchNewsletterData( postId );
-
-						// Check for sync errors after refreshing the HTML.
-						fetchSyncErrors( postId );
-					}
 				} );
 		}
 	}, [ isSaving, isAutosaving ] );

diff --git a/src/newsletter-editor/store.js b/src/newsletter-editor/store.js
@@ -126,6 +126,7 @@ export const fetchNewsletterData = async postId => {
 		updateNewsletterDataError( error );
 	}
 	updateIsRetrieving( false );
+	return true;
 };
 
 // Dispatcher to fetch any errors from the most recent sync attempt.
@@ -147,6 +148,7 @@ export const fetchSyncErrors = async postId => {
 		updateNewsletterDataError( error );
 	}
 	updateIsRetrieving( false );
+	return true;
 }
 
 // Dispatcher to fetch send lists and sublists from the connected ESP and update the newsletterData in store.