From 76b0247108a0597809a543a8c925aa7ae14c6b18 Mon Sep 17 00:00:00 2001 From: Aymeric Giraudet Date: Tue, 29 Oct 2024 11:01:44 +0100 Subject: [PATCH] fix(insights): deduplicate view events (#6414) * fix(insights): deduplicate view events * fix tests and bundlesize --- bundlesize.config.json | 2 +- .../__tests__/createInsightsMiddleware.ts | 129 ++++++++++++++++++ .../middlewares/createInsightsMiddleware.ts | 26 ++++ tests/common/widgets/hits/insights.ts | 8 +- .../common/widgets/infinite-hits/insights.ts | 8 +- 5 files changed, 162 insertions(+), 11 deletions(-) diff --git a/bundlesize.config.json b/bundlesize.config.json index 0a3f49a9c9..744e2f5b46 100644 --- a/bundlesize.config.json +++ b/bundlesize.config.json @@ -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", diff --git a/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts b/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts index 476c74f06e..e636f7a902 100644 --- a/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts +++ b/packages/instantsearch.js/src/middlewares/__tests__/createInsightsMiddleware.ts @@ -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 () => { diff --git a/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts b/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts index 3b6b4a6695..04e63313b4 100644 --- a/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts +++ b/packages/instantsearch.js/src/middlewares/createInsightsMiddleware.ts @@ -438,6 +438,18 @@ export function createInsightsMiddleware< }; } + const viewedObjectIDs = new Set(); + 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( @@ -445,6 +457,20 @@ export function createInsightsMiddleware< 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) { diff --git a/tests/common/widgets/hits/insights.ts b/tests/common/widgets/hits/insights.ts index 404b3891f4..ba721e90e3 100644 --- a/tests/common/widgets/hits/insights.ts +++ b/tests/common/widgets/hits/insights.ts @@ -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', { @@ -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); } }); @@ -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', { diff --git a/tests/common/widgets/infinite-hits/insights.ts b/tests/common/widgets/infinite-hits/insights.ts index a744cdf304..739a65a799 100644 --- a/tests/common/widgets/infinite-hits/insights.ts +++ b/tests/common/widgets/infinite-hits/insights.ts @@ -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', { @@ -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); } }); @@ -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', {