Fix tracks identity cookie cache #9249
Merged
Conversation
This file contains 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
alefesouza
requested review from
a team and
malithsen
and removed request for
a team
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +844 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
malithsen
requested changes
Aug 13, 2024
There was a problem hiding this comment.
Thanks for working on this @alefesouza
This is a creative approach. I've noted some concerns in the comments.
Primarily, I don't love the fact that we are recording events as a side effect of the getTracksIdentity call.
What do you think of, removing the backend tracking for page views and instead recording from frontend when the page has loaded, using recordUserEvent function?
malithsen
requested changes
Aug 15, 2024
malithsen
requested changes
Aug 19, 2024
There was a problem hiding this comment.
Nice work so far Alefe.
woocommerce_after_main_content hook only runs on the product page. So with WooPay disabled, the page view events from the cart and the checkout pages are no longer recorded as the frontend script is not added.
Additionally, do you think we can use existing JS scripts to call the recordUserEvent function on page load instead of dynamically adding a new script? I haven't checked if this is possible, and the current approach is okay if that's not possible.
@malithsen thanks, changed to
I don't think so because this function comes from the |
malithsen
requested changes
Aug 19, 2024
There was a problem hiding this comment.
When placing an order using WooPayments, checkout_order_placed event is recorded via the checkout_order_processed function. This internally uses the maybe_record_wcpay_shopper_event function. Above event is no longer recorded. I haven't found why exactly but I am guessing because this event is emitted between page transitions, frontend scripts never gets a chance to be added on to the footer.
Sorry, I missed this earlier.
|
@malithsen fixed on 784b47f. |
malithsen
approved these changes
Aug 19, 2024
jessy-p
pushed a commit
that referenced
this pull request
Aug 19, 2024
6 tasks
rafaelzaleski
pushed a commit
that referenced
this pull request
Aug 27, 2024
rafaelzaleski
pushed a commit
that referenced
this pull request
Aug 27, 2024
This reverts commit 2ce5bc7.
This was referenced Sep 26, 2024
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 p1722880351733569-slack-C02CCT3F1QA
Changes proposed in this Pull Request
This PR uses the same approach introduced by #8185 to prevent breaking the cache for the products, cart and checkout pages.
Testing instructions
tk_aicookie.Set-Cookieheader (this breaks the cache)./wp-admin/admin-ajax.phprequest.get_identityaction and thewcpay_product_page_viewevent.tk_aicookie added by theget_identityrequest.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge