Address the WordPress.org review: scope the admin notices, fix the duplicate-install fatal

.github/workflows/ci.yml

@@ -117,6 +117,13 @@ jobs:
       - name: Run the test suite with the WooCommerce store rules active
         run: composer run test:woocommerce
 
+      # Two of this plugin's storage decisions are only wrong on multisite —
+      # a network-wide usermeta key standing in for a per-site one, and a
+      # notice hook that does not fire in Network Admin. Neither is visible to
+      # a single-site run.
+      - name: Run the test suite as a multisite network
+        run: composer run test:multisite
+
   test-woocommerce:
     name: WooCommerce integration
     runs-on: ubuntu-latest

citecue.php

@@ -3,7 +3,7 @@
  * Plugin Name:       CiteCue AI Auto-Fix
  * Plugin URI:        https://github.com/citecue/wordpress-plugin
  * Description:       Serves CiteCue-optimized versions of your pages to AI bots and crawlers, adds CiteCue's enriched SEO metadata to your live pages, publishes your llms.txt, and lets CiteCue push brand-building draft content into WordPress.
- * Version:           1.1.0
+ * Version:           1.1.1
  * Requires at least: 5.8
  * Requires PHP:      7.4
  * Author:            CiteCue
@@ -19,44 +19,126 @@
 	exit;
 }
 
