Make the admin notification mechanics contextually aware (of the user and of the admin screen)

changelog.txt

@@ -3,6 +3,7 @@
 = 8.0.0 - xxxx-xx-xx =
 * Fix - Safeguards added to the Subscriptions Totals template used in the My Account area, to guard against fatal errors that could arise in unusual conditions.
 * Fix - Correctly load product names with HTML on the cart and checkout shipping rates.
+* Fix - Improve our admin notice handling to ensure notices are displayed to the intended admin user.
 
 = 7.9.0 - 2025-01-10 =
 * Add - Compatibility with WooCommerce's new preview email feature introduced in 9.6.

includes/admin/wcs-admin-functions.php

@@ -13,59 +13,141 @@
 }
 
 /**
- * Store a message to display via @see wcs_display_admin_notices().
+ * Registers an admin notice to be displayed once to the current (or other specified) user.
  *
- * @param string The message to display
- * @since 1.0.0 - Migrated from WooCommerce Subscriptions v2.0
+ * @see   wcs_display_admin_notices()
+ * @since 1.0.0 Migrated from WooCommerce Subscriptions v2.0.
+ * @since 7.2.0 Added support for specifying the target user and context.
+ *
+ * @param string      $message     The message to display.
+ * @param string      $notice_type Either 'success' or 'error'.
+ * @param int|null    $user_id     The specific user who should see this message. If not specified, defaults to the current user.
+ * @param string|null $screen_id   The screen ID for which the message should be displayed. If not specified, it will show on the next admin page load.
+ *
+ * @return void
  */
