Cookie Consent: Record consent log versions#49996
Draft
chihsuan wants to merge 3 commits into
Draft
Conversation
Summary: - Add policy and banner version columns to the consent log table. - Store configured log versions when writing consent log records. - Expose only a narrow log-version accessor and keep full config private. - Add the package changelog entry. Rationale: - Consent records need to preserve the policy and banner versions that were in effect when consent was captured so later copy or category changes do not weaken proof-of-consent records. - The table version bump lets dbDelta migrate existing installations. - A narrow accessor avoids widening the full cookie-consent config into the package public API. Tests: - php -l projects/packages/cookie-consent/src/class-consent-log-controller.php - php -l projects/packages/cookie-consent/src/class-cookie-consent.php - isolated PHP harness for default, configured, and invalid log version inserts - PATH="$HOME/.nvm/versions/node/v24.15.0/bin:$PATH" /Users/chihsuan/Library/pnpm/pnpm --dir projects/packages/cookie-consent test - PATH="$HOME/.nvm/versions/node/v24.15.0/bin:$PATH" /Users/chihsuan/Library/pnpm/pnpm jetpack test php packages/cookie-consent - composer phpcs:lint -- projects/packages/cookie-consent/src/class-consent-log-controller.php projects/packages/cookie-consent/src/class-cookie-consent.php - PATH="$HOME/.nvm/versions/node/v24.15.0/bin:$PATH" /Users/chihsuan/Library/pnpm/pnpm jetpack phan packages/cookie-consent --allow-polyfill-parser (fails on existing trunk tests/php PHPUnit scaffold issues)
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. 🤷 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the cookie-consent “proof of consent” logging by persisting the policy and banner version values that were in effect at the time a consent event was recorded, and exposes a small accessor for retrieving those configured version values.
Changes:
- Add
policy_versionandbanner_versioncolumns to the consent log DB table and bump the table schema version for dbDelta migration. - Persist configured log versions on each consent-log insert, defaulting to
'1'when unset/invalid. - Add a public
Cookie_Consent::get_log_versions()accessor and add a cookie-consent changelog entry.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| projects/packages/cookie-consent/src/class-cookie-consent.php | Adds a public accessor for log versions and introduces default log.*_version config keys. |
| projects/packages/cookie-consent/src/class-consent-log-controller.php | Updates schema version, adds DB columns, writes versions on insert, and exposes versions in the REST schema. |
| projects/packages/cookie-consent/changelog/add-consent-log-versions | Records the user-facing change in the package changelog. |
Contributor
|
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! |
github-actions
Bot
added
the
[Status] Needs Author Reply
Normalize Cookie_Consent::get_log_versions() so callers receive non-empty strings, then add PHPUnit coverage for defaults, filtered overrides, scalar normalization, and invalid config values. Addresses review comment 3480280243. Addresses review comment 3480280267.
Make REST route endpoint indexes explicit so Phan no longer reports mixed keyed and unkeyed entries, and remove file-level suppressions that CI reports as unused.
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.
Fixes WOOA7S-1606
Proposed changes
policy_versionandbanner_versioncolumns to the cookie consent log table and bump the consent-log DB version so existing tables are migrated bydbDelta.log.policy_versionandlog.banner_versionvalues on each consent log record, defaulting each value to1.Cookie_Consent::get_log_versions()accessor while keeping the full cookie-consent config private.Cookie_Consent::get_log_versions()values to non-empty strings and add PHPUnit coverage for default, filtered, scalar, empty, and invalid values.Related product discussion/links
Does this pull request change what data or activity we track or use?
Yes. Cookie consent log records now include the policy and banner version strings in effect when consent was captured. These values come from cookie-consent configuration and are intended as proof-of-consent metadata.
Testing instructions
git diff --name-only origin/trunk...HEADlists only:projects/packages/cookie-consent/changelog/add-consent-log-versionsprojects/packages/cookie-consent/src/class-consent-log-controller.phpprojects/packages/cookie-consent/src/class-cookie-consent.phpprojects/packages/cookie-consent/tests/php/Cookie_Consent_Log_Versions_Test.phpphp -l projects/packages/cookie-consent/src/class-consent-log-controller.phpphp -l projects/packages/cookie-consent/src/class-cookie-consent.phpphp -l projects/packages/cookie-consent/tests/php/Cookie_Consent_Log_Versions_Test.phpprojects/packages/cookie-consent/vendor/bin/phpunit-select-config projects/packages/cookie-consent/phpunit.#.xml.dist --colors=always --filter Cookie_Consent_Log_Versions_TestPATH="$HOME/.nvm/versions/node/v24.15.0/bin:$PATH" /Users/chihsuan/Library/pnpm/pnpm jetpack test php packages/cookie-consentcomposer phpcs:lint -- projects/packages/cookie-consent/src/class-consent-log-controller.php projects/packages/cookie-consent/src/class-cookie-consent.php projects/packages/cookie-consent/tests/php/Cookie_Consent_Log_Versions_Test.php1versions when nologconfig is supplied and configured versions whenjetpack_cookie_consent_configsupplieslog.policy_version/log.banner_version.Validation note: local
phan packages/cookie-consent --allow-polyfill-parserstill exits nonzero on the existing current-trunk PHPUnit scaffold diagnostics underprojects/packages/cookie-consent/tests/php/*. The CI-reportedPhanPluginMixedKeyNoKeyand unused-suppression diagnostics fromf23774f4fbare addressed in5a8bbb6284.