From dccb87d15e598dd8fa3b62f5fdba518ec000947f Mon Sep 17 00:00:00 2001 From: kristianwind Date: Mon, 28 Sep 2026 21:45:29 +0200 Subject: [PATCH] Document the plugin, and check that the documentation still describes it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five files under docs/, the names the family agreed: README, SETUP, ARCHITECTURE, CONTRACT, RELEASE. Plus ci/check-docs.php, ported from woo-security-manager 52dd99d, wired into the static job. CONTRACT.md is generated from the source rather than written from memory, and that was the right instruction: my first sweep read only single-quoted hook names and missed two rows, one of them update_plugins_{$hostname} โ€” the hook behind the live-shop collision, written with interpolation and therefore invisible to a pattern built from the concept of the line. Nine hooks documented: six wc_edm_* filters this plugin offers, three wpmgr_* it both offers and answers through its copy of the frozen shared-menu.php. Two hooks the checker structurally cannot see are listed by hand with a note saying so: wc_edm_prune_audit_log (add_action on a class constant) and update_plugins_{$hostname} (interpolated). A generated list beats a remembered one and is still a list of what the generator can see. The checker needed two fixes before it was safe to run here. ๐Ÿ”ด The published version reads HOOK_PREFIX from ci/plugin.conf for the documented side, and refuses without it โ€” then tests the source side against a hardcoded 'nlsec_'. Ported verbatim that is not a green over nothing, which would be easy to spot. Measured in this tree with CONTRACT.md documenting no hook at all: 3 hook(s) in the shipped source and not in docs/CONTRACT.md: wpmgr_dashboard_cards, wpmgr_menu_glyph, wpmgr_menu_title Red, specific, and blind to all six wc_edm_* hooks โ€” because 'wpmgr_' is the same string in every tree and 'nlsec_' is the one that is not. Fix those three and it goes green over six undocumented contract hooks, with a red run behind you as evidence that it works. A partially blind check that fails is more convincing than one that passes. Note which step disarmed the guard. The floor at 20 shipped PHP files refused to run in this 14-file tree, and that refusal was the only thing standing in the way; the porting instructions were to add HOOK_PREFIX and adjust the floor. Doing as told is what removed the protection โ€” two wrongs cancelling into a pass, in the order CLAUDE.md predicts, because the floor is the error you can see and the prefix is not. Both numbers now come from the conf (HOOK_PREFIX, MIN_SHIPPED_PHP_FILES) and both refuse rather than defaulting. The inherited literal 20 is gone: a second hardcoded number per plugin is a second thing to re-measure on every port. Ten selftest assertions, 58 total. Four failure modes, the two refusals, the counts on the pass, and the row that matters most: ๐Ÿ”ด A hook name in a COMMENT must not read as a hook call. CONTRACT.md documents a filter by showing the apply_filters() a consumer makes, and the same example sits in a docblock โ€” so a checker that reads comments reports this plugin consuming a filter it answers, in the file explaining why it must not. That assertion is a control asserting a PASS, which is the kind that can quietly become unable to fail, so it names the specific success line rather than reading an exit code, and it is proven both ways: with token_get_all's T_COMMENT branch made inert, exactly one of 58 assertions fails and it is that one. docs/TESTING-AND-RELEASES.md is folded into RELEASE.md and deleted rather than left beside it โ€” two copies of a rule is how the copies start to disagree. Folding surfaced that it still documented the pre-rename Update URI (kw/woocommerce-email-discount-manager) and still recommended, as future work, the split into a releases-only repository that has already happened. Both corrected in place, with the correction dated. docs/ is not in ci/dist-files.sh โ€” measured, 0 docs files in the zip. static=0 selftest=0 site=0; site still 69 assertions on WordPress 7.1.2 with WooCommerce 11.1.2. ci/ and docs/ ship nothing, so no version bump. Co-Authored-By: Claude Opus 5 (1M context) --- ci/check-docs.php | 452 +++++++++++++++++++++++++++++++++++ ci/plugin.conf | 1 + ci/selftest.sh | 104 ++++++++ ci/static.sh | 12 + docs/ARCHITECTURE.md | 138 +++++++++++ docs/CONTRACT.md | 111 +++++++++ docs/README.md | 70 ++++++ docs/RELEASE.md | 271 +++++++++++++++++++++ docs/SETUP.md | 96 ++++++++ docs/TESTING-AND-RELEASES.md | 263 -------------------- 10 files changed, 1255 insertions(+), 263 deletions(-) create mode 100644 ci/check-docs.php create mode 100644 docs/ARCHITECTURE.md create mode 100644 docs/CONTRACT.md create mode 100644 docs/README.md create mode 100644 docs/RELEASE.md create mode 100644 docs/SETUP.md delete mode 100644 docs/TESTING-AND-RELEASES.md diff --git a/ci/check-docs.php b/ci/check-docs.php new file mode 100644 index 0000000..f3ba2c5 --- /dev/null +++ b/ci/check-docs.php @@ -0,0 +1,452 @@ + + */ + +/** + * MIN_SHIPPED_PHP_FILES from ci/plugin.conf. Refuses rather than defaulting: + * an unset floor parses to 0 and every comparison against it passes, which is + * the same bug as a scan over zero files one level up. + */ +function shipped_php_floor() { + $conf = dirname( __DIR__ ) . '/ci/plugin.conf'; + + if ( is_file( $conf ) && preg_match( '/^MIN_SHIPPED_PHP_FILES=([0-9]+)/m', (string) file_get_contents( $conf ), $m ) && (int) $m[1] > 0 ) { + return (int) $m[1]; + } + + fwrite( STDERR, "::error::No usable MIN_SHIPPED_PHP_FILES in ci/plugin.conf โ€” refusing to run with no floor.\n" ); + exit( 1 ); +} + +/** + * This plugin's own hook prefix, read from ci/plugin.conf. + * + * Refuses rather than guessing, and is deliberately NOT derived from + * TEXT_DOMAIN: here the domain is wc-email-discount and the hooks are wc_edm_, + * and in the tree this file came from the domain is nolimit-security and the + * hooks are nlsec_. A checker that guessed would be wrong in both. + */ +function hook_prefix() { + static $prefix = null; + + if ( null !== $prefix ) { + return $prefix; + } + + $conf = dirname( __DIR__ ) . '/ci/plugin.conf'; + $prefix = ''; + + if ( is_file( $conf ) && preg_match( '/^HOOK_PREFIX=([A-Za-z0-9_]+)/m', (string) file_get_contents( $conf ), $m ) ) { + $prefix = strtolower( $m[1] ); + } + + if ( '' === $prefix ) { + fwrite( STDERR, "::error::No HOOK_PREFIX in ci/plugin.conf โ€” refusing to guess which hooks are ours.\n" ); + exit( 1 ); + } + + return $prefix; +} + +/** + * Is this hook part of a contract, or is it internal wiring? + * + * ๐Ÿ”ด A contract is a name SOMEBODY ELSE has to know. That is the whole rule, + * and getting it wrong in either direction ruins the check. + * + * The first version required every hook in the tree, which meant 50 core and + * WooCommerce names โ€” `admin_menu`, `init`, `wp_login`, `woocommerce_login_form` + * โ€” that we merely listen to. Listening to `init` is not a promise to anyone; + * WordPress documents `init`. Requiring them would have made CONTRACT.md a + * transcript of our own `add_action` calls, and the first person to read it + * would stop, which is worse than not having the check. + * + * So the test is the PREFIX, because the prefix is exactly "is this a name the + * family owns": + * + * do_action / apply_filters on nlsec_* or wpmgr_* + * we are the source, or we are asking the family. Document it. + * + * add_filter / add_action on wpmgr_* + * we are answering a name another plugin defined. It crosses a + * repository boundary, which is the class of value nobody's checks cover. + * Document it. + * + * add_filter / add_action on nlsec_* + * our own wiring. If it is also offered, the first rule already caught + * it; if it is not, nobody outside needs to know. + * + * anything else + * core, WooCommerce, a third party. Not ours to document. + * + * Narrowing it this way is not loosening it. It found three real gaps on the + * first run that the noisy version had buried under fifty core hooks: + * `wpmgr_ai_usage` (a family-wide hook this plugin EMITS and had never told + * anybody about), `nlsec_public_rest_routes` (a genuine extension point a form + * plugin needs when a shop locks REST down) and `nlsec_pending_wp_config`. + * + * @param string $call do_action | apply_filters | add_filter | add_action + * @param string $hook The hook name. + * @return bool + */ +function is_contract( $call, $hook ) { + // ๐Ÿ”ด From ci/plugin.conf, never a literal. The version of this file + // published as woo-security-manager 52dd99d asked plugin.conf for the + // prefix on the DOCUMENTED side (and refuses without it, correctly) and + // still tested the SOURCE side against a hardcoded 'nlsec_'. Ported here + // verbatim that is not a green-over-nothing โ€” it is worse. Measured on this + // tree with docs/CONTRACT.md documenting no hook at all: + // + // 3 hook(s) in the shipped source and not in docs/CONTRACT.md: + // wpmgr_dashboard_cards, wpmgr_menu_glyph, wpmgr_menu_title + // + // Red, specific, and blind to all six wc_edm_* hooks โ€” because 'wpmgr_' is + // the same string in every tree and 'nlsec_' is the one that is not. Fix + // those three and the check goes green over six undocumented contract + // hooks, with a red run behind you as evidence that it works. A partially + // blind check that fails is more convincing than one that passes. + // + // Note which step disarmed the guard: the floor at 20 shipped PHP files + // refused to run in this 14-file tree, which is the only thing that stopped + // it, and the porting instructions were to add HOOK_PREFIX and fix the + // floor. Doing as told is what removed the protection. Two wrongs + // cancelling into a pass, in the documented order. + $ours = 0 === strpos( $hook, hook_prefix() ); + $family = 0 === strpos( $hook, 'wpmgr_' ); + + if ( ! $ours && ! $family ) { + return false; + } + + if ( 'do_action' === $call || 'apply_filters' === $call ) { + return true; + } + + // add_filter / add_action: only when the name is the family's, i.e. we are + // answering somebody else. + return $family; +} + +$root = isset( $argv[1] ) ? rtrim( $argv[1], '/' ) : '.'; +$docs = $root . '/docs'; + +$fails = 0; + +function fail( $message ) { + global $fails; + $fails++; + printf( "::error::%s\n", $message ); +} + +/* + * 1. The set exists and is not a stub. + * + * A floor, not a mere existence test: an empty file passes `is_file()`, and a + * heading with nothing under it passes any check written from the concept of + * "is it documented". The numbers are per-file because the documents have + * genuinely different jobs โ€” README is one screen on purpose. + */ +$required = array( + 'README.md' => 25, + 'SETUP.md' => 40, + 'ARCHITECTURE.md' => 60, + 'CONTRACT.md' => 50, + 'RELEASE.md' => 40, +); + +$lines_total = 0; + +foreach ( $required as $file => $floor ) { + $path = $docs . '/' . $file; + + if ( ! is_file( $path ) ) { + fail( sprintf( 'docs/%s is missing.', $file ) ); + continue; + } + + $lines = count( file( $path, FILE_IGNORE_NEW_LINES ) ); + $lines_total += $lines; + + if ( $lines < $floor ) { + fail( sprintf( 'docs/%s has %d lines, under the floor of %d โ€” a stub is not documentation.', $file, $lines, $floor ) ); + } +} + +if ( $fails > 0 ) { + printf( "\n%d problem(s) with the document set.\n", $fails ); + exit( 1 ); +} + +printf( " %d documents, %d lines.\n", count( $required ), $lines_total ); + +/* + * 2. The version in the docs is this plugin's version. + * + * Named value, not "a version is mentioned": a check that merely finds a + * version-shaped string passes on last year's. + */ +$mains = glob( $root . '/*.php' ); +$version = ''; + +foreach ( (array) $mains as $main ) { + if ( preg_match( '/^\s*\*\s*Version:\s*([0-9][^\s]*)/m', (string) file_get_contents( $main ), $m ) ) { + $version = $m[1]; + break; + } +} + +if ( '' === $version ) { + fail( 'No Version: header found in the plugin main file โ€” nothing to compare the docs against.' ); +} else { + $readme = (string) file_get_contents( $docs . '/README.md' ); + + if ( false === strpos( $readme, $version ) ) { + fail( sprintf( 'docs/README.md does not mention version %s.', $version ) ); + } else { + printf( " docs/README.md names version %s, which is the header's.\n", $version ); + } +} + +/* + * 3. CONTRACT.md against the source, both directions. + */ +$listing = $root . '/ci/dist-files.sh'; + +if ( ! is_file( $listing ) ) { + fail( 'ci/dist-files.sh is missing โ€” cannot tell which files ship.' ); + exit( 1 ); +} + +exec( escapeshellcmd( $listing ) . ' 2>/dev/null', $shipped, $rc ); + +if ( 0 !== $rc ) { + fail( 'ci/dist-files.sh failed โ€” refusing to guess which files ship.' ); + exit( 1 ); +} + +$php = array_values( array_filter( $shipped, static function ( $f ) { + return '.php' === substr( $f, -4 ); +} ) ); + +// The floor comes from ci/plugin.conf, not from a literal. It arrived here as +// 20 โ€” measured in the tree this file was written in โ€” and refused to run in +// this 14-file one. A second hardcoded number per plugin is a second thing to +// re-measure on every port, and the conf already carries this one. +// +// The floor is not the fault: a copied kit that reads zero files prints a +// plausible number and passes. Refuse instead of reporting a green over +// nothing โ€” and refuse rather than warn, because a warning next to a plausible +// number is read as a plausible number. +$min_php = shipped_php_floor(); + +if ( count( $php ) < $min_php ) { + fail( sprintf( 'Only %d shipped PHP files found, under the floor of %d. That is too few to be this tree โ€” check the prefix and the paths before lowering this.', count( $php ), $min_php ) ); + exit( 1 ); +} + +$in_code = array(); +$anywhere = array(); + +foreach ( $php as $file ) { + $path = $root . '/' . $file; + + if ( ! is_file( $path ) ) { + continue; + } + + $source = (string) file_get_contents( $path ); + + // ๐Ÿ”ด Strip comments. See the header: our own docblocks contain the exact + // calls this check looks for, and reading them turns the file that + // explains a fault into the file that reports it. + $code = ''; + + foreach ( token_get_all( $source ) as $token ) { + if ( is_array( $token ) ) { + if ( T_COMMENT === $token[0] || T_DOC_COMMENT === $token[0] ) { + continue; + } + + $code .= $token[1]; + continue; + } + + $code .= $token; + } + + if ( preg_match_all( '/(do_action|apply_filters|add_filter|add_action)\(\s*\x27([a-z0-9_]+)\x27/', $code, $m, PREG_SET_ORDER ) ) { + foreach ( $m as $hit ) { + if ( is_contract( $hit[1], $hit[2] ) ) { + $in_code[ $hit[2] ] = true; + } + } + } + + /* + * ๐Ÿ”ด A second, wider set, and the two directions need different ones. + * + * "Must be documented" is the narrow question โ€” contract hooks only. + * "Has this row gone stale" is a different one, and asking it against the + * narrow set is wrong: it reported `nlsec_request_reputation` as no longer + * existing, which is this plugin's most important contract. The only + * occurrence in shipped code is `add_filter()` โ€” we ANSWER it, and the + * consumer applies it โ€” so the contract classifier correctly declines to + * demand it and the staleness check then read that as absence. + * + * Same for the five cron hooks, which the document names under a heading + * that says they are explicitly NOT a contract. + * + * So: a documented name is stale only when it appears NOWHERE in the + * shipped source. Every string literal, not just the ones in hook calls. + */ + if ( preg_match_all( '/\x27([a-z0-9_]+)\x27/', $code, $m ) ) { + foreach ( $m[1] as $literal ) { + $anywhere[ $literal ] = true; + } + } +} + +// WordPress core hooks, and our own scheduled jobs, are named in the document +// under headings of their own rather than in the tables. Listing them here is +// what stops the check reporting them as undocumented forever โ€” which is how a +// floor gets lowered until the check reads nothing. +$contract = (string) file_get_contents( $docs . '/CONTRACT.md' ); + +$documented = array(); + +if ( preg_match_all( '/`([a-z0-9_]+)`/', $contract, $m ) ) { + foreach ( $m[1] as $name ) { + $documented[ $name ] = true; + } +} + +$core = array( 'plugins_api', 'rest_api_init' ); + +$undocumented = array(); + +foreach ( array_keys( $in_code ) as $hook ) { + if ( ! isset( $documented[ $hook ] ) ) { + $undocumented[] = $hook; + } +} + +sort( $undocumented ); + +// The other direction is narrower on purpose: CONTRACT.md legitimately names +// things that are not hooks (fields, codes, functions), so only names carrying +// this plugin's own prefix are held to existing in the source. +// The prefix comes from ci/plugin.conf, which every plugin in the family has, +// rather than from a pattern over the hook names โ€” a copied kit that guesses +// the prefix from its own contents is how a check ends up reading zero files +// in the next tree and printing a plausible number. +$prefix = ''; + +if ( is_file( $root . '/ci/plugin.conf' ) && preg_match( '/^HOOK_PREFIX=([A-Za-z0-9_]+)/m', (string) file_get_contents( $root . '/ci/plugin.conf' ), $m ) ) { + $prefix = strtolower( $m[1] ); +} + +if ( '' === $prefix ) { + fail( 'No HOOK_PREFIX in ci/plugin.conf โ€” refusing to guess which documented names are ours.' ); + exit( 1 ); +} + +$stale = array(); + +foreach ( array_keys( $documented ) as $name ) { + // Long enough to be a hook rather than the bare prefix itself, which is + // also a table-name literal in the source. + if ( 0 !== strpos( $name, $prefix ) || strlen( $name ) <= strlen( $prefix ) + 3 ) { + continue; + } + + if ( isset( $anywhere[ $name ] ) ) { + continue; + } + + /* + * ๐Ÿ”ด A dynamic hook is built by concatenation, so the source carries the + * literal PREFIX and the document has to carry the full name a reader + * needs โ€” `update_option_{$option}`, `site_transient_{$transient}`, + * `plugin_action_links_{$file}`. Compared literally, that row is correct + * in both files and fails forever, and the fix somebody reaches for is + * deleting the row rather than the rule. Found by Nolimit Cache, whose + * run was 56/56 with exactly one such false positive. + */ + $dynamic = false; + + foreach ( array_keys( $anywhere ) as $literal ) { + if ( strlen( $literal ) > strlen( $prefix ) && 0 === strpos( $name, $literal ) ) { + $dynamic = true; + break; + } + } + + if ( ! $dynamic ) { + $stale[] = $name; + } +} + +sort( $stale ); + +if ( $undocumented ) { + fail( sprintf( + '%d hook(s) in the shipped source and not in docs/CONTRACT.md: %s', + count( $undocumented ), + implode( ', ', $undocumented ) + ) ); +} + +if ( $stale ) { + fail( sprintf( + '%d name(s) in docs/CONTRACT.md that no longer exist in the source: %s', + count( $stale ), + implode( ', ', $stale ) + ) ); +} + +if ( ! $undocumented && ! $stale ) { + printf( + " %d hooks in %d shipped PHP files, all %d in docs/CONTRACT.md (prefix %s, %d core).\n", + count( $in_code ), + count( $php ), + count( $in_code ), + '' !== $prefix ? $prefix . '*' : '?', + count( $core ) + ); +} + +if ( $fails > 0 ) { + printf( "\n%d documentation problem(s).\n", $fails ); + exit( 1 ); +} + +exit( 0 ); diff --git a/ci/plugin.conf b/ci/plugin.conf index 8540a2f..a7e02b3 100644 --- a/ci/plugin.conf +++ b/ci/plugin.conf @@ -109,3 +109,4 @@ OLD_PLUGIN_NAME="WooCommerce Email Discount Manager" # function scope, so a `return` partway through skips its own floor and the # run reports green having tested two thirds of what it claims. MIN_SITE_ASSERTIONS=60 +HOOK_PREFIX=wc_edm_ diff --git a/ci/selftest.sh b/ci/selftest.sh index 26abc72..232b751 100755 --- a/ci/selftest.sh +++ b/ci/selftest.sh @@ -370,6 +370,110 @@ else bad "only $pairs pairs compared โ€” the agreement check has almost nothing to look at" fi +# ------------------------------------------------------------------------ docs +# ๐Ÿ”ด ci/check-docs.php has four failure modes and one subtle exclusion, and the +# exclusion is the one that would regress unnoticed: it strips comments before +# reading the source. Without that, a CONTRACT.md example shown in a docblock +# reads as a real hook call โ€” so a file documenting a contract is reported as +# breaking it. Nolimit Security proved all four by hand in one sitting; four +# commands in a commit message are not an assertion that runs again next month. +# +# Unlike the git-identity rows, these fixtures cannot trip the checker over +# themselves: it reads only what ci/dist-files.sh ships, and ci/ and tests/ are +# not in that list. Verified rather than assumed โ€” dist-files.sh returns no +# ci/ path at all. +echo +echo "โ†’ ci/check-docs.php must catch a document that stopped describing this tree" +cd "$work/repo" +docs_verdict() { ( . ci/lib.sh && ci_load_conf && ci_php ci/check-docs.php . ) 2>&1 || true; } + +# 1. a hook in the source that no document mentions. +cp includes/class-helpers.php "$work/helpers.orig" +printf '\n// Added by ci/selftest.sh.\nfunction wc_edm_selftest_probe() {\n\treturn apply_filters( %s, true );\n}\n' \ + "'wc_edm_selftest_undocumented'" >> includes/class-helpers.php +if [ "$(grep -c 'wc_edm_selftest_undocumented' includes/class-helpers.php)" -lt 1 ]; then + bad "could not stage the undocumented-hook fixture" +else + printf '%s' "$(docs_verdict)" | grep -q 'not in docs/CONTRACT.md: wc_edm_selftest_undocumented' \ + && ok "flagged a hook the source offers and CONTRACT.md does not mention" \ + || bad "NOT FLAGGED: an undocumented hook โ€” a sibling cannot discover it without reading our source" +fi +cp "$work/helpers.orig" includes/class-helpers.php + +# 2. ๐Ÿ”ด the control that matters: the same name, in a COMMENT only. The checker +# must stay GREEN, and this asserts the specific success line rather than an +# exit code โ€” a refusal, a missing file or a fatal all exit non-zero too, and +# any of them would read as "the comment was ignored". +printf '\n// apply_filters( %s, true ) โ€” an example in prose, not a call.\n' \ + "'wc_edm_selftest_in_a_comment'" >> includes/class-helpers.php +if [ "$(grep -c 'wc_edm_selftest_in_a_comment' includes/class-helpers.php)" -lt 1 ]; then + bad "could not stage the comment-only fixture" +else + comment_out=$(docs_verdict) + if printf '%s' "$comment_out" | grep -qE 'all [0-9]+ in docs/CONTRACT.md'; then + ok "a hook name in a comment is not a hook call (comment stripping still works)" + else + printf '%s\n' "$comment_out" | sed 's/^/ /' + bad "a hook name in a COMMENT was read as a call โ€” a docblock documenting a contract now breaks the build" + fi +fi +cp "$work/helpers.orig" includes/class-helpers.php + +# 3. the other direction: a row for a hook that no longer exists. Silent without +# this check, and the row reads as deliberate. +cp docs/CONTRACT.md "$work/contract.orig" +printf '\n| `wc_edm_selftest_renamed_away` | `bool $x` | `true` | nothing |\n' >> docs/CONTRACT.md +if [ "$(grep -c 'wc_edm_selftest_renamed_away' docs/CONTRACT.md)" -lt 1 ]; then + bad "could not stage the stale-row fixture" +else + printf '%s' "$(docs_verdict)" | grep -q 'wc_edm_selftest_renamed_away' \ + && ok "flagged a CONTRACT.md row whose hook is not in the source" \ + || bad "NOT FLAGGED: a stale CONTRACT.md row โ€” the document describes a tree that is not this one" +fi +cp "$work/contract.orig" docs/CONTRACT.md + +# 4. a document emptied to its heading. "Exists" passes over a stub. +cp docs/SETUP.md "$work/setup.orig" +printf '# Setting it up\n' > docs/SETUP.md +printf '%s' "$(docs_verdict)" | grep -q 'docs/SETUP.md has 1 lines' \ + && ok "flagged a document emptied to its heading" \ + || bad "NOT FLAGGED: a one-line SETUP.md โ€” 'the file exists' is not 'the file says anything'" +cp "$work/setup.orig" docs/SETUP.md + +# 5. the version drifting away from the header. +sed -i 's/^Version 1\.[0-9]*\.[0-9]*/Version 9.9.9/' docs/README.md +if [ "$(grep -c '^Version 9.9.9' docs/README.md)" -lt 1 ]; then + bad "could not stage the version-drift fixture" +else + printf '%s' "$(docs_verdict)" | grep -q 'does not mention version' \ + && ok "flagged docs naming a version the header does not" \ + || bad "NOT FLAGGED: docs at 9.9.9 against a different header โ€” the docs describe another release" +fi +git checkout -q -- docs/README.md 2>/dev/null || cp "$work/README.md.orig" docs/README.md + +# 6. both conf values must refuse rather than default. An unset floor parses to +# 0 and every comparison against it passes, and a guessed prefix classifies +# every hook in the tree as somebody else's. +cp ci/plugin.conf "$work/conf.orig" +for key in HOOK_PREFIX MIN_SHIPPED_PHP_FILES; do + grep -v "^$key=" "$work/conf.orig" > ci/plugin.conf + if [ "$(grep -c "^$key=" ci/plugin.conf)" -ne 0 ]; then + bad "could not stage the missing-$key fixture" + continue + fi + printf '%s' "$(docs_verdict)" | grep -q "No .*$key" \ + && ok "refuses to run with no $key rather than guessing" \ + || bad "ran without $key โ€” a guessed prefix or a zero floor passes over everything" +done +cp "$work/conf.orig" ci/plugin.conf + +# And the pass must carry both numbers. "The documentation matches" over zero +# hooks in zero files is the same green as over nine in fourteen. +docs_clean=$(docs_verdict) +printf '%s' "$docs_clean" | grep -qE '^ [0-9]+ hooks in [0-9]+ shipped PHP files, all [0-9]+ in docs/CONTRACT.md' \ + && ok "the docs check reports how many hooks and how many files it read" \ + || bad "the docs check printed no counts โ€” it cannot tell 9 hooks in 14 files from 0 in 0" + # ------------------------------------------------------------------- plugin.conf echo echo "โ†’ the floors must be required, not defaulted" diff --git a/ci/static.sh b/ci/static.sh index 48b4242..f328625 100755 --- a/ci/static.sh +++ b/ci/static.sh @@ -451,6 +451,18 @@ for field in 'Plugin Name' 'Description' 'Author' 'Author URI' 'Plugin URI'; do done [ "$fails" = "$before" ] && echo " name, description, author and links are all filled in" +ci_step "the documentation still describes this tree" +# A document nobody checks goes stale, and the expensive half is not "does the +# file exist" โ€” it is whether docs/CONTRACT.md and the source still name the +# same hooks, in both directions. A hook we offer and never documented is a +# name a sibling plugin cannot discover without reading our source; a row whose +# hook we since renamed is a comment describing a tree that is not this one. +# +# Ported from woo-security-manager 52dd99d. Its floors and its hook prefix come +# from ci/plugin.conf and it refuses rather than defaulting โ€” see the note in +# the file about why the prefix cannot be derived from TEXT_DOMAIN. +ci_php ci/check-docs.php . || fails=$((fails + 1)) + ci_step "the compatibility headers still tell the truth" # Advisory, never fatal. WordPress ships a new version whether or not anything # here is wrong, and a build that goes red on release day for a header nobody diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md new file mode 100644 index 0000000..e11e14b --- /dev/null +++ b/docs/ARCHITECTURE.md @@ -0,0 +1,138 @@ +# Architecture + +## The files that ship + +18 files. `ci/`, `tests/` and `docs/` are not among them โ€” `ci/dist-files.sh` +decides, and a static check asserts that every PHP file the code loads is in +that list. + +| file | what it is | +| --- | --- | +| `woocommerce-email-discount-manager.php` | header, constants, boot order, activation and deactivation | +| `includes/class-db.php` | the five tables, `dbDelta()` schema, and every query | +| `includes/class-helpers.php` | matching an email to an assignment | +| `includes/class-frontend.php` | cart totals and shipping filtering | +| `includes/class-admin.php` | the menu, the three tabs, the dashboard card | +| `includes/class-admin-groups.php` | the Groups tab | +| `includes/class-admin-emails.php` | the Emails tab | +| `includes/class-admin-shipping.php` | the Shipping tab | +| `includes/class-admin-audit.php` | the audit log viewer | +| `includes/class-audit-log.php` | writing and pruning the audit log | +| `includes/class-rate-limiter.php` | per-method REST rate limiting | +| `includes/class-rest-api.php` | the `wc-edm/v1` routes | +| `includes/class-updater.php` | the release-feed update check | +| `includes/shared-menu.php` | the family's shared parent menu โ€” byte-identical in all eight plugins | +| `assets/css/admin.css`, `assets/js/admin.js`, `assets/icon.svg` | admin UI and the icon WordPress draws on the Updates screen | +| `README.txt` | the readme WordPress parses | + +## Boot order + +The order matters and two of the steps are deliberately before the WooCommerce +check. + +1. **File load.** Constants are defined: `WC_EDM_VERSION`, + `WC_EDM_PLUGIN_FILE`, `WC_EDM_PLUGIN_DIR`, `WC_EDM_PLUGIN_URL`. +2. **`plugins_loaded`** โ†’ `init()`. +3. **The updater loads first**, before the WooCommerce check. A shop with + WooCommerce switched off for an afternoon must not quietly stop receiving + fixes for this plugin. +4. **The shared menu title filter is added**, also before that check, and for a + related reason: the shared parent menu is registered by whichever family + plugin boots first, and its label must not depend on whether *this* plugin + got past a gate. +5. **WooCommerce check.** Missing โ†’ an admin notice, and `init()` returns. The + plugin is dormant, not broken. +6. Text domain, then `includes()`. A file that failed to load is collected and + reported as a notice rather than fataling. +7. **`WC_EDM_DB` and `WC_EDM_Helpers` are required to continue.** Without + either, `init()` returns and the notice stands โ€” everything else builds on + them. +8. `init` priority 5 โ†’ `WC_EDM_DB::maybe_upgrade()`, so a schema change lands + after an update that skipped re-activation. +9. Admin, frontend, audit log and REST objects are constructed, each behind a + `class_exists()` check. + +The shared menu is registered under `WPMGR_MENU_SLUG` with a +`function_exists( 'wpmgr_ensure_menu' )` fallback, so a site where an older +sibling's copy of `shared-menu.php` loads first still gets a menu. + +## Where data lives + +### Tables + +Five, all prefixed `wp_wc_edm_` (whatever the site's `$wpdb->prefix` is), +created by `dbDelta()` with foreign keys added afterwards: + +- `wp_wc_edm_groups` โ€” discount groups +- `wp_wc_edm_group_categories` โ€” group โ†’ product category +- `wp_wc_edm_emails` โ€” email patterns and their assignment +- `wp_wc_edm_shipping_rules` โ€” group โ†’ shipping method, show or hide +- `wp_wc_edm_audit_log` โ€” admin actions + +### Options + +- `wc_edm_db_version` โ€” schema version, the only thing `maybe_upgrade()` reads + +That is the complete list. This plugin stores no settings in options; the three +tabs write to the tables. + +### Transients + +- `wc_edm_release_feed` โ€” the update feed response +- `wc_edm_card_counts` โ€” the two numbers on the dashboard card, 5 minutes + +### Scheduled events + +One: `wc_edm_prune_audit_log`, daily, scheduled on `init` if not already +present and unscheduled on deactivation. It deletes audit rows older than the +retention window. + +## Lifecycle + +**Activate** (`register_activation_hook`) โ€” refuses with an error if WooCommerce +is not active, then creates the tables. + +**Deactivate** (`register_deactivation_hook`) โ€” unschedules the prune event. +Tables, options and transients stay. + +**Uninstall** โ€” ๐Ÿ”ด **nothing happens.** There is no `uninstall.php` and no +`register_uninstall_hook`, so deleting the plugin in wp-admin removes the files +and leaves all five tables, both options and the transients in the database. +Deliberate: deleting a plugin by accident should not destroy a shop's discount +configuration. The cost is that a genuine clean removal is manual โ€” drop the +five `wp_wc_edm_*` tables and delete `wc_edm_db_version` by hand. + +## The customer-facing path + +`woocommerce_cart_calculate_fees` (priority 20) adds the discount as a negative +fee. `woocommerce_before_calculate_totals` (priority 20) applies a coupon for +the Coupon assignment type. `woocommerce_package_rates` (priority 20, 2 args) +filters shipping. + +The shipping filter's shape is the part worth reading in full, because a subtle +version of it shipped free delivery to strangers for several releases: + +```php +$match = $this->get_current_match(); + +if ( $match && 'group' === $match->assignment_type && $match->group_id ) { + $rates = $this->apply_group_shipping_rules( $rates, (int) $match->group_id ); +} else { + $rates = $this->remove_restricted_rates( $rates ); +} +``` + +The `else` is the fix. A customer who matches nothing must still have the +reserved methods taken away; before 1.6.0 that branch did not exist, so the +restriction only applied to people who matched a group โ€” i.e. to nobody who +mattered. `apply_group_shipping_rules()` also falls back to +`remove_restricted_rates()` when a group has no rules at all, for the same +reason. + +## Not present + +- No drop-in, no mu-plugin, nothing that runs before WordPress boots. +- Nothing hooks `the_content` or buffers output. +- No cron beyond the one prune event. +- No custom post types, no taxonomies, no user meta. +- No `WP_CLI` commands. diff --git a/docs/CONTRACT.md b/docs/CONTRACT.md new file mode 100644 index 0000000..68b1df3 --- /dev/null +++ b/docs/CONTRACT.md @@ -0,0 +1,111 @@ +# Contract + +Every hook this plugin offers, and every family hook it answers. Version 1.6.3. + +A contract is a name somebody else has to know. Core hooks this plugin merely +listens to โ€” `init`, `admin_menu`, `wp_enqueue_scripts` โ€” are not here; +WordPress documents those, and listening to `init` is not a promise to anyone. +The exception is at the bottom, and it is there because it is shared with every +sibling plugin on the same update host. + +The tables and the source are compared in both directions by +`ci/check-docs.php`, which runs in CI. A hook added without a row here fails +the build, and so does a row whose name no longer exists in the source. + +## Hooks this plugin provides + +All six are filters, all read a setting that has a working default, and all are +answered from `apply_filters()` in shipped code. A shop that adds no code gets +the default and never needs this table. + +| hook | signature | default | if you are absent | +| --- | --- | --- | --- | +| `wc_edm_audit_log_enabled` | `bool $enabled` | `true` | the audit log records admin actions | +| `wc_edm_audit_log_retention_days` | `int $days` | `30` | rows older than 30 days are pruned daily; the value is floored at 1, so `0` means 1 day, not "keep forever" | +| `wc_edm_client_ip` | `string $ip` | the resolved address | the plugin's own resolution is used โ€” override this behind a proxy | +| `wc_edm_rate_limit_enabled` | `bool $enabled` | `true` | REST routes are rate limited | +| `wc_edm_rate_limit_max_requests` | `int $default, string $method` | per method | the built-in allowance applies; floored at 1 | +| `wc_edm_rate_limit_window` | `int $seconds, string $method` | `MINUTE_IN_SECONDS` | a 60-second window; floored at 1 | + +**What a consumer must not do.** Do not return a non-scalar: each of these is +cast (`(bool)`, `(int)`, `(string)`) and a bad type becomes a wrong value rather +than an error. Do not assume `wc_edm_rate_limit_*` can switch limiting off by +returning `0` โ€” they are floored at 1; use `wc_edm_rate_limit_enabled`. +Returning `false` from `wc_edm_audit_log_enabled` on a shop that has a +compliance reason to keep the log is a decision, not a performance tweak. + +## The family's shared hooks + +These cross a repository boundary, which is the class of value nobody's checks +cover, so both directions are written out. + +| hook | this plugin | what that means | +| --- | --- | --- | +| `wpmgr_menu_title` | **answers** it (main file) and **provides** it (our copy of `shared-menu.php`) | returns `Nolimit`, so the shared parent menu is labelled the same whichever sibling registered it | +| `wpmgr_dashboard_cards` | **answers** it (`class-admin.php`) and **provides** it (our copy of `shared-menu.php`) | adds this plugin's card โ€” colour `#1a7a52`, two counts, cached 5 minutes | +| `wpmgr_menu_glyph` | **provides** it only, via our copy of `shared-menu.php` | this plugin does not override the glyph; it answers with the shared default `โˆž` | + +**Why "provides" and "answers" are both true.** `includes/shared-menu.php` is +byte-identical in all eight family plugins and frozen at a SHA in +`ci/plugin.conf`. Whichever copy loads first is the one that runs, so this +plugin is simultaneously one of the eight possible providers of these three +hooks and a consumer of whichever copy won. That is why the label is set with a +filter and not by editing the shared file: editing it would make a site with +three of eight plugins updated show a different menu title depending on +activation order, and change its mind when a plugin is deactivated. + +**What a consumer must not do.** Do not edit `shared-menu.php` to change any of +these โ€” it reaches every site in activation order and the frozen SHA will fail +the build in all eight trees. Answer the filter from your own plugin. +`wpmgr_menu_title` must be idempotent: every sibling adds a callback and the +order is not defined. + +## One core hook, because it is shared + +| hook | this plugin | why it is in a contract document | +| --- | --- | --- | +| `update_plugins_{$hostname}` | **answers** it, `add_filter( โ€ฆ, 10, 3 )` | ๐Ÿ”ด the filter name is built from the *hostname*, not the repository, so every plugin updating from `gitea.nolimit.dk` lands on the same hook and WordPress runs every callback for every one of them | + +Answer it unconditionally and you offer your release to your neighbour. On a +live shop in September 2026 the subscriptions plugin was told version 1.4.4 was +an update for itself; 1.4.4 was this plugin's. Load order decides who wins, so +a rename is enough to flip which plugin gets corrupted โ€” and flipped toward one +with auto-updates on, it installs a stranger's zip over itself unattended. + +The guard is two halves and both are required: + +```php +add_filter( "update_plugins_{$host}", array( $this, 'offer_update' ), 10, 3 ); + +// and inside offer_update(): +if ( '' !== $plugin_file && plugin_basename( WC_EDM_PLUGIN_FILE ) !== $plugin_file ) { + return $update; +} +``` + +Leave the argument count at 2 and `$plugin_file` never arrives, so the guard +silently does nothing โ€” the same shape of bug it fixes. **No integration test +can catch this**, because a test site installs one plugin from that host and the +collision cannot arise; the assertion is the defence. + +`plugins_api` is also answered and is safe, because it guards on `$args->slug`. + +## What the checker cannot see, and is therefore listed by hand + +`ci/check-docs.php` reads hook names out of the shipped source as string +literals. Two names in this tree are not literals at the call site: + +| hook | how it is written | why it is missed | +| --- | --- | --- | +| `wc_edm_prune_audit_log` | `add_action( self::CRON_HOOK, โ€ฆ )` | a class constant, resolved at runtime | +| `update_plugins_{$hostname}` | `add_filter( "update_plugins_{$host}", โ€ฆ )` | interpolated from a variable | + +Both rows above are real and both would survive a rename of the constant or the +variable without the checker noticing. `wc_edm_prune_audit_log` is the daily +audit-log prune event; it is internal wiring rather than an extension point, +but it is a name that appears in `wp_next_scheduled()` output and in any cron +viewer, so it is written down. + +This section exists because the instrument's blind spot is worth stating next +to its results. A generated list is better than a remembered one, and it is +still a list of what the generator can see. diff --git a/docs/README.md b/docs/README.md new file mode 100644 index 0000000..9f0b4f3 --- /dev/null +++ b/docs/README.md @@ -0,0 +1,70 @@ +# Nolimit Discounts + +Version 1.6.3 ยท a WooCommerce plugin ยท slug `woocommerce-email-discount-manager` + +Gives a customer a discount, and decides which shipping methods they may +choose, based on the email address they check out with. + +## What it does + +A shop puts in email patterns and says what each one is worth. At checkout the +plugin looks at the customer's email, finds the most specific pattern that +matches, and applies it. + +Three kinds of assignment, one per email pattern: + +| type | effect | +| --- | --- | +| **Group** | the customer gets a discount group's percent, on the product categories attached to that group | +| **Coupon** | an existing WooCommerce coupon is applied automatically | +| **Custom** | a flat percent off the whole cart | + +Patterns can be a full address (`john@example.com`), a domain +(`@company.com` โ€” everyone there), or a fragment (`john` โ€” any address +containing it). When more than one matches, the most specific wins: exact +address, then domain, then fragment. + +## Shipping + +A discount group can be given shipping rules, which is the half worth reading +twice because it decides what a stranger sees. + +A method a group is told to **show** is *reserved* for that group. Customers +who match no group do not see it. Before 1.6.0 a method set up for approved +company addresses stayed on the checkout for everybody, so free delivery meant +for one customer could be chosen by anyone who found it โ€” the bug that release +fixed. + +The plugin never leaves a customer with no shipping method at all. If applying +the rules would empty the list, every method comes back. + +## What it is not + +Facts that take a while to establish by reading the source, so they are written +down here: + +- **Not a coupon engine.** The Coupon assignment type applies a coupon + WooCommerce already has. This plugin does not create, price or validate one. +- **No public REST surface.** There is a REST API, and every route requires + `manage_woocommerce`. Nothing is readable by an anonymous visitor. +- **No telemetry.** Nothing is sent anywhere. The only outbound request is the + update check against the release feed, and it asks for a version number. +- **No block editor or shortcode.** The customer-facing side is the cart and + checkout calculation and nothing else. +- **Data survives uninstall.** There is no `uninstall.php` and no uninstall + hook, so deleting the plugin leaves its five tables and its options in the + database. That is deliberate โ€” deleting a plugin by accident should not + destroy a shop's discount configuration โ€” but it means a clean removal is a + manual job. See ARCHITECTURE.md. + +## Where the rest is + +| document | what is in it | +| --- | --- | +| [SETUP.md](SETUP.md) | installing it, and how to tell it is actually working | +| [ARCHITECTURE.md](ARCHITECTURE.md) | files, boot order, where data lives, what activation does | +| [CONTRACT.md](CONTRACT.md) | every hook this plugin offers and every one it answers | +| [RELEASE.md](RELEASE.md) | how a release is cut, and what is checked before it is called done | + +Requires WordPress 5.8+, WooCommerce 6.0+, PHP 7.4+. Tested against WordPress +7.1.2 and WooCommerce 11.1.2 on PHP 7.4, 8.3 and 8.4. diff --git a/docs/RELEASE.md b/docs/RELEASE.md new file mode 100644 index 0000000..b3bc740 --- /dev/null +++ b/docs/RELEASE.md @@ -0,0 +1,271 @@ +# Releasing + +One release a day at most, cut by hand. Version 1.6.3 is current. + +## The sequence + +1. **Branch**, one change per branch. +2. **Bump the version if anything shipped changes.** It lives in four places + and a static check asserts they agree: the `Version:` header, + `WC_EDM_VERSION`, `README.txt`'s `Stable tag`, and a new `README.txt` + changelog entry. A `ci/` or `docs/` change ships nothing, so it gets no + bump. + โš ๏ธ Add a changelog entry; never rewrite an existing one. An entry describes + what was true for that release, and editing it makes the changelog describe + a release that never happened. +3. **Run all three suites locally**, unpiped, and read the numbers: + `./ci/static.sh` ยท `./ci/selftest.sh` ยท `./ci/site-test.sh` +4. **Push and open a PR.** `origin` has two push URLs, so one push reaches both + GitHub and Gitea โ€” read both halves of the output, it can fail for one and + succeed for the other. +5. **Wait for CI on both forges**, then merge. +6. **Verify the effect on `main`, not the sha.** `git show origin/main:` + and look for the thing you changed. A squash or rebase gives every commit a + new sha, so an ancestor check can report a missing commit that is sitting + there. +7. **`git push gitea main`.** A merge commit is created on GitHub and nothing + carries it to Gitea. This is a step, not a repair โ€” it is needed after every + merge. +8. **`./ci/build-zip.sh`**, then publish the release to + `kw/nolimit-discounts-dist` and attach the zip. +9. **Verify the download anonymously** and compare it byte-for-byte with the + local build. Never paste a URL Gitea built: its `ROOT_URL` is a raw IP on + plain HTTP, so `browser_download_url` points around the Cloudflare tunnel. + Rebuild the URL from the hostname and keep Gitea's path. + +Reading the release JSON: use a JSON parser, never `sed`. A greedy +`"id":([0-9]+)` matches the last id in the response, and the payload embeds an +author object and an assets array with ids of their own. **Assert the object you +found is the one you wanted** (`tag_name == $TAG`) before writing to it โ€” a 404 +is loud, an id that belongs to something else is not. + +## What is verified before it is called done + +| suite | what it proves | the number to read | +| --- | --- | --- | +| `ci/static.sh` | syntax, guards, text domain, no secrets, every loaded file ships, one version everywhere, the two compat files agree, no tracked file writes a git identity, the docs still describe this tree | a count on every passing line | +| `ci/selftest.sh` | **the checkers themselves** โ€” see below | `all N assertions passed` | +| `ci/site-test.sh` | a real WordPress + WooCommerce + MySQL install, this plugin activated, `tests/smoke.php` run through `wp eval-file` | `All 69 assertions passed`, floored at 60 by the caller | +| `.github/workflows/compat.yml` | the matrix: PHP 7.4 / 8.3 / 8.4, WP latest and nightly, WC latest and beta | one job per cell | + +## The checkers are themselves tested, and that is unusual + +`ci/selftest.sh` exists because a check that examined nothing prints the same +line as one that examined everything, so a hollow check survives verification +indefinitely. It plants a fault and asserts that **this** check is the one that +fails โ€” by exit status and by the specific message, never by a filename or a log +line that merely sits near the signal. + +Every assertion carries a **landing check**: a `grep -c` proving the plant is in +the file *before* its verdict is believed. A `sed` or a `printf` that silently +matched nothing leaves the tree in its control state, every row passes, and the +run reads as "this check cannot be broken" โ€” the same hollow success as a scan +over zero files, wearing the costume of the most rigorous thing in the suite. + +What is proven **both ways** (red on a planted fault, green on a control): + +- the credential scan โ€” one fixture per shape it claims to catch, plus + lookalike controls that must not fire, and one fixture per directory and file + *type*, because a scope change re-dates every earlier measurement +- the direct-access guard โ€” a file mentioning `ABSPATH` only in a comment is + still flagged, which the grep version of that check passed +- `ci/check-textdomain.php` โ€” three zero-verdicts, and an above-the-real-count + floor +- the workflow guard โ€” an unparseable workflow, an ungated multi-cell matrix, + and parsing zero files +- the git-identity scan โ€” sixteen rows: five writes, three repairs, two reads, + the scoped `git -c` form, a commented mention, and a write outside `ci/` +- the compat-value agreement โ€” three fields drifted one at a time +- the documentation checker โ€” see below +- the floors โ€” each broken once and watched to fail + +What is proven **red only**, and says so: the form check asserting no check +consumes an `xargs` exit status. `xargs` returns 123 when any batch found +nothing, which `if` reads as clean โ€” but this tree's files fit one batch, so +nothing here can measure the fault. That assertion is a drift check on the +shape, not evidence the replacement is correct. + +## Documentation + +`ci/check-docs.php` asserts the five files in `docs/` exist, clear a length +floor, name the current version, and that `docs/CONTRACT.md` and the source +agree in both directions. `docs/` is not in `ci/dist-files.sh`, so none of it +ships. + +Its two floors and its hook prefix come from `ci/plugin.conf` +(`MIN_SHIPPED_PHP_FILES`, `HOOK_PREFIX`) and it refuses to run rather than +defaulting. The prefix is not derivable from `TEXT_DOMAIN` โ€” here the domain is +`wc-email-discount` and the hooks are `wc_edm_`. + +โš ๏ธ **When porting this checker to another plugin, change the prefix before +lowering the floor.** The floor is the error you can see; the prefix is not. The +version published as `woo-security-manager` `52dd99d` read the prefix from the +conf on the documented side and still tested the source side against a +hardcoded `nlsec_`. In this tree, with the floor fixed and an empty +`CONTRACT.md`, it reported **3** missing hooks instead of **9** โ€” red, specific, +and blind to every one of this plugin's own. A partially blind check that fails +is more convincing than one that passes. + +--- + +## Proving an update actually arrives + +`ci/update-smoke.sh` installs the **previous** version into a throwaway shop +and then gets out of the way: WordPress's own update check finds the new one, +downloads it and installs it. + +```sh +FROM_VERSION=1.4.0 ci/update-smoke.sh +``` + +Nothing else can stand in for it. Every piece of the chain can be individually +correct and the update still not arrive: + +- a package URL on an address the shop cannot reach, +- a zip whose top-level directory is not the plugin slug โ€” which installs a + *second* copy beside the first and leaves the old one active, +- a version WordPress does not consider newer than what is installed. + +It checks all three, plus that the plugin is still active afterwards and that +WordPress stops offering the update once it is applied. + +Run it after a release. + +--- + +## How the automatic updates work + +The plugin header carries: + +``` +Update URI: https://gitea.nolimit.dk/kw/nolimit-discounts-dist +``` + +(It said `kw/woocommerce-email-discount-manager` until 2026-09-28. The source +repository was renamed to `kw/nolimit-discounts` and releases were split into a +separate `-dist` repository โ€” the move this document's own "one-time setup" +section below recommended, carried out and then not written down here.) + +`includes/class-updater.php` hooks `update_plugins_gitea.nolimit.dk`, the +filter WordPress fires for plugins whose `Update URI` points at that host. It +reads `/api/v1/repos/kw//releases/latest`, takes the tag as the version +and the attached `.zip` as the package, and hands both to WordPress. WordPress +does the version comparison, the download, the unpacking and the rollback. + +That means all of this works with no extra code: + +- the update notice on **Plugins** and **Dashboard โ†’ Updates** +- the **update now** link +- the per-plugin **Enable auto-updates** link +- `wp plugin update woocommerce-email-discount-manager` over WP-CLI + +### Turning on automatic updates + +Plugins screen โ†’ the **Automatic updates** column on the right โ†’ **Enable +auto-updates**. WordPress then installs new versions by itself on its +twice-daily check. + +That link is not free for a plugin outside wordpress.org. +`WP_Plugins_List_Table` marks a plugin as `update-supported` only if it finds +it in the update transient's `response` list (an update is waiting) or its +`no_update` list (we were asked, there is none): + +```php +// wp-admin/includes/class-wp-plugins-list-table.php +$plugin_data = array_merge( (array) $plugin_info->response[ $plugin_file ], array( 'update-supported' => true ), $plugin_data ); +$plugin_data = array_merge( (array) $plugin_info->no_update[ $plugin_file ], array( 'update-supported' => true ), $plugin_data ); +} elseif ( empty( $plugin_data['update-supported'] ) ) { + $plugin_data['update-supported'] = false; +``` + +A plugin in neither list gets "Auto-updates are not available for this plugin" +and no toggle โ€” while looking entirely healthy otherwise. That is why +`offer_update()` answers even when the installed version is already current: +the answer lands in `no_update`, and the link stays. `tests/smoke.php` builds +the real list table and asserts the flag rather than trusting the reasoning. + +### "Check again" really checks again + +The feed is cached for fifteen minutes, and the cache is dropped entirely when +wp-admin is loaded with `force-check`. + +It was six hours, on the reasoning that WordPress checks twice a day anyway. +That reasoning was wrong. WordPress gates its own check **by screen**: + +| Screen | How often WordPress re-checks | +| --- | --- | +| Dashboard โ†’ Updates | every minute | +| Plugins | every hour | +| Background / cron | every 2โ€“12 hours | + +This filter only runs when that check runs, so WordPress is already the rate +limiter. A cache longer than its cadence does nothing but hide new releases +underneath it โ€” a release published, and a shop shown nothing. + +From the command line, `wp transient delete wc_edm_release_feed` does the same. + +### The icon + +`assets/icon.svg`, handed to WordPress in the offer's `icons` key. Without it +Dashboard โ†’ Updates shows a generic grey plug. WordPress takes the first of +`svg`, `2x`, `1x`, `default` that is set. + +It is served from the shop's own copy of the plugin, not from the release host: +one fewer thing that has to be reachable for wp-admin to render correctly, and +it cannot go mixed-content on an HTTPS shop. The mark is drawn as paths rather +than ``, because an SVG that leans on a font renders differently +everywhere โ€” and this one is shown at about 40px, which is the size where that +matters. + +`Update URI` earns its place a second way: since WordPress 5.8, a plugin that +declares one is **never asked about on wordpress.org**. Without it, a .org +plugin that happened to share this slug could be pushed onto a shop running +this one. + +### The two repositories, and why there are two + +Every shop reads the release feed without credentials, so whatever `Update URI` +points at must be **public**. + +That is now a repository of its own: `kw/nolimit-discounts-dist` holds releases +and nothing else, and `kw/nolimit-discounts` holds the source. The updater +derives everything from the header, so splitting them was a one-line change โ€” +and it means the source repository's visibility is a separate decision from the +feed's. + +โš ๏ธ The source repository is **still public** as of 1.6.3. Making it private is +safe only once no installation is still pointing its `Update URI` at the old +address; an install on a pre-1.5.0 build reads its feed from +`kw/nolimit-discounts`, and making that private would silently stop its +updates. The old repository's feed still resolves and tops out at v1.4.6, +which carries the neighbour guard. + +### The download address is rebuilt, not taken + +The updater ignores the `browser_download_url` the API hands back and builds +the address from the `Update URI` header instead. + +That field is filled from the forge's own configured root URL, which is not +necessarily the address the outside world reaches it on. `gitea.nolimit.dk` +returns `http://57.129.89.117:25001/...` there โ€” plain HTTP, a raw IP, a +non-standard port, and around the Cloudflare tunnel in front of it. A shop +handed that would be downloading plugin code over an unencrypted connection to +an address nothing guarantees will keep working. + +The paths Gitea reports are correct; only the host is wrong. So the host comes +from the header โ€” the one place that already had to be right for any of this +to work โ€” and the path is kept. + +This is worth knowing because of *how* it hides: a CI job on the same network +reaches that raw IP perfectly well, so a verification step that follows the +reported URL goes green. Only a real client outside fails. The release check +therefore downloads the address **the plugin builds**, and opens the zip to +confirm it is the plugin rather than an error page with a 200 on it. + +### It reports nothing + +The update check sends a plain unauthenticated GET for a file every visitor to +that repository can read. The user agent is overridden on purpose: WordPress's +default one is `WordPress/7.1; https://the-shop.example`, which would tell the +update host which shops are running this plugin. Nothing else about the site +leaves it, ever. diff --git a/docs/SETUP.md b/docs/SETUP.md new file mode 100644 index 0000000..eda02ff --- /dev/null +++ b/docs/SETUP.md @@ -0,0 +1,96 @@ +# Setting it up + +## Installing + +Updates come from the plugin's own release feed, not wordpress.org. + +1. Download `woocommerce-email-discount-manager.zip` from the latest release at + `https://gitea.nolimit.dk/kw/nolimit-discounts-dist/releases`. +2. **Plugins โ†’ Add New โ†’ Upload Plugin.** +3. Activate. WooCommerce must already be active โ€” activation stops with an + error message if it is not, rather than activating into a broken state. + +After the first install, the plugin offers its own updates in **Dashboard โ†’ +Updates** like any other. Nothing else needs configuring for that to work. + +### If the update fails with "could not copy file" + +The plugin directory is not writable by the user PHP runs as. This is a file +ownership problem on the server, not a fault in the release โ€” WordPress cannot +overwrite files it does not own. Check it as the web server's user rather than +as root, because root can write everything and the check will pass for the +wrong reason: + +``` +docker exec sh -c 'find /var/www/html/wp-content/plugins/woocommerce-email-discount-manager -type f | wc -l' +docker exec -u 33 sh -c 'find /var/www/html/wp-content/plugins/woocommerce-email-discount-manager -type f -writable | wc -l' +``` + +The two numbers must be equal. `33` is `www-data` on the Debian-based images; +confirm with `ps -o user,args -e | grep apache` before trusting it. + +## The three screens + +**WooCommerce โ†’ Nolimit Discounts** (also reachable under the shared **Nolimit** +menu) has three tabs. + +### Groups + +A group is a name, a discount percent, and a list of WooCommerce product +categories. The percent applies to products in those categories and nothing +else. Categories are ordered by drag and drop; the order is cosmetic. + +### Emails + +One row per email pattern. Each row picks **one** of Group, Coupon or Custom โ€” +they are exclusive, and the form will not let a row carry two. + +This is the screen where a decision is actually required: **how specific should +the pattern be?** A domain pattern (`@company.com`) gives the discount to every +current and future address at that company, including ones nobody has seen yet. +A fragment pattern (`john`) matches `john@anywhere.com` and also +`johnson@example.org`. If a discount is worth real money, use the full address. + +### Shipping + +Per group, per shipping method, one of **show** or **hide**. + +The consequence worth understanding before using this tab: a method any group +is told to **show** becomes reserved. Customers who match no group stop seeing +it everywhere on the shop, not only inside that group. That is the point โ€” it +is how "free delivery for approved addresses" stays unavailable to everybody +else โ€” but it means adding one show rule changes what strangers see. + +## How to tell it is working + +Not "no errors". A specific number on a specific screen: + +1. **WooCommerce โ†’ Nolimit Discounts โ†’ Emails.** The row you added is listed + with its type and its value. If the tab is empty after saving, the write + failed and nothing else below will work. + +2. **The Nolimit dashboard card.** **Nolimit** in the admin menu shows a card + for this plugin with two counts โ€” groups and email assignments. They should + equal what the Groups and Emails tabs show. The card is cached for five + minutes, so wait that long before reading it as disagreement. + +3. **The real test, in a cart.** Log in as (or check out with) an address that + matches a pattern and put a product from a covered category in the cart. + The cart totals show a **negative fee line** carrying the group's name. No + line means no match โ€” check the pattern, not the percent. + +4. **Shipping, if you set any rules.** With rules in place, compare the + checkout as a matching customer and in a private window as a stranger. The + reserved method must be absent for the stranger. Both should show at least + one method; if the stranger sees *everything*, the rules emptied the list + and the never-leave-them-stranded fallback returned all of them. + +5. **Updates.** **Dashboard โ†’ Updates** names this plugin with a version and + draws its icon. A broken image there means the release is missing its + `assets/` โ€” report it rather than pressing update. + +## Settings that need no decision + +There is no settings page beyond the three tabs. Rate limiting on the REST API +and the audit log's retention are on by default and adjustable only by filter +โ€” see CONTRACT.md. A shop that does not want to write code never touches them. diff --git a/docs/TESTING-AND-RELEASES.md b/docs/TESTING-AND-RELEASES.md deleted file mode 100644 index aac1b95..0000000 --- a/docs/TESTING-AND-RELEASES.md +++ /dev/null @@ -1,263 +0,0 @@ -# Testing, releases and automatic updates - -Three questions, answered by three things in this repository. - -| Question | Answer | -| --- | --- | -| Does it still work on the newest WordPress and WooCommerce? | `ci/site-test.sh`, run weekly by `compat.yml` against `latest`, `nightly` and `beta` | -| Can updates be pushed out automatically? | Yes โ€” `includes/class-updater.php` plus `release.yml` | -| What happens when *I* press update in wp-admin? | Nothing surprising, because the release was tested against those versions before it was published | - ---- - -## Running the tests - -```sh -ci/static.sh # syntax, hygiene, versions, headers โ€” seconds -ci/site-test.sh # a real WordPress + WooCommerce โ€” a few minutes -ci/build-zip.sh # what a shop actually installs -FROM_VERSION=1.4.0 \ - ci/update-smoke.sh # let WordPress update itself from the live feed -``` - -`ci/site-test.sh` takes three knobs: - -```sh -WP_VERSION=latest # or nightly, or an exact version like 7.0 -WC_VERSION=latest # or beta, or an exact version like 11.1.0 -PHP_VERSION=8.3 # any tag of the wordpress:cli image -``` - -It builds a throwaway site in Docker containers, installs WordPress and -WooCommerce at those versions, copies in **exactly the files the release zip -will contain**, activates the plugin, and runs `tests/fixtures.php` and -`tests/smoke.php` inside it with `wp eval-file`. - -The tests use a real cart, real coupons and real `WC_Shipping_Rate` objects. -That is the point. A mocked cart keeps passing after WooCommerce has changed -underneath it, which is the one failure this whole directory exists to catch. - -Everything is torn down afterwards โ€” containers, volumes and the network โ€” so -running it twice leaves nothing behind. - -### Deprecations count as failures - -The site test switches on `WP_DEBUG` and fails the build on any notice logged -from this plugin's directory. That is how a WordPress or PHP upgrade actually -breaks a plugin: nothing fails, a notice appears, and two releases later it is -a fatal. Notices from WordPress core and WooCommerce are ignored โ€” failing on -those would make the build red for something nobody here can fix, and a build -that is always red is a build that gets ignored. - ---- - -## The weekly compatibility run - -`compat.yml` runs the matrix every Monday at 06:00 and on demand: - -| WordPress | WooCommerce | PHP | What it tells you | -| --- | --- | --- | --- | -| latest | latest | 8.3 | today's shop | -| **nightly** | latest | 8.3 | the next WordPress, weeks early | -| latest | **beta** | 8.3 | the next WooCommerce, weeks early | -| latest | latest | 7.4 | the floor the plugin header promises | -| latest | latest | 8.4 | where deprecations appear first | - -The `beta` cell falls back to the stable release when there is no open beta, so -it is never silently skipped. - -When the whole matrix passes, a second job compares `Tested up to:` and -`WC tested up to:` against the current releases and opens a pull request if -they are behind. It runs only after the matrix is green, because those headers -are a claim โ€” WooCommerce shows `WC tested up to` to the shop owner as an -untested-plugin banner, and that banner is what they read before deciding -whether to press update. - -The schedule fires on both forges; the matrix job skips the scheduled run on -Gitea so the single self-hosted runner is not asked to repeat GitHub's work. -`workflow_dispatch` still works there. - ---- - -## Cutting a release - -1. Bump `Version:` in the plugin header. `ci/static.sh` checks that - `WC_EDM_VERSION` and the readme's `Stable tag:` agree with it โ€” WordPress - compares the update feed against the header and nothing else, so a - disagreement means the update is offered again immediately after it is - installed, forever. -2. Add a changelog section to `README.txt`. -3. Merge to `main`. -4. Tag it **annotated**, because the message is the release notes: - ```sh - git tag -a v1.4.0 -m "..." - git push origin v1.4.0 # origin pushes to GitHub and Gitea both - ``` - -`release.yml` then runs the static checks, installs the plugin into a real -WordPress + WooCommerce and runs the full test suite, builds the zip, publishes -the release, and finally **reads the feed back anonymously** to prove a shop -can actually download it. - -Two things in there are guards against bugs that are invisible without them: - -- **The tag message is re-fetched before it is read.** `actions/checkout` - fetches every tag correctly and then overwrites the triggering one with the - commit SHA, turning an annotated tag into a lightweight one. Reading it then - returns the *commit* message โ€” on a merge commit, "Merge pull request #12 - from โ€ฆ". Nothing fails; the release is just published with the wrong text. -- **The anonymous read-back sends no credentials.** The two ways the feed - breaks โ€” a private repository and a release with no zip attached โ€” both - leave a release page that looks perfectly fine to anyone logged in. - ---- - -## Proving an update actually arrives - -`ci/update-smoke.sh` installs the **previous** version into a throwaway shop -and then gets out of the way: WordPress's own update check finds the new one, -downloads it and installs it. - -```sh -FROM_VERSION=1.4.0 ci/update-smoke.sh -``` - -Nothing else can stand in for it. Every piece of the chain can be individually -correct and the update still not arrive: - -- a package URL on an address the shop cannot reach, -- a zip whose top-level directory is not the plugin slug โ€” which installs a - *second* copy beside the first and leaves the old one active, -- a version WordPress does not consider newer than what is installed. - -It checks all three, plus that the plugin is still active afterwards and that -WordPress stops offering the update once it is applied. - -Run it after a release. - ---- - -## How the automatic updates work - -The plugin header carries: - -``` -Update URI: https://gitea.nolimit.dk/kw/woocommerce-email-discount-manager -``` - -`includes/class-updater.php` hooks `update_plugins_gitea.nolimit.dk`, the -filter WordPress fires for plugins whose `Update URI` points at that host. It -reads `/api/v1/repos/kw//releases/latest`, takes the tag as the version -and the attached `.zip` as the package, and hands both to WordPress. WordPress -does the version comparison, the download, the unpacking and the rollback. - -That means all of this works with no extra code: - -- the update notice on **Plugins** and **Dashboard โ†’ Updates** -- the **update now** link -- the per-plugin **Enable auto-updates** link -- `wp plugin update woocommerce-email-discount-manager` over WP-CLI - -### Turning on automatic updates - -Plugins screen โ†’ the **Automatic updates** column on the right โ†’ **Enable -auto-updates**. WordPress then installs new versions by itself on its -twice-daily check. - -That link is not free for a plugin outside wordpress.org. -`WP_Plugins_List_Table` marks a plugin as `update-supported` only if it finds -it in the update transient's `response` list (an update is waiting) or its -`no_update` list (we were asked, there is none): - -```php -// wp-admin/includes/class-wp-plugins-list-table.php -$plugin_data = array_merge( (array) $plugin_info->response[ $plugin_file ], array( 'update-supported' => true ), $plugin_data ); -$plugin_data = array_merge( (array) $plugin_info->no_update[ $plugin_file ], array( 'update-supported' => true ), $plugin_data ); -} elseif ( empty( $plugin_data['update-supported'] ) ) { - $plugin_data['update-supported'] = false; -``` - -A plugin in neither list gets "Auto-updates are not available for this plugin" -and no toggle โ€” while looking entirely healthy otherwise. That is why -`offer_update()` answers even when the installed version is already current: -the answer lands in `no_update`, and the link stays. `tests/smoke.php` builds -the real list table and asserts the flag rather than trusting the reasoning. - -### "Check again" really checks again - -The feed is cached for fifteen minutes, and the cache is dropped entirely when -wp-admin is loaded with `force-check`. - -It was six hours, on the reasoning that WordPress checks twice a day anyway. -That reasoning was wrong. WordPress gates its own check **by screen**: - -| Screen | How often WordPress re-checks | -| --- | --- | -| Dashboard โ†’ Updates | every minute | -| Plugins | every hour | -| Background / cron | every 2โ€“12 hours | - -This filter only runs when that check runs, so WordPress is already the rate -limiter. A cache longer than its cadence does nothing but hide new releases -underneath it โ€” a release published, and a shop shown nothing. - -From the command line, `wp transient delete wc_edm_release_feed` does the same. - -### The icon - -`assets/icon.svg`, handed to WordPress in the offer's `icons` key. Without it -Dashboard โ†’ Updates shows a generic grey plug. WordPress takes the first of -`svg`, `2x`, `1x`, `default` that is set. - -It is served from the shop's own copy of the plugin, not from the release host: -one fewer thing that has to be reachable for wp-admin to render correctly, and -it cannot go mixed-content on an HTTPS shop. The mark is drawn as paths rather -than ``, because an SVG that leans on a font renders differently -everywhere โ€” and this one is shown at about 40px, which is the size where that -matters. - -`Update URI` earns its place a second way: since WordPress 5.8, a plugin that -declares one is **never asked about on wordpress.org**. Without it, a .org -plugin that happened to share this slug could be pushed onto a shop running -this one. - -### One-time setup - -The repository on Gitea must be **public**, because every shop reads that feed -without credentials. Settings โ†’ make it public. Nothing in a GPL plugin zip is -secret, and the release workflow fails loudly with this exact instruction if it -is not done. - -If you would rather keep the source private, create a separate public repo that -holds only releases and point `Update URI` at that instead โ€” the updater -derives everything else from the header, so that is a one-line change. - -### The download address is rebuilt, not taken - -The updater ignores the `browser_download_url` the API hands back and builds -the address from the `Update URI` header instead. - -That field is filled from the forge's own configured root URL, which is not -necessarily the address the outside world reaches it on. `gitea.nolimit.dk` -returns `http://57.129.89.117:25001/...` there โ€” plain HTTP, a raw IP, a -non-standard port, and around the Cloudflare tunnel in front of it. A shop -handed that would be downloading plugin code over an unencrypted connection to -an address nothing guarantees will keep working. - -The paths Gitea reports are correct; only the host is wrong. So the host comes -from the header โ€” the one place that already had to be right for any of this -to work โ€” and the path is kept. - -This is worth knowing because of *how* it hides: a CI job on the same network -reaches that raw IP perfectly well, so a verification step that follows the -reported URL goes green. Only a real client outside fails. The release check -therefore downloads the address **the plugin builds**, and opens the zip to -confirm it is the plugin rather than an error page with a 200 on it. - -### It reports nothing - -The update check sends a plain unauthenticated GET for a file every visitor to -that repository can read. The user agent is overridden on purpose: WordPress's -default one is `WordPress/7.1; https://the-shop.example`, which would tell the -update host which shops are running this plugin. Nothing else about the site -leaves it, ever.