fix(mailchimp): fatal error with invalid API keys (#1690)

* fix(mailchimp): fatal error with invalid API keys * fix: 400 is an error status code too * fix: return an error if cache refresh fails * Update includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php Co-authored-by: Adam Cassis <[email protected]> * fix: php lint * fix: clear cache on API key change * fix: properly show errors from trying to refresh cache * fix: add key for all lists error * fix: avoid PHP deprecated warning --------- Co-authored-by: Adam Cassis <[email protected]>
Automattic · Nov 11, 2024 · b7fd7e6 · b7fd7e6
1 parent 01dd9ac
commit b7fd7e6
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 7 deletions.
diff --git a/includes/class-newspack-newsletters-settings.php b/includes/class-newspack-newsletters-settings.php
@@ -287,6 +287,11 @@ private static function render_oauth_authorization() {
 	private static function render_lists_table() {
 		$lists = Newspack_Newsletters_Subscription::get_lists();
 		if ( is_wp_error( $lists ) || empty( $lists ) ) {
+			?>
+			<div class="notice notice-error">
+				<p><?php echo esc_html( is_wp_error( $lists ) ? $lists->get_error_message() : __( 'No list data found.', 'newspack-newsletters' ) ); ?></p>
+			</div>
+			<?php
 			return;
 		}
 		?>

diff --git a/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp-cached-data.php b/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp-cached-data.php
@@ -82,6 +82,9 @@ public static function init() {
 		add_action( 'wp_ajax_' . self::AJAX_ACTION, [ __CLASS__, 'handle_dispatch_refresh' ] );
 		add_action( 'wp_ajax_nopriv_' . self::AJAX_ACTION, [ __CLASS__, 'handle_dispatch_refresh' ] );
 
+		// Invalidate all cached data if API key changes.
+		add_action( 'update_option_newspack_mailchimp_api_key', [ __CLASS__, 'invalidate_cache' ] );
+
 		add_action( self::CRON_HOOK, [ __CLASS__, 'handle_cron' ] );
 		add_filter( 'cron_schedules', [ __CLASS__, 'add_cron_interval' ] ); // phpcs:ignore
 
@@ -285,10 +288,14 @@ private static function update_cache( $list_id, $data ) {
 	/**
 	 * Clears the cache errors for a given list
 	 *
-	 * @param string $list_id The List ID.
+	 * @param string|null $list_id The List ID, or null for all lists.
 	 * @return void
 	 */
-	private static function clear_errors( $list_id ) {
+	private static function clear_errors( $list_id = null ) {
+		if ( ! $list_id ) {
+			delete_option( self::ERRORS_OPTION );
+			return;
+		}
 		$errors = get_option( self::ERRORS_OPTION, [] );
 		if ( isset( $errors[ $list_id ] ) ) {
 			unset( $errors[ $list_id ] );
@@ -300,11 +307,22 @@ private static function clear_errors( $list_id ) {
 	/**
 	 * Stores the last error for a given list, if the cache is older than self::SURFACE_ERRORS_AFTER
 	 *
-	 * @param string $list_id The List ID.
-	 * @param string $error The error message.
+	 * @param string|null $list_id The List ID, or null for all lists.
+	 * @param string      $error The error message.
 	 */
-	private static function maybe_add_error( $list_id, $error ) {
-		Newspack_Newsletters_Logger::log( 'Mailchimp cache: handling error while fetching cache for list ' . $list_id );
+	private static function maybe_add_error( $list_id = null, $error = '' ) {
+		Newspack_Newsletters_Logger::log(
+			sprintf(
+				'Mailchimp cache: handling error while fetching cache for %s',
+				$list_id ? 'list ' . $list_id : 'all lists'
+			)
+		);
+		if ( ! $list_id ) {
+			$list_id = 'lists';
+		}
+		if ( ! $error ) {
+			$error = __( 'Unknown error', 'newspack_newsletters' );
+		}
 		$cache_date = get_option( self::get_cache_date_key( $list_id ) );
 		if ( $cache_date && ( time() - $cache_date ) > self::SURFACE_ERRORS_AFTER ) {
 			$errors             = get_option( self::ERRORS_OPTION, [] );
@@ -518,6 +536,16 @@ private static function refresh_cached_data( $list_id ) {
 		}
 	}
 
+	/**
+	 * Invalidate cached data by clearing the cache date key for all lists.
+	 */
+	public static function invalidate_cache() {
+		Newspack_Newsletters_Logger::log( 'Mailchimp cache: Invalidating cached data' );
+		delete_option( self::get_cache_date_key() );
+		delete_option( self::get_lists_cache_key() );
+		self::clear_errors();
+	}
+
 	/**
 	 * Handles the cron job and triggers the async requests to refresh the cache for all lists
 	 *
@@ -529,6 +557,7 @@ public static function handle_cron() {
 			$lists = self::fetch_lists(); // Force a cache refresh.
 		} catch ( Exception $e ) {
 			Newspack_Newsletters_Logger::log( 'Mailchimp cache: Error refreshing lists cache: ' . $e->getMessage() );
+			self::maybe_add_error( null, $e->getMessage() );
 			return;
 		}
 

diff --git a/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php b/includes/service-providers/mailchimp/class-newspack-newsletters-mailchimp.php
@@ -1426,7 +1426,7 @@ public function validate( $result, $preferred_error = null ) {
 				throw new Exception( esc_html__( 'A Mailchimp error has occurred.', 'newspack-newsletters' ) );
 			}
 		}
-		if ( ! empty( $result['status'] ) && in_array( $result['status'], [ 400, 404 ] ) ) {
+		if ( ! empty( $result['status'] ) && (int) $result['status'] >= 400 ) {
 			if ( $preferred_error && ! Newspack_Newsletters::debug_mode() ) {
 				if ( ! empty( $result['detail'] ) ) {
 					$preferred_error .= ' ' . $result['detail'];