public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Scopes: Don't use redirect if you're on an active scope navigation (#115149)

Browse Source 
* Don't use redirectUrl if we are on an active scope navigation

* Remove superflous test
This commit is contained in:
Tobias Skarhed
2025-12-11 17:42:47 +01:00
committed by GitHub
parent fe4c615b3d
commit f63c2cb2dd
6 changed files with 146 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
devenv/scopes/scopes-config.yaml
+6
View File
@@ -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
devenv/scopes/scopes.go
+7
View File
@@ -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,
e2e-playwright/dashboard-cujs/scope-redirect.spec.ts
+44 -25
View File
@@ -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/);
});
});
});
e2e-playwright/utils/scope-helpers.ts
+53 -3
View File
@@ -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<Promise<Response>> = [];
for (const scope of scopes) {
e2e-playwright/utils/scopes.ts
+18
View File
@@ -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,
},
];
};
public/app/features/scopes/selector/ScopesSelectorService.ts
+18 -8
View File
@@ -372,13 +372,7 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
// Redirect to the scope node's redirect URL if it exists, otherwise redirect to the first scope navigation.
private redirectAfterApply = (scopeNode: ScopeNode | undefined) => {
// 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<ScopesSelectorServi
return isCurrentPath(currentPath, s.spec.url);
});
// Only redirect to redirectPath if we are not currently on an active scope navigation
if (
!activeScopeNavigation &&
scopeNode &&
scopeNode.spec.redirectPath &&
typeof scopeNode.spec.redirectPath === 'string' &&
// Don't redirect if we're already on the target path
!isCurrentPath(currentPath, scopeNode.spec.redirectPath)
) {
locationService.push(scopeNode.spec.redirectPath);
return;
}
// Redirect to first scopeNavigation if current URL isn't a scopeNavigation
if (!activeScopeNavigation && this.dashboardsService.state.scopeNavigations.length > 0) {
// Redirect to the first available scopeNavigation
const firstScopeNavigation = this.dashboardsService.state.scopeNavigations[0];
@@ -396,7 +404,9 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
'url' in firstScopeNavigation.spec &&
typeof firstScopeNavigation.spec.url === 'string' &&
// Only redirect to dashboards TODO: Remove this once Logs Drilldown has Scopes support
firstScopeNavigation.spec.url.includes('/d/')
firstScopeNavigation.spec.url.includes('/d/') &&
// Don't redirect if we're already on the target path
!isCurrentPath(currentPath, firstScopeNavigation.spec.url)
) {
locationService.push(firstScopeNavigation.spec.url);
}