+/*
+ * Stand down for a pre-WordPress.org copy, which cannot stand down for us.
+ *
+ * Releases before the move to WordPress.org unpacked to citecue/, and the only
+ * one that ever did is 1.0.0 — which predates the guard below and so defines
+ * the constants and runs its requires unconditionally. The directory's copy
+ * installs as citecue-ai-auto-fix/, so a site carrying the old one gains a
+ * second plugin rather than an upgrade, and `require_once` does not save us:
+ * the two copies are two paths, so the second to load redeclares every class
+ * and takes the site down.
+ *
+ * Which of them is second is not a coin toss. activate_plugin() sorts
+ * active_plugins before storing it, '-' sorts before '/', so
+ * citecue-ai-auto-fix/citecue.php is always included first and citecue/ is
+ * always the one that fatals. The guard below therefore never gets the chance
+ * to fire in the case it was written for: by the time 1.0.0 runs, it is this
+ * copy's classes it is redeclaring, and 1.0.0 has no guard to check.
+ *
+ * So this copy yields instead. The site keeps running — on 1.0.0, which is the
+ * worse version but a working one — and the notice says which directory to
+ * delete to get this one back. Deleting it is also what makes this branch stop
+ * running, so the check confirms the file is really still there rather than
+ * trusting a stale active_plugins entry, which would strand the site on a copy
+ * that is no longer installed.
+ */
+$citecue_legacy_is_running = ( static function () {
+	$legacy = 'citecue/citecue.php';
+
+	if ( plugin_basename( __FILE__ ) === $legacy ) {
+		return false;
+	}
+
+	$active = (array) get_option( 'active_plugins', array() );
+	if ( is_multisite() ) {
+		$active = array_merge( $active, array_keys( (array) get_site_option( 'active_sitewide_plugins', array() ) ) );
+	}
+
+	return in_array( $legacy, $active, true ) && file_exists( WP_PLUGIN_DIR . '/' . $legacy );
+} )();
+
+if ( $citecue_legacy_is_running ) {
+	$citecue_legacy_notice = static function () {
+		if ( ! current_user_can( 'activate_plugins' ) ) {
+			return;
+		}
+		// Duplicated in the guard below rather than shared through a helper:
+		// this is the one file that can legitimately be included twice, and a
+		// named function here is a redeclaration waiting to happen.
+		$screen = function_exists( 'get_current_screen' ) ? get_current_screen() : null;
+		if ( ! $screen || ( 'plugins' !== $screen->id && 'plugins-network' !== $screen->id ) ) {
+			return;
+		}
+		printf(
+			'<div class="notice notice-error"><p>%s</p></div>',
+			esc_html(
+				sprintf(
+					/* translators: 1: the older plugin directory, e.g. citecue/. 2: this plugin's directory. */
+					__( 'CiteCue AI Auto-Fix is installed twice. An older copy in %1$s is the one WordPress is running, so the copy in %2$s has not loaded. Deactivate and delete the older one to switch to this version — your settings and connection are stored in the database and carry over untouched.', 'citecue-ai-auto-fix' ),
+					'citecue/',
+					dirname( plugin_basename( __FILE__ ) ) . '/'
+				)
+			)
+		);
+	};
+
+	add_action( 'admin_notices', $citecue_legacy_notice );
+	add_action( 'network_admin_notices', $citecue_legacy_notice );
+
+	unset( $citecue_legacy_is_running, $citecue_legacy_notice );
+	return;
+}
+
+unset( $citecue_legacy_is_running );
+
 /*
  * Stand down if another copy of this plugin already loaded.
  *
- * Releases before the move to WordPress.org shipped an archive that unpacked
- * to citecue/. The directory's copy installs as citecue-ai-auto-fix/, so a
- * site carrying the old one gains a second plugin rather than an upgrade —
- * and `require_once` does not save us, because the two copies are two paths.
- * Both would run their requires, the second would redeclare every class, and
- * the site would go down with a fatal error on the next request.
+ * The case above is the one duplicate this plugin has actually shipped. This
+ * one catches the rest: a GitHub "Download ZIP" unpacks to
+ * wordpress-plugin-main/, and any directory sorting after
+ * citecue-ai-auto-fix/ loads second, sees the constant and does nothing —
+ * which turns a white screen into an admin notice naming the directory to
+ * delete.
  *
- * The copy that loses the race does nothing and says so, which turns a white
- * screen into an admin notice naming the directory to delete.
+ * The notice belongs on a Plugins screen and nowhere else: deleting a plugin
+ * directory is a Plugins-screen job, and nothing about this is urgent enough to
+ * follow an administrator through the rest of their dashboard. Both Plugins
+ * screens count, though — a network-activated copy can only be deactivated from
+ * Network Admin, and `admin_notices` does not fire there at all.
  */
 if ( defined( 'CITECUE_VERSION' ) ) {
-	add_action(
-		'admin_notices',
-		static function () {
-			if ( ! current_user_can( 'activate_plugins' ) ) {
-				return;
-			}
-			printf(
-				'<div class="notice notice-error"><p>%s</p></div>',
-				esc_html(
-					sprintf(
-						/* translators: 1: plugin file that is running, e.g. citecue/citecue.php. 2: duplicate plugin file that did not load. */
-						__( 'CiteCue AI Auto-Fix is installed twice. WordPress is running %1$s, so the copy in %2$s did not load. Deactivate and delete whichever of the two you do not want to keep.', 'citecue-ai-auto-fix' ),
-						plugin_basename( CITECUE_PLUGIN_FILE ),
-						plugin_basename( __FILE__ )
-					)
-				)
-			);
+	$citecue_duplicate_notice = static function () {
+		if ( ! current_user_can( 'activate_plugins' ) ) {
+			return;
+		}
+		$screen = function_exists( 'get_current_screen' ) ? get_current_screen() : null;
+		if ( ! $screen || ( 'plugins' !== $screen->id && 'plugins-network' !== $screen->id ) ) {
+			return;
 		}
-	);
+		printf(
+			'<div class="notice notice-error"><p>%s</p></div>',
+			esc_html(
+				sprintf(
+					/* translators: 1: plugin file that is running, e.g. citecue/citecue.php. 2: duplicate plugin file that did not load. */
+					__( 'CiteCue AI Auto-Fix is installed twice. WordPress is running %1$s, so the copy in %2$s did not load. Deactivate and delete whichever of the two you do not want to keep.', 'citecue-ai-auto-fix' ),
+					plugin_basename( CITECUE_PLUGIN_FILE ),
+					plugin_basename( __FILE__ )
+				)
+			)
+		);
+	};
+
+	add_action( 'admin_notices', $citecue_duplicate_notice );
+	add_action( 'network_admin_notices', $citecue_duplicate_notice );
 
+	unset( $citecue_duplicate_notice );
 	return;
 }
 
-define( 'CITECUE_VERSION', '1.1.0' );
+define( 'CITECUE_VERSION', '1.1.1' );
 define( 'CITECUE_PLUGIN_FILE', __FILE__ );
 define( 'CITECUE_PLUGIN_DIR', plugin_dir_path( __FILE__ ) );
 

composer.json

@@ -44,9 +44,11 @@
 		"phpcbf": "phpcbf",
 		"test": [
 			"@test:core",
-			"@test:woocommerce"
+			"@test:woocommerce",
+			"@test:multisite"
 		],
 		"test:core": "phpunit",
-		"test:woocommerce": "CITECUE_STUB_WOOCOMMERCE=1 phpunit"
+		"test:woocommerce": "CITECUE_STUB_WOOCOMMERCE=1 phpunit",
+		"test:multisite": "WP_MULTISITE=1 phpunit"
 	}
 }

