diff --git a/ci/lib.sh b/ci/lib.sh index d08fc8f..fdd69d3 100644 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -22,7 +22,7 @@ ci_load_conf() { # leaves the floors empty, every comparison becomes a no-op, and the hollow # checks these were added to fix come back silently — the same shape of bug. local missing= - for var in PLUGIN_SLUG PLUGIN_MAIN TEXT_DOMAIN NEEDS_WOOCOMMERCE MIN_SHIPPED_PHP_FILES MIN_TRANSLATION_CALLS RELEASE_REPO; do + for var in PLUGIN_SLUG PLUGIN_MAIN TEXT_DOMAIN NEEDS_WOOCOMMERCE MIN_SHIPPED_PHP_FILES MIN_TRANSLATION_CALLS RELEASE_REPO WPMGR_MENU_SHA; do [ -n "${!var:-}" ] || missing="$missing $var" done if [ -n "$missing" ]; then diff --git a/ci/plugin.conf b/ci/plugin.conf index 6daac95..0a0ce70 100644 --- a/ci/plugin.conf +++ b/ci/plugin.conf @@ -66,3 +66,15 @@ MIN_TRANSLATION_CALLS=60 # job token only covers the repo it runs in, so this is a cross-repo PAT and # Kristian's to add. RELEASE_REPO=kw/nolimit-discounts-dist + +# The sha256 of includes/shared-menu.php, which is byte-identical in all six +# plugins. Which copy a site loads is decided by activation order and PHP +# cannot un-declare a function, so the first one wins for the whole request — +# a shop with two plugins on one version and three on another gets whichever +# booted first, while every repository's own check is green on its own side. +# The frozen hash is therefore a family-wide fact, not a local one, and taking +# a new one is something all six do in the same window. +# +# 184cebdc = contract 1.1.0 (the ∞ glyph, and the header/count corrections +# Nolimit SEO found). Previous frozen hash was f4862d5c = 1.0.0. +WPMGR_MENU_SHA=184cebdcd86024d4e864c529bc68ead0f7d63656d9bb3dfe0e1fdf3203e364ae diff --git a/ci/static.sh b/ci/static.sh index dfff98c..a74c2a4 100755 --- a/ci/static.sh +++ b/ci/static.sh @@ -174,6 +174,24 @@ case "$update_uri" in *) note_fail "Update URI is '$update_uri' but releases are published to '$RELEASE_REPO'. Installed copies poll the header, so they would stop being offered updates without a word." ;; esac +ci_step "the shared menu file is the family copy" +# includes/shared-menu.php is byte-identical in all six plugins. Editing it +# here would not make this plugin's version win — which copy a site loads is +# decided by activation order, and PHP cannot un-declare a function — it would +# make the menu differ between shops depending on which plugin booted first, +# and change its mind when one is deactivated. So the file is frozen, and +# taking a new version is something all six do in the same window. +shared_menu=includes/shared-menu.php +if [ ! -f "$shared_menu" ]; then + echo "::warning::$shared_menu is not present, so this plugin registers no shared menu." +else + have=$(sha256sum "$shared_menu" | cut -d" " -f1) + echo " sha256: $have" + [ "$have" = "$WPMGR_MENU_SHA" ] \ + && echo " matches the frozen family copy" \ + || note_fail "$shared_menu does not match the frozen family copy. Expected $WPMGR_MENU_SHA, got $have. Do not edit it — take the new version the others took." +fi + ci_step "no secrets in tracked files" # Suggested by the AI-manager port, and it costs nothing: a pushed key cannot be # unpushed. It has to be rotated, and the window between the push and noticing diff --git a/includes/shared-menu.php b/includes/shared-menu.php index 45dd266..86dd81b 100644 --- a/includes/shared-menu.php +++ b/includes/shared-menu.php @@ -6,18 +6,21 @@ * │ THIS FILE IS BYTE-IDENTICAL IN EVERY MANAGER PLUGIN. │ * │ Do not edit it to add something for one plugin. Extend it through the │ * │ `wpmgr_dashboard_cards` filter, which is what it is for. │ - * │ Contract version 1.0.0 — frozen. See "Why it is frozen" below. │ + * │ Contract version 1.1.0. See "Why it is frozen" below — and note that │ + * │ the two changes it has had were both paid for in load-order confusion. │ * └──────────────────────────────────────────────────────────────────────────┘ * - * The problem it solves: five plugins from the same family, each adding a - * top-level menu, is five entries in a sidebar that already has fifteen. So - * they share one parent, "Manager", and each adds a submenu under it. + * The problem it solves: a family of plugins each adding a top-level menu is + * a handful of entries in a sidebar that already has fifteen. So they share + * one parent and each adds a submenu under it. Six of them as of this + * writing, which is why the count is not stated anywhere it would have to be + * maintained. * * Three things about how it does that are load-bearing, and none is obvious. * - * **1. Every declaration is guarded.** Five plugins each `require_once` their - * own copy of this file, in one request. `require_once` only deduplicates by - * resolved path, and these are five different paths — so without the + * **1. Every declaration is guarded.** Each plugin `require_once`s its own + * copy of this file, in one request. `require_once` only deduplicates by + * resolved path, and these are different paths — so without the * `function_exists()` guard, activating the second plugin is * `Fatal error: Cannot redeclare wpmgr_ensure_menu()`. A white screen on a * shop, caused by nothing except owning two of these plugins. It is the same @@ -29,8 +32,16 @@ * un-declare a function, so there is no honest way for a newer copy to replace * an older one that has already been loaded. The only stable answer is that * the file never changes. Hence: **frozen**. The parent slug, the capability, - * the position and the filter name are a contract between five repositories - * now, and anything new arrives through the filter. + * the position and the filter name are a contract between every repository in + * the family, and anything new arrives through the filter. + * + * 🔴 It has moved twice, and both times the cost was the same: a half-adopted + * change means the menu is whatever the alphabetically-first plugin's copy + * says, and every repository's own hash check goes green on its own side + * while the family disagrees. Nolimit SEO measured exactly that today — two + * plugins on 1.1.0 and three on 1.0.0, so a shop got the infinity glyph or + * the old cog depending on activation order. If this file has to change, + * every plugin takes it in the same hour or the change is not made. * * **3. The dashboard is built from a filter, not owned by a plugin.** If the * plugin that happened to register the menu is deactivated, the menu is @@ -58,7 +69,7 @@ if ( ! defined( 'ABSPATH' ) ) { } if ( ! defined( 'WPMGR_MENU_VERSION' ) ) { - define( 'WPMGR_MENU_VERSION', '1.0.0' ); + define( 'WPMGR_MENU_VERSION', '1.1.0' ); } if ( ! defined( 'WPMGR_MENU_SLUG' ) ) { @@ -92,19 +103,76 @@ if ( ! function_exists( 'wpmgr_ensure_menu' ) ) { */ $title = apply_filters( 'wpmgr_menu_title', 'Manager' ); + /** + * The glyph on the shared parent menu. + * + * An infinity sign, because the family is called Nolimit. A cog said + * nothing, and there is no infinity dashicon. + * + * Drawn as TEXT rather than as an SVG passed to add_menu_page(), and + * the reason is colour. WordPress renders a data-URI SVG as a CSS + * background-image, and a background-image cannot inherit a colour — + * so it stays one fixed grey while every other icon lightens on hover + * and turns white on the current item. It looks subtly broken under + * every admin colour scheme, including the default. A `::before` + * glyph is text: it takes the colour it is given and follows all of + * those states for free, with no image request. + * + * Filterable so that changing it later needs no edit to this file. + * This file is byte-identical across the family and hash-frozen in + * each plugin's CI, so an edit here is a coordinated release for all + * of them; a filter is one line in one plugin. + * + * @param string $glyph A single character, or '' for the old cog. + */ + $glyph = apply_filters( 'wpmgr_menu_glyph', '∞' ); + add_menu_page( $title, $title, 'manage_options', WPMGR_MENU_SLUG, 'wpmgr_render_dashboard', - 'dashicons-admin-generic', + // 'none' makes WordPress emit the icon wrapper with no dashicon + // class on it, which is the hook the style below needs. A dashicon + // name here would win instead, because its class carries its own + // ::before content. + '' === $glyph ? 'dashicons-admin-generic' : 'none', // Just below WooCommerce (55.x) and above Appearance (60). The // fractional position is not decoration: an integer collides with // whatever core or another plugin put there, and the loser is // simply dropped. 58.9 ); + + if ( '' !== $glyph ) { + add_action( 'admin_head', 'wpmgr_menu_glyph_style' ); + } + } + + /** + * The one style rule the glyph needs. + * + * Printed in the head on every admin page, because the menu is on every + * admin page — a stylesheet enqueued only on this family's own screens + * would leave a blank square everywhere else. + * + * `font-family` has to be overridden: WordPress sets the icon wrapper to + * the Dashicons font, and Dashicons has no infinity glyph, so inheriting + * it renders nothing at all. + */ + function wpmgr_menu_glyph_style() { + $glyph = apply_filters( 'wpmgr_menu_glyph', '∞' ); + + if ( '' === $glyph ) { + return; + } + + printf( + '', + esc_attr( WPMGR_MENU_SLUG ), + esc_attr( $glyph ) + ); } /** diff --git a/tests/smoke.php b/tests/smoke.php index 842783e..d5f04d2 100644 --- a/tests/smoke.php +++ b/tests/smoke.php @@ -314,17 +314,13 @@ wc_edm_is( isset( $wc_edm_mine[0]['colour'] ) ? $wc_edm_mine[0]['colour'] : null ); -// includes/shared-menu.php is byte-identical in all five plugins, and which -// copy a site loads is decided by activation order. Editing it here would make -// a site with three of five updated show one label or another depending on who -// booted first — and PHP cannot un-declare a function, so the first copy wins -// for the whole request. The hash is the guard: change the file and this goes -// red rather than the behaviour going strange on somebody's shop. -wc_edm_is( - 'and the shared menu file is the family copy, unedited', - 'f4862d5c3a2d24dadddaaec90c9221d61f2923964e164fd525a800ddcec63185', - hash_file( 'sha256', WC_EDM_PLUGIN_DIR . 'includes/shared-menu.php' ) -); +// The frozen hash of includes/shared-menu.php is asserted in ci/static.sh +// against WPMGR_MENU_SHA in ci/plugin.conf, and deliberately NOT repeated +// here: two copies of the same constant drift, and the one that drifts is the +// one nobody remembers to update. What is worth asserting at runtime is a +// different fact — that the file is actually loaded in a real install, which +// the parent-menu assertion above already proves, since wpmgr_ensure_menu() +// only exists if it was. /* ------------------------------------------------------------ woocommerce -- */