From 58e9f22c1b84cb235d4618b081ac838f90fe3ee2 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Thu, 21 Nov 2024 11:27:01 +0100 Subject: [PATCH] Sidecar: Specify where it can be opened and when it automatically closes (#96683) --- .../src/services/LocationService.tsx | 13 ++ .../services/SidecarContext_EXPERIMENTAL.ts | 20 +-- .../SidecarService_EXPERIMENTAL.test.ts | 76 ++++++++++- .../services/SidecarService_EXPERIMENTAL.ts | 122 +++++++++++++++--- 4 files changed, 188 insertions(+), 43 deletions(-) diff --git a/packages/grafana-runtime/src/services/LocationService.tsx b/packages/grafana-runtime/src/services/LocationService.tsx index 6a148b15482..f3a26c5b110 100644 --- a/packages/grafana-runtime/src/services/LocationService.tsx +++ b/packages/grafana-runtime/src/services/LocationService.tsx @@ -1,5 +1,6 @@ import * as H from 'history'; import React, { useContext } from 'react'; +import { BehaviorSubject, Observable } from 'rxjs'; import { deprecationWarning, UrlQueryMap, urlUtil } from '@grafana/data'; import { attachDebugger, createLogger } from '@grafana/ui'; @@ -21,6 +22,7 @@ export interface LocationService { getHistory: () => H.History; getSearch: () => URLSearchParams; getSearchObject: () => UrlQueryMap; + getLocationObservable: () => Observable; /** * This is from the old LocationSrv interface @@ -31,6 +33,7 @@ export interface LocationService { /** @internal */ export class HistoryWrapper implements LocationService { private readonly history: H.History; + private locationObservable: BehaviorSubject; constructor(history?: H.History) { // If no history passed create an in memory one if being called from test @@ -40,6 +43,12 @@ export class HistoryWrapper implements LocationService { ? H.createMemoryHistory({ initialEntries: ['/'] }) : H.createBrowserHistory({ basename: config.appSubUrl ?? '/' })); + this.locationObservable = new BehaviorSubject(this.history.location); + + this.history.listen((location) => { + this.locationObservable.next(location); + }); + this.partial = this.partial.bind(this); this.push = this.push.bind(this); this.replace = this.replace.bind(this); @@ -48,6 +57,10 @@ export class HistoryWrapper implements LocationService { this.getLocation = this.getLocation.bind(this); } + getLocationObservable() { + return this.locationObservable.asObservable(); + } + getHistory() { return this.history; } diff --git a/packages/grafana-runtime/src/services/SidecarContext_EXPERIMENTAL.ts b/packages/grafana-runtime/src/services/SidecarContext_EXPERIMENTAL.ts index 9f8d6e42d9e..05f4579d0ba 100644 --- a/packages/grafana-runtime/src/services/SidecarContext_EXPERIMENTAL.ts +++ b/packages/grafana-runtime/src/services/SidecarContext_EXPERIMENTAL.ts @@ -1,15 +1,12 @@ import { createContext, useContext } from 'react'; import { useObservable } from 'react-use'; -import { locationService as mainLocationService } from './LocationService'; import { SidecarService_EXPERIMENTAL, sidecarServiceSingleton_EXPERIMENTAL } from './SidecarService_EXPERIMENTAL'; export const SidecarContext_EXPERIMENTAL = createContext( sidecarServiceSingleton_EXPERIMENTAL ); -const HIDDEN_ROUTES = ['/login']; - /** * This is the main way to interact with the sidecar service inside a react context. It provides a wrapper around the * service props so that even though they are observables we just pass actual values to the components. @@ -27,32 +24,25 @@ export function useSidecar_EXPERIMENTAL() { const initialContext = useObservable(service.initialContextObservable, service.initialContext); const activePluginId = useObservable(service.activePluginIdObservable, service.activePluginId); const locationService = service.getLocationService(); - const forceHidden = HIDDEN_ROUTES.includes(mainLocationService.getLocation().pathname); return { - activePluginId: forceHidden ? undefined : activePluginId, - initialContext: forceHidden ? undefined : initialContext, + activePluginId, + initialContext, locationService, // TODO: currently this allows anybody to open any app, in the future we should probably scope this to the // current app but that means we will need to incorporate this better into the plugin platform APIs which // we will do once the functionality is reasonably stable openApp: (pluginId: string, context?: unknown) => { - if (forceHidden) { - return; - } return service.openApp(pluginId, context); }, openAppV2: (pluginId: string, path?: string) => { - if (forceHidden) { - return; - } return service.openAppV2(pluginId, path); }, + openAppV3: (options: { pluginId: string; path?: string; follow?: boolean }) => { + return service.openAppV3(options); + }, closeApp: () => service.closeApp(), isAppOpened: (pluginId: string) => { - if (forceHidden) { - return false; - } return service.isAppOpened(pluginId); }, }; diff --git a/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts b/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts index afa9f3aaf18..b41e0f7f70e 100644 --- a/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts +++ b/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.test.ts @@ -1,17 +1,28 @@ +import * as H from 'history'; + import { config } from '../config'; +import { HistoryWrapper } from './LocationService'; import { SidecarService_EXPERIMENTAL } from './SidecarService_EXPERIMENTAL'; describe('SidecarService_EXPERIMENTAL', () => { - beforeEach(() => { + let mainLocationService: HistoryWrapper; + let sidecarService: SidecarService_EXPERIMENTAL; + + beforeAll(() => { config.featureToggles.appSidecar = true; }); - afterEach(() => { - config.featureToggles.appSidecar = undefined; + + afterAll(() => { + 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 = new SidecarService_EXPERIMENTAL(); sidecarService.openApp('pluginId', { filter: 'test' }); expect(sidecarService.activePluginId).toBe('pluginId'); @@ -25,7 +36,6 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('has the correct state after opening and closing an app v2', () => { - const sidecarService = new SidecarService_EXPERIMENTAL(); sidecarService.openAppV2('pluginId', '/test'); expect(sidecarService.activePluginId).toBe('pluginId'); @@ -37,8 +47,19 @@ describe('SidecarService_EXPERIMENTAL', () => { expect(sidecarService.getLocationService().getLocation().pathname).toBe('/'); }); + it('has the correct state after opening and closing an app v3', () => { + sidecarService.openAppV3({ pluginId: 'pluginId', path: '/test' }); + + expect(sidecarService.activePluginId).toBe('pluginId'); + expect(sidecarService.getLocationService().getLocation().pathname).toBe('/a/pluginId/test'); + + sidecarService.closeApp(); + expect(sidecarService.activePluginId).toBe(undefined); + expect(sidecarService.initialContext).toBe(undefined); + expect(sidecarService.getLocationService().getLocation().pathname).toBe('/'); + }); + it('reports correct opened state', () => { - const sidecarService = new SidecarService_EXPERIMENTAL(); expect(sidecarService.isAppOpened('pluginId')).toBe(false); sidecarService.openApp('pluginId'); @@ -49,7 +70,6 @@ describe('SidecarService_EXPERIMENTAL', () => { }); it('reports correct opened state v2', () => { - const sidecarService = new SidecarService_EXPERIMENTAL(); expect(sidecarService.isAppOpened('pluginId')).toBe(false); sidecarService.openAppV2('pluginId'); @@ -58,4 +78,46 @@ describe('SidecarService_EXPERIMENTAL', () => { sidecarService.closeApp(); expect(sidecarService.isAppOpened('pluginId')).toBe(false); }); + + it('reports correct opened state v3', () => { + expect(sidecarService.isAppOpened('pluginId')).toBe(false); + + sidecarService.openAppV3({ pluginId: 'pluginId' }); + expect(sidecarService.isAppOpened('pluginId')).toBe(true); + + sidecarService.closeApp(); + expect(sidecarService.isAppOpened('pluginId')).toBe(false); + }); + + it('autocloses on not allowed routes', () => { + sidecarService.openAppV3({ pluginId: 'pluginId' }); + expect(sidecarService.isAppOpened('pluginId')).toBe(true); + mainLocationService.push('/config'); + + expect(sidecarService.isAppOpened('pluginId')).toBe(false); + }); + + it('autocloses on when changing route', () => { + sidecarService.openAppV3({ pluginId: 'pluginId' }); + expect(sidecarService.isAppOpened('pluginId')).toBe(true); + mainLocationService.push('/a/other-app'); + + expect(sidecarService.isAppOpened('pluginId')).toBe(false); + }); + + it('does not autocloses when set to follow', () => { + sidecarService.openAppV3({ pluginId: 'pluginId', follow: true }); + expect(sidecarService.isAppOpened('pluginId')).toBe(true); + mainLocationService.push('/a/other-app'); + + expect(sidecarService.isAppOpened('pluginId')).toBe(true); + }); + + it('autocloses on not allowed routes when set to follow', () => { + sidecarService.openAppV3({ pluginId: 'pluginId', follow: true }); + expect(sidecarService.isAppOpened('pluginId')).toBe(true); + mainLocationService.push('/config'); + + expect(sidecarService.isAppOpened('pluginId')).toBe(false); + }); }); diff --git a/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.ts b/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.ts index 98c4a1ff1a0..2656f1d7448 100644 --- a/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.ts +++ b/packages/grafana-runtime/src/services/SidecarService_EXPERIMENTAL.ts @@ -5,7 +5,17 @@ import { BehaviorSubject, map, Observable } from 'rxjs'; import { reportInteraction } from '../analytics/utils'; import { config } from '../config'; -import { HistoryWrapper, LocationService, locationService } from './LocationService'; +import { HistoryWrapper, locationService as mainLocationService, LocationService } from './LocationService'; + +// Only allow sidecar to be opened on these routes. It does not seem to make sense to keep the sidecar opened on +// config/admin pages for example. +// At this moment let's be restrictive about where the sidecar can show and add more routes if there is a need. +const ALLOW_ROUTES = [ + /(^\/d\/)/, // dashboards + /^\/explore/, // explore + explore metrics + /^\/a\/[^\/]+/, // app plugins + /^\/alerting/, +]; /** * This is a service that handles state and operation of a sidecar feature (sideview to render a second app in grafana). @@ -19,15 +29,26 @@ import { HistoryWrapper, LocationService, locationService } from './LocationServ */ export class SidecarService_EXPERIMENTAL { private _initialContext: BehaviorSubject; - private memoryLocationService: LocationService; - private history: LocationStorageHistory; - constructor() { + private sidecarLocationService: LocationService; + private mainLocationService: LocationService; + + // If true we don't close the sidecar when user navigates to another app or part of Grafana from where the sidecar + // was opened. + private follow = false; + + // Keep track of where the sidecar was originally opened for autoclose behaviour. + private mainLocationWhenOpened: string | undefined; + + private mainOnAllowedRoute = false; + + constructor(mainLocationService: LocationService) { this._initialContext = new BehaviorSubject(undefined); - // We need a local ref for this so we can tap into the location changes and drive rerendering of components based - // on it without having a parent Router. - this.history = createLocationStorageHistory({ storageKey: 'grafana.sidecar.history' }); - this.memoryLocationService = new HistoryWrapper(this.history); + this.mainLocationService = mainLocationService; + this.sidecarLocationService = new HistoryWrapper( + createLocationStorageHistory({ storageKey: 'grafana.sidecar.history' }) + ); + this.handleMainLocationChanges(); } private assertFeatureEnabled() { @@ -39,6 +60,48 @@ export class SidecarService_EXPERIMENTAL { return true; } + private updateMainLocationWhenOpened() { + const pathname = this.mainLocationService.getLocation().pathname; + for (const route of ALLOW_ROUTES) { + const match = pathname.match(route)?.[0]; + if (match) { + this.mainLocationWhenOpened = match; + return; + } + } + } + + /** + * Every time the main location changes we check if we should keep the sidecar open or close it based on list + * of allowed routes and also based on the follow flag when opening the app. + */ + private handleMainLocationChanges() { + this.mainOnAllowedRoute = ALLOW_ROUTES.some((prefix) => + this.mainLocationService.getLocation().pathname.match(prefix) + ); + + this.mainLocationService.getLocationObservable().subscribe((location) => { + if (!this.activePluginId) { + return; + } + this.mainOnAllowedRoute = ALLOW_ROUTES.some((prefix) => location.pathname.match(prefix)); + + if (!this.mainOnAllowedRoute) { + this.closeApp(); + return; + } + + // We check if we moved to some other app or part of grafana from where we opened the sidecar. + const isTheSameLocation = Boolean( + this.mainLocationWhenOpened && location.pathname.startsWith(this.mainLocationWhenOpened) + ); + + if (!(isTheSameLocation || this.follow)) { + this.closeApp(); + } + }); + } + /** * Get current app id of the app in sidecar. This is most probably provisional. In the future * this should be driven by URL addressing so that routing for the apps don't change. Useful just internally @@ -47,7 +110,7 @@ export class SidecarService_EXPERIMENTAL { * @experimental */ get activePluginIdObservable() { - return this.history.getLocationObservable().pipe( + return this.sidecarLocationService.getLocationObservable().pipe( map((val) => { return getPluginIdFromUrl(val?.pathname || ''); }) @@ -75,11 +138,11 @@ export class SidecarService_EXPERIMENTAL { * @experimental */ get activePluginId() { - return getPluginIdFromUrl(this.memoryLocationService.getLocation().pathname); + return getPluginIdFromUrl(this.sidecarLocationService.getLocation().pathname); } getLocationService() { - return this.memoryLocationService; + return this.sidecarLocationService; } /** @@ -88,26 +151,41 @@ export class SidecarService_EXPERIMENTAL { * @experimental */ openApp(pluginId: string, context?: unknown) { - if (!this.assertFeatureEnabled()) { + if (!(this.assertFeatureEnabled() && this.mainOnAllowedRoute)) { return; } this._initialContext.next(context); - this.memoryLocationService.push({ pathname: `/a/${pluginId}` }); - - reportInteraction('sidecar_service_open_app', { pluginId, version: 1 }); + this.openAppV3({ pluginId, follow: false }); } /** * Opens an app in a sidecar. You can also relative path inside the app to open. + * @deprecated * @experimental */ openAppV2(pluginId: string, path?: string) { - if (!this.assertFeatureEnabled()) { + this.openAppV3({ pluginId, path, follow: false }); + } + + /** + * Opens an app in a sidecar. You can also relative path inside the app to open. + * @param options.pluginId Plugin ID of the app to open + * @param options.path Relative path inside the app to open + * @param options.follow If true, the sidecar will stay open even if the main location change to another app or + * Grafana section + * + * @experimental + */ + openAppV3(options: { pluginId: string; path?: string; follow?: boolean }) { + if (!(this.assertFeatureEnabled() && this.mainOnAllowedRoute)) { return; } - this.memoryLocationService.push({ pathname: `/a/${pluginId}${path || ''}` }); - reportInteraction('sidecar_service_open_app', { pluginId, version: 2 }); + this.follow = options.follow || false; + + this.updateMainLocationWhenOpened(); + this.sidecarLocationService.push({ pathname: `/a/${options.pluginId}${options.path || ''}` }); + reportInteraction('sidecar_service_open_app', { pluginId: options.pluginId, follow: options.follow }); } /** @@ -118,8 +196,10 @@ export class SidecarService_EXPERIMENTAL { return; } + this.follow = false; + this.mainLocationWhenOpened = undefined; this._initialContext.next(undefined); - this.memoryLocationService.replace({ pathname: '/' }); + this.sidecarLocationService.replace({ pathname: '/' }); reportInteraction('sidecar_service_close_app'); } @@ -160,7 +240,7 @@ function getPluginIdFromUrl(url: string) { function getMainAppPluginId() { // TODO: not great but we have to get a handle on the other locationService used for the main view and easiest way // right now is through this global singleton - const { pathname } = locationService.getLocation(); + const { pathname } = mainLocationService.getLocation(); // A naive way to sort of simulate core features being an app and having an appID let mainApp = getPluginIdFromUrl(pathname); @@ -258,4 +338,4 @@ function createLocationStorageHistory(options: LocalStorageHistoryOptions): Loca }; } -export const sidecarServiceSingleton_EXPERIMENTAL = new SidecarService_EXPERIMENTAL(); +export const sidecarServiceSingleton_EXPERIMENTAL = new SidecarService_EXPERIMENTAL(mainLocationService);