Skip to content

Commit d5fbefc

Browse files
barryhughesmattallan
barryhughes
and
mattallan
authored
Make the admin notification mechanics contextually aware (of the user and of the admin screen) (#767)
* Make the admin notification mechanics contextually aware (of the user and screen). * Minor fixes/tidy. * Update changelog.txt Co-authored-by: Matt Allan <[email protected]> * Only determine the screen ID once per invocation. * Improve the way in which we test if the notice queue is empty (before clearing the queue). * Do not accept new admin notices for users who are not logged in. If a plugin tries to call wcs_add_admin_notice() and no user is currently logged in, then a transient will be created and (if this happens periodically) it will A) not expire B) continue to grow in size. This change guards against that possibility. * Correct typo. Co-authored-by: Matt Allan <[email protected]> --------- Co-authored-by: Matt Allan <[email protected]>
1 parent 5569c09 commit d5fbefc

File tree

4 files changed

+302
-22
lines changed

4 files changed

+302
-22
lines changed

‎changelog.txt

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* Fix - Recommend WooPayments when there is no available payment gateway.
55
* 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.
66
* Fix - Correctly load product names with HTML on the cart and checkout shipping rates.
7+
* Fix - Improve our admin notice handling to ensure notices are displayed to the intended admin user.
78

89
= 7.9.0 - 2025-01-10 =
910
* Add - Compatibility with WooCommerce's new preview email feature introduced in 9.6.

‎includes/admin/wcs-admin-functions.php

+104-22
Original file line numberDiff line numberDiff line change
@@ -13,59 +13,141 @@
1313
}
1414

1515
/**
16-
* Store a message to display via @see wcs_display_admin_notices().
16+
* Registers an admin notice to be displayed once to the current (or other specified) user.
1717
*
18-
* @param string The message to display
19-
* @since 1.0.0 - Migrated from WooCommerce Subscriptions v2.0
18+
* @see wcs_display_admin_notices()
19+
* @since 1.0.0 Migrated from WooCommerce Subscriptions v2.0.
20+
* @since 7.2.0 Added support for specifying the target user and context.
21+
*
22+
* @param string $message The message to display.
23+
* @param string $notice_type Either 'success' or 'error'.
24+
* @param int|null $user_id The specific user who should see this message. If not specified, defaults to the current user.
25+
* @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.
26+
*
27+
* @return void
2028
*/
21-
function wcs_add_admin_notice( $message, $notice_type = 'success' ) {
29+
function wcs_add_admin_notice( $message, $notice_type = 'success', $user_id = null, $screen_id = null ) {
30+
$user_id = (int) ( null === $user_id ? get_current_user_id() : $user_id );
31+
32+
if ( $user_id < 1 ) {
33+
wc_get_logger()->warning(
34+
sprintf(
35+
/* Translators: %1$s: notice type ('success' or 'error'), %2$s: notice text. */
36+
'Admin notices can only be added if a user is currently logged in. Attempted (%1$s) notice: "%2$s"',
37+
$notice_type,
38+
$message
39+
),
40+
array(
41+
'backtrace' => true,
42+
'user_id' => $user_id,
43+
)
44+
);
45+
46+
return;
47+
}
2248

23-
$notices = get_transient( '_wcs_admin_notices' );
49+
$notices = get_transient( '_wcs_admin_notices_' . $user_id );
2450

25-
if ( false === $notices ) {
51+
if ( ! is_array( $notices ) ) {
2652
$notices = array();
2753
}
2854

29-
$notices[ $notice_type ][] = $message;
55+
$notices[ $notice_type ][] = array(
56+
'message' => $message,
57+
'screen_id' => $screen_id,
58+
);
3059

31-
set_transient( '_wcs_admin_notices', $notices, 60 * 60 );
60+
set_transient( '_wcs_admin_notices_' . $user_id, $notices, HOUR_IN_SECONDS );
3261
}
3362

3463
/**
35-
* Display any notices added with @see wcs_add_admin_notice()
64+
* Display any admin notices added with wcs_add_admin_notice().
65+
*
66+
* @see wcs_add_admin_notice()
67+
* @since 1.0.0 Migrated from WooCommerce Subscriptions v2.0.
68+
* @since 7.2.0 Supports contextual awareness of the user and screen.
3669
*
37-
* This method is also hooked to 'admin_notices' to display notices there.
70+
* @param bool $clear If the message queue should be cleared after rendering the message(s). Defaults to true.
3871
*
39-
* @since 1.0.0 - Migrated from WooCommerce Subscriptions v2.0
72+
* @return void
4073
*/
4174
function wcs_display_admin_notices( $clear = true ) {
75+
$user_id = get_current_user_id();
76+
$notices = get_transient( '_wcs_admin_notices_' . $user_id );
77+
78+
if ( ! is_array( $notices ) || empty( $notices ) ) {
79+
return;
80+
}
4281

43-
$notices = get_transient( '_wcs_admin_notices' );
82+
/**
83+
* Normalizes, sanitizes and outputs the provided notices.
84+
*
85+
* @param array &$notices The notice data.
86+
* @param string $class The CSS notice class to be applied (typically 'updated' or 'error').
87+
*
88+
* @return void
89+
*/
90+
$handle_notices = static function ( &$notices, $class ) {
91+
$notice_output = array();
92+
$screen_id = false;
4493

45-
if ( false !== $notices && ! empty( $notices ) ) {
94+
foreach ( $notices as $index => $notice ) {
95+
// Ensure the notice data now has the expected shape. If it does not, remove it.
96+
if ( ! is_array( $notice ) || ! isset( $notice['message'] ) || ! array_key_exists( 'screen_id', $notice ) ) {
97+
unset( $notices[ $index ] );
98+
continue;
99+
}
46100

47-
if ( ! empty( $notices['success'] ) ) {
48-
array_walk( $notices['success'], 'esc_html' );
49-
echo '<div id="moderated" class="updated"><p>' . wp_kses_post( implode( "</p>\n<p>", $notices['success'] ) ) . '</p></div>';
101+
// We only need to determine the current screen ID once.
102+
if ( false === $screen_id ) {
103+
$screen = get_current_screen();
104+
$screen_id = $screen instanceof WP_Screen ? $screen->id : '';
105+
}
106+
107+
// Should the notice display in the current screen context?
108+
if ( is_string( $notice['screen_id'] ) && $screen_id !== $notice['screen_id'] ) {
109+
continue;
110+
}
111+
112+
$notice_output[] = esc_html( $notice['message'] );
113+
unset( $notices[ $index ] );
50114
}
51115

52-
if ( ! empty( $notices['error'] ) ) {
53-
array_walk( $notices['error'], 'esc_html' );
54-
echo '<div id="moderated" class="error"><p>' . wp_kses_post( implode( "</p>\n<p>", $notices['error'] ) ) . '</p></div>';
116+
// $notice_output may be empty if some notices were withheld, due to not matching the screen context.
117+
if ( ! empty( $notice_output ) ) {
118+
echo '<div id="moderated" class="' . esc_attr( $class ) . '"><p>' . wp_kses_post( implode( "</p>\n<p>", $notice_output ) ) . '</p></div>';
55119
}
120+
};
121+
122+
if ( ! empty( $notices['success'] ) ) {
123+
$handle_notices( $notices['success'], 'updated' );
124+
}
125+
126+
if ( ! empty( $notices['error'] ) ) {
127+
$handle_notices( $notices['error'], 'error' );
128+
}
129+
130+
// Under certain circumstances, the caller may not wish for the rendered messages to be cleared from the queue.
131+
if ( false === $clear ) {
132+
return;
56133
}
57134

58-
if ( false !== $clear ) {
135+
// If all notices were rendered, clear the queue. If only some were rendered, clear what we can.
136+
if ( empty( $notices['success'] ) && empty( $notices['error'] ) ) {
59137
wcs_clear_admin_notices();
138+
} else {
139+
set_transient( '_wcs_admin_notices_' . $user_id, $notices, HOUR_IN_SECONDS );
60140
}
61141
}
142+
62143
add_action( 'admin_notices', 'wcs_display_admin_notices' );
63144

64145
/**
65146
* Delete any admin notices we stored for display later.
66147
*
67-
* @since 1.0.0 - Migrated from WooCommerce Subscriptions v2.0
148+
* @since 1.0.0 Migrated from WooCommerce Subscriptions v2.0.
149+
* @since 7.2.0 Became user aware.
68150
*/
69151
function wcs_clear_admin_notices() {
70-
delete_transient( '_wcs_admin_notices' );
152+
delete_transient( '_wcs_admin_notices_' . get_current_user_id() );
71153
}

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

+5
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,11 @@ public static function upgrade() {
129129
wp_unschedule_hook( 'wcs_cleanup_big_logs' );
130130
}
131131

132+
if ( version_compare( self::$active_version, '8.0.0', '<' ) ) {
133+
// As of Subscriptions 7.2.0 (Core 8.0.0), admin notices are stored one transient per-user.
134+
delete_transient( '_wcs_admin_notices' );
135+
}
136+
132137
self::upgrade_complete();
133138
}
134139

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

+192
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
<?php
2+
3+
/**
4+
* Tests for the functions contained in includes/admin/wcs-admin-functions.php.
5+
*/
6+
class WCS_Admin_Functions_Test extends WP_UnitTestCase {
7+
/**
8+
* User ID of an administrator-level test user.
9+
*
10+
* @var int
11+
*/
12+
private static $admin_id;
13+
14+
/**
15+
* User ID of a contributor-level test user.
16+
*
17+
* @var int
18+
*/
19+
private static $contributor_id;
20+
21+
/**
22+
* Ensure the admin functions are loaded in preparation for our tests.
23+
*
24+
* @return void
25+
*/
26+
public static function set_up_before_class() {
27+
parent::set_up_before_class();
28+
29+
if ( ! function_exists( 'wcs_admin_notice' ) ) {
30+
require_once __DIR__ . '/../../includes/admin/wcs-admin-functions.php';
31+
}
32+
33+
self::$admin_id = self::factory()->user->create( array( 'role' => 'administrator' ) );
34+
self::$contributor_id = self::factory()->user->create( array( 'role' => 'contributor' ) );
35+
}
36+
37+
/**
38+
* Admin notices should target a specific user, and should not be visible to other users.
39+
*
40+
* @see wcs_add_admin_notice()
41+
* @see wcs_clear_admin_notices()
42+
* @see wcs_display_admin_notices()
43+
*
44+
* @return void
45+
*/
46+
public function test_wcs_admin_notice_visibility_by_user() {
47+
$message_text = 'The first rule of subscription club, is you do not talk about subscription club.';
48+
49+
wp_set_current_user( self::$admin_id );
50+
wcs_add_admin_notice( $message_text );
51+
52+
wp_set_current_user( self::$contributor_id );
53+
$this->assertEquals(
54+
'',
55+
$this->capture_wcs_admin_notice_text(),
56+
'The message was not exposed to the wrong user.'
57+
);
58+
59+
wp_set_current_user( self::$admin_id );
60+
$this->assertStringContainsString(
61+
$message_text,
62+
$this->capture_wcs_admin_notice_text(),
63+
'The expected message was shared with the user.'
64+
);
65+
66+
wcs_add_admin_notice( $message_text, 'success', self::$contributor_id );
67+
$this->assertEquals(
68+
'',
69+
$this->capture_wcs_admin_notice_text(),
70+
'The message (which does not target the current user) is not inadvertently shown to the current user.'
71+
);
72+
73+
wp_set_current_user( self::$contributor_id );
74+
$this->assertStringContainsString(
75+
$message_text,
76+
$this->capture_wcs_admin_notice_text(),
77+
'The expected message was shared with the correct user.'
78+
);
79+
}
80+
81+
/**
82+
* Admin notices should not be accepted if a user is not actually logged in.
83+
*
84+
* This covers an edge case that generally should not arise. However, if it did, we would want to avoid
85+
* a scenario in which a '_wcs_admin_notices_0' transient is created and starts to balloon in size.
86+
*
87+
* @return void
88+
*/
89+
public function test_wcs_admin_notices_are_only_added_when_a_user_is_logged_in() {
90+
$logged_messages = [];
91+
$logging_monitor = function ( $message ) use ( &$logged_messages ) {
92+
$logged_messages[] = $message;
93+
};
94+
95+
add_filter( 'woocommerce_logger_log_message', $logging_monitor );
96+
wp_set_current_user( 0 );
97+
wcs_add_admin_notice( "You're gonna need a bigger subscription." );
98+
remove_filter( 'woocommerce_logger_log_message', $logging_monitor );
99+
100+
$this->assertEquals(
101+
'',
102+
$this->capture_wcs_admin_notice_text(),
103+
'If a user is not logged in, admin notifications are not accepted.'
104+
);
105+
106+
$this->assertStringContainsString(
107+
'Admin notices can only be added if a user is currently logged in',
108+
$logged_messages[0],
109+
'If an attempt is made to add an admin notice when nobody is logged in, a warning is logged.'
110+
);
111+
}
112+
113+
/**
114+
* Admin notices can target a specific admin screen, and should not render outside of that context.
115+
*
116+
* @see wcs_add_admin_notice()
117+
* @see wcs_clear_admin_notices()
118+
* @see wcs_display_admin_notices()
119+
*
120+
* @return void
121+
*/
122+
public function test_wcs_admin_notice_visibility_by_screen() {
123+
global $current_screen;
124+
125+
$original_screen = $current_screen;
126+
$message_text = 'The second rule of subscription club, is you DO NOT talk about subscription club.';
127+
128+
wp_set_current_user( self::$admin_id );
129+
wcs_add_admin_notice( $message_text, 'error', null, 'subscriptions-dashboard' );
130+
131+
$this->assertEquals(
132+
'',
133+
$this->capture_wcs_admin_notice_text(),
134+
'The message was not exposed outside of the specified screen.'
135+
);
136+
137+
$test_screen = WP_Screen::get( 'subscriptions-dashboard' );
138+
set_current_screen( $test_screen );
139+
140+
$this->assertStringContainsString(
141+
$message_text,
142+
$this->capture_wcs_admin_notice_text(),
143+
'The message was displayed in the context of the specified screen.'
144+
);
145+
146+
set_current_screen( $original_screen );
147+
}
148+
149+
/**
150+
* Admin notices generally act as 'flash messages' and are removed from the queue after they
151+
* have rendered. However, the system also allows for them to stay in the queue.
152+
*
153+
* @return void
154+
*/
155+
public function test_wcs_admin_notice_queue_clearance() {
156+
wp_set_current_user( self::$admin_id );
157+
$message_text = "That's no moon, it's a subscription notice.";
158+
wcs_add_admin_notice( $message_text, 'error' );
159+
160+
$this->assertStringContainsString(
161+
esc_html( $message_text ),
162+
$this->capture_wcs_admin_notice_text( false ),
163+
'The admin notice is displayed as expected.'
164+
);
165+
166+
$this->assertStringContainsString(
167+
esc_html( $message_text ),
168+
$this->capture_wcs_admin_notice_text(),
169+
'The admin notice is displayed a second time, because it was not cleared last time.'
170+
);
171+
172+
$this->assertEquals(
173+
'',
174+
$this->capture_wcs_admin_notice_text(),
175+
'The admin notice does not display, because it was cleared from the queue.'
176+
);
177+
}
178+
179+
/**
180+
* Equivalent to calling wcs_display_admin_notices() directly, except the function output is
181+
* captured and returned in a string.
182+
*
183+
* @param bool $clear If the message queue should be cleared after getting/displaying the messages.
184+
*
185+
* @return string
186+
*/
187+
private function capture_wcs_admin_notice_text( $clear = true ) {
188+
ob_start();
189+
wcs_display_admin_notices( $clear );
190+
return (string) ob_get_clean();
191+
}
192+
}

0 commit comments

Comments
 (0)