diff --git a/.changeset/analytics-millenial-whoop.md b/.changeset/analytics-millenial-whoop.md new file mode 100644 index 0000000000..265e0072ce --- /dev/null +++ b/.changeset/analytics-millenial-whoop.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': patch +--- + +Fixed a bug that could cause `navigate` analytics events to be misattributed to the plugin mounted on the root route (e.g. the `home` plugin at `/`) when the route that was navigated to wasn't associated with a routable extension. diff --git a/.github/vale/Vocab/Backstage/accept.txt b/.github/vale/Vocab/Backstage/accept.txt index efd6186405..ed09c73c91 100644 --- a/.github/vale/Vocab/Backstage/accept.txt +++ b/.github/vale/Vocab/Backstage/accept.txt @@ -213,6 +213,7 @@ middleware minikube Minikube Minio +misattributed misconfiguration misconfigured mkdocs diff --git a/packages/core-app-api/src/routing/RouteTracker.test.tsx b/packages/core-app-api/src/routing/RouteTracker.test.tsx index 6bf58a117e..25d5cfbcfb 100644 --- a/packages/core-app-api/src/routing/RouteTracker.test.tsx +++ b/packages/core-app-api/src/routing/RouteTracker.test.tsx @@ -25,30 +25,46 @@ import { createPlugin, createRouteRef, } from '@backstage/core-plugin-api'; +import { MATCH_ALL_ROUTE } from './collectors'; describe('RouteTracker', () => { + const routeRef0 = createRouteRef({ + id: 'home:root', + }); const routeRef1 = createRouteRef({ id: 'route1', }); const routeRef2 = createRouteRef({ id: 'route2', }); + const plugin0 = createPlugin({ id: 'home' }); const plugin1 = createPlugin({ id: 'plugin1' }); + const plugin2 = createPlugin({ id: 'plugin2' }); const routeObjects: BackstageRouteObject[] = [ + { + path: '', + element:
home page
, + routeRefs: new Set([routeRef0]), + plugins: new Set([plugin0]), + caseSensitive: false, + children: [MATCH_ALL_ROUTE], + }, { path: '/path/:p1/:p2', element: go, routeRefs: new Set([routeRef1]), plugins: new Set([plugin1]), caseSensitive: false, + children: [MATCH_ALL_ROUTE], }, { path: '/path2/:param', element:
hi there
, routeRefs: new Set([routeRef2]), - plugins: new Set(), + plugins: new Set([plugin2]), caseSensitive: false, + children: [MATCH_ALL_ROUTE], }, ]; @@ -92,8 +108,8 @@ describe('RouteTracker', () => { - {routeObjects.map(({ routeRefs, ...props }) => ( - + {routeObjects.map(({ path, element }) => ( + ))} @@ -109,11 +125,110 @@ describe('RouteTracker', () => { }, context: { extension: 'App', - pluginId: 'root', + pluginId: 'plugin2', routeRef: 'route2', }, subject: '/path2/hello', value: undefined, }); }); + + it('should capture path query and hash', async () => { + render( + + + + + , + ); + + expect(mockedAnalytics.captureEvent).toHaveBeenCalledWith({ + action: 'navigate', + attributes: { + p1: 'foo', + p2: 'bar', + }, + context: { + extension: 'App', + pluginId: 'plugin1', + routeRef: 'route1', + }, + subject: '/path/foo/bar?q=1#header-1', + value: undefined, + }); + }); + + it('should match the root path and send relevant context', async () => { + render( + + + + + , + ); + + expect(mockedAnalytics.captureEvent).toHaveBeenCalledWith({ + action: 'navigate', + attributes: {}, + context: { + extension: 'App', + pluginId: 'home', + routeRef: 'home:root', + }, + subject: '/', + value: undefined, + }); + }); + + it('should return default context when it would have otherwise matched on the root path', async () => { + render( + + + + + Non-extension} + /> + + + , + ); + + expect(mockedAnalytics.captureEvent).toHaveBeenCalledWith({ + action: 'navigate', + attributes: {}, + context: { + extension: 'App', + pluginId: 'root', + routeRef: 'unknown', + }, + subject: '/not-routable-extension', + value: undefined, + }); + }); + + it('should return parent route context on navigating to a sub-route', async () => { + render( + + + + + , + ); + + expect(mockedAnalytics.captureEvent).toHaveBeenCalledWith({ + action: 'navigate', + attributes: { + param: 'param-value', + }, + context: { + extension: 'App', + pluginId: 'plugin2', + routeRef: 'route2', + }, + subject: '/path2/param-value/sub-route', + value: undefined, + }); + }); }); diff --git a/packages/core-app-api/src/routing/RouteTracker.tsx b/packages/core-app-api/src/routing/RouteTracker.tsx index f1910728e3..69903268ac 100644 --- a/packages/core-app-api/src/routing/RouteTracker.tsx +++ b/packages/core-app-api/src/routing/RouteTracker.tsx @@ -38,12 +38,11 @@ const getExtensionContext = ( const matches = matchRoutes(routes, { pathname }); // Of the matching routes, get the last (e.g. most specific) instance of - // the BackstageRouteObject. - + // the BackstageRouteObject that contains a routeRef. Filtering by routeRef + // ensures subRouteRefs are aligned to their parent routes' context. const routeMatch = matches ?.filter(match => match?.route.routeRefs?.size > 0) .pop(); - const routeObject = routeMatch?.route; // If there is no route object, then allow inheritance of default context. @@ -51,6 +50,12 @@ const getExtensionContext = ( return undefined; } + // If the matched route is the root route (no path), and the pathname is + // not the path of the homepage, then inherit from the default context. + if (routeObject.path === '' && pathname !== '/') { + return undefined; + } + // If there is a single route ref, use it. let routeRef: RouteRef | undefined; if (routeObject.routeRefs.size === 1) { @@ -66,7 +71,7 @@ const getExtensionContext = ( const params = Object.entries( routeMatch?.params || {}, ).reduce((acc, [key, value]) => { - if (value !== undefined) { + if (value !== undefined && key !== '*') { acc[key] = value; } return acc;