From f63c2cb2dda408426d4f48640e74e8395d3af98f Mon Sep 17 00:00:00 2001 From: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> Date: Thu, 11 Dec 2025 17:42:47 +0100 Subject: [PATCH] Scopes: Don't use redirect if you're on an active scope navigation (#115149) * Don't use redirectUrl if we are on an active scope navigation * Remove superflous test --- devenv/scopes/scopes-config.yaml | 6 ++ devenv/scopes/scopes.go | 7 ++ .../dashboard-cujs/scope-redirect.spec.ts | 69 ++++++++++++------- e2e-playwright/utils/scope-helpers.ts | 56 ++++++++++++++- e2e-playwright/utils/scopes.ts | 18 +++++ .../scopes/selector/ScopesSelectorService.ts | 26 ++++--- 6 files changed, 146 insertions(+), 36 deletions(-) diff --git a/devenv/scopes/scopes-config.yaml b/devenv/scopes/scopes-config.yaml index 54842e5818e..36fd6645fa9 100644 --- a/devenv/scopes/scopes-config.yaml +++ b/devenv/scopes/scopes-config.yaml @@ -83,6 +83,12 @@ tree: nodeType: leaf linkId: test-case-2 linkType: scope + test-case-redirect: + title: Test case with redirect + nodeType: leaf + linkId: shoe-org + linkType: scope + redirectPath: /d/dcb9f5e9-8066-4397-889e-864b99555dbb #Reliability dashboard clusters: title: Clusters nodeType: container diff --git a/devenv/scopes/scopes.go b/devenv/scopes/scopes.go index 1f8a698ccda..335540cbef5 100644 --- a/devenv/scopes/scopes.go +++ b/devenv/scopes/scopes.go @@ -67,10 +67,12 @@ type ScopeFilterConfig struct { type TreeNode struct { Title string `yaml:"title"` SubTitle string `yaml:"subTitle,omitempty"` + Description string `yaml:"description,omitempty"` NodeType string `yaml:"nodeType"` LinkID string `yaml:"linkId,omitempty"` LinkType string `yaml:"linkType,omitempty"` DisableMultiSelect bool `yaml:"disableMultiSelect,omitempty"` + RedirectPath string `yaml:"redirectPath,omitempty"` Children map[string]TreeNode `yaml:"children,omitempty"` } @@ -259,6 +261,7 @@ func (c *Client) createScopeNode(name string, node TreeNode, parentName string) spec := v0alpha1.ScopeNodeSpec{ Title: node.Title, SubTitle: node.SubTitle, + Description: node.Description, NodeType: nodeType, DisableMultiSelect: node.DisableMultiSelect, } @@ -272,6 +275,10 @@ func (c *Client) createScopeNode(name string, node TreeNode, parentName string) spec.LinkType = linkType } + if node.RedirectPath != "" { + spec.RedirectPath = node.RedirectPath + } + resource := v0alpha1.ScopeNode{ TypeMeta: metav1.TypeMeta{ APIVersion: apiVersion, diff --git a/e2e-playwright/dashboard-cujs/scope-redirect.spec.ts b/e2e-playwright/dashboard-cujs/scope-redirect.spec.ts index c0e6e55b5b0..952e8a3da63 100644 --- a/e2e-playwright/dashboard-cujs/scope-redirect.spec.ts +++ b/e2e-playwright/dashboard-cujs/scope-redirect.spec.ts @@ -8,6 +8,7 @@ test.use({ scopeFilters: true, groupByVariable: true, reloadDashboardsOnParamsChange: true, + useScopesNavigationEndpoint: true, }, }); @@ -61,31 +62,6 @@ test.describe('Scope Redirect Functionality', () => { }); }); - test('should fall back to scope navigation when no redirectUrl', async ({ page, gotoDashboardPage }) => { - const scopes = testScopesWithRedirect(); - - await test.step('Navigate to dashboard and open scopes selector', async () => { - await gotoDashboardPage({ uid: 'cuj-dashboard-1' }); - await openScopesSelector(page, scopes); - }); - - await test.step('Select scope without redirectUrl', async () => { - // Select the scope without redirectUrl directly - await selectScope(page, 'sn-redirect-fallback', scopes[1]); - }); - - await test.step('Apply scopes and verify fallback behavior', async () => { - await applyScopes(page, [scopes[1]]); - - // Should stay on current dashboard since no redirectUrl is provided - // The scope navigation fallback should not redirect (as per existing behavior) - await expect(page).toHaveURL(/\/d\/cuj-dashboard-1/); - - // Verify the scope was applied - await expect(page).toHaveURL(/scopes=scope-sn-redirect-fallback/); - }); - }); - test('should not redirect when reloading page on dashboard not in dashboard list', async ({ page, gotoDashboardPage, @@ -171,4 +147,47 @@ test.describe('Scope Redirect Functionality', () => { await expect(page).not.toHaveURL(/scopes=/); }); }); + + test('should not redirect to redirectPath when on active scope navigation', async ({ page, gotoDashboardPage }) => { + const scopes = testScopesWithRedirect(); + + await test.step('Set up scope navigation to dashboard-1', async () => { + // First, apply a scope that creates scope navigation to dashboard-1 (without redirectPath) + await gotoDashboardPage({ uid: 'cuj-dashboard-1' }); + await openScopesSelector(page, scopes); + await selectScope(page, 'sn-redirect-setup', scopes[2]); + await applyScopes(page, [scopes[2]]); + + // Verify we're on dashboard-1 with the scope applied + await expect(page).toHaveURL(/\/d\/cuj-dashboard-1/); + await expect(page).toHaveURL(/scopes=scope-sn-redirect-setup/); + }); + + await test.step('Navigate to dashboard-1 to be on active scope navigation', async () => { + // Navigate to dashboard-1 which is now a scope navigation target + await gotoDashboardPage({ + uid: 'cuj-dashboard-1', + queryParams: new URLSearchParams({ scopes: 'scope-sn-redirect-setup' }), + }); + + // Verify we're on dashboard-1 + await expect(page).toHaveURL(/\/d\/cuj-dashboard-1/); + }); + + await test.step('Apply scope with redirectPath and verify no redirect', async () => { + // Now apply a different scope that has redirectPath + // Since we're on an active scope navigation, it should NOT redirect + await openScopesSelector(page, scopes); + await selectScope(page, 'sn-redirect-with-navigation', scopes[3]); + await applyScopes(page, [scopes[3]]); + + // Verify the new scope was applied + await expect(page).toHaveURL(/scopes=scope-sn-redirect-with-navigation/); + + // Since we're already on the active scope navigation (dashboard-1), + // we should NOT redirect to redirectPath (dashboard-3) + await expect(page).toHaveURL(/\/d\/cuj-dashboard-1/); + await expect(page).not.toHaveURL(/\/d\/cuj-dashboard-3/); + }); + }); }); diff --git a/e2e-playwright/utils/scope-helpers.ts b/e2e-playwright/utils/scope-helpers.ts index 749577ecb05..fc88a79d8fa 100644 --- a/e2e-playwright/utils/scope-helpers.ts +++ b/e2e-playwright/utils/scope-helpers.ts @@ -156,13 +156,18 @@ export async function applyScopes(page: Page, scopes?: TestScope[]) { return; } - const url: string = + const dashboardBindingsUrl: string = '**/apis/scope.grafana.app/v0alpha1/namespaces/*/find/scope_dashboard_bindings?' + scopes.map((scope) => `scope=scope-${scope.name}`).join('&'); + const scopeNavigationsUrl: string = + '**/apis/scope.grafana.app/v0alpha1/namespaces/*/find/scope_navigations?' + + scopes.map((scope) => `scope=scope-${scope.name}`).join('&'); + const groups: string[] = ['Most relevant', 'Dashboards', 'Something else', '']; - await page.route(url, async (route) => { + // Mock scope_dashboard_bindings endpoint + await page.route(dashboardBindingsUrl, async (route) => { await route.fulfill({ status: 200, contentType: 'application/json', @@ -215,7 +220,52 @@ export async function applyScopes(page: Page, scopes?: TestScope[]) { }); }); - const responsePromise = page.waitForResponse((response) => response.url().includes(`/find/scope_dashboard_bindings`)); + // Mock scope_navigations endpoint + await page.route(scopeNavigationsUrl, async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + apiVersion: 'scope.grafana.app/v0alpha1', + items: scopes.flatMap((scope) => { + const navigations: Array<{ + kind: string; + apiVersion: string; + metadata: { name: string; resourceVersion: string; creationTimestamp: string }; + spec: { url: string; scope: string }; + status: { title: string }; + }> = []; + + // Create a scope navigation if dashboardUid is provided + if (scope.dashboardUid && scope.addLinks) { + navigations.push({ + kind: 'ScopeNavigation', + apiVersion: 'scope.grafana.app/v0alpha1', + metadata: { + name: `scope-${scope.name}-nav`, + resourceVersion: '1', + creationTimestamp: 'stamp', + }, + spec: { + url: `/d/${scope.dashboardUid}`, + scope: `scope-${scope.name}`, + }, + status: { + title: scope.dashboardTitle ?? scope.title, + }, + }); + } + + return navigations; + }), + }), + }); + }); + + const responsePromise = page.waitForResponse( + (response) => + response.url().includes(`/find/scope_dashboard_bindings`) || response.url().includes(`/find/scope_navigations`) + ); const scopeRequestPromises: Array> = []; for (const scope of scopes) { diff --git a/e2e-playwright/utils/scopes.ts b/e2e-playwright/utils/scopes.ts index c5b141d4eb7..73a29a5da95 100644 --- a/e2e-playwright/utils/scopes.ts +++ b/e2e-playwright/utils/scopes.ts @@ -124,5 +124,23 @@ export const testScopesWithRedirect = (): TestScope[] => { dashboardTitle: 'CUJ Dashboard 2', addLinks: true, }, + { + name: 'sn-redirect-setup', + title: 'Setup Navigation', + // No redirectPath - used to set up scope navigation to dashboard-1 + filters: [{ key: 'namespace', operator: 'equals', value: 'setup-nav' }], + dashboardUid: 'cuj-dashboard-1', // Creates scope navigation to this dashboard + dashboardTitle: 'CUJ Dashboard 1', + addLinks: true, + }, + { + name: 'sn-redirect-with-navigation', + title: 'Redirect With Navigation', + redirectPath: '/d/cuj-dashboard-3', // Redirect target + filters: [{ key: 'namespace', operator: 'equals', value: 'redirect-with-nav' }], + dashboardUid: 'cuj-dashboard-1', // Creates scope navigation to this dashboard + dashboardTitle: 'CUJ Dashboard 1', + addLinks: true, + }, ]; }; diff --git a/public/app/features/scopes/selector/ScopesSelectorService.ts b/public/app/features/scopes/selector/ScopesSelectorService.ts index 6f2bf86b7e0..abd837c7fb7 100644 --- a/public/app/features/scopes/selector/ScopesSelectorService.ts +++ b/public/app/features/scopes/selector/ScopesSelectorService.ts @@ -372,13 +372,7 @@ export class ScopesSelectorService extends ScopesServiceBase { - // Check if the selected scope has a redirect path - if (scopeNode && scopeNode.spec.redirectPath && typeof scopeNode.spec.redirectPath === 'string') { - locationService.push(scopeNode.spec.redirectPath); - return; - } - - // Redirect to first scopeNavigation if current URL isn't a scopeNavigation + // Check if we are currently on an active scope navigation const currentPath = locationService.getLocation().pathname; const activeScopeNavigation = this.dashboardsService.state.scopeNavigations.find((s) => { if (!('url' in s.spec) || typeof s.spec.url !== 'string') { @@ -387,6 +381,20 @@ export class ScopesSelectorService extends ScopesServiceBase 0) { // Redirect to the first available scopeNavigation const firstScopeNavigation = this.dashboardsService.state.scopeNavigations[0]; @@ -396,7 +404,9 @@ export class ScopesSelectorService extends ScopesServiceBase