diff --git a/ci/update-smoke.sh b/ci/update-smoke.sh index 3dce184..cac320f 100755 --- a/ci/update-smoke.sh +++ b/ci/update-smoke.sh @@ -92,13 +92,39 @@ echo " status: $status" [ "$now" = "$expected" ] || { echo "Expected $expected after the update." >&2; exit 1; } [ "$status" = active ] || { echo "The plugin is no longer active after updating itself." >&2; exit 1; } -# A zip whose top-level directory is not the slug installs a SECOND copy -# beside the first instead of replacing it, and the old one stays active. -count=$(docker exec "$id-cli" sh -c 'ls /var/www/html/wp-content/plugins | wc -l') +ci_step "one copy of the plugin, not two" +# A zip whose top-level directory is not the slug installs a SECOND copy beside +# the first instead of replacing it, and the old one stays active. This used to +# count the directories into a variable and then never look at it — a check +# that could not fail, sitting under a comment explaining exactly what it was +# for. docker exec "$id-cli" ls /var/www/html/wp-content/plugins | sed 's/^/ /' +copies=$(docker exec "$id-cli" sh -c \ + "ls -d /var/www/html/wp-content/plugins/$PLUGIN_SLUG* 2>/dev/null | wc -l" | tr -d '[:space:]') +echo " directories starting with $PLUGIN_SLUG: $copies" +[ "$copies" = 1 ] || { + echo "Expected exactly one, got $copies. The zip unpacked to a directory that is not the slug." >&2 + exit 1 +} + +ci_step "the code that is now running reports the new version" +# Files can be replaced while the running code still reports the old version — +# an opcache that was not invalidated, or a constant defined somewhere else. +# The constant name is read out of the plugin header rather than hard-coded: +# this script is copied between plugins, and a name that does not exist over +# there prints an empty value and carries on cheerfully. +const=$(sed -n -E "s/^[[:space:]]*define\([[:space:]]*'([A-Z_]*VERSION)'.*/\1/p" "$PLUGIN_MAIN" | head -1) +if [ -n "$const" ]; then + running=$(wp eval "echo defined( '$const' ) ? $const : '';" | tr -d '[:space:]') + echo " $const = ${running:-}" + [ "$running" = "$expected" ] || { + echo "The files say $expected but the running code says '${running:-undefined}'." >&2 + exit 1 + } +else + echo " $PLUGIN_MAIN defines no *_VERSION constant — nothing to cross-check." +fi -ci_step "and it still works afterwards" -wp eval 'echo WC_EDM_VERSION;' | sed 's/^/ WC_EDM_VERSION: /' wp plugin list --name="$PLUGIN_SLUG" --field=update | grep -qx none \ && echo " WordPress no longer offers an update — the header and the feed agree." diff --git a/docs/REUSE.md b/docs/REUSE.md index 648df72..143c498 100644 --- a/docs/REUSE.md +++ b/docs/REUSE.md @@ -126,6 +126,20 @@ measures something else. It cost two rounds here, once on If a script reports something you just fixed as still missing, `git status` before you debug the script. +## Make a check fail once before you believe it + +Three checks in this directory turned out to be incapable of failing, and all +three sat under a comment explaining exactly what they were for: + +- `tests/smoke.php` printed `FAIL` and exited 0 for four releases. +- `ci/static.sh`'s PHP lint passed while linting an empty directory. +- `ci/update-smoke.sh` counted the plugin directories into a variable and never + looked at it — under a comment about a zip installing a second copy. + +None of them were caught by reading the code, and a green build is exactly what +a broken check looks like. So: break it on purpose once, watch it go red, then +put it back. It costs one run. + ## Two guards worth understanding before you move them **`.gitea/workflows/` hides `.github/workflows/` completely.** Gitea uses the @@ -144,9 +158,17 @@ the *last* id in the payload, and a release object carries an embedded `author` and, once anything is attached, an `assets` array — each with an id of its own. `ci/publish-release.sh` uses a four-line `json_field()` over `python3` instead. Measured here: the greedy pattern returns the asset ids 1/2/3/4 for -releases whose real ids are 416/417/419/421. Four releases were published -correctly anyway, purely because this instance's author id is **-1** and the -minus sign stops `[0-9]+` from matching. That is not a safety margin. +releases whose real ids are 416/417/419/421. + +Four releases were published correctly anyway, and the reason is **who +published them, not which instance**: a release created by the Actions job +token has the ghost user as its author (`"id":-1`), and the minus sign stops +`[0-9]+` from matching. A release created by a real user — a cross-repo PAT, or +one published by hand — has `"id":1` and the greedy match lands on it. Do not +read the `-1` as a safety margin; it lasts until the first release somebody +publishes by hand. And on a re-run the lookup fetches a response that *has* +assets, where the last id is an asset id: positive, on any instance, with any +token. **A forge's own download URL is not the address your users can reach.** `browser_download_url` is built from the forge's configured root URL; diff --git a/includes/class-updater.php b/includes/class-updater.php index 94c0c0b..e9c4d8e 100644 --- a/includes/class-updater.php +++ b/includes/class-updater.php @@ -72,6 +72,9 @@ class WC_EDM_Updater { // And "Check again" has to actually check again. add_action( 'admin_init', array( $this, 'forget_on_force_check' ) ); + + // "View version details" next to the update notice. + add_filter( 'plugins_api', array( $this, 'plugin_details' ), 10, 3 ); } /** @@ -155,6 +158,58 @@ class WC_EDM_Updater { ); } + /** + * Answer the "View version details" modal. + * + * WordPress builds that link as + * plugin-install.php?tab=plugin-information&plugin=, which calls + * plugins_api() — and plugins_api() asks **wordpress.org**. This plugin is + * not there, so without this filter the link opens a modal containing an + * error, on the same screen that is otherwise telling the shop owner an + * update is ready. Nothing warns you: the link looks like every other + * plugin's. + * + * @param false|object|array $result Response so far. + * @param string $action Requested action. + * @param object $args Request arguments. + * @return false|object|array + */ + public function plugin_details( $result, $action, $args ) { + $slug = dirname( plugin_basename( WC_EDM_PLUGIN_FILE ) ); + + if ( 'plugin_information' !== $action || empty( $args->slug ) || $args->slug !== $slug ) { + return $result; + } + + require_once ABSPATH . 'wp-admin/includes/plugin.php'; + $plugin = get_plugin_data( WC_EDM_PLUGIN_FILE, false, false ); + $release = $this->latest_release(); + + $notes = $release && '' !== $release['notes'] ? $release['notes'] : ''; + + return (object) array( + 'name' => $plugin['Name'], + 'slug' => $slug, + 'version' => $release ? $release['version'] : $plugin['Version'], + 'author' => $plugin['Author'], + 'author_profile'=> $plugin['AuthorURI'], + 'homepage' => $plugin['PluginURI'], + 'requires' => $plugin['RequiresWP'], + 'requires_php' => $plugin['RequiresPHP'], + 'tested' => $release ? $release['tested'] : '', + 'download_link' => $release ? $release['package'] : '', + 'icons' => array( 'svg' => WC_EDM_PLUGIN_URL . 'assets/icon.svg' ), + 'sections' => array( + 'description' => wpautop( esc_html( $plugin['Description'] ) ), + // Plain text from the release notes, not markdown: rendering it + // as HTML would mean trusting whatever is in a release body. + 'changelog' => '' !== $notes + ? wpautop( esc_html( $notes ) ) + : wpautop( esc_html__( 'Release notes are published with each release.', 'wc-email-discount' ) ), + ), + ); + } + /** * Drop the cached feed. */ @@ -288,6 +343,9 @@ class WC_EDM_Updater { 'package' => $base . '/releases/download/' . rawurlencode( $tag ) . '/' . rawurlencode( $zip ), 'url' => $base . '/releases/tag/' . rawurlencode( $tag ), 'tested' => $tested, + // The release notes are the changelog. Kept so the "View version + // details" modal has something true to show. + 'notes' => isset( $body['body'] ) ? (string) $body['body'] : '', ); } diff --git a/tests/smoke.php b/tests/smoke.php index 23f0a69..ca8903a 100644 --- a/tests/smoke.php +++ b/tests/smoke.php @@ -347,6 +347,56 @@ set_transient( $offered = wc_edm()->updater->offer_update( false, get_plugin_data( WC_EDM_PLUGIN_FILE, false, false ) ); wc_edm_is( 'the current version is answered too, not passed over', WC_EDM_VERSION, is_array( $offered ) ? $offered['version'] : null ); +// The "View version details" link next to the update notice. WordPress builds +// it as plugin-install.php?tab=plugin-information&plugin=, which calls +// plugins_api() — and plugins_api() asks wordpress.org, where this plugin does +// not exist. Unanswered, that link opens a modal reading "Plugin not found." +// on the same screen that is telling the shop owner an update is ready. +// Measured on a live shop before this was written. +set_transient( + WC_EDM_Updater::TRANSIENT, + array( + 'version' => '99.0.0', + 'package' => 'https://gitea.nolimit.dk/kw/demo/releases/download/v99.0.0/demo.zip', + 'url' => 'https://gitea.nolimit.dk/kw/demo', + 'tested' => '7.1', + 'notes' => "v99.0.0 — a headline\n\nA line of release notes.", + ), + 600 +); + +// plugins_api() lives in the admin includes, which a WP-CLI context does not +// load. wp-admin has it; this does not. +require_once ABSPATH . 'wp-admin/includes/plugin-install.php'; + +$wc_edm_slug = dirname( plugin_basename( WC_EDM_PLUGIN_FILE ) ); +$details = plugins_api( 'plugin_information', array( 'slug' => $wc_edm_slug ) ); + +wc_edm_ok( + 'the details modal is answered here, not by wordpress.org', + ! is_wp_error( $details ), + is_wp_error( $details ) ? $details->get_error_message() : '' +); +wc_edm_is( 'and it is about this plugin', $wc_edm_slug, is_wp_error( $details ) ? null : $details->slug ); +wc_edm_is( 'it reports the version from the feed', '99.0.0', is_wp_error( $details ) ? null : $details->version ); +wc_edm_ok( + 'the changelog is the release notes', + ! is_wp_error( $details ) && false !== strpos( $details->sections['changelog'], 'A line of release notes' ), + 'a modal with an empty changelog is barely better than an error' +); +wc_edm_ok( + 'and the notes are escaped rather than rendered', + ! is_wp_error( $details ) && false === strpos( $details->sections['changelog'], 'updater->plugin_details( false, 'plugin_information', (object) array( 'slug' => 'akismet' ) ), + 'answering for somebody else would replace their modal with ours' +); + +delete_transient( WC_EDM_Updater::TRANSIENT ); + /* * The "Enable auto-updates" link on the Plugins screen. *