From c2e0020b978eaf43a3ee6d6478e365c48740dd6f Mon Sep 17 00:00:00 2001 From: Rajib Quayum Date: Mon, 11 Aug 2025 14:00:28 -0400 Subject: [PATCH] feat: adds transform path and can save to VisitListener, fixes and adds tests Signed-off-by: Rajib Quayum chore: update API reports --- plugins/home/report.api.md | 18 + .../src/components/VisitListener.test.tsx | 322 +++++++++++------- plugins/home/src/components/VisitListener.tsx | 75 +++- 3 files changed, 281 insertions(+), 134 deletions(-) diff --git a/plugins/home/report.api.md b/plugins/home/report.api.md index 2f70f1a9c9..3e6ff5de04 100644 --- a/plugins/home/report.api.md +++ b/plugins/home/report.api.md @@ -264,6 +264,13 @@ export type Visit = { entityRef?: string; }; +// @public +export type VisitCanSaveFunction = ({ + pathname, +}: { + pathname: string; +}) => boolean; + // @public export interface VisitDisplayContextValue { // (undocumented) @@ -319,11 +326,15 @@ export const VisitListener: ({ toEntityRef, visitName, enrichVisit, + transformPathname, + canSave, }: { children?: ReactNode; toEntityRef?: ({ pathname }: { pathname: string }) => string | undefined; visitName?: ({ pathname }: { pathname: string }) => string; enrichVisit?: VisitEnrichmentFunction; + transformPathname?: VisitTransformPathnameFunction; + canSave?: VisitCanSaveFunction; }) => JSX.Element; // @public @@ -382,6 +393,13 @@ export type VisitsWebStorageApiOptions = { errorApi: ErrorApi; }; +// @public +export type VisitTransformPathnameFunction = ({ + pathname, +}: { + pathname: string; +}) => string; + // @public export const WelcomeTitle: ({ language, diff --git a/plugins/home/src/components/VisitListener.test.tsx b/plugins/home/src/components/VisitListener.test.tsx index c9de01b45f..1661dc202c 100644 --- a/plugins/home/src/components/VisitListener.test.tsx +++ b/plugins/home/src/components/VisitListener.test.tsx @@ -49,7 +49,33 @@ const mockVisitsApi = { }; describe('', () => { - afterEach(jest.resetAllMocks); + beforeEach(() => { + jest + .spyOn(window, 'requestAnimationFrame') + .mockImplementation((cb: FrameRequestCallback): number => { + cb(0); + return 0; + }); + }); + + afterEach(() => { + jest.restoreAllMocks(); + jest.resetAllMocks(); + }); + + it('uses requestAnimationFrame to defer visit saving', async () => { + const pathname = '/catalog/default/component/test-component'; + + await renderInTestApp( + + + , + { routeEntries: [pathname] }, + ); + + expect(window.requestAnimationFrame).toHaveBeenCalledTimes(1); + await waitFor(() => expect(mockVisitsApi.save).toHaveBeenCalledTimes(1)); + }); it('registers a visit', async () => { const pathname = '/catalog/default/component/playback-order'; @@ -131,144 +157,182 @@ describe('', () => { ); }); - describe('requestId tests', () => { - beforeEach(() => { - // Mock requestAnimationFrame to execute immediately - global.requestAnimationFrame = jest.fn(callback => { - callback(0); - return 1; - }); - global.cancelAnimationFrame = jest.fn(); - }); + it('saves base visit when no enrichment function is provided', async () => { + const pathname = '/catalog/default/component/test-component'; - afterEach(() => { - jest.restoreAllMocks(); - }); + await renderInTestApp( + + + , + { routeEntries: [pathname] }, + ); - it('uses requestAnimationFrame to defer visit saving', async () => { - const pathname = '/catalog/default/component/test-component'; - - await renderInTestApp( - - - , - { routeEntries: [pathname] }, - ); - - expect(global.requestAnimationFrame).toHaveBeenCalledTimes(1); - await waitFor(() => expect(mockVisitsApi.save).toHaveBeenCalledTimes(1)); - }); - - it('saves base visit when no enrichment function is provided', async () => { - const pathname = '/catalog/default/component/test-component'; - - await renderInTestApp( - - - , - { routeEntries: [pathname] }, - ); - - await waitFor(() => - expect(mockVisitsApi.save).toHaveBeenCalledWith({ - visit: { - pathname, - entityRef: 'component:default/test-component', - name: 'test-component', - }, - }), - ); - }); - - it('enriches visit with additional data when enrichVisit function is provided', async () => { - const pathname = '/catalog/default/component/test-component'; - const enrichVisit: VisitEnrichmentFunction = jest.fn(async _visit => ({ - customProperty: 'custom-value', - category: 'test-category', - priority: 1, - })); - - await renderInTestApp( - - - , - { routeEntries: [pathname] }, - ); - - await waitFor(() => { - expect(enrichVisit).toHaveBeenCalledWith({ + await waitFor(() => + expect(mockVisitsApi.save).toHaveBeenCalledWith({ + visit: { pathname, entityRef: 'component:default/test-component', name: 'test-component', - }); - expect(mockVisitsApi.save).toHaveBeenCalledWith({ - visit: { - pathname, - entityRef: 'component:default/test-component', - name: 'test-component', - customProperty: 'custom-value', - category: 'test-category', - priority: 1, - }, - }); + }, + }), + ); + }); + + it('enriches visit with additional data when enrichVisit function is provided', async () => { + const pathname = '/catalog/default/component/test-component'; + const enrichVisit: VisitEnrichmentFunction = jest.fn(async _visit => ({ + customProperty: 'custom-value', + category: 'test-category', + priority: 1, + })); + + await renderInTestApp( + + + , + { routeEntries: [pathname] }, + ); + + await waitFor(() => { + expect(enrichVisit).toHaveBeenCalledWith({ + pathname, + entityRef: 'component:default/test-component', + name: 'test-component', }); - }); - - it('handles synchronous enrichment function', async () => { - const pathname = '/catalog/default/component/test-component'; - const enrichVisit: VisitEnrichmentFunction = jest.fn(_visit => ({ - syncProperty: 'sync-value', - })); - - await renderInTestApp( - - - , - { routeEntries: [pathname] }, - ); - - await waitFor(() => { - expect(enrichVisit).toHaveBeenCalledWith({ + expect(mockVisitsApi.save).toHaveBeenCalledWith({ + visit: { pathname, entityRef: 'component:default/test-component', name: 'test-component', - }); - expect(mockVisitsApi.save).toHaveBeenCalledWith({ - visit: { - pathname, - entityRef: 'component:default/test-component', - name: 'test-component', - syncProperty: 'sync-value', - }, - }); - }); - }); - - it('enrichment function can override base visit properties', async () => { - const pathname = '/catalog/default/component/test-component'; - const enrichVisit: VisitEnrichmentFunction = jest.fn(async _visit => ({ - name: 'Overridden Name', - entityRef: 'overridden:ref/value', - customField: 'additional-data', - })); - - await renderInTestApp( - - - , - { routeEntries: [pathname] }, - ); - - await waitFor(() => { - expect(mockVisitsApi.save).toHaveBeenCalledWith({ - visit: { - pathname, - name: 'Overridden Name', - entityRef: 'overridden:ref/value', - customField: 'additional-data', - }, - }); + customProperty: 'custom-value', + category: 'test-category', + priority: 1, + }, }); }); }); + + it('handles synchronous enrichment function', async () => { + const pathname = '/catalog/default/component/test-component'; + const enrichVisit: VisitEnrichmentFunction = jest.fn(_visit => ({ + syncProperty: 'sync-value', + })); + + await renderInTestApp( + + + , + { routeEntries: [pathname] }, + ); + + await waitFor(() => { + expect(enrichVisit).toHaveBeenCalledWith({ + pathname, + entityRef: 'component:default/test-component', + name: 'test-component', + }); + expect(mockVisitsApi.save).toHaveBeenCalledWith({ + visit: { + pathname, + entityRef: 'component:default/test-component', + name: 'test-component', + syncProperty: 'sync-value', + }, + }); + }); + }); + + it('enrichment function can override base visit properties', async () => { + const pathname = '/catalog/default/component/test-component'; + const enrichVisit: VisitEnrichmentFunction = jest.fn(async _visit => ({ + name: 'Overridden Name', + entityRef: 'overridden:ref/value', + customField: 'additional-data', + })); + + await renderInTestApp( + + + , + { routeEntries: [pathname] }, + ); + + await waitFor(() => { + expect(mockVisitsApi.save).toHaveBeenCalledWith({ + visit: { + pathname, + name: 'Overridden Name', + entityRef: 'overridden:ref/value', + customField: 'additional-data', + }, + }); + }); + }); + + it('is able to override transformPathname and change the pathname', async () => { + const pathname = '/catalog/default/component/playback-order-2/sub-path'; + + const transformPathnameOverride = ({ + pathname: mypathname, + }: { + pathname: string; + }) => mypathname.replace('/sub-path', ''); + + await renderInTestApp( + + + , + { routeEntries: [pathname] }, + ); + + await waitFor(() => + expect(mockVisitsApi.save).toHaveBeenCalledWith({ + visit: { + pathname: '/catalog/default/component/playback-order-2', + entityRef: 'component:default/playback-order-2', + name: 'playback-order-2', + }, + }), + ); + }); + + it('is able to override canSave and save under set conditions', async () => { + const pathname = '/catalog'; + + const canSaveOverride = ({ pathname: path }: { pathname: string }) => + path === '/catalog'; + + await renderInTestApp( + + + , + { routeEntries: [pathname] }, + ); + + await waitFor(() => + expect(mockVisitsApi.save).toHaveBeenCalledWith({ + visit: { + pathname, + entityRef: undefined, + name: 'catalog', + }, + }), + ); + }); + + it('is able to override canSave and not save under set conditions', async () => { + const pathname = '/catalog'; + + const canSaveOverride = ({ pathname: path }: { pathname: string }) => + path !== '/catalog'; + + await renderInTestApp( + + + , + { routeEntries: [pathname] }, + ); + + await waitFor(() => expect(mockVisitsApi.save).not.toHaveBeenCalled()); + }); }); diff --git a/plugins/home/src/components/VisitListener.tsx b/plugins/home/src/components/VisitListener.tsx index e7a8cf4a22..534e6805b2 100644 --- a/plugins/home/src/components/VisitListener.tsx +++ b/plugins/home/src/components/VisitListener.tsx @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { ReactNode, useEffect } from 'react'; +import { ReactNode, useEffect, useRef } from 'react'; import { useLocation } from 'react-router-dom'; @@ -93,6 +93,48 @@ export type VisitEnrichmentFunction = ( visit: VisitInput, ) => Record | Promise>; +/** + * @public + * Type definition for the transform pathname function + * This allows transforming the pathname before it is considered for any other processing + */ +export type VisitTransformPathnameFunction = ({ + pathname, +}: { + pathname: string; +}) => string; + +/** + * @internal + * Default implementation of visit pathname transform function + */ +const getTransformPathname = + (): VisitTransformPathnameFunction => + ({ pathname }: { pathname: string }): string => { + return pathname; + }; + +/** + * @public + * Type definition for the can save function + * This allows checking whether a visit can be saved + */ +export type VisitCanSaveFunction = ({ + pathname, +}: { + pathname: string; +}) => boolean; + +/** + * @internal + * Default implementation of visit can save function + */ +const getCanSave = + (): VisitCanSaveFunction => + (_: { pathname: string }): boolean => { + return true; + }; + /** * @public * Component responsible for listening to location changes and calling @@ -103,25 +145,40 @@ export const VisitListener = ({ toEntityRef, visitName, enrichVisit, + transformPathname, + canSave, }: { children?: ReactNode; toEntityRef?: ({ pathname }: { pathname: string }) => string | undefined; visitName?: ({ pathname }: { pathname: string }) => string; enrichVisit?: VisitEnrichmentFunction; + transformPathname?: VisitTransformPathnameFunction; + canSave?: VisitCanSaveFunction; }): JSX.Element => { + const previousVisitPathname = useRef(''); const visitsApi = useApi(visitsApiRef); const { pathname } = useLocation(); const toEntityRefImpl = toEntityRef ?? getToEntityRef(); const visitNameImpl = visitName ?? getVisitName(); + const transformPathnameImpl = transformPathname ?? getTransformPathname(); + const canSaveImpl = canSave ?? getCanSave(); useEffect(() => { + const visitPathname = transformPathnameImpl({ pathname }); + if (previousVisitPathname.current === visitPathname) { + return () => {}; + } + previousVisitPathname.current = visitPathname; + if (!canSaveImpl({ pathname: visitPathname })) { + return () => {}; + } // Wait for the browser to finish with paint with the assumption react // has finished with dom reconciliation. const requestId = requestAnimationFrame(async () => { const baseVisit = { - name: visitNameImpl({ pathname }), - pathname, - entityRef: toEntityRefImpl({ pathname }), + name: visitNameImpl({ pathname: visitPathname }), + pathname: visitPathname, + entityRef: toEntityRefImpl({ pathname: visitPathname }), }; let visitToSave = baseVisit; @@ -141,7 +198,15 @@ export const VisitListener = ({ }); }); return () => cancelAnimationFrame(requestId); - }, [visitsApi, pathname, toEntityRefImpl, visitNameImpl, enrichVisit]); + }, [ + visitsApi, + pathname, + toEntityRefImpl, + visitNameImpl, + enrichVisit, + transformPathnameImpl, + canSaveImpl, + ]); return <>{children}; };