includes/class-citecue-admin.php

@@ -20,6 +20,19 @@
  */
 class Citecue_Admin {
 
+	/**
+	 * User-option prefix recording a notice this user has dismissed.
+	 *
+	 * A user option rather than user meta, because on multisite the usermeta
+	 * table is shared across the whole network while the condition these
+	 * notices report on comes from per-site options. Stored as meta, one
+	 * administrator dismissing the prompt on one site in a network would
+	 * silence an unrelated, still-true prompt on all the others;
+	 * update_user_option() prefixes the key with the current blog's, so each
+	 * site gets its own answer.
+	 */
+	const DISMISSED_OPTION_PREFIX = 'citecue_dismissed_';
+
 	/**
 	 * Plugin container.
 	 *
@@ -45,6 +58,7 @@ public function register() {
 		add_action( 'admin_menu', array( $this, 'add_menu' ) );
 		add_action( 'admin_init', array( $this, 'register_settings' ) );
 		add_action( 'admin_init', array( $this, 'maybe_claim_connect' ) );
+		add_action( 'admin_init', array( $this, 'maybe_dismiss_notice' ) );
 		add_action( 'admin_notices', array( $this, 'notices' ) );
 		add_action( 'admin_post_citecue_connect_start', array( $this, 'handle_connect_start' ) );
 		add_action( 'admin_post_citecue_disconnect', array( $this, 'handle_disconnect' ) );
@@ -119,17 +133,45 @@ private function redirect_with( $code ) {
 	}
 
 	/**
-	 * Admin notices: action feedback plus a persistent auth-failure warning.
+	 * Whether the screen being rendered is one of the plugin's own.
+	 *
+	 * This plugin has no business putting messages on the comment queue, the
+	 * media library or anyone's post editor, so every notice it emits is gated
+	 * on this. Two screens qualify: the settings page the message is about, and
+	 * the Plugins list, which is where an administrator looks when a plugin
+	 * needs attention and the only screen on which some of these are actionable.
+	 *
+	 * @return bool
+	 */
+	private function is_plugin_screen() {
+		$screen = function_exists( 'get_current_screen' ) ? get_current_screen() : null;
+		if ( ! $screen ) {
+			return false;
+		}
+
+		$id = (string) $screen->id;
+
+		return 'plugins' === $id || false !== strpos( $id, 'citecue' );
+	}
+
+	/**
+	 * Admin notices: action feedback plus the two conditions worth interrupting
+	 * an administrator over.
+	 *
+	 * Confined to the plugin's own screens — see is_plugin_screen(). Nothing
+	 * here is urgent enough to follow someone around their whole dashboard, and
+	 * the settings screen states all of it a second time in its status card, so
+	 * a dismissed or unseen notice costs no information.
 	 *
 	 * @return void
 	 */
 	public function notices() {
-		if ( ! current_user_can( 'manage_options' ) ) {
+		if ( ! current_user_can( 'manage_options' ) || ! $this->is_plugin_screen() ) {
 			return;
 		}
 
 		if ( get_option( 'citecue_auth_failed' ) ) {
-			echo '<div class="notice notice-error"><p><strong>' . esc_html__( 'CiteCue:', 'citecue-ai-auto-fix' ) . '</strong> '
+			echo '<div class="notice notice-error is-dismissible"><p><strong>' . esc_html__( 'CiteCue:', 'citecue-ai-auto-fix' ) . '</strong> '
 				. esc_html__( 'the API key was rejected, so optimized pages are not being served to AI crawlers. Update the key in the CiteCue settings.', 'citecue-ai-auto-fix' )
 				. ' <a href="' . esc_url( $this->settings_url() ) . '">' . esc_html__( 'Open settings', 'citecue-ai-auto-fix' ) . '</a></p></div>';
 		}
@@ -183,21 +225,19 @@ public function notices() {
 	 * CiteCue learns the capability only from the connect exchange, so a site
 	 * that connected before this release injects enriched metadata while the
 	 * app still reports the channel as unable to — and tells the customer their
-	 * "Live" fix is not reaching human visitors. One reconnect fixes it. Shown
-	 * on the Plugins and CiteCue screens only: it is worth acting on, but it is
-	 * not an error, and it has no business following an administrator around
-	 * their whole dashboard.
+	 * "Live" fix is not reaching human visitors. One reconnect fixes it.
+	 *
+	 * It is advice, not an error, so it can be turned off for good: the same
+	 * state is on the settings screen's status card either way, and a message
+	 * an administrator has read and decided against should not keep arriving.
 	 *
 	 * @return void
 	 */
 	private function seo_head_reconnect_notice() {
 		if ( ! $this->plugin->settings->needs_seo_head_reconnect() ) {
 			return;
 		}
-
-		$screen = function_exists( 'get_current_screen' ) ? get_current_screen() : null;
-		$here   = $screen ? (string) $screen->id : '';
-		if ( 'plugins' !== $here && false === strpos( $here, 'citecue' ) ) {
+		if ( $this->notice_is_dismissed( 'seo_head_reconnect' ) ) {
 			return;
 		}
 
@@ -211,11 +251,79 @@ private function seo_head_reconnect_notice() {
 				<strong><?php esc_html_e( 'CiteCue:', 'citecue-ai-auto-fix' ); ?></strong>
 				<?php echo esc_html( $message ); ?>
 			</p>
-			<p><?php $this->action_button( 'citecue_connect_start', __( 'Reconnect to CiteCue', 'citecue-ai-auto-fix' ) ); ?></p>
+			<p>
+				<?php $this->action_button( 'citecue_connect_start', __( 'Reconnect to CiteCue', 'citecue-ai-auto-fix' ) ); ?>
+				<a href="<?php echo esc_url( $this->dismiss_url( 'seo_head_reconnect' ) ); ?>"><?php esc_html_e( 'Dismiss permanently', 'citecue-ai-auto-fix' ); ?></a>
+			</p>
 		</div>
 		<?php
 	}
 
+	/**
+	 * Whether this user has dismissed a notice for good.
+	 *
+	 * Per user rather than per site: one administrator deciding they do not
+	 * want to be told again is not a decision to make on their colleagues'
+	 * behalf. Per site as well as per user — see DISMISSED_OPTION_PREFIX.
+	 *
+	 * @param string $notice Notice key.
+	 * @return bool
+	 */
+	private function notice_is_dismissed( $notice ) {
+		return (bool) get_user_option( self::DISMISSED_OPTION_PREFIX . $notice, get_current_user_id() );
+	}
+
+	/**
+	 * The link that dismisses a notice for good.
+	 *
+	 * @param string $notice Notice key.
+	 * @return string
+	 */
+	private function dismiss_url( $notice ) {
+		return wp_nonce_url(
+			add_query_arg( 'citecue_dismiss', $notice, $this->current_admin_url() ),
+			'citecue_dismiss_' . $notice
+		);
+	}
+
+	/**
+	 * The admin URL currently being rendered, for links that come back here.
+	 *
+	 * @return string
+	 */
+	private function current_admin_url() {
+		$screen = function_exists( 'get_current_screen' ) ? get_current_screen() : null;
+
+		return ( $screen && 'plugins' === $screen->id ) ? admin_url( 'plugins.php' ) : $this->settings_url();
+	}
+
+	/**
+	 * Records a dismissal and reloads the screen without the query arguments.
+	 *
+	 * @return void
+	 */
+	public function maybe_dismiss_notice() {
+		if ( ! isset( $_GET['citecue_dismiss'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- the nonce is checked below, once there is something to check it against.
+			return;
+		}
+		if ( ! current_user_can( 'manage_options' ) ) {
+			return;
+		}
+
+		$notice = sanitize_key( wp_unslash( $_GET['citecue_dismiss'] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- checked on the next line.
+		check_admin_referer( 'citecue_dismiss_' . $notice );
+
+		if ( 'seo_head_reconnect' !== $notice ) {
+			return;
+		}
+
+		update_user_option( get_current_user_id(), self::DISMISSED_OPTION_PREFIX . $notice, time() );
+
+		$back = wp_get_referer();
+		wp_safe_redirect( $back ? remove_query_arg( array( 'citecue_dismiss', '_wpnonce' ), $back ) : $this->settings_url() );
+		exit;
+	}
+
 	/**
 	 * Completes a handshake when CiteCue redirects back with a one-time code.
 	 *