From 41c120ba89f693f23e10e0342d5f98f9f243352e Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Fri, 10 Jan 2025 11:09:12 +0100 Subject: [PATCH] Sidecar: Fix cases when sidecar wouldn't open depending on the first loaded route (#98679) Update mainOnAllowedRoute even if no activeId --- .../SidecarService_EXPERIMENTAL.test.ts | 45 +++++++++++++++---- .../services/SidecarService_EXPERIMENTAL.ts | 3 +- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts b/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts index b41e0f7f70e..6e6c7a0d1a8 100644 --- a/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts +++ b/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts @@ -5,10 +5,16 @@ import { config } from '../config'; import { HistoryWrapper } from './LocationService'; import { SidecarService_EXPERIMENTAL } from './SidecarService_EXPERIMENTAL'; -describe('SidecarService_EXPERIMENTAL', () => { - let mainLocationService: HistoryWrapper; - let sidecarService: SidecarService_EXPERIMENTAL; +function setup() { + const mainLocationService = new HistoryWrapper(H.createMemoryHistory({ initialEntries: ['/explore'] })); + const sidecarService = new SidecarService_EXPERIMENTAL(mainLocationService); + return { + mainLocationService, + sidecarService, + }; +} +describe('SidecarService_EXPERIMENTAL', () => { beforeAll(() => { config.featureToggles.appSidecar = true; }); @@ -17,12 +23,8 @@ describe('SidecarService_EXPERIMENTAL', () => { config.featureToggles.appSidecar = false; }); - beforeEach(() => { - mainLocationService = new HistoryWrapper(H.createMemoryHistory({ initialEntries: ['/explore'] })); - sidecarService = new SidecarService_EXPERIMENTAL(mainLocationService); - }); - it('has the correct state after opening and closing an app', () => { + const { sidecarService } = setup(); sidecarService.openApp('pluginId', { filter: 'test' }); expect(sidecarService.activePluginId).toBe('pluginId'); @@ -36,6 +38,7 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('has the correct state after opening and closing an app v2', () => { + const { sidecarService } = setup(); sidecarService.openAppV2('pluginId', '/test'); expect(sidecarService.activePluginId).toBe('pluginId'); @@ -48,6 +51,7 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('has the correct state after opening and closing an app v3', () => { + const { sidecarService } = setup(); sidecarService.openAppV3({ pluginId: 'pluginId', path: '/test' }); expect(sidecarService.activePluginId).toBe('pluginId'); @@ -60,6 +64,7 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('reports correct opened state', () => { + const { sidecarService } = setup(); expect(sidecarService.isAppOpened('pluginId')).toBe(false); sidecarService.openApp('pluginId'); @@ -70,6 +75,7 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('reports correct opened state v2', () => { + const { sidecarService } = setup(); expect(sidecarService.isAppOpened('pluginId')).toBe(false); sidecarService.openAppV2('pluginId'); @@ -80,6 +86,7 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('reports correct opened state v3', () => { + const { sidecarService } = setup(); expect(sidecarService.isAppOpened('pluginId')).toBe(false); sidecarService.openAppV3({ pluginId: 'pluginId' }); @@ -90,6 +97,7 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('autocloses on not allowed routes', () => { + const { sidecarService, mainLocationService } = setup(); sidecarService.openAppV3({ pluginId: 'pluginId' }); expect(sidecarService.isAppOpened('pluginId')).toBe(true); mainLocationService.push('/config'); @@ -98,6 +106,7 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('autocloses on when changing route', () => { + const { sidecarService, mainLocationService } = setup(); sidecarService.openAppV3({ pluginId: 'pluginId' }); expect(sidecarService.isAppOpened('pluginId')).toBe(true); mainLocationService.push('/a/other-app'); @@ -106,6 +115,7 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('does not autocloses when set to follow', () => { + const { sidecarService, mainLocationService } = setup(); sidecarService.openAppV3({ pluginId: 'pluginId', follow: true }); expect(sidecarService.isAppOpened('pluginId')).toBe(true); mainLocationService.push('/a/other-app'); @@ -114,10 +124,29 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('autocloses on not allowed routes when set to follow', () => { + const { sidecarService, mainLocationService } = setup(); sidecarService.openAppV3({ pluginId: 'pluginId', follow: true }); expect(sidecarService.isAppOpened('pluginId')).toBe(true); mainLocationService.push('/config'); expect(sidecarService.isAppOpened('pluginId')).toBe(false); }); + + it('autocloses on not allowed routes when set to follow', () => { + const { sidecarService, mainLocationService } = setup(); + sidecarService.openAppV3({ pluginId: 'pluginId', follow: true }); + expect(sidecarService.isAppOpened('pluginId')).toBe(true); + mainLocationService.push('/config'); + + expect(sidecarService.isAppOpened('pluginId')).toBe(false); + }); + + it('opens sidecar even if starting route is not allowed and then it changes', () => { + const mainLocationService = new HistoryWrapper(H.createMemoryHistory({ initialEntries: ['/login'] })); + const sidecarService = new SidecarService_EXPERIMENTAL(mainLocationService); + mainLocationService.push('/explore'); + + sidecarService.openAppV3({ pluginId: 'pluginId', follow: true }); + expect(sidecarService.isAppOpened('pluginId')).toBe(true); + }); }); diff --git a/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.ts b/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.ts index 2656f1d7448..8e00fbb42e0 100644 --- a/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.ts +++ b/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.ts @@ -81,10 +81,11 @@ export class SidecarService_EXPERIMENTAL { ); this.mainLocationService.getLocationObservable().subscribe((location) => { + this.mainOnAllowedRoute = ALLOW_ROUTES.some((prefix) => location.pathname.match(prefix)); + if (!this.activePluginId) { return; } - this.mainOnAllowedRoute = ALLOW_ROUTES.some((prefix) => location.pathname.match(prefix)); if (!this.mainOnAllowedRoute) { this.closeApp();