Repository navigation
Add lint/test tooling and CI, make GTM configurable, document bindings - #25
Merged
Merged
Conversation
composer audit flagged high-severity advisories in squizlabs/ php_codesniffer, phpcsstandards/phpcsutils and wp-coding-standards/wpcs. All three are dev-only and never shipped in the release zip, but they run on every lint, locally and soon in CI. Lock-only update within the existing constraints: WPCS 3.4.1, PHP_CodeSniffer 3.13.6, PHPCSUtils 1.2.3. composer audit is now clean. Refs #14 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
composer lint ran bare phpcs with no ruleset and no path, so it exited 3 without checking anything. phpcs.xml.dist selects the WordPress standard, the jc prefix and the jc-core-functionality text domain, and excludes vendor/, node_modules/ and tests/ (the PHPUnit bootstrap stubs core functions by their real, unprefixed names). Only the prefix-length check is disabled: renaming every function and the public jc_csp_* filters to satisfy a 3-character minimum would break existing hooks. Declares Requires at least 6.5 (block bindings, Requires Plugins and script modules all need it) and Requires PHP 8.0 (the live server), so WordPress refuses to activate on anything older and the ruleset's minimum_wp_version has a source. lint and lint-fix now resolve phpcs and phpcbf from vendor/bin through Composer. Refs #14 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The 23 errors left after the phpcbf pass needed judgment rather than auto-fixing: function docblocks with @param/@return, sentence-case inline comments, $quoteTitle -> $quote_title, and the plugin header's @Package tag. jc_add_json_save_point() drops its unused $path parameter; WordPress still passes it, and PHP ignores extra arguments to userland callbacks. Hook registrations move below the functions they register, which is where WPCS expects them; they still run at file load, so behavior is unchanged. composer lint now exits 0. Refs #14 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Brings the two test files that drifted in line with wp-scripts format, so the CI formatting check starts from a clean tree. acf-json/ is ignored: ACF rewrites those files in its own layout every time a field group or post type is saved in the admin, so reformatting them would be undone on the next save and then fail CI. CHANGELOG.md is generated by commit-and-tag-version and is ignored for the same reason. Refs #14 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
npm test ran lint-staged, which had no config, so it did nothing and passed. It now runs both suites (node:test, then PHPUnit). lint:php is added alongside the existing JS and package.json linters. husky installs a pre-commit hook on npm install (prepare) that runs lint-staged: phpcs on staged PHP, wp-scripts format on staged JS, JSON, YAML and Markdown. Same tools CI runs, so the hook and CI cannot disagree. Matches rcid-core-functionality. package.json keys are reordered to satisfy lint:pkg-json, which CI will run; no values change. Refs #14 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Nothing checked a change before it was tagged: release.yml only runs on a v*.*.* tag. CI now runs on every pull request and on main: - php-lint on PHP 8.0, the plugin's minimum and the live server's version: php -l on every plugin file, composer validate, composer lint. PHPUnit 12 needs 8.3, so this job relaxes the platform check to install the dev tools; PHPUnit is never run here. - tests on PHP 8.3: npm test (node:test, then PHPUnit). - js: lint:pkg-json, and wp-scripts format must leave the tree unchanged, the same command the pre-commit hook runs. Based on rcid-core-functionality's lint.yml, plus the tests job. Closes the optional CI item in #14. Refs #14 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The container ID was hard-coded twice and GTM loaded for everyone, including the site owner, so every admin visit was counted in analytics. Staging had no way to turn it off. jc_gtm_container_id() defaults to the live container (GTM-WNP9BDSD), so updating changes nothing on the live site. Override it with a JC_GTM_ID constant in wp-config.php or the jc_gtm_container_id filter; '' disables GTM. Anything that is not a GTM-[A-Z0-9]+ ID is treated as '', so a typo turns GTM off rather than printing an arbitrary string into a script. Users with manage_options get neither snippet. The ID is escaped for each context: wp_json_encode() in the inline script, esc_url() on the noscript iframe. Fixes #15 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The info page documented only jc/copyright. It now covers jc/user-data (its key and userId arguments, what each key returns, and an example block), and shows the active GTM container, that admins are skipped, and how to change or disable it with JC_GTM_ID. jc_add_json_load_point() removed ACF's theme load path behind a comment reading "(optional)". It is deliberate: the plugin is the single source of the content model, so a theme's leftover acf-json/ must not load duplicates or overrides. The docblock now says so, and how to add a theme path without undoing it. Fixes #16 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
GtmTest pins the behavior #15 changed: - the default stays the live container, so updating changes nothing - the loader goes through the script API and gets the CSP nonce - administrators get neither snippet; other logged-in users still do - the filter and the JC_GTM_ID constant override the ID (the constant tests run in separate processes, since a constant cannot be unset) - '' and invalid IDs, including script-breakout and markup payloads, print nothing at all SettingsPageTest now checks the jc/user-data example (#16) and the GTM line, including its disabled state. Against main, 12 of these fail or error. Refs #15, #16 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
README gains the JC_GTM_ID / filter configuration and the admin skip, and a Development section covering npm test, composer lint, the pre-commit hook and what CI checks. CLAUDE.md drops the stale "no phpcs.xml" and "npm test does nothing" notes, records the PHP 8.0 vs PHPUnit 8.3 split, and the deliberate ACF load-path removal. ROADMAP ticks sections 5 and 6. Refs #14, #15, #16 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
node --test only understands glob patterns from Node 21; Node 20, which CI uses, read "tests/**/*.test.js" as a literal path and failed with "Could not find". The test files all sit directly in tests/, so an unquoted tests/*.test.js, expanded by the shell, works on any Node. Refs #14 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Oct 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes the v1.2.0 milestone: working lint and test tooling with CI (#14), a configurable GTM container that skips admins (#15), and the missing documentation (#16).
Closes #14
Closes #15
Closes #16
What changed
Tooling (#14)
phpcs.xml.dist: the WordPress standard,jcprefix,jc-core-functionalitytext domain andminimum_wp_version6.5. It excludesvendor/,node_modules/andtests/.composer lintandcomposer lint-fixnow use it.$quoteTitle→$quote_title, an unused parameter, and hook registrations moved below their functions.composer auditis clean.wp-scripts formaton staged JS, JSON, YAML and Markdown.npm testnow runs node:test, then PHPUnit.lint:phpis added..prettierignore: excludesacf-json/andCHANGELOG.md..github/workflows/ci.ymlruns on every PR and onmain, with three jobs:php-lint(PHP 8.0):php -lon every plugin file,composer validate,composer lint.tests(PHP 8.3):npm test.js:lint:pkg-json, plus a check thatwp-scripts formatchanges nothing.Requires at least: 6.5andRequires PHP: 8.0.package.json: keys reordered to satisfylint:pkg-json; no values changed.GTM (#15):
lib/functions/gtm.phpjc_gtm_container_id()defaults toGTM-WNP9BDSD. It can be overridden with aJC_GTM_IDconstant or thejc_gtm_container_idfilter.'', or anything that isn'tGTM-[A-Z0-9]+, disables GTM.manage_options.wp_json_encode()in the inline script, andadd_query_arg()+esc_url()on the<noscript>iframe.Docs (#16)
jc/user-data: thekey/userIdarguments, what each key returns, and an example block.jc_add_json_load_point(): its docblock explains that removing the theme's ACF load path is deliberate. The plugin is the only source of the content model, so a theme's leftoveracf-json/can't load duplicates or override it.Tests
GtmTest: 13 tests, with theJC_GTM_IDconstant tests in separate processes.SettingsPageTest: 2 more tests.current_user_can(),add_query_arg(), and awp_print_inline_script_tag()that runs the attributes filter like core does.Why
composer lintran barephpcswith no ruleset or path, so it exited 3 without checking anything.npm testran an unconfiguredlint-stagedand always passed. Nothing checked a change before a release tag, and the dev lint tools had high-severity advisories.jc/user-datawas undocumented, and the ACF load-path removal was labelled "(optional)" with no explanation.PHP versions: PHPUnit 12 needs PHP 8.3, but the live server runs 8.0, so CI tests on 8.3 and checks that the plugin's own files parse on 8.0. The
php-lintjob installs dev tools with--ignore-platform-req=php; PHPUnit is never run there.How to test
CI: this PR is the workflow's first run. All three jobs should pass.
GTM regression check: to see the new GTM tests catch the old code, run
git show main:lib/functions/gtm.php > lib/functions/gtm.php && composer test. Expect 10 GTM failures/errors; restore withgit checkout lib/functions/gtm.php. Againstmain'sgtm.php,settings.phpandgeneral.phptogether, 12 of the new tests fail or error.In WordPress, after deploying:
GTM-WNP9BDSD, and the inline loader should carry anonce.Breaking changes
npm installnow installs it. Commits with badly formatted PHP fail locally until fixed, andgit commit --no-verifybypasses it.Migration steps
define( 'JC_GTM_ID', '' );towp-config.phpto keep test traffic out of analytics.🤖 Generated with Claude Code