fix(insights): pass clickAnalytics automatically#1080
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 68a9008:
|
Haroenv
left a comment
There was a problem hiding this comment.
things that could be changed
| params: expect.objectContaining({ clickAnalytics: true }), | ||
| }), | ||
| expect.objectContaining({ | ||
| params: expect.objectContaining({ clickAnalytics: true }), |
There was a problem hiding this comment.
is this behaviour expected (facet search with clickAnalytics: true)
There was a problem hiding this comment.
looks like no queryID is returned anyway, it's likely ignored by the engine
There was a problem hiding this comment.
Do we want to filter it out to avoid sending an unnecessary larger payload?
There was a problem hiding this comment.
to prevent wrong clickAnalytics, you don't want to say the search is tracked, when there's no clicks happen
There was a problem hiding this comment.
We discussed and this is unlikely, and we'll deal with it when there ever is a queryID in sffv
packages/autocomplete-plugin-algolia-insights/src/createAlgoliaInsightsPlugin.ts
Outdated
Show resolved
Hide resolved
packages/autocomplete-plugin-algolia-insights/src/__tests__/createAlgoliaInsightsPlugin.test.ts
Outdated
Show resolved
Hide resolved
| params: expect.objectContaining({ clickAnalytics: true }), | ||
| }), | ||
| expect.objectContaining({ | ||
| params: expect.objectContaining({ clickAnalytics: true }), |
There was a problem hiding this comment.
Do we want to filter it out to avoid sending an unnecessary larger payload?
Summary
adds a new mechanism in preResolve to check the context for any extra query parameters to set. The insights plugin will set
clickAnalytics: trueas one of those parameters to apply by default.To debate: should clickAnalytics be true for facet searches? We don't send the same automatic events if the items don't have objectID (as click is different). I'm guessing we can ignore that, as searchForFacetValues doesn't appear in analytics.
To debate: should the key be
queryParameters,searchParametersorquery.params?Results
We automatically pass
clickAnalytics: trueparameter if the insights plugin is used.FX-2191