Skip to content

Commit

Permalink
fix(insights): deduplicate view events (#6414)
Browse files Browse the repository at this point in the history
* fix(insights): deduplicate view events

* fix tests and bundlesize
  • Loading branch information
aymeric-giraudet authored Oct 29, 2024
1 parent f82ebab commit 76b0247
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 11 deletions.
2 changes: 1 addition & 1 deletion bundlesize.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
},
{
"path": "packages/vue-instantsearch/vue2/umd/index.js",
"maxSize": "68.5 kB"
"maxSize": "68.75 kB"
},
{
"path": "packages/vue-instantsearch/vue3/umd/index.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1326,6 +1326,135 @@ describe('insights', () => {
}).toWarnDev();
expect(insightsClient).toHaveBeenCalledTimes(numberOfCalls); // still the same
});

it('does not send view events that were previously sent for the current query', () => {
const { insightsClient, instantSearchInstance } = createTestEnvironment();

instantSearchInstance.use(
createInsightsMiddleware({
insightsClient,
})
);

insightsClient('setUserToken', 'token');

instantSearchInstance.sendEventToInsights({
insightsMethod: 'viewedObjectIDs',
widgetType: 'ais.customWidget',
eventType: 'view',
payload: {
index: 'my-index',
eventName: 'My Hits Viewed',
objectIDs: ['obj1'],
},
});

instantSearchInstance.sendEventToInsights({
insightsMethod: 'viewedObjectIDs',
widgetType: 'ais.customWidget',
eventType: 'view',
payload: {
index: 'my-index',
eventName: 'My Hits Viewed',
objectIDs: ['obj1'],
},
});

expect(
insightsClient.mock.calls.filter(
(call) => call[0] === 'viewedObjectIDs'
)
).toHaveLength(1);
});

it('clears saved view events when the query changes', () => {
const { insightsClient, instantSearchInstance } = createTestEnvironment();

instantSearchInstance.use(
createInsightsMiddleware({
insightsClient,
})
);

insightsClient('setUserToken', 'token');

instantSearchInstance.sendEventToInsights({
insightsMethod: 'viewedObjectIDs',
widgetType: 'ais.customWidget',
eventType: 'view',
payload: {
index: 'my-index',
eventName: 'My Hits Viewed',
objectIDs: ['obj1'],
},
});

instantSearchInstance.mainHelper!.derivedHelpers[0].emit('result', {
results: { queryId: '2' },
});

instantSearchInstance.sendEventToInsights({
insightsMethod: 'viewedObjectIDs',
widgetType: 'ais.customWidget',
eventType: 'view',
payload: {
index: 'my-index',
eventName: 'My Hits Viewed',
objectIDs: ['obj1'],
},
});

expect(
insightsClient.mock.calls.filter(
(call) => call[0] === 'viewedObjectIDs'
)
).toHaveLength(2);
});

it("only sends view events that haven't been sent yet for current query", () => {
const { insightsClient, instantSearchInstance } = createTestEnvironment();

instantSearchInstance.use(
createInsightsMiddleware({
insightsClient,
})
);

insightsClient('setUserToken', 'token');

instantSearchInstance.sendEventToInsights({
insightsMethod: 'viewedObjectIDs',
widgetType: 'ais.customWidget',
eventType: 'view',
payload: {
index: 'my-index',
eventName: 'My Hits Viewed',
objectIDs: ['obj1'],
},
});

instantSearchInstance.sendEventToInsights({
insightsMethod: 'viewedObjectIDs',
widgetType: 'ais.customWidget',
eventType: 'view',
payload: {
index: 'my-index',
eventName: 'My Hits Viewed',
objectIDs: ['obj1', 'obj2'],
},
});

expect(
insightsClient.mock.calls.filter(
(call) => call[0] === 'viewedObjectIDs'
)
).toHaveLength(2);
expect(insightsClient).toHaveBeenLastCalledWith(
'viewedObjectIDs',
expect.objectContaining({ objectIDs: ['obj2'] }),
expect.any(Object)
);
});
});

test("does not write to the URL on load when there's an existing anonymous cookie", async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,13 +438,39 @@ export function createInsightsMiddleware<
};
}

const viewedObjectIDs = new Set<string>();
let lastQueryId: string | undefined;
instantSearchInstance.mainHelper!.derivedHelpers[0].on(
'result',
({ results }) => {
if (!results.queryID || results.queryID !== lastQueryId) {
lastQueryId = results.queryID;
viewedObjectIDs.clear();
}
}
);

instantSearchInstance.sendEventToInsights = (event: InsightsEvent) => {
if (onEvent) {
onEvent(
event,
insightsClientWithLocalCredentials as TInsightsClient
);
} else if (event.insightsMethod) {
if (event.insightsMethod === 'viewedObjectIDs') {
const payload = event.payload as {
objectIDs: string[];
};
const difference = payload.objectIDs.filter(
(objectID) => !viewedObjectIDs.has(objectID)
);
if (difference.length === 0) {
return;
}
difference.forEach((objectID) => viewedObjectIDs.add(objectID));
payload.objectIDs = difference;
}

// Source is used to differentiate events sent by instantsearch from those sent manually.
(event.payload as any).algoliaSource = ['instantsearch'];
if ($$automatic) {
Expand Down
8 changes: 3 additions & 5 deletions tests/common/widgets/hits/insights.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export function createInsightsTests(

// View event called for each index once
{
expect(window.aa).toHaveBeenCalledTimes(3);
expect(window.aa).toHaveBeenCalledTimes(2);
expect(window.aa).toHaveBeenCalledWith(
'viewedObjectIDs',
{
Expand Down Expand Up @@ -130,9 +130,7 @@ export function createInsightsTests(
await wait(0);
});

// @TODO: This is a bug, we should not send a view event when the results are the same.
// see: https://github.com/algolia/instantsearch/issues/5442
expect(window.aa).toHaveBeenCalledTimes(3);
expect(window.aa).toHaveBeenCalledTimes(2);
}
});

Expand Down Expand Up @@ -195,7 +193,7 @@ export function createInsightsTests(

// View event called for each index, batched in chunks of 20
{
expect(window.aa).toHaveBeenCalledTimes(6);
expect(window.aa).toHaveBeenCalledTimes(4);
expect(window.aa).toHaveBeenCalledWith(
'viewedObjectIDs',
{
Expand Down
8 changes: 3 additions & 5 deletions tests/common/widgets/infinite-hits/insights.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export function createInsightsTests(

// View event called for each index once
{
expect(window.aa).toHaveBeenCalledTimes(3);
expect(window.aa).toHaveBeenCalledTimes(2);
expect(window.aa).toHaveBeenCalledWith(
'viewedObjectIDs',
{
Expand Down Expand Up @@ -130,9 +130,7 @@ export function createInsightsTests(
await wait(0);
});

// @TODO: This is a bug, we should not send a view event when the results are the same.
// see: https://github.com/algolia/instantsearch/issues/5442
expect(window.aa).toHaveBeenCalledTimes(3);
expect(window.aa).toHaveBeenCalledTimes(2);
}
});

Expand Down Expand Up @@ -195,7 +193,7 @@ export function createInsightsTests(

// View event called for each index, batched in chunks of 20
{
expect(window.aa).toHaveBeenCalledTimes(6);
expect(window.aa).toHaveBeenCalledTimes(4);
expect(window.aa).toHaveBeenCalledWith(
'viewedObjectIDs',
{
Expand Down

0 comments on commit 76b0247

Please sign in to comment.