-function wcs_add_admin_notice( $message, $notice_type = 'success' ) {
+function wcs_add_admin_notice( $message, $notice_type = 'success', $user_id = null, $screen_id = null ) {
+	$user_id = (int) ( null === $user_id ? get_current_user_id() : $user_id );
+
+	if ( $user_id < 1 ) {
+		wc_get_logger()->warning(
+			sprintf(
+				/* Translators: %1$s: notice type ('success' or 'error'), %2$s: notice text. */
+				'Admin notices can only be added if a user is currently logged in. Attemped (%1$s) notice: "%2$s"',
+				$notice_type,
+				$message
+			),
+			array(
+				'backtrace' => true,
+				'user_id'   => $user_id,
+			)
+		);
+
+		return;
+	}
 
-	$notices = get_transient( '_wcs_admin_notices' );
+	$notices = get_transient( '_wcs_admin_notices_' . $user_id );
 
-	if ( false === $notices ) {
+	if ( ! is_array( $notices ) ) {
 		$notices = array();
 	}
 
-	$notices[ $notice_type ][] = $message;
+	$notices[ $notice_type ][] = array(
+		'message'   => $message,
+		'screen_id' => $screen_id,
+	);
 
-	set_transient( '_wcs_admin_notices', $notices, 60 * 60 );
+	set_transient( '_wcs_admin_notices_' . $user_id, $notices, HOUR_IN_SECONDS );
 }
 
 /**
- * Display any notices added with @see wcs_add_admin_notice()
+ * Display any admin notices added with wcs_add_admin_notice().
+ *
+ * @see   wcs_add_admin_notice()
+ * @since 1.0.0 Migrated from WooCommerce Subscriptions v2.0.
+ * @since 7.2.0 Supports contextual awareness of the user and screen.
  *
- * This method is also hooked to 'admin_notices' to display notices there.
+ * @param bool $clear If the message queue should be cleared after rendering the message(s). Defaults to true.
  *
- * @since 1.0.0 - Migrated from WooCommerce Subscriptions v2.0
+ * @return void
  */
 function wcs_display_admin_notices( $clear = true ) {
+	$user_id = get_current_user_id();
+	$notices = get_transient( '_wcs_admin_notices_' . $user_id );
+
+	if ( ! is_array( $notices ) || empty( $notices ) ) {
+		return;
+	}
 
-	$notices = get_transient( '_wcs_admin_notices' );
+	/**
+	 * Normalizes, sanitizes and outputs the provided notices.
+	 *
+	 * @param array  &$notices The notice data.
+	 * @param string $class    The CSS notice class to be applied (typically 'updated' or 'error').
+	 *
+	 * @return void
+	 */
+	$handle_notices = static function ( &$notices, $class ) {
+		$notice_output = array();
+		$screen_id     = false;
 
-	if ( false !== $notices && ! empty( $notices ) ) {
+		foreach ( $notices as $index => $notice ) {
+			// Ensure the notice data now has the expected shape. If it does not, remove it.
+			if ( ! is_array( $notice ) || ! isset( $notice['message'] ) || ! array_key_exists( 'screen_id', $notice ) ) {
+				unset( $notices[ $index ] );
+				continue;
+			}
 
-		if ( ! empty( $notices['success'] ) ) {
-			array_walk( $notices['success'], 'esc_html' );
-			echo '<div id="moderated" class="updated"><p>' . wp_kses_post( implode( "</p>\n<p>", $notices['success'] ) ) . '</p></div>';
+			// We only need to determine the current screen ID once.
+			if ( false === $screen_id ) {
+				$screen    = get_current_screen();
+				$screen_id = $screen instanceof WP_Screen ? $screen->id : '';
+			}
+
+			// Should the notice display in the current screen context?
+			if ( is_string( $notice['screen_id'] ) && $screen_id !== $notice['screen_id'] ) {
+				continue;
+			}
+
+			$notice_output[] = esc_html( $notice['message'] );
+			unset( $notices[ $index ] );
 		}
 
-		if ( ! empty( $notices['error'] ) ) {
-			array_walk( $notices['error'], 'esc_html' );
-			echo '<div id="moderated" class="error"><p>' . wp_kses_post( implode( "</p>\n<p>", $notices['error'] ) ) . '</p></div>';
+		// $notice_output may be empty if some notices were withheld, due to not matching the screen context.
+		if ( ! empty( $notice_output ) ) {
+			echo '<div id="moderated" class="' . esc_attr( $class ) . '"><p>' . wp_kses_post( implode( "</p>\n<p>", $notice_output ) ) . '</p></div>';
 		}
+	};
+
+	if ( ! empty( $notices['success'] ) ) {
+		$handle_notices( $notices['success'], 'updated' );
+	}
+
+	if ( ! empty( $notices['error'] ) ) {
+		$handle_notices( $notices['error'], 'error' );
+	}
+
+	// Under certain circumstances, the caller may not wish for the rendered messages to be cleared from the queue.
+	if ( false === $clear ) {
+		return;
 	}
 
-	if ( false !== $clear ) {
+	// If all notices were rendered, clear the queue. If only some were rendered, clear what we can.
+	if ( empty( $notices['success'] ) && empty( $notices['error'] ) ) {
 		wcs_clear_admin_notices();
+	} else {
+		set_transient( '_wcs_admin_notices_' . $user_id, $notices, HOUR_IN_SECONDS );
 	}
 }
+
 add_action( 'admin_notices', 'wcs_display_admin_notices' );
 
 /**
  * Delete any admin notices we stored for display later.
  *
- * @since 1.0.0 - Migrated from WooCommerce Subscriptions v2.0
+ * @since 1.0.0 Migrated from WooCommerce Subscriptions v2.0.
+ * @since 7.2.0 Became user aware.
  */
 function wcs_clear_admin_notices() {
-	delete_transient( '_wcs_admin_notices' );
+	delete_transient( '_wcs_admin_notices_' . get_current_user_id() );
 }

includes/upgrades/class-wc-subscriptions-upgrader.php

@@ -129,6 +129,11 @@ public static function upgrade() {
 			wp_unschedule_hook( 'wcs_cleanup_big_logs' );
 		}
 
+		if ( version_compare( self::$active_version, '8.0.0', '<' ) ) {
+			// As of Subscriptions 7.2.0 (Core 8.0.0), admin notices are stored one transient per-user.
+			delete_transient( '_wcs_admin_notices' );
+		}
+
 		self::upgrade_complete();
 	}
 

tests/unit/test-wcs-admin-functions.php

@@ -0,0 +1,192 @@
+<?php
+
+/**
+ * Tests for the functions contained in includes/admin/wcs-admin-functions.php.
+ */
+class WCS_Admin_Functions_Test extends WP_UnitTestCase {
+	/**
+	 * User ID of an administrator-level test user.
+	 *
+	 * @var int
+	 */
+	private static $admin_id;
+
+	/**
+	 * User ID of a contributor-level test user.
+	 *
+	 * @var int
+	 */
+	private static $contributor_id;
+
+	/**
+	 * Ensure the admin functions are loaded in preparation for our tests.
+	 *
+	 * @return void
+	 */
+	public static function set_up_before_class() {
+		parent::set_up_before_class();
+
+		if ( ! function_exists( 'wcs_admin_notice' ) ) {
+			require_once __DIR__ . '/../../includes/admin/wcs-admin-functions.php';
+		}
+
+		self::$admin_id       = self::factory()->user->create( array( 'role' => 'administrator' ) );
+		self::$contributor_id = self::factory()->user->create( array( 'role' => 'contributor' ) );
+	}
+
+	/**
+	 * Admin notices should target a specific user, and should not be visible to other users.
+	 *
+	 * @see wcs_add_admin_notice()
+	 * @see wcs_clear_admin_notices()
+	 * @see wcs_display_admin_notices()
+	 *
+	 * @return void
+	 */
+	public function test_wcs_admin_notice_visibility_by_user() {
+		$message_text = 'The first rule of subscription club, is you do not talk about subscription club.';
+
+		wp_set_current_user( self::$admin_id );
+		wcs_add_admin_notice( $message_text );
+
+		wp_set_current_user( self::$contributor_id );
+		$this->assertEquals(
+			'',
+			$this->capture_wcs_admin_notice_text(),
+			'The message was not exposed to the wrong user.'
+		);
+
+		wp_set_current_user( self::$admin_id );
+		$this->assertStringContainsString(
+			$message_text,
+			$this->capture_wcs_admin_notice_text(),
+			'The expected message was shared with the user.'
+		);
+
+		wcs_add_admin_notice( $message_text, 'success', self::$contributor_id );
+		$this->assertEquals(
+			'',
+			$this->capture_wcs_admin_notice_text(),
+			'The message (which does not target the current user) is not inadvertently shown to the current user.'
+		);
+
+		wp_set_current_user( self::$contributor_id );
+		$this->assertStringContainsString(
+			$message_text,
+			$this->capture_wcs_admin_notice_text(),
+			'The expected message was shared with the correct user.'
+		);
+	}
+
+	/**
+	 * Admin notices should not be accepted if a user is not actually logged in.
+	 *
+	 * This covers an edge case that generally should not arise. However, if it did, we would want to avoid
+	 * a scenario in which a '_wcs_admin_notices_0' transient is created and starts to balloon in size.
+	 *
+	 * @return void
+	 */
+	public function test_wcs_admin_notices_are_only_added_when_a_user_is_logged_in() {
+		$logged_messages = [];
+		$logging_monitor = function ( $message ) use ( &$logged_messages ) {
+			$logged_messages[] = $message;
+		};
+
+		add_filter( 'woocommerce_logger_log_message', $logging_monitor );
+		wp_set_current_user( 0 );
+		wcs_add_admin_notice( "You're gonna need a bigger subscription." );
+		remove_filter( 'woocommerce_logger_log_message', $logging_monitor );
+
+		$this->assertEquals(
+			'',
+			$this->capture_wcs_admin_notice_text(),
+			'If a user is not logged in, admin notifications are not accepted.'
+		);
+
+		$this->assertStringContainsString(
+			'Admin notices can only be added if a user is currently logged in',
+			$logged_messages[0],
+			'If an attempt is made to add an admin notice when nobody is logged in, a warning is logged.'
+		);
+	}
+
+	/**
+	 * Admin notices can target a specific admin screen, and should not render outside of that context.
+	 *
+	 * @see wcs_add_admin_notice()
+	 * @see wcs_clear_admin_notices()
+	 * @see wcs_display_admin_notices()
+	 *
+	 * @return void
+	 */
+	public function test_wcs_admin_notice_visibility_by_screen() {
+		global $current_screen;
+
+		$original_screen = $current_screen;
+		$message_text    = 'The second rule of subscription club, is you DO NOT talk about subscription club.';
+
+		wp_set_current_user( self::$admin_id );
+		wcs_add_admin_notice( $message_text, 'error', null, 'subscriptions-dashboard' );
+
+		$this->assertEquals(
+			'',
+			$this->capture_wcs_admin_notice_text(),
+			'The message was not exposed outside of the specified screen.'
+		);
+
+		$test_screen = WP_Screen::get( 'subscriptions-dashboard' );
+		set_current_screen( $test_screen );
+
+		$this->assertStringContainsString(
+			$message_text,
+			$this->capture_wcs_admin_notice_text(),
+			'The message was displayed in the context of the specified screen.'
+		);
+
+		set_current_screen( $original_screen );
+	}
+
+	/**
+	 * Admin notices generally act as 'flash messages' and are removed from the queue after they
+	 * have rendered. However, the system also allows for them to stay in the queue.
+	 *
+	 * @return void
+	 */
+	public function test_wcs_admin_notice_queue_clearance() {
+		wp_set_current_user( self::$admin_id );
+		$message_text = "That's no moon, it's a subscription notice.";
+		wcs_add_admin_notice( $message_text, 'error' );
+
+		$this->assertStringContainsString(
+			esc_html( $message_text ),
+			$this->capture_wcs_admin_notice_text( false ),
+			'The admin notice is displayed as expected.'
+		);
+
+		$this->assertStringContainsString(
+			esc_html( $message_text ),
+			$this->capture_wcs_admin_notice_text(),
+			'The admin notice is displayed a second time, because it was not cleared last time.'
+		);
+
+		$this->assertEquals(
+			'',
+			$this->capture_wcs_admin_notice_text(),
+			'The admin notice does not display, because it was cleared from the queue.'
+		);
+	}
+
+	/**
+	 * Equivalent to calling wcs_display_admin_notices() directly, except the function output is
+	 * captured and returned in a string.
+	 *
+	 * @param bool $clear If the message queue should be cleared after getting/displaying the messages.
+	 *
+	 * @return string
+	 */
+	private function capture_wcs_admin_notice_text( $clear = true ) {
+		ob_start();
+		wcs_display_admin_notices( $clear );
+		return (string) ob_get_clean();
+	}
+}