Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(insights): pass clickAnalytics automatically #1080

Merged
merged 7 commits into from
Feb 2, 2023
Merged

fix(insights): pass clickAnalytics automatically #1080

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
711f796
fix(insights): pass clickAnalytics automatically
Haroenv Feb 1, 2023
9d12090
docs(examples): remove manual clickAnalytics
Haroenv Feb 1, 2023
b2d3cfe
make conditional
Haroenv Feb 2, 2023
9e2a4ca
test
Haroenv Feb 2, 2023
61a1785
lint
Haroenv Feb 2, 2023
a4e5137
wording changes
Haroenv Feb 2, 2023
68a9008
Merge branch 'next' into wip/insights-auto-clickAnalytics
Haroenv Feb 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions cypress/test-apps/js/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ const querySuggestionsPlugin = createQuerySuggestionsPlugin({
indexName: 'instant_search_demo_query_suggestions',
getSearchParams({ state }) {
return recentSearchesPlugin.data.getAlgoliaSearchParams({
clickAnalytics: true,
hitsPerPage: state.query ? 5 : 10,
});
},
Expand Down Expand Up @@ -85,7 +84,6 @@ autocomplete({
indexName: 'instant_search',
query,
params: {
clickAnalytics: true,
attributesToSnippet: ['name:10', 'description:35'],
snippetEllipsisText: '…',
},
Expand Down
3 changes: 1 addition & 2 deletions examples/playground/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx h */
import {
autocomplete,
Expand Down Expand Up @@ -37,7 +38,6 @@ const querySuggestionsPlugin = createQuerySuggestionsPlugin({
indexName: 'instant_search_demo_query_suggestions',
getSearchParams({ state }) {
return recentSearchesPlugin.data.getAlgoliaSearchParams({
clickAnalytics: true,
hitsPerPage: state.query ? 5 : 10,
});
},
Expand Down Expand Up @@ -79,7 +79,6 @@ autocomplete<ProductHit>({
indexName: 'instant_search',
query,
params: {
clickAnalytics: true,
attributesToSnippet: ['name:10', 'description:35'],
snippetEllipsisText: '…',
},
Expand Down
1 change: 1 addition & 0 deletions examples/playground/categoriesPlugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @jsxRuntime classic */
/** @jsx h */
import { AutocompletePlugin, getAlgoliaFacets } from '@algolia/autocomplete-js';
import { SearchClient } from 'algoliasearch/lite';
Expand Down
6 changes: 3 additions & 3 deletions examples/playground/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@
</div>
</div>

<script src="env.ts"></script>
<script src="darkMode.ts"></script>
<script src="app.tsx"></script>
<script type="module" src="env.ts"></script>
<script type="module" src="darkMode.ts"></script>
<script type="module" src="app.tsx"></script>
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
</body>
</html>
1 change: 0 additions & 1 deletion examples/query-suggestions-with-hits/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ autocomplete<ProductHit>({
indexName: 'instant_search',
query,
params: {
clickAnalytics: true,
attributesToSnippet: ['name:10'],
snippetEllipsisText: '…',
},
Expand Down
1 change: 0 additions & 1 deletion examples/recently-viewed-items/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ autocomplete<ProductHit>({
indexName: 'instant_search',
query,
params: {
clickAnalytics: true,
attributesToSnippet: ['name:10', 'description:35'],
snippetEllipsisText: '…',
},
Expand Down
1 change: 0 additions & 1 deletion examples/reshape/productsPlugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ export const productsPlugin = {
indexName: 'instant_search',
query,
params: {
clickAnalytics: true,
attributesToSnippet: ['name:10', 'description:35'],
snippetEllipsisText: '…',
hitsPerPage: 15,
Expand Down
1 change: 0 additions & 1 deletion examples/tags-in-searchbox/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ autocomplete<ProductHit | Tag<TagExtraData>>({
indexName: 'instant_search',
query,
params: {
clickAnalytics: true,
attributesToSnippet: ['name:10'],
snippetEllipsisText: '…',
filters: mapToAlgoliaFilters(tagsByFacet),
Expand Down
1 change: 0 additions & 1 deletion examples/tags-with-hits/app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ autocomplete<ProductHit | Tag<TagExtraData>>({
indexName: 'instant_search',
query,
params: {
clickAnalytics: true,
attributesToSnippet: ['name:10'],
snippetEllipsisText: '…',
filters: mapToAlgoliaFilters(tagsByFacet),
Expand Down
6 changes: 5 additions & 1 deletion packages/autocomplete-core/src/onInput.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,11 @@ export function onInput<TItem extends BaseItem>({
...setters,
})
).then((itemsOrDescription) =>
preResolve<TItem>(itemsOrDescription, source.sourceId)
preResolve<TItem>(
itemsOrDescription,
source.sourceId,
store.getState()
)
);
})
)
Expand Down
31 changes: 28 additions & 3 deletions packages/autocomplete-core/src/resolve.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ import {
} from '@algolia/client-search';
import type { SearchClient } from 'algoliasearch/lite';

import { BaseItem, InternalAutocompleteSource } from './types';
import {
AutocompleteState,
BaseItem,
InternalAutocompleteSource,
} from './types';
import { mapToAlgoliaResponse } from './utils';

function isDescription<TItem extends BaseItem>(
Expand Down Expand Up @@ -56,15 +60,36 @@ type RequestDescriptionPreResolvedCustom<TItem extends BaseItem> = {

export function preResolve<TItem extends BaseItem>(
itemsOrDescription: TItem[] | TItem[][] | RequesterDescription<TItem>,
sourceId: string
sourceId: string,
state: AutocompleteState<TItem>
):
| RequestDescriptionPreResolved<TItem>
| RequestDescriptionPreResolvedCustom<TItem> {
if (isRequesterDescription<TItem>(itemsOrDescription)) {
const contextParameters =
itemsOrDescription.requesterId === 'algolia'
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
? Object.assign(
{},
...Object.keys(state.context).map((key) => {
return (state.context[key] as Record<string, unknown>)
?.queryParameters;
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
})
)
: {};

return {
...itemsOrDescription,
requests: itemsOrDescription.queries.map((query) => ({
query,
query:
itemsOrDescription.requesterId === 'algolia'
? {
...query,
params: {
...contextParameters,
...query.params,
},
}
: query,
sourceId,
transformResponse: itemsOrDescription.transformResponse,
})),
Expand Down
96 changes: 96 additions & 0 deletions packages/autocomplete-js/src/__tests__/requester.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,4 +692,100 @@ describe('requester', () => {
`);
});
});

test('allows plugins to pass extra parameters to the search client', async () => {
const container = document.createElement('div');
const panelContainer = document.createElement('div');

document.body.appendChild(panelContainer);

const searchClient = createSearchClient({
search: jest.fn(() =>
Promise.resolve(
createMultiSearchResponse<{ label: string }>(
{
hits: [{ objectID: '1', label: 'Hit 1' }],
},
{ facetHits: [{ count: 2, value: 'Hit 2' }] }
)
)
),
});

autocomplete({
container,
panelContainer,
getSources({ query }) {
return [
{
sourceId: 'hits',
getItems() {
return getAlgoliaResults({
searchClient,
queries: [
{
indexName: 'indexName',
query,
},
],
});
},
templates: {
item({ item }) {
return JSON.stringify(item);
},
},
},
{
sourceId: 'facets',
getItems() {
return getAlgoliaFacets({
searchClient,
queries: [
{
indexName: 'indexName',
facet: 'categories',
params: {
facetQuery: query,
},
},
],
});
},
templates: {
item({ item }) {
return JSON.stringify(item);
},
},
},
];
},
}).setContext({
myPlugin: {
queryParameters: {
extraParam: true,
},
},
});

const input = container.querySelector<HTMLInputElement>('.aa-Input');

fireEvent.input(input, { target: { value: 'a' } });

await waitFor(() => {
expect(
panelContainer.querySelector<HTMLElement>('.aa-Panel')
).toBeInTheDocument();
});

expect(searchClient.search).toHaveBeenCalledTimes(1);
expect(searchClient.search).toHaveBeenCalledWith([
expect.objectContaining({
params: expect.objectContaining({ extraParam: true }),
}),
expect.objectContaining({
params: expect.objectContaining({ extraParam: true }),
}),
]);
});
});
88 changes: 88 additions & 0 deletions ...es/autocomplete-plugin-algolia-insights/src/__tests__/createAlgoliaInsightsPlugin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import { createAutocomplete } from '@algolia/autocomplete-core';
import {
getAlgoliaFacets,
getAlgoliaResults,
} from '@algolia/autocomplete-preset-algolia';
import { noop } from '@algolia/autocomplete-shared';
import userEvent from '@testing-library/user-event';
import insightsClient from 'search-insights';

import {
createMultiSearchResponse,
createPlayground,
createSearchClient,
createSource,
runAllMicroTasks,
} from '../../../../test/utils';
Expand Down Expand Up @@ -79,6 +85,88 @@ describe('createAlgoliaInsightsPlugin', () => {
);
});

test('sets clickAnalytics: true for getAlgoliaResults and getAlgoliaFacets', async () => {
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
const insightsClient = jest.fn();
const insightsPlugin = createAlgoliaInsightsPlugin({ insightsClient });

const searchClient = createSearchClient({
search: jest.fn(() =>
Promise.resolve(
createMultiSearchResponse<{ label: string }>(
{
hits: [{ objectID: '1', label: 'Hit 1' }],
},
{
facetHits: [{ count: 2, value: 'Hit 2' }],
}
)
)
),
});

const playground = createPlayground(createAutocomplete, {
plugins: [insightsPlugin],
getSources({ query }) {
return [
{
sourceId: 'hits',
getItems() {
return getAlgoliaResults({
searchClient,
queries: [
{
indexName: 'indexName',
query,
},
],
});
},
templates: {
item({ item }) {
return JSON.stringify(item);
},
},
},
{
sourceId: 'facets',
getItems() {
return getAlgoliaFacets({
searchClient,
queries: [
{
indexName: 'indexName',
facet: 'categories',
params: {
facetQuery: query,
},
},
],
});
},
templates: {
item({ item }) {
return JSON.stringify(item);
},
},
},
];
},
});

userEvent.type(playground.inputElement, 'a');
await runAllMicroTasks();

expect(searchClient.search).toHaveBeenCalledTimes(1);
expect(searchClient.search).toHaveBeenCalledWith([
expect.objectContaining({
params: expect.objectContaining({ clickAnalytics: true }),
}),
expect.objectContaining({
params: expect.objectContaining({ clickAnalytics: true }),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this behaviour expected (facet search with clickAnalytics: true)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like no queryID is returned anyway, it's likely ignored by the engine

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to filter it out to avoid sending an unnecessary larger payload?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to prevent wrong clickAnalytics, you don't want to say the search is tracked, when there's no clicks happen

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed and this is unlikely, and we'll deal with it when there ever is a queryID in sffv

}),
]);
});

describe('onItemsChange', () => {
test('sends a `viewedObjectIDs` event by default', async () => {
const insightsClient = jest.fn();
Expand Down
9 changes: 8 additions & 1 deletion packages/autocomplete-plugin-algolia-insights/src/createAlgoliaInsightsPlugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,14 @@ export function createAlgoliaInsightsPlugin(
subscribe({ setContext, onSelect, onActive }) {
insightsClient('addAlgoliaAgent', 'insights-plugin');

setContext({ algoliaInsightsPlugin: { insights } });
setContext({
algoliaInsightsPlugin: {
queryParameters: {
Haroenv marked this conversation as resolved.
Show resolved Hide resolved
clickAnalytics: true,
},
insights,
},
});

onSelect(({ item, state, event }) => {
if (!isAlgoliaInsightsHit(item)) {
Expand Down