From 4600bd2e77c0e21f7f10dd03b9bfd999f1122d32 Mon Sep 17 00:00:00 2001 From: Victor Marin <36818606+mdvictor@users.noreply.github.com> Date: Thu, 10 Oct 2024 19:12:26 +0300 Subject: [PATCH] Scopes: Open dashboard list when a scope is selected (#94464) * Open dashboard list when a scope is selected * refactor * test * remove localstorage key * add checks on open/close methods * remove redundant statement * improve dashboards listing --- .../core/components/AppChrome/AppChrome.tsx | 4 +- .../scopes/internal/ScopesDashboardsScene.tsx | 96 +++++++++++++------ public/app/features/scopes/internal/const.ts | 1 - .../scopes/tests/dashboardsList.test.ts | 17 +++- .../features/scopes/tests/utils/assertions.ts | 2 + .../features/scopes/tests/utils/render.tsx | 2 - .../features/scopes/tests/utils/selectors.ts | 1 + public/app/features/scopes/utils.ts | 4 +- 8 files changed, 88 insertions(+), 39 deletions(-) delete mode 100644 public/app/features/scopes/internal/const.ts diff --git a/public/app/core/components/AppChrome/AppChrome.tsx b/public/app/core/components/AppChrome/AppChrome.tsx index 133deae94d3..9c323a7ba42 100644 --- a/public/app/core/components/AppChrome/AppChrome.tsx +++ b/public/app/core/components/AppChrome/AppChrome.tsx @@ -34,7 +34,9 @@ export function AppChrome({ children }: Props) { const dockedMenuLocalStorageState = store.getBool(DOCKED_LOCAL_STORAGE_KEY, true); const menuDockedAndOpen = !state.chromeless && state.megaMenuDocked && state.megaMenuOpen; const scopesDashboardsState = useScopesDashboardsState(); - const isScopesDashboardsOpen = Boolean(scopesDashboardsState?.isEnabled && scopesDashboardsState?.isPanelOpened); + const isScopesDashboardsOpen = Boolean( + scopesDashboardsState?.isEnabled && scopesDashboardsState?.isPanelOpened && !scopesDashboardsState?.isReadOnly + ); const isSingleTopNav = config.featureToggles.singleTopNav; useMediaQueryChange({ breakpoint: dockedMenuBreakpoint, diff --git a/public/app/features/scopes/internal/ScopesDashboardsScene.tsx b/public/app/features/scopes/internal/ScopesDashboardsScene.tsx index e838286df3f..b5450af8630 100644 --- a/public/app/features/scopes/internal/ScopesDashboardsScene.tsx +++ b/public/app/features/scopes/internal/ScopesDashboardsScene.tsx @@ -1,5 +1,6 @@ import { css, cx } from '@emotion/css'; import { isEqual } from 'lodash'; +import { finalize, from, Subscription } from 'rxjs'; import { GrafanaTheme2, ScopeDashboardBinding } from '@grafana/data'; import { SceneComponentProps, SceneObjectBase, SceneObjectRef, SceneObjectState } from '@grafana/scenes'; @@ -10,7 +11,6 @@ import { ScopesDashboardsTree } from './ScopesDashboardsTree'; import { ScopesDashboardsTreeSearch } from './ScopesDashboardsTreeSearch'; import { ScopesSelectorScene } from './ScopesSelectorScene'; import { fetchDashboards } from './api'; -import { DASHBOARDS_OPENED_KEY } from './const'; import { SuggestedDashboardsFoldersMap } from './types'; import { filterFolders, getScopeNamesFromSelectedScopes, groupDashboards } from './utils'; @@ -26,6 +26,7 @@ export interface ScopesDashboardsSceneState extends SceneObjectState { isLoading: boolean; isPanelOpened: boolean; isEnabled: boolean; + isReadOnly: boolean; scopesSelected: boolean; searchQuery: string; } @@ -36,8 +37,9 @@ export const getInitialDashboardsState: () => Omit Omit { static Component = ScopesDashboardsSceneRenderer; + private dashboardsFetchingSub: Subscription | undefined; + constructor() { super({ selector: null, @@ -52,35 +56,44 @@ export class ScopesDashboardsScene extends SceneObjectBase { - if (this.state.isEnabled && this.state.isPanelOpened) { - this.fetchDashboards(); - } - const resolvedSelector = this.state.selector?.resolve(); + if (resolvedSelector?.state.scopes.length ?? 0 > 0) { + this.fetchDashboards(); + this.openPanel(); + } + if (resolvedSelector) { this._subs.add( resolvedSelector.subscribeToState((newState, prevState) => { - if ( - this.state.isEnabled && - this.state.isPanelOpened && - !newState.isLoadingScopes && - (prevState.isLoadingScopes || newState.scopes !== prevState.scopes) - ) { + const newScopeNames = getScopeNamesFromSelectedScopes(newState.scopes ?? []); + const oldScopeNames = getScopeNamesFromSelectedScopes(prevState.scopes ?? []); + + if (!isEqual(newScopeNames, oldScopeNames)) { this.fetchDashboards(); + + if (newState.scopes.length > 0) { + this.openPanel(); + } else { + this.closePanel(); + } } }) ); } + + return () => { + this.dashboardsFetchingSub?.unsubscribe(); + }; }); } public async fetchDashboards() { const scopeNames = getScopeNamesFromSelectedScopes(this.state.selector?.resolve().state.scopes ?? []); - if (isEqual(scopeNames, this.state.forScopeNames)) { - return; - } + this.dashboardsFetchingSub?.unsubscribe(); + + this.setState({ forScopeNames: scopeNames }); if (scopeNames.length === 0) { return this.setState({ @@ -95,18 +108,26 @@ export class ScopesDashboardsScene extends SceneObjectBase { + this.setState({ isLoading: false }); + }) + ) + .subscribe((dashboards) => { + const folders = groupDashboards(dashboards); + const filteredFolders = filterFolders(folders, this.state.searchQuery); - this.setState({ - dashboards, - folders, - filteredFolders, - forScopeNames: scopeNames, - isLoading: false, - scopesSelected: scopeNames.length > 0, - }); + this.setState({ + dashboards, + folders, + filteredFolders, + isLoading: false, + scopesSelected: scopeNames.length > 0, + }); + + this.dashboardsFetchingSub?.unsubscribe(); + }); } public changeSearchQuery(searchQuery: string) { @@ -148,14 +169,19 @@ export class ScopesDashboardsScene extends SceneObjectBase) { - const { dashboards, filteredFolders, isLoading, isPanelOpened, isEnabled, searchQuery, scopesSelected } = + const { dashboards, filteredFolders, isLoading, isPanelOpened, isEnabled, isReadOnly, searchQuery, scopesSelected } = model.useState(); const styles = useStyles2(getStyles); - if (!isEnabled || !isPanelOpened) { + if (!isEnabled || !isPanelOpened || isReadOnly) { return null; } diff --git a/public/app/features/scopes/internal/const.ts b/public/app/features/scopes/internal/const.ts deleted file mode 100644 index 643a106960d..00000000000 --- a/public/app/features/scopes/internal/const.ts +++ /dev/null @@ -1 +0,0 @@ -export const DASHBOARDS_OPENED_KEY = 'grafana.scopes.dashboards.opened'; diff --git a/public/app/features/scopes/tests/dashboardsList.test.ts b/public/app/features/scopes/tests/dashboardsList.test.ts index e5215a89f86..3b5f2fe1885 100644 --- a/public/app/features/scopes/tests/dashboardsList.test.ts +++ b/public/app/features/scopes/tests/dashboardsList.test.ts @@ -12,7 +12,9 @@ import { expectDashboardInDocument, expectDashboardLength, expectDashboardNotInDocument, + expectDashboardsClosed, expectDashboardSearchValue, + expectDashboardsOpen, expectDashboardsSearch, expectNoDashboardsForFilter, expectNoDashboardsForScope, @@ -45,9 +47,20 @@ describe('Dashboards list', () => { await resetScenes(); }); - it('Does not fetch dashboards list when the list is not expanded', async () => { + it('Opens container and fetches dashboards list when a scope is selected', async () => { + expectDashboardsClosed(); await updateScopes(['mimir']); - expect(fetchDashboardsSpy).not.toHaveBeenCalled(); + expectDashboardsOpen(); + expect(fetchDashboardsSpy).toHaveBeenCalled(); + }); + + it('Closes container when no scopes are selected', async () => { + await updateScopes(['mimir']); + expectDashboardsOpen(); + await updateScopes(['mimir', 'loki']); + expectDashboardsOpen(); + await updateScopes([]); + expectDashboardsClosed(); }); it('Fetches dashboards list when the list is expanded', async () => { diff --git a/public/app/features/scopes/tests/utils/assertions.ts b/public/app/features/scopes/tests/utils/assertions.ts index 785f72e005f..cfd54c4ad78 100644 --- a/public/app/features/scopes/tests/utils/assertions.ts +++ b/public/app/features/scopes/tests/utils/assertions.ts @@ -1,6 +1,7 @@ import { getMock, locationReloadSpy } from './mocks'; import { getDashboard, + getDashboardsContainer, getDashboardsExpand, getDashboardsSearch, getNotFoundForFilter, @@ -62,6 +63,7 @@ export const expectResultCloudOpsNotSelected = () => expectRadioNotChecked(getRe export const expectDashboardsDisabled = () => expectDisabled(getDashboardsExpand); export const expectDashboardsClosed = () => expectNotInDocument(queryDashboardsContainer); +export const expectDashboardsOpen = () => expectInDocument(getDashboardsContainer); export const expectNoDashboardsSearch = () => expectNotInDocument(queryDashboardsSearch); export const expectDashboardsSearch = () => expectInDocument(getDashboardsSearch); export const expectNoDashboardsNoScopes = () => expectInDocument(getNotFoundNoScopes); diff --git a/public/app/features/scopes/tests/utils/render.tsx b/public/app/features/scopes/tests/utils/render.tsx index 538ed293770..71d60ff6492 100644 --- a/public/app/features/scopes/tests/utils/render.tsx +++ b/public/app/features/scopes/tests/utils/render.tsx @@ -12,7 +12,6 @@ import { DashboardDataDTO, DashboardDTO, DashboardMeta } from 'app/types'; import { initializeScopes, scopesDashboardsScene, scopesSelectorScene } from '../../instance'; import { getInitialDashboardsState } from '../../internal/ScopesDashboardsScene'; import { initialSelectorState } from '../../internal/ScopesSelectorScene'; -import { DASHBOARDS_OPENED_KEY } from '../../internal/const'; import { clearMocks } from './actions'; @@ -160,7 +159,6 @@ export async function resetScenes() { await jest.runOnlyPendingTimersAsync(); jest.useRealTimers(); scopesSelectorScene?.setState(initialSelectorState); - localStorage.removeItem(DASHBOARDS_OPENED_KEY); scopesDashboardsScene?.setState(getInitialDashboardsState()); cleanup(); } diff --git a/public/app/features/scopes/tests/utils/selectors.ts b/public/app/features/scopes/tests/utils/selectors.ts index 44f24b5fbac..b3174c0c6e3 100644 --- a/public/app/features/scopes/tests/utils/selectors.ts +++ b/public/app/features/scopes/tests/utils/selectors.ts @@ -38,6 +38,7 @@ export const getSelectorApply = () => screen.getByTestId(selectors.selector.appl export const getSelectorCancel = () => screen.getByTestId(selectors.selector.cancel); export const getDashboardsExpand = () => screen.getByTestId(selectors.dashboards.expand); +export const getDashboardsContainer = () => screen.getByTestId(selectors.dashboards.container); export const queryDashboardsContainer = () => screen.queryByTestId(selectors.dashboards.container); export const queryDashboardsSearch = () => screen.queryByTestId(selectors.dashboards.search); export const getDashboardsSearch = () => screen.getByTestId(selectors.dashboards.search); diff --git a/public/app/features/scopes/utils.ts b/public/app/features/scopes/utils.ts index 85d01626fa8..29ddd616382 100644 --- a/public/app/features/scopes/utils.ts +++ b/public/app/features/scopes/utils.ts @@ -25,12 +25,12 @@ export function disableScopes() { export function exitScopesReadOnly() { scopesSelectorScene?.exitReadOnly(); - scopesDashboardsScene?.enable(); + scopesDashboardsScene?.exitReadOnly(); } export function enterScopesReadOnly() { scopesSelectorScene?.enterReadOnly(); - scopesDashboardsScene?.disable(); + scopesDashboardsScene?.enterReadOnly(); } export function getClosestScopesFacade(scene: SceneObject): ScopesFacade | null {