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.