feat(headless SSR commerce): Allow setting analytics 'capture' property through navigator provider #4995
Conversation
fbeaudoincoveo
requested review from
olamothe,
fpbrault,
y-lakhdar and
louis-bompart
| request.headers.get('Cookie') | ||
| ); | ||
| const clientId = clientIdCookie ?? randomUUID(); | ||
| const clientId = await getClientId(request); |
There was a problem hiding this comment.
- TODO: Implement client ID with cookie in NextJS sample
Pull Request ReportPR Title❌ Title should follow the conventional commit spec: Example: Live demo linksBundle Size
|
fbeaudoincoveo
temporarily deployed
to
PR Artifacts
GitHub Actions
Inactive
| @@ -25,7 +25,9 @@ export const fromAnalyticsStateToAnalyticsParams = ( | |||
| ...(s.userDisplayName && {userDisplayName: s.userDisplayName}), | |||
| ...(s.deviceId && {deviceId: s.deviceId}), | |||
| ...(s.trackingId && {trackingId: s.trackingId}), | |||
| ...{capture: true}, | |||
| ...{ | |||
| capture: navigatorContext.capture ?? navigatorContext.clientId !== '', | |||
There was a problem hiding this comment.
If the clientId is '', requests will fail, so this is a better fallback than true IMO.
| const totalItemsInCart = await externalCartService.getTotalCount(); | ||
| const cookie = await coveo_visitorId.parse(request.headers.get('Cookie')); |
There was a problem hiding this comment.
This was a pretty weak implementation that didn't take into account the possibility that using the cookie could be prevented by the user's privacy settings.
We're now setting the cookie conditionally in each route that leverages Coveo instead.
| * | ||
| * Must be set left undefined or set to `false` if the `clientId` is an empty string, otherwise requests will fail. | ||
| * | ||
| * Should also be set to `false` to comply with regulations if the user has somehow indicated that they do not wish to |
There was a problem hiding this comment.
Regulations varies depending on regions and all, let's not provide legal advices and focus on what the parameter does only.
There was a problem hiding this comment.
@jpmarceau what do you think? Is there some documentation we could/should link to instead?
There was a problem hiding this comment.
You could link to https://docs.coveo.com/en/3348
But I think it would be fair to do as Louis suggests, only stating what the param does. I would expect readers to consult the documentation site rather than reference documentation for such considerations.
| @@ -151,7 +151,8 @@ export const buildQuerySuggestRequest = ( | |||
| ? {referrer: navigatorContext.referrer} | |||
| : {}), | |||
| }, | |||
| capture: state.configuration.analytics.enabled, | |||
| capture: | |||
There was a problem hiding this comment.
Needs to be tested (UT)
| @@ -25,7 +25,9 @@ export const fromAnalyticsStateToAnalyticsParams = ( | |||
| ...(s.userDisplayName && {userDisplayName: s.userDisplayName}), | |||
| ...(s.deviceId && {deviceId: s.deviceId}), | |||
| ...(s.trackingId && {trackingId: s.trackingId}), | |||
| ...{capture: true}, | |||
| ...{ | |||
| capture: navigatorContext.capture ?? navigatorContext.clientId !== '', | |||
There was a problem hiding this comment.
Needs to be tested (UT)
fbeaudoincoveo
temporarily deployed
to
PR Artifacts
GitHub Actions
Inactive
fbeaudoincoveo
temporarily deployed
to
PR Artifacts
GitHub Actions
Inactive
I'm also including a more full-fledged implementation of the clientId in the Remix SSR sample.
https://coveord.atlassian.net/browse/KIT-3988
@jpmarceau I'm adding you as a reviewer mainly for the Remix SSR sample part (although there's also a bit of new reference docs in this PR).