Cookie Consent: config-driven core with per-feature toggles#50114
Cookie Consent: config-driven core with per-feature toggles#50114chihsuan wants to merge 14 commits into
Conversation
Cookie_Consent::get_config() now resolves Config_Schema once and stashes the result instead of double-resolving through the jetpack_cookie_consent_config filter. Drops that filter and the temporary get_default_config()/normalize_config() delegates now that nothing needs them. get_copy()/get_consent_categories() read the stash by default, and the banner/CCPA templates reuse the already-resolved copy/categories instead of re-resolving on every render. Also locks in that consent.categories is derived from the resolved copy (not pristine defaults) when categories aren't explicitly supplied, which regressed under the old double-resolve path.
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
There was a problem hiding this comment.
Pull request overview
This PR refactors the cookie-consent package toward third-party consumption by introducing a schema-driven configuration resolver and a config-driven Cookie_Consent::init( array $config = [] ) entry point with per-feature hook gating.
Changes:
- Add
Config_Schemaas a single source of truth for config shape/defaults and aresolve()routine that stashes resolved config. - Update
Cookie_Consent::init()to accept config, support a masterenabledswitch, and register hooks conditionally perfeatures.*. - Update consent-log initialization to accept injected
logconfig, and add/adjust PHPUnit coverage for schema resolution, feature toggles, and asset enqueue behavior.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/packages/cookie-consent/tests/php/Init_Feature_Toggles_Test.php | New tests asserting feature toggles gate hook registration. |
| projects/packages/cookie-consent/tests/php/Enqueue_Assets_Test.php | New tests asserting enqueue emissions are gated and geo config contract is stable. |
| projects/packages/cookie-consent/tests/php/Cookie_Consent_Log_Versions_Test.php | Updates tests to inject config via stashed config instead of removed filters. |
| projects/packages/cookie-consent/tests/php/Consent_Log_Controller_Test.php | Updates controller tests for injected log config and new config injection approach. |
| projects/packages/cookie-consent/tests/php/Consent_Log_Controller_Log_Versions_Test.php | Updates log-version tests to use config injection helper. |
| projects/packages/cookie-consent/tests/php/Config_Test.php | Updates config tests to use public get_config() and stashed defaults behavior. |
| projects/packages/cookie-consent/tests/php/Config_Schema_Test.php | New unit tests for Config_Schema::schema() and resolve() behavior. |
| projects/packages/cookie-consent/tests/php/Config_Normalization_Test.php | Migrates normalization coverage from Cookie_Consent to Config_Schema. |
| projects/packages/cookie-consent/tests/php/class-testcase.php | Adds helpers to set/reset the private config stash for tests via reflection. |
| projects/packages/cookie-consent/src/schema/class-config-schema.php | Introduces schema + resolver for configuration defaults, coercion, and validation. |
| projects/packages/cookie-consent/src/cookie-banner-content.php | Allows templates to accept pre-resolved $copy/$categories (fallback when absent). |
| projects/packages/cookie-consent/src/class-cookie-consent.php | Implements config-driven init(), feature gating, stashed config, and updated asset emission. |
| projects/packages/cookie-consent/src/class-consent-log-controller.php | Accepts injected log config; uses it for IP mode/retention and updates docs accordingly. |
| projects/packages/cookie-consent/src/ccpa-content.php | Allows template to accept pre-resolved $copy (fallback when absent). |
| projects/packages/cookie-consent/README.md | Documents new init( $config ) API, master switch, feature toggles, and config structure. |
| projects/packages/cookie-consent/changelog/wooa7s-1639-config-driven-core | Adds changelog entry for the new config-driven core API. |
…build - init(): latch on the first call including when enabled=false, so the master switch is a sticky no-op that a later caller can't re-enable; docblock now matches the behavior. - init(): drop the dead `|| $features['geo']` enqueue branch. Geo emission lives after the banner guard in enqueue_assets(), so geo alone enqueues nothing. - Config_Schema::resolve(): build schema() once and index into it instead of rebuilding it ~5x per resolve (each build re-materializes copy/category/country defaults); remove the now-unused defaults() wrapper. - Tests: assert enabled=false leaves the durable side effects (consent-log cron, Boost filter, CCPA REST hook) unregistered; cover injected retention_days and its malformed-value fallback; invert the geo-only enqueue expectation.
…nums Config_Schema now owns the GEO_PROVIDERS, IP_MODES, and DEFAULT_IP_MODE constants; the schema descriptor, resolve_geo(), and Consent_Log_Controller::get_ip_mode() all read their allowed values from them, so each enum is declared exactly once instead of being re-implemented per validator. - resolve_geo() validates against GEO_PROVIDERS and resets to the default provider, dropping the hardcoded 'wpcom'/'custom' literals. - Consent_Log_Controller drops its local IP_MODES/DEFAULT_IP_MODE constants and consumes Config_Schema::ip_modes()/default_ip_mode(). - Tests: an unknown injected ip_mode falls back via the schema; the ip_mode accessors are asserted to match the schema enum so the two can't drift.
…onsent-config-driven-core # Conflicts: # projects/packages/cookie-consent/README.md
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
Fixes WOOA7S-1639
Proposed changes
Part of the Cookie Consent all-sites hardening. Today
Cookie_Consent::init()takes no arguments and wires up everything at once. This makes the core config-driven: a consumer passes a config toinit(), anenabledswitch turns the whole feature on/off, and each capability is an independent toggle. Called with no arguments, behavior is unchanged.Cookie_Consent::init( array $config = [] )— resolves the config once, bails early ifenabledisfalse, and registers each feature (banner,ccpa_page,footer_links,consent_log,tracks,geo) only when its toggle is on.Config_Schema— one declarative schema (types, defaults, validation) thatresolve()applies. Stateless; reads no options.logsettings (ip_mode,retention_days, versions) as an argument instead of reaching into globals.jetpack_cookie_consent_configandjetpack_cookie_consent_log_retention_daysfilters as override points for code that doesn't own theinit()call.init()is the primary path (for the host plugin); the config filter runs over the resolved config and is re-resolved throughConfig_Schema, so third-party input is still validated.jetpack_cookie_consent_configstays a documented, versioned public API.Related product discussion/links
Does this pull request change what data or activity we track or use?
No. Behavior-preserving refactor — default IP handling stays
drop, log retention stays 30 days.Testing instructions
Unit tests (the primary check):
Try the new API — the package runs inside a consumer (e.g. premium-analytics). To exercise the toggles, drop a mu-plugin on a site that has the package, e.g.
wp-content/mu-plugins/cookie-consent-config.php:Add
?preview_cookie_consent=1to a front-end URL to force the banner to show outside a GDPR region. With the selective config, confirm no CCPA page is auto-created andstats.wp.com/w.jsis not enqueued, while the banner still renders. With enabled = false, confirm nothing is registered.Third-party override — with the package booted, confirm another plugin can still adjust config without owning the
init()call: