-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: use the inferQueryID feature of search-insights #180
Conversation
e52e642
to
b908ba9
Compare
Refactor the code in `recommend-config.js` to use the `URL` constructor instead of the `generateProductUrl` function for generating URLs. This change ensures that the URLs are correctly generated with the queryID and indexName parameters for analytics. Additionally, it improves the performance by using the `href` property of the `URL` object instead of concatenating strings.
queryID: item.__queryID, | ||
indexName: item.__indexName, | ||
}); | ||
item.url = new URL(item.url, window.location.origin).href; |
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.
This is not needed, it's done automatically by the browser, I'm removing it.
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.
Nice cleanup 🙌
Thank you for the improvement |
I'm still tracking the subject.
|
Actually, the frontend libraries don't need to be updated, as they use the version that we load manually, so we're good on the 3rd point. The problem now comes from the 2nd point: opting out is done with the |
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.
I've confirmed with the Events team that it's the expected behaviour and that they won't make further changes for now. Let's merge then!
So userHasOptedOut
is now set to true
when the user opt-out of tracking, which permits to not use his browser's localStorage in that case (the consequence being that no insights events are sent for those users)
Since v2.15.0, search-insights can keep track of the queryID automatically.
This PR moves the SFRA cartridge to this new built-in method.
Changes
inferQueryID
parameteruserHasOptedOut
when users have refused the tracking. This is becausesearch-insights.js
now systematically uses the browser's local storage to keep track of queryIDs, and since it's not technically necessary data, we need users' consent