-
Notifications
You must be signed in to change notification settings - Fork 800
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
[WooCommerce Analytics] Support for Search and Landing Page #40698
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 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! |
Thanks for adding support for landing page and search. I tested entering all the pages and the session_started event with landing page prop works well, but there's one case that might need to be checked. Here's my step:
There's also another case that
|
Thanks @ianlin for catching that. It seems to be an inherited issue. The object was not accessible in the checkout-flow file. I merge those actions inside the universal file. Now it should work Can we have another round? |
Thanks for the change. I tested the first scenario from the #40698 (comment) above:
Right now the |
Thanks @ianlin for the review. I cannot reproduce the error you mentioned. recording.movCan you make sure you tested by removing the session from the cookie? The reason it doesn't appear in Tracks is because the lifetime of the tracks is 2h maximum so maybe your session_started was recorded before that. |
Thanks for the hint, I did not manually remove the session cookie, I only closed and reopened the incognito page. So here's what I've discovered: Scenario A: Remove session cookie and go to cart page ✅
Scenario B: Do not remove session cookie, close and reopen incognito page, and go the cart page
Scenario C: Remove session cookie and go to checkout page
Scenario D: Remove session cookie, close and reopen incognito page, and go the checkout page
Sorry if I mistakenly test this functionality 🙏, I was trying to test all the cases that I can think of. Also, what I have done to apply your new commits was:
Did I miss something? |
Hey @ianlin Thanks again for testing the PR. I appreciate the thorough testing :) I will go over your cases one by one:
Worked for me. Attached video: Screen.Recording.2024-12-30.at.19.31.58.mov
Very tricky to solve this. And a good catch on your side. Seems that the session was sent before the redirect happened. So the cookie is set and record_event method is called. However, is never sending an event because the redirect prevents wp_enqueue_js to load. In the next request to cart, after the redirection happens. The session cookie is set already, but session_started record_event never happens as explained before. Now I fixed it using Screen.Recording.2024-12-30.at.21.07.23.mov
This I'm a bit confused. In title, you mention "go the checkout page". So it means is same as scenario C. However, in the details, you mention "Go to cart page" so it's scenario B. In any case, I believe now it's working finally. Can we test one round more? |
Proposed changes:
woocommerceanalytics_search
eventwoocommerceanalytics_session_id
intowoocommerceanalytics_session
since now it stores more dataOther information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
Prerequisites
--
woocommerceanalytics_session_started
is triggered with asession_id
andlanding_page
prop with a clean URL.session_id
andlanding_page
props?s={query}
or by adding Search Products block somewhere)woocommerceanalytics_search
is triggered with asearch_query
prop indicating the search query andqty
prop indicating the results count. Also,session_id
andlanding_page
props match the expected.woocommerceanalytics_session
session cookie