refactor: Simplify breadcrumb keys (#19370)

* Rework breadcrumb keys with IDs to include scene

* Use the breadcrumb key in data-attr

* Remove `globalKey`

* Fix selectors

* Use joined breadcrumb key for fragment and `data-attr`

* Fix missing `sceneBreadcrumbs` keys

.github/dependabot.yml

@@ -1,6 +1,6 @@
 version: 2
 updates:
-  - package-ecosystem: "github-actions"
-    directory: "/"
-    schedule:
-      interval: weekly
+    - package-ecosystem: 'github-actions'
+      directory: '/'
+      schedule:
+          interval: weekly

cypress/e2e/dashboard.cy.ts

@@ -14,9 +14,9 @@ describe('Dashboard', () => {
     it('Dashboards loaded', () => {
         cy.get('h1').should('contain', 'Dashboards')
         // Breadcrumbs work
-        cy.get('[data-attr=breadcrumb-0]').should('contain', 'Hogflix')
-        cy.get('[data-attr=breadcrumb-1]').should('contain', 'Hogflix Demo App')
-        cy.get('[data-attr=breadcrumb-2]').should('have.text', 'Dashboards')
+        cy.get('[data-attr=breadcrumb-organization]').should('contain', 'Hogflix')
+        cy.get('[data-attr=breadcrumb-project]').should('contain', 'Hogflix Demo App')
+        cy.get('[data-attr=breadcrumb-Dashboards]').should('have.text', 'Dashboards')
     })
 
     // FIXME: this test works in real, but not in cypress
@@ -123,10 +123,10 @@ describe('Dashboard', () => {
 
         cy.get('.InsightCard').its('length').should('be.gte', 2)
         // Breadcrumbs work
-        cy.get('[data-attr=breadcrumb-0]').should('contain', 'Hogflix')
-        cy.get('[data-attr=breadcrumb-1]').should('contain', 'Hogflix Demo App')
-        cy.get('[data-attr=breadcrumb-2]').should('have.text', 'Dashboards')
-        cy.get('[data-attr=breadcrumb-3]').should('have.text', TEST_DASHBOARD_NAME + 'UnnamedCancelSave')
+        cy.get('[data-attr=breadcrumb-organization]').should('contain', 'Hogflix')
+        cy.get('[data-attr=breadcrumb-project]').should('contain', 'Hogflix Demo App')
+        cy.get('[data-attr=breadcrumb-Dashboards]').should('have.text', 'Dashboards')
+        cy.get('[data-attr^="breadcrumb-Dashboard:"]').should('have.text', TEST_DASHBOARD_NAME + 'UnnamedCancelSave')
     })
 
     it('Click on a dashboard item dropdown and view graph', () => {

cypress/e2e/insights.cy.ts

@@ -23,10 +23,10 @@ describe('Insights', () => {
     it('Saving an insight sets breadcrumbs', () => {
         createInsight('insight name')
 
-        cy.get('[data-attr=breadcrumb-0]').should('contain', 'Hogflix')
-        cy.get('[data-attr=breadcrumb-1]').should('contain', 'Hogflix Demo App')
-        cy.get('[data-attr=breadcrumb-2]').should('have.text', 'Product analytics')
-        cy.get('[data-attr=breadcrumb-3]').should('have.text', 'insight name')
+        cy.get('[data-attr=breadcrumb-organization]').should('contain', 'Hogflix')
+        cy.get('[data-attr=breadcrumb-project]').should('contain', 'Hogflix Demo App')
+        cy.get('[data-attr=breadcrumb-SavedInsights]').should('have.text', 'Product analytics')
+        cy.get('[data-attr^="breadcrumb-Insight:"]').should('have.text', 'insight name')
     })
 
     it('Can change insight name', () => {

frontend/src/layout/navigation-3000/components/TopBar.tsx

@@ -12,7 +12,7 @@ import React, { useLayoutEffect, useState } from 'react'
 
 import { breadcrumbsLogic } from '~/layout/navigation/Breadcrumbs/breadcrumbsLogic'
 import { navigationLogic } from '~/layout/navigation/navigationLogic'
-import { FinalizedBreadcrumb } from '~/types'
+import { Breadcrumb as IBreadcrumb } from '~/types'
 
 import { navigation3000Logic } from '../navigationLogic'
 
@@ -88,17 +88,13 @@ export function TopBar(): JSX.Element | null {
                 <div className="TopBar3000__breadcrumbs">
                     {breadcrumbs.length > 1 && (
                         <div className="TopBar3000__trail">
-                            {breadcrumbs.slice(0, -1).map((breadcrumb, index) => (
-                                <React.Fragment key={breadcrumb.name || '…'}>
-                                    <Breadcrumb breadcrumb={breadcrumb} index={index} />
+                            {breadcrumbs.slice(0, -1).map((breadcrumb) => (
+                                <React.Fragment key={joinBreadcrumbKey(breadcrumb.key)}>
+                                    <Breadcrumb breadcrumb={breadcrumb} />
                                     <div className="TopBar3000__separator" />
                                 </React.Fragment>
                             ))}
-                            <Breadcrumb
-                                breadcrumb={breadcrumbs[breadcrumbs.length - 1]}
-                                index={breadcrumbs.length - 1}
-                                here
-                            />
+                            <Breadcrumb breadcrumb={breadcrumbs[breadcrumbs.length - 1]} here />
                         </div>
                     )}
                     <Here breadcrumb={breadcrumbs[breadcrumbs.length - 1]} />
@@ -110,30 +106,31 @@ export function TopBar(): JSX.Element | null {
 }
 
 interface BreadcrumbProps {
-    breadcrumb: FinalizedBreadcrumb
-    index: number
+    breadcrumb: IBreadcrumb
     here?: boolean
 }
 
-function Breadcrumb({ breadcrumb, index, here }: BreadcrumbProps): JSX.Element {
+function Breadcrumb({ breadcrumb, here }: BreadcrumbProps): JSX.Element {
     const { renameState } = useValues(breadcrumbsLogic)
     const { tentativelyRename, finishRenaming } = useActions(breadcrumbsLogic)
     const [popoverShown, setPopoverShown] = useState(false)
 
+    const joinedKey = joinBreadcrumbKey(breadcrumb.key)
+
     let nameElement: JSX.Element
     if (breadcrumb.name != null && breadcrumb.onRename) {
         nameElement = (
             <EditableField
                 name="item-name-small"
-                value={renameState && renameState[0] === breadcrumb.globalKey ? renameState[1] : breadcrumb.name}
-                onChange={(newName) => tentativelyRename(breadcrumb.globalKey, newName)}
+                value={renameState && renameState[0] === joinedKey ? renameState[1] : breadcrumb.name}
+                onChange={(newName) => tentativelyRename(joinedKey, newName)}
                 onSave={(newName) => {
                     void breadcrumb.onRename?.(newName)
                 }}
-                mode={renameState && renameState[0] === breadcrumb.globalKey ? 'edit' : 'view'}
+                mode={renameState && renameState[0] === joinedKey ? 'edit' : 'view'}
                 onModeToggle={(newMode) => {
                     if (newMode === 'edit') {
-                        tentativelyRename(breadcrumb.globalKey, breadcrumb.name as string)
+                        tentativelyRename(joinedKey, breadcrumb.name as string)
                     } else {
                         finishRenaming()
                     }
@@ -160,7 +157,7 @@ function Breadcrumb({ breadcrumb, index, here }: BreadcrumbProps): JSX.Element {
             onClick={() => {
                 breadcrumb.popover && setPopoverShown(!popoverShown)
             }}
-            data-attr={`breadcrumb-${index}`}
+            data-attr={`breadcrumb-${joinedKey}`}
             to={breadcrumb.path}
         >
             {nameElement}
@@ -193,23 +190,25 @@ function Breadcrumb({ breadcrumb, index, here }: BreadcrumbProps): JSX.Element {
 }
 
 interface HereProps {
-    breadcrumb: FinalizedBreadcrumb
+    breadcrumb: IBreadcrumb
 }
 
 function Here({ breadcrumb }: HereProps): JSX.Element {
     const { renameState } = useValues(breadcrumbsLogic)
     const { tentativelyRename, finishRenaming } = useActions(breadcrumbsLogic)
 
+    const joinedKey = joinBreadcrumbKey(breadcrumb.key)
+
     return (
         <h1 className="TopBar3000__here" data-attr="top-bar-name">
             {breadcrumb.name == null ? (
                 <LemonSkeleton className="w-40 h-4" />
             ) : breadcrumb.onRename ? (
                 <EditableField
                     name="item-name-large"
-                    value={renameState && renameState[0] === breadcrumb.globalKey ? renameState[1] : breadcrumb.name}
+                    value={renameState && renameState[0] === joinedKey ? renameState[1] : breadcrumb.name}
                     onChange={(newName) => {
-                        tentativelyRename(breadcrumb.globalKey, newName)
+                        tentativelyRename(joinedKey, newName)
                         if (breadcrumb.forceEditMode) {
                             // In this case there's no "Save" button, we update on input
                             void breadcrumb.onRename?.(newName)
@@ -218,16 +217,12 @@ function Here({ breadcrumb }: HereProps): JSX.Element {
                     onSave={(newName) => {
                         void breadcrumb.onRename?.(newName)
                     }}
-                    mode={
-                        breadcrumb.forceEditMode || (renameState && renameState[0] === breadcrumb.globalKey)
-                            ? 'edit'
-                            : 'view'
-                    }
+                    mode={breadcrumb.forceEditMode || (renameState && renameState[0] === joinedKey) ? 'edit' : 'view'}
                     onModeToggle={
                         !breadcrumb.forceEditMode
                             ? (newMode) => {
                                   if (newMode === 'edit') {
-                                      tentativelyRename(breadcrumb.globalKey, breadcrumb.name as string)
+                                      tentativelyRename(joinedKey, breadcrumb.name as string)
                                   } else {
                                       finishRenaming()
                                   }
@@ -245,3 +240,7 @@ function Here({ breadcrumb }: HereProps): JSX.Element {
         </h1>
     )
 }
+
+function joinBreadcrumbKey(key: IBreadcrumb['key']): string {
+    return Array.isArray(key) ? key.map(String).join(':') : String(key)
+}

frontend/src/layout/navigation/Breadcrumbs/breadcrumbsLogic.tsx

@@ -11,7 +11,7 @@ import { userLogic } from 'scenes/userLogic'
 
 import { OrganizationSwitcherOverlay } from '~/layout/navigation/OrganizationSwitcher'
 import { ProjectSwitcherOverlay } from '~/layout/navigation/ProjectSwitcher'
-import { Breadcrumb, FinalizedBreadcrumb } from '~/types'
+import { Breadcrumb } from '~/types'
 
 import type { breadcrumbsLogicType } from './breadcrumbsLogicType'
 
@@ -67,20 +67,18 @@ export const breadcrumbsLogic = kea<breadcrumbsLogicType>([
             (s) => [
                 // We're effectively passing the selector through to the scene logic, and "recalculating"
                 // this every time it's rendered. Caching will happen within the scene's breadcrumb selector.
-                (state, props) => {
+                (state, props): Breadcrumb[] => {
                     const activeSceneLogic = sceneLogic.selectors.activeSceneLogic(state, props)
                     const activeScene = s.activeScene(state, props)
-                    const sceneConfig = s.sceneConfig(state, props)
                     if (activeSceneLogic && 'breadcrumbs' in activeSceneLogic.selectors) {
                         const activeLoadedScene = sceneLogic.selectors.activeLoadedScene(state, props)
                         return activeSceneLogic.selectors.breadcrumbs(
                             state,
                             activeLoadedScene?.paramsToProps?.(activeLoadedScene?.sceneParams) || props
                         )
-                    } else if (sceneConfig?.name) {
-                        return [{ name: sceneConfig.name }]
                     } else if (activeScene) {
-                        return [{ name: identifierToHuman(activeScene) }]
+                        const sceneConfig = s.sceneConfig(state, props)
+                        return [{ name: sceneConfig?.name ?? identifierToHuman(activeScene), key: activeScene }]
                     } else {
                         return []
                     }
@@ -168,24 +166,8 @@ export const breadcrumbsLogic = kea<breadcrumbsLogicType>([
         ],
         breadcrumbs: [
             (s) => [s.appBreadcrumbs, s.sceneBreadcrumbs],
-            (appBreadcrumbs, sceneBreadcrumbs): FinalizedBreadcrumb[] => {
-                const breadcrumbs = Array<FinalizedBreadcrumb>(appBreadcrumbs.length + sceneBreadcrumbs.length)
-                const globalPathSoFar: string[] = []
-                for (let i = 0; i < appBreadcrumbs.length; i++) {
-                    globalPathSoFar.push(String(appBreadcrumbs[i].key))
-                    breadcrumbs[i] = {
-                        ...appBreadcrumbs[i],
-                        globalKey: globalPathSoFar.join('.'),
-                    }
-                }
-                for (let i = 0; i < sceneBreadcrumbs.length; i++) {
-                    globalPathSoFar.push(String(sceneBreadcrumbs[i].key))
-                    breadcrumbs[i + appBreadcrumbs.length] = {
-                        ...sceneBreadcrumbs[i],
-                        globalKey: globalPathSoFar.join('.'),
-                    }
-                }
-                return breadcrumbs
+            (appBreadcrumbs, sceneBreadcrumbs): Breadcrumb[] => {
+                return appBreadcrumbs.concat(sceneBreadcrumbs)
             },
         ],
         firstBreadcrumb: [(s) => [s.breadcrumbs], (breadcrumbs) => breadcrumbs[0]],

frontend/src/scenes/actions/actionLogic.ts

@@ -66,7 +66,7 @@ export const actionLogic = kea<actionLogicType>([
                     path: urls.actions(),
                 },
                 {
-                    key: action?.id || 'new',
+                    key: [Scene.Action, action?.id || 'new'],
                     name: inProgressName ?? (action?.name || ''),
                     onRename: async (name: string) => {
                         const id = action?.id

frontend/src/scenes/apps/appMetricsSceneLogic.ts

@@ -188,7 +188,7 @@ export const appMetricsSceneLogic = kea<appMetricsSceneLogicType>([
                     path: urls.projectApps(),
                 },
                 {
-                    key: pluginConfigId,
+                    key: [Scene.AppMetrics, pluginConfigId],
                     name: pluginConfig?.plugin_info?.name,
                     path: urls.appMetrics(pluginConfigId),
                 },

frontend/src/scenes/batch_exports/batchExportLogic.ts

@@ -233,7 +233,7 @@ export const batchExportLogic = kea<batchExportLogicType>([
                     path: urls.batchExports(),
                 },
                 {
-                    key: config?.id || 'loading',
+                    key: [Scene.BatchExport, config?.id || 'loading'],
                     name: config?.name,
                 },
             ],

frontend/src/scenes/cohorts/cohortSceneLogic.ts

@@ -29,7 +29,7 @@ export const cohortSceneLogic = kea<cohortSceneLogicType>([
                         path: urls.cohorts(),
                     },
                     {
-                        key: cohortId || 'loading',
+                        key: [Scene.Cohort, cohortId || 'loading'],
                         name: cohortId && cohortId !== 'new' ? cohortsById[cohortId]?.name || 'Untitled' : 'Untitled',
                     },
                 ]

frontend/src/scenes/dashboard/dashboardLogic.tsx

@@ -742,7 +742,7 @@ export const dashboardLogic = kea<dashboardLogicType>([
                     path: urls.dashboards(),
                 },
                 {
-                    key: dashboard?.id || 'new',
+                    key: [Scene.Dashboard, dashboard?.id || 'new'],
                     name: dashboard?.id ? dashboard.name || 'Unnamed' : null,
                     onRename: async (name) => {
                         if (dashboard) {

frontend/src/scenes/data-management/definition/definitionLogic.ts

@@ -133,7 +133,7 @@ export const definitionLogic = kea<definitionLogicType>([
                         path: isEvent ? urls.eventDefinitions() : urls.propertyDefinitions(),
                     },
                     {
-                        key: definition?.id || 'new',
+                        key: [isEvent ? Scene.EventDefinition : Scene.PropertyDefinition, definition?.id || 'new'],
                         name: definition?.id !== 'new' ? getPropertyLabel(definition?.name) || 'Untitled' : 'Untitled',
                     },
                 ]

frontend/src/scenes/data-warehouse/new_table/dataWarehouseTableLogic.tsx

@@ -108,7 +108,7 @@ export const dataWarehouseTableLogic = kea<dataWarehouseTableLogicType>([
                     path: urls.dataWarehouseExternal(),
                 },
                 {
-                    key: 'new',
+                    key: [Scene.DataWarehouseTable, 'new'],
                     name: 'New',
                 },
             ],

frontend/src/scenes/early-access-features/earlyAccessFeatureLogic.ts

@@ -129,7 +129,7 @@ export const earlyAccessFeatureLogic = kea<earlyAccessFeatureLogicType>([
                     path: urls.earlyAccessFeatures(),
                 },
                 {
-                    key: earlyAccessFeature.id || 'new',
+                    key: [Scene.EarlyAccessFeature, earlyAccessFeature.id || 'new'],
                     name: earlyAccessFeature.name,
                 },
             ],

frontend/src/scenes/experiments/experimentLogic.tsx

@@ -650,7 +650,7 @@ export const experimentLogic = kea<experimentLogicType>([
                     path: urls.experiments(),
                 },
                 {
-                    key: experimentId,
+                    key: [Scene.Experiment, experimentId],
                     name: experiment?.name || 'New',
                     path: urls.experiment(experimentId || 'new'),
                 },

frontend/src/scenes/feature-flags/featureFlagLogic.ts

@@ -944,7 +944,7 @@ export const featureFlagLogic = kea<featureFlagLogicType>([
                     name: 'Feature Flags',
                     path: urls.featureFlags(),
                 },
-                { key: featureFlag.id || 'unknown', name: featureFlag.key || 'Unnamed' },
+                { key: [Scene.FeatureFlag, featureFlag.id || 'unknown'], name: featureFlag.key || 'Unnamed' },
             ],
         ],
         propertySelectErrors: [

frontend/src/scenes/groups/groupLogic.ts

@@ -117,7 +117,7 @@ export const groupLogic = kea<groupLogicType>([
                     path: urls.groups(String(groupTypeIndex)),
                 },
                 {
-                    key: `${groupTypeIndex}-${groupKey}`,
+                    key: [Scene.Group, `${groupTypeIndex}-${groupKey}`],
                     name: groupDisplayId(groupKey, groupData?.group_properties || {}),
                     path: urls.group(String(groupTypeIndex), groupKey),
                 },

frontend/src/scenes/insights/insightSceneLogic.tsx

@@ -109,7 +109,7 @@ export const insightSceneLogic = kea<insightSceneLogicType>([
                     path: urls.savedInsights(),
                 },
                 {
-                    key: insight?.short_id || 'new',
+                    key: [Scene.Insight, insight?.short_id || 'new'],
                     name:
                         insight?.name ||
                         summarizeInsight(insight?.query, filters, {

frontend/src/scenes/notebooks/notebookSceneLogic.ts

@@ -36,7 +36,7 @@ export const notebookSceneLogic = kea<notebookSceneLogicType>([
                     path: urls.notebooks(),
                 },
                 {
-                    key: notebook?.short_id || 'new',
+                    key: [Scene.Notebook, notebook?.short_id || 'new'],
                     name: notebook ? notebook?.title || 'Unnamed' : loading ? null : 'Notebook not found',
                 },
             ],

frontend/src/scenes/persons/personsLogic.tsx

@@ -256,7 +256,7 @@ export const personsLogic = kea<personsLogicType>([
                 ]
                 if (showPerson) {
                     breadcrumbs.push({
-                        key: person.id || 'unknown',
+                        key: [Scene.Person, person.id || 'unknown'],
                         name: asDisplay(person),
                     })
                 }

frontend/src/scenes/session-recordings/detail/sessionRecordingDetailLogic.ts

@@ -23,7 +23,7 @@ export const sessionRecordingDetailLogic = kea<sessionRecordingDetailLogicType>(
                     path: urls.replay(),
                 },
                 {
-                    key: sessionRecordingId,
+                    key: [Scene.ReplaySingle, sessionRecordingId],
                     name: sessionRecordingId ?? 'Not Found',
                     path: sessionRecordingId ? urls.replaySingle(sessionRecordingId) : undefined,
                 },

frontend/src/scenes/session-recordings/playlist/sessionRecordingsPlaylistSceneLogic.ts

@@ -152,7 +152,7 @@ export const sessionRecordingsPlaylistSceneLogic = kea<sessionRecordingsPlaylist
                     path: urls.replay(ReplayTabs.Playlists),
                 },
                 {
-                    key: playlist?.short_id || 'new',
+                    key: [Scene.ReplayPlaylist, playlist?.short_id || 'new'],
                     name: playlist?.name || playlist?.derived_name || 'Unnamed',
                     onRename: async (name: string) => {
                         if (!playlist) {