-
Notifications
You must be signed in to change notification settings - Fork 842
WooCommerce Analytics: Deduplicate product_checkout events #46497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
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! |
6db6d3c to
fa497be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements deduplication for product_checkout events to prevent them from firing on every page refresh during a checkout session. The changes introduce session-based tracking that fires events once per cart state and optionally tracks JS session changes when ClickHouse is enabled.
Key changes:
- Adds WC Session-based deduplication to track checkout events once per cart state
- Implements JS session ID tracking for sessionTracking feature support
- Automatically resets tracking state when cart changes (add/remove items, quantity updates)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
projects/packages/woocommerce-analytics/src/class-universal.php |
Adds deduplication logic with should_track_checkout, mark_checkout_tracked, and reset_checkout_tracking_state methods; updates checkout_process to fire events via record_event() with session-based checks |
projects/packages/woocommerce-analytics/src/class-woo-analytics-trait.php |
Adds get_js_session_id() method and JS_SESSION_COOKIE_NAME constant to support JS session tracking |
projects/packages/woocommerce-analytics/tests/php/mocks/woocommerce-functions.php |
Adds mock WC instance management for testing session functionality |
projects/packages/woocommerce-analytics/tests/php/mocks/class-wc-session.php |
Introduces mock WC_Session class for testing session storage |
projects/packages/woocommerce-analytics/tests/php/bootstrap.php |
Includes new WC_Session mock in test bootstrap |
projects/packages/woocommerce-analytics/tests/php/Universal_Test.php |
Adds tests for reset_checkout_tracking_state and validates session key constants |
projects/packages/woocommerce-analytics/changelog/fix-deduplicate-product-checkout-event |
Documents the change as a patch-level modification |
Comments suppressed due to low confidence (3)
projects/packages/woocommerce-analytics/src/class-woo-analytics-trait.php:342
- The
get_js_session_idmethod lacks test coverage. Consider adding tests to verify: (1) returns null when ClickHouse is disabled, (2) returns null when cookie is not set, (3) correctly extracts session_id from valid cookie JSON, (4) handles invalid JSON gracefully. This is particularly important given the security and error-handling concerns around cookie parsing.
}
// phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- We're just reading and JSON-decoding the cookie.
$cookie_value = wp_unslash( $_COOKIE[ $cookie_name ] );
$cookie_data = json_decode( urldecode( $cookie_value ), true );
return $cookie_data['session_id'] ?? null;
}
/**
* Gather relevant product information
*
* @param \WC_Product $product product.
* @return array
*/
projects/packages/woocommerce-analytics/src/class-woo-analytics-trait.php:341
- Reading the cookie value with
wp_unslashis appropriate, but the phpcs ignore comment claims the input doesn't need sanitization because "we're just reading and JSON-decoding the cookie". However, the cookie value comes from the client and could be maliciously crafted. Whilejson_decodeis generally safe, the decoded value should be validated. Consider adding explicit type checking to ensure the decoded value is an array and that the session_id field, if present, is a string to prevent unexpected behavior from malicious input.
* Gather relevant product information
*
* @param \WC_Product $product product.
* @return array
projects/packages/woocommerce-analytics/src/class-woo-analytics-trait.php:341
- The
get_js_session_idmethod does not validate that the JSON decoding was successful before attempting to access the array. If the cookie contains invalid JSON,json_decodereturnsnull, and attempting to access$cookie_data['session_id']will cause a PHP warning about trying to access an array offset on null. Add a check to ensure$cookie_datais an array before accessing its elements.
* @param \WC_Product $product product.
* @return array
projects/packages/woocommerce-analytics/src/class-universal.php
Outdated
Show resolved
Hide resolved
projects/packages/woocommerce-analytics/src/class-woo-analytics-trait.php
Show resolved
Hide resolved
projects/packages/woocommerce-analytics/src/class-universal.php
Outdated
Show resolved
Hide resolved
projects/packages/woocommerce-analytics/src/class-universal.php
Outdated
Show resolved
Hide resolved
Code Coverage SummaryCoverage changed in 2 files.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
fa497be to
b809f3b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
projects/packages/woocommerce-analytics/src/class-woo-analytics-trait.php
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
projects/packages/woocommerce-analytics/src/class-woo-analytics-trait.php
Show resolved
Hide resolved
projects/packages/woocommerce-analytics/src/class-woo-analytics-trait.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
projects/packages/woocommerce-analytics/src/class-woo-analytics-trait.php
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
- Fire product_checkout PHP-side via record_event() for consistency with product_purchase - Add session-based deduplication to prevent duplicate events on page refresh - Reset tracking state when cart changes (add/remove items, quantity updates) - Include JS session ID tracking for sessionTracking feature support - Add unit tests for reset_checkout_tracking_state and session key constants 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…s-trait.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…s-trait.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
WC_Session::save_data() is not declared on the abstract class. WooCommerce automatically saves session data at request end via shutdown hook. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Initial plan * Add comprehensive tests for checkout deduplication logic Co-authored-by: chihsuan <4344253+chihsuan@users.noreply.github.com> * Refactor cookie setup into helper methods to reduce duplication Co-authored-by: chihsuan <4344253+chihsuan@users.noreply.github.com> * Address code review feedback: improve test patterns Co-authored-by: chihsuan <4344253+chihsuan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: chihsuan <4344253+chihsuan@users.noreply.github.com>
Add a helper method that conditionally calls setAccessible() only on PHP < 8.1, where it's required for private method access. PHP 8.1+ makes private/protected methods accessible by default via reflection, and PHP 8.5+ deprecates the method. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
d7bd5b3 to
3eba662
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Simplified the condition for detecting session changes by removing the check for a current session ID. Now, it directly compares the current session ID with the last session ID to determine if a new session has started.
Related to WOOA7S-929
Problem
The
product_checkoutevent currently fires every time the checkout page loads, causing duplicate events on page refresh. This is inconsistent with theproduct_purchaseevent, which only fires on the final purchase. Since two events are often analyzed together, this inconsistency can lead to misleading insights.Current behavior:
Expected behavior:
Proposed changes:
product_checkoutevents using WC Session storage to track if checkout has already been recorded for the current cart stateproduct_checkoutPHP-side viarecord_event()for consistency withproduct_purchaseevent (both now fire server-side)Technical approach
wca_checkout_trackedflag in WC Session whenproduct_checkoutfiresOther information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
No, this PR does not change what data we track. It reduces duplicate events while preserving meaningful tracking:
Testing instructions:
product_checkoutevent should fire (check debug.log)woocommerceanalytics_sessioncookie session id value (simulate new session)