fix: address review feedback on the pre-release notice

- Use the core is-dismissible close button and drop the hand-rolled one.
- Keep i18n one translatable string per message.
- Persist dismissal keyed to PR PLUGIN_VERSION so it returns on newer builds.
- Fold is_pre_release into PreReleaseNotice and allow build metadata.
- Deny without capability via wp_send_json_error(403) or wp_die('', 403).
- Rename the dismiss script to pre-release-notice.js and persist on X clicks.

Author: faisalahammad

assets/js/admin-notice.js assets/js/pre-release-notice.jsassets/js/admin-notice.js renamed to assets/js/pre-release-notice.js

@@ -10,22 +10,31 @@
 		for ( const notice of notices ) {
 			notice.addEventListener( 'click', ( event ) => {
 				const dismiss = event.target.closest(
-					'[data-antispam-bee-dismiss], [data-antispam-bee-dismiss-link]'
+					'.notice-dismiss, [data-antispam-bee-dismiss-link]'
 				);
 
 				if ( ! dismiss ) {
 					return;
 				}
 
-				event.preventDefault();
+				// Core already hides and removes the notice for its own close
+				// button, so only block the Dismiss link's default navigation.
+				if ( ! dismiss.closest( '.notice-dismiss' ) ) {
+					event.preventDefault();
+				}
 
-				const href = dismiss.dataset.antispamBeeDismissLink;
+				const href = notice.dataset.antispamBeeDismissLink;
 
+				// The notice may already be gone once core has handled the close
+				// button, so only hide it here when the AJAX call finished before
+				// core's own removal.
 				wp.ajax.post( antispamBeePreReleaseNotice.action, {
 					_ajax_nonce: antispamBeePreReleaseNotice.nonce,
 				} )
 					.done( () => {
-						notice.remove();
+						if ( notice.isConnected ) {
+							notice.style.display = 'none';
+						}
 					} )
 					.fail( () => {
 						if ( href ) {

src/Admin/PreReleaseNotice.php

@@ -7,7 +7,6 @@
 
 namespace AntispamBee\Admin;
 
-use AntispamBee\Helpers\StringHelper;
 use const AntispamBee\MAIN_PLUGIN_FILE;
 use const AntispamBee\PLUGIN_VERSION;
 
@@ -92,27 +91,34 @@ public static function maybe_render( string $hook_suffix = '' ): void {
 			self::DISMISS_ACTION
 		);
 
+		$version_label = wp_kses_post(
+			sprintf(
+				/* translators: %s: installed version, already wrapped in code tags. */
+				__( 'You are running version %s.', 'antispam-bee' ),
+				'<code>' . esc_html( PLUGIN_VERSION ) . '</code>'
+			)
+		);
+
 		printf(
-			'<div class="notice notice-warning" data-antispam-bee-pre-release-notice>' .
+			'<div class="notice notice-warning is-dismissible" data-antispam-bee-pre-release-notice data-antispam-bee-dismiss-link="%5$s">' .
 			'<p><strong>%1$s</strong></p>' .
-			'<p>%2$s <code>%3$s</code></p>' .
-			'<p>%4$s</p>' .
-			'<p><a class="button" href="%5$s" target="_blank" rel="noopener noreferrer">%6$s</a> ' .
-			'<a class="button-link" href="%7$s" data-antispam-bee-dismiss-link="%7$s">%8$s</a></p>' .
-			'<button type="button" class="notice-dismiss" data-antispam-bee-dismiss aria-label="%9$s"><span class="screen-reader-text">%9$s</span></button>' .
+			'<p>%2$s</p>' .
+			'<p>%3$s</p>' .
+			'<p><a class="button" href="%4$s" target="_blank" rel="noopener noreferrer">%6$s</a> ' .
+			'<a class="button-link" href="%5$s">%7$s</a></p>' .
 			'</div>',
 			esc_html__( 'Antispam Bee is a pre-release version', 'antispam-bee' ),
-			esc_html__( 'You are running version', 'antispam-bee' ),
-			esc_html( PLUGIN_VERSION ),
+			// The version label is sanitized by wp_kses_post() above.
+			// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
+			$version_label,
 			esc_html__(
 				'This is a pre-release and not intended for production. Please test it and report any issues you find.',
 				'antispam-bee'
 			),
 			esc_url( self::FEEDBACK_URL ),
-			esc_html__( 'Report a bug', 'antispam-bee' ),
 			esc_url( $dismiss_url ),
-			esc_html__( 'Dismiss', 'antispam-bee' ),
-			esc_html__( 'Dismiss this notice', 'antispam-bee' )
+			esc_html__( 'Report a bug', 'antispam-bee' ),
+			esc_html__( 'Dismiss', 'antispam-bee' )
 		);
 	}
 
@@ -128,7 +134,7 @@ public static function maybe_enqueue_assets( string $hook_suffix = '' ): void {
 
 		wp_enqueue_script(
 			'antispam-bee-pre-release-notice',
-			plugin_dir_url( MAIN_PLUGIN_FILE ) . 'assets/js/admin-notice.js',
+			plugin_dir_url( MAIN_PLUGIN_FILE ) . 'assets/js/pre-release-notice.js',
 			[ 'wp-util' ],
 			PLUGIN_VERSION,
 			true
@@ -152,7 +158,7 @@ public static function maybe_enqueue_assets( string $hook_suffix = '' ): void {
 	 * @return bool Whether to show the notice.
 	 */
 	private static function should_show( string $hook_suffix ): bool {
-		if ( ! StringHelper::is_pre_release( PLUGIN_VERSION ) ) {
+		if ( ! self::is_pre_release( PLUGIN_VERSION ) ) {
 			return false;
 		}
 
@@ -164,33 +170,61 @@ private static function should_show( string $hook_suffix ): bool {
 			return false;
 		}
 
-		return false === (bool) get_user_meta( get_current_user_id(), self::DISMISSED_META_KEY, true );
+		return get_user_meta( get_current_user_id(), self::DISMISSED_META_KEY, true ) !== PLUGIN_VERSION;
+	}
+
+	/**
+	 * Whether a plugin version string marks a pre-release.
+	 *
+	 * A version is a pre-release if the measured number is followed by a
+	 * semantic versioning pre-release suffix, e.g. `3.0.0-RC.1` or
+	 * `3.0.0-beta.2`. The stable `3.0.0` has no such suffix. Build metadata
+	 * after the pre-release suffix, e.g. `3.0.0-beta.2+build`, still marks a
+	 * pre-release.
+	 *
+	 * @param string $version The version string.
+	 *
+	 * @return bool Whether the version is a pre-release.
+	 */
+	public static function is_pre_release( string $version ): bool {
+		return 1 === preg_match( '/^[0-9]+(?:\.[0-9]+){0,2}-[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$/', $version );
 	}
 
 	/**
 	 * Persist the dismissal, then acknowledge an AJAX request or redirect.
 	 *
-	 * Both `wp_send_json_success()` and the redirect end the request, so the
-	 * non-AJAX branch is the only one that reaches the redirect and `exit`.
+	 * `wp_send_json_success()` ends an AJAX request, so the non-AJAX branch is
+	 * the only one that reaches the redirect and `exit`.
 	 */
 	public static function handle_dismiss(): void {
 		if ( ! current_user_can( 'manage_options' ) ) {
-			wp_die( esc_html__( 'You do not have permission to do this.', 'antispam-bee' ), 403 );
+			$message = esc_html__( 'You do not have permission to do this.', 'antispam-bee' );
+
+			if ( wp_doing_ajax() ) {
+				wp_send_json_error( $message, 403 );
+			}
+
+			// The message is escaped by esc_html__() above.
+			// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
+			wp_die( $message, '', 403 );
 		}
 
 		$is_ajax = wp_doing_ajax();
 
 		if ( $is_ajax ) {
 			check_ajax_referer( self::DISMISS_ACTION );
-			update_user_meta( get_current_user_id(), self::DISMISSED_META_KEY, 1 );
-			wp_send_json_success();
 		} else {
 			check_admin_referer( self::DISMISS_ACTION );
-			update_user_meta( get_current_user_id(), self::DISMISSED_META_KEY, 1 );
+		}
 
-			wp_safe_redirect( wp_get_referer() ? wp_get_referer() : admin_url() );
+		update_user_meta( get_current_user_id(), self::DISMISSED_META_KEY, PLUGIN_VERSION );
 
-			exit;
+		if ( $is_ajax ) {
+			wp_send_json_success();
 		}
+
+		wp_safe_redirect( wp_get_referer() ? wp_get_referer() : admin_url() );
+
+		exit;
 	}
 }

src/Helpers/StringHelper.php

@@ -22,16 +22,21 @@ class PreReleaseNoticeTest extends TestCase {
 	 * @param bool $can_manage Whether the mock user may manage options.
 	 * @param bool $dismissed  Whether the mock user dismissed the notice.
 	 */
-	private function mock_dependencies( bool $can_manage = true, bool $dismissed = false ): void {
+	private function mock_dependencies( bool $can_manage = true, ?string $dismissed_version = null ): void {
 		when( 'esc_html' )->returnArg();
 		when( 'esc_url' )->returnArg();
+		when( 'wp_kses_post' )->returnArg();
 		when( 'wp_nonce_url' )->returnArg();
 		when( 'admin_url' )->returnArg();
 		when( 'current_user_can' )->justReturn( $can_manage );
 		when( 'get_current_user_id' )->justReturn( 1 );
 		when( 'get_user_meta' )->alias(
-			static function ( $id, $key ) use ( $dismissed ) {
-				return $key === PreReleaseNotice::DISMISSED_META_KEY ? $dismissed : false;
+			static function ( $id, $key ) use ( $dismissed_version ) {
+				if ( $key !== PreReleaseNotice::DISMISSED_META_KEY ) {
+					return false;
+				}
+
+				return null === $dismissed_version ? '' : $dismissed_version;
 			}
 		);
 		when( 'plugin_dir_url' )->returnArg();
@@ -95,19 +100,19 @@ private function render_for( string $hook_suffix ): string {
 	public function test_renders_on_the_settings_page(): void {
 		self::mock_dependencies();
 
-		self::assertStringContainsString(
-			'pre-release',
-			self::render_for( 'settings_page_antispam_bee' )
-		);
+		$output = self::render_for( 'settings_page_antispam_bee' );
+
+		self::assertStringContainsString( 'pre-release', $output );
+		self::assertStringContainsString( 'is-dismissible', $output );
+		self::assertStringContainsString( '<code>3.0.0-beta.2</code>', $output );
+		self::assertStringNotContainsString( 'data-antispam-bee-dismiss ', $output );
+		self::assertStringNotContainsString( '<button type="button" class="notice-dismiss"', $output );
 	}
 
 	public function test_renders_on_the_plugins_list(): void {
 		self::mock_dependencies();
 
-		self::assertStringContainsString(
-			'pre-release',
-			self::render_for( 'plugins.php' )
-		);
+		self::assertStringContainsString( 'pre-release', self::render_for( 'plugins.php' ) );
 	}
 
 	public function test_renders_using_the_global_hook_suffix(): void {
@@ -154,12 +159,18 @@ public function test_does_not_render_without_manage_options_capability(): void {
 		self::assertSame( '', self::render_for( 'plugins.php' ) );
 	}
 
-	public function test_does_not_render_when_dismissed_by_the_user(): void {
-		self::mock_dependencies( true, true );
+	public function test_does_not_render_when_dismissed_for_the_installed_version(): void {
+		self::mock_dependencies( true, '3.0.0-beta.2' );
 
 		self::assertSame( '', self::render_for( 'plugins.php' ) );
 	}
 
+	public function test_renders_again_for_a_newer_prerelease_version(): void {
+		self::mock_dependencies( true, '3.0.0-RC.1' );
+
+		self::assertStringContainsString( 'pre-release', self::render_for( 'plugins.php' ) );
+	}
+
 	public function test_enqueues_assets_on_the_plugins_list(): void {
 		self::mock_dependencies();
 		$enqueued = [];
@@ -207,12 +218,18 @@ static function ( $id, $key, $value ) use ( &$updated ) {
 		when( 'wp_send_json_success' )->alias(
 			static function () use ( &$sent ) {
 				$sent = true;
+
+				throw new RuntimeException( '__ANTISPAM_BEE_EXPECTED_HALT__' );
 			}
 		);
 
-		PreReleaseNotice::handle_dismiss();
+		self::assert_and_terminates(
+			static function () {
+				PreReleaseNotice::handle_dismiss();
+			}
+		);
 
-		self::assertSame( [ [ 1, PreReleaseNotice::DISMISSED_META_KEY, 1 ] ], $updated );
+		self::assertSame( [ [ 1, PreReleaseNotice::DISMISSED_META_KEY, \AntispamBee\PLUGIN_VERSION ] ], $updated );
 		self::assertTrue( $sent, 'The AJAX request must be acknowledged' );
 	}
 
@@ -243,14 +260,38 @@ static function () {
 			}
 		);
 
-		self::assertSame( [ [ 1, PreReleaseNotice::DISMISSED_META_KEY, 1 ] ], $updated );
+		self::assertSame( [ [ 1, PreReleaseNotice::DISMISSED_META_KEY, \AntispamBee\PLUGIN_VERSION ] ], $updated );
 		self::assertSame( 'https://example.com/wp-admin/plugins.php', $target );
 	}
 
-	public function test_handle_dismiss_denies_without_capability(): void {
+	public function test_handle_dismiss_denies_ajax_without_capability(): void {
 		self::mock_dependencies( false );
 		when( 'wp_doing_ajax' )->justReturn( true );
 
+		$captured = null;
+		when( 'wp_send_json_error' )->alias(
+			static function ( ...$args ) use ( &$captured ) {
+				$captured = $args;
+
+				throw new RuntimeException( '__ANTISPAM_BEE_EXPECTED_HALT__' );
+			}
+		);
+
+		self::assert_and_terminates(
+			static function () {
+				PreReleaseNotice::handle_dismiss();
+			}
+		);
+
+		self::assertNotNull( $captured, 'The handler must refuse users without the capability' );
+		self::assertCount( 2, $captured );
+		self::assertSame( 403, $captured[1], 'The AJAX denial must carry an HTTP 403 status' );
+	}
+
+	public function test_handle_dismiss_denies_form_request_without_capability(): void {
+		self::mock_dependencies( false );
+		when( 'wp_doing_ajax' )->justReturn( false );
+
 		$captured = null;
 		self::stub_terminator( 'wp_die', static function ( ...$args ) use ( &$captured ) {
 			$captured = $args;
@@ -263,6 +304,41 @@ static function () {
 		);
 
 		self::assertNotNull( $captured, 'The handler must refuse users without the capability' );
-		self::assertSame( 403, $captured[1] );
+		self::assertCount( 3, $captured );
+		self::assertSame( '', $captured[1], 'The page title must be an empty string' );
+		self::assertSame( 403, $captured[2], 'The non-AJAX denial must carry an HTTP 403 status' );
+	}
+
+	/**
+	 * Data provider for pre-release detection.
+	 *
+	 * @return array<string, array{string, bool}>
+	 */
+	public static function data_is_pre_release(): array {
+		return [
+			'stable 3.0.0'            => [ '3.0.0', false ],
+			'stable 1.2.3'            => [ '1.2.3', false ],
+			'stable with prefix'      => [ 'v2.11.13', false ],
+			'rc suffix'               => [ '3.0.0-RC.1', true ],
+			'beta suffix'             => [ '3.0.0-beta.2', true ],
+			'alpha suffix'            => [ '3.0.0-alpha', true ],
+			'mixed case suffix'       => [ '3.0.0-Rc1', true ],
+			'build metadata'          => [ '3.0.0-beta.2+build', true ],
+			'empty string'            => [ '', false ],
+			'not a version'           => [ 'foo', false ],
+			'suffix without number'   => [ '-beta', false ],
+		];
+	}
+
+	/**
+	 * Test is_pre_release() against a range of version strings.
+	 *
+	 * @param string $version Version string.
+	 * @param bool   $expected Expected result.
+	 *
+	 * @dataProvider data_is_pre_release
+	 */
+	public function test_is_pre_release( string $version, bool $expected ): void {
+		self::assertSame( $expected, PreReleaseNotice::is_pre_release( $version ) );
 	}
 }