Scopes: Don't redirect on URL sync (#113851)
* Add toggle for redirection when applying a scope * Add e2e tests * Update unit tests
This commit is contained in:
@@ -85,4 +85,89 @@ test.describe('Scope Redirect Functionality', () => {
|
||||
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,
|
||||
}) => {
|
||||
const scopes = testScopesWithRedirect();
|
||||
|
||||
await test.step('Navigate to dashboard and select scope', async () => {
|
||||
await gotoDashboardPage({ uid: 'cuj-dashboard-1' });
|
||||
await openScopesSelector(page, scopes);
|
||||
await selectScope(page, 'sn-redirect-fallback', scopes[1]);
|
||||
await applyScopes(page, [scopes[1]]);
|
||||
|
||||
// Verify the scope was applied
|
||||
await expect(page).toHaveURL(/scopes=scope-sn-redirect-fallback/);
|
||||
});
|
||||
|
||||
await test.step('Navigate to dashboard not in scope dashboard list', async () => {
|
||||
// Navigate to a dashboard that is not in the scope dashboard bindings
|
||||
// Preserve the scope parameter using queryParams
|
||||
await gotoDashboardPage({
|
||||
uid: 'cuj-dashboard-3',
|
||||
queryParams: new URLSearchParams({ scopes: 'scope-sn-redirect-fallback' }),
|
||||
});
|
||||
|
||||
// Verify we're on cuj-dashboard-3 with the scope still applied
|
||||
await expect(page).toHaveURL(/\/d\/cuj-dashboard-3/);
|
||||
await expect(page).toHaveURL(/scopes=scope-sn-redirect-fallback/);
|
||||
});
|
||||
|
||||
await test.step('Reload page and verify no redirect', async () => {
|
||||
// Reload the page with the scope still selected
|
||||
await page.reload();
|
||||
|
||||
// Wait for the page to load
|
||||
await page.waitForLoadState('networkidle');
|
||||
|
||||
// Should stay on the same dashboard (cuj-dashboard-3), not redirect
|
||||
await expect(page).toHaveURL(/\/d\/cuj-dashboard-3/);
|
||||
|
||||
// Verify the scope is still applied
|
||||
await expect(page).toHaveURL(/scopes=scope-sn-redirect-fallback/);
|
||||
});
|
||||
});
|
||||
|
||||
test('should not redirect when deselecting scopes on dashboard not in dashboard list', async ({
|
||||
page,
|
||||
gotoDashboardPage,
|
||||
}) => {
|
||||
const scopes = testScopesWithRedirect();
|
||||
|
||||
await test.step('Navigate to dashboard and select scope', async () => {
|
||||
await gotoDashboardPage({ uid: 'cuj-dashboard-1' });
|
||||
await openScopesSelector(page, scopes);
|
||||
await selectScope(page, 'sn-redirect-fallback', scopes[1]);
|
||||
await applyScopes(page, [scopes[1]]);
|
||||
|
||||
// Verify the scope was applied
|
||||
await expect(page).toHaveURL(/scopes=scope-sn-redirect-fallback/);
|
||||
});
|
||||
|
||||
await test.step('Navigate to dashboard not in scope dashboard list', async () => {
|
||||
// Navigate to a dashboard that is not in the scope dashboard bindings
|
||||
// Preserve the scope parameter using queryParams
|
||||
await gotoDashboardPage({
|
||||
uid: 'cuj-dashboard-3',
|
||||
queryParams: new URLSearchParams({ scopes: 'scope-sn-redirect-fallback' }),
|
||||
});
|
||||
|
||||
// Verify we're on cuj-dashboard-3 with the scope still applied
|
||||
await expect(page).toHaveURL(/\/d\/cuj-dashboard-3/);
|
||||
await expect(page).toHaveURL(/scopes=scope-sn-redirect-fallback/);
|
||||
});
|
||||
|
||||
await test.step('Deselect scopes and verify no redirect', async () => {
|
||||
// Click the clear button to remove all scopes
|
||||
await page.getByTestId('scopes-selector-input-clear').click();
|
||||
|
||||
// Should stay on the same dashboard (cuj-dashboard-3), not redirect
|
||||
await expect(page).toHaveURL(/\/d\/cuj-dashboard-3/);
|
||||
|
||||
// Verify the scope is no longer in the URL
|
||||
await expect(page).not.toHaveURL(/scopes=/);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -100,7 +100,7 @@ describe('ScopesService', () => {
|
||||
|
||||
service = new ScopesService(selectorService, dashboardsService, locationService);
|
||||
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], undefined, 'node1');
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], undefined, 'node1', false);
|
||||
});
|
||||
|
||||
it('should read scope_parent for backward compatibility', () => {
|
||||
@@ -111,7 +111,7 @@ describe('ScopesService', () => {
|
||||
|
||||
service = new ScopesService(selectorService, dashboardsService, locationService);
|
||||
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], 'parent1', undefined);
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], 'parent1', undefined, false);
|
||||
});
|
||||
|
||||
it('should prefer scope_node when both scope_node and scope_parent exist', () => {
|
||||
@@ -123,7 +123,7 @@ describe('ScopesService', () => {
|
||||
service = new ScopesService(selectorService, dashboardsService, locationService);
|
||||
|
||||
// Should call with parent1 as parentNodeId and node1 as scopeNodeId
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], 'parent1', 'node1');
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1'], 'parent1', 'node1', false);
|
||||
// Should preload node1 (not parent1)
|
||||
expect(selectorService.resolvePathToRoot).toHaveBeenCalledWith('node1', expect.anything());
|
||||
});
|
||||
@@ -158,7 +158,7 @@ describe('ScopesService', () => {
|
||||
|
||||
service = new ScopesService(selectorService, dashboardsService, locationService);
|
||||
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1', 'scope2'], undefined, 'node1');
|
||||
expect(selectorService.changeScopes).toHaveBeenCalledWith(['scope1', 'scope2'], undefined, 'node1', false);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -157,7 +157,8 @@ export class ScopesService implements ScopesContextValue {
|
||||
}
|
||||
|
||||
public changeScopes = (scopeNames: string[], parentNodeId?: string, scopeNodeId?: string) =>
|
||||
this.selectorService.changeScopes(scopeNames, parentNodeId, scopeNodeId);
|
||||
// Don't redirect on apply for initial load from URL. We only want to redirect when selecting from the selector
|
||||
this.selectorService.changeScopes(scopeNames, parentNodeId, scopeNodeId, false);
|
||||
|
||||
public setReadOnly = (readOnly: boolean) => {
|
||||
if (this.state.readOnly !== readOnly) {
|
||||
|
||||
@@ -341,14 +341,15 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
|
||||
return this.collapseNode(scopeNodeId);
|
||||
};
|
||||
|
||||
changeScopes = (scopeNames: string[], parentNodeId?: string, scopeNodeId?: string) => {
|
||||
changeScopes = (scopeNames: string[], parentNodeId?: string, scopeNodeId?: string, redirectOnApply?: boolean) => {
|
||||
return this.applyScopes(
|
||||
scopeNames.map((id, index) => ({
|
||||
scopeId: id,
|
||||
// Only the first scope gets the scopeNodeId
|
||||
scopeNodeId: index === 0 ? scopeNodeId : undefined,
|
||||
parentNodeId,
|
||||
}))
|
||||
})),
|
||||
redirectOnApply
|
||||
);
|
||||
};
|
||||
|
||||
@@ -356,7 +357,7 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
|
||||
* Apply the selected scopes. Apart from setting the scopes it also fetches the scope metadata and also loads the
|
||||
* related dashboards.
|
||||
*/
|
||||
private applyScopes = async (scopes: SelectedScope[]) => {
|
||||
private applyScopes = async (scopes: SelectedScope[], redirectOnApply = true) => {
|
||||
// Skip if we are trying to apply the same scopes as are already applied.
|
||||
if (
|
||||
this.state.appliedScopes.length === scopes.length &&
|
||||
@@ -372,7 +373,9 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
|
||||
// We call this even if we have 0 scope because in that case it also closes the dashboard drawer.
|
||||
this.dashboardsService.fetchDashboards(scopes.map((s) => s.scopeId)).then(() => {
|
||||
const selectedScopeNode = scopes[0]?.scopeNodeId ? this.state.nodes[scopes[0]?.scopeNodeId] : undefined;
|
||||
this.redirectAfterApply(selectedScopeNode);
|
||||
if (redirectOnApply) {
|
||||
this.redirectAfterApply(selectedScopeNode);
|
||||
}
|
||||
});
|
||||
|
||||
if (scopes.length > 0) {
|
||||
@@ -428,7 +431,7 @@ export class ScopesSelectorService extends ScopesServiceBase<ScopesSelectorServi
|
||||
}
|
||||
};
|
||||
|
||||
public removeAllScopes = () => this.applyScopes([]);
|
||||
public removeAllScopes = () => this.applyScopes([], false);
|
||||
|
||||
private addRecentScopes = (scopes: Scope[], parentNode?: ScopeNode) => {
|
||||
if (scopes.length === 0) {
|
||||
|
||||
@@ -262,7 +262,7 @@ describe('Dashboards list', () => {
|
||||
expectDashboardLength('billing-usage', 1);
|
||||
});
|
||||
|
||||
it('redirects to the first scope navigation if your current dashboard is not a scope navigation', async () => {
|
||||
it('does not redirect when scopes are set programmatically on dashboard not in scope navigation list', async () => {
|
||||
// Render another dashboard, which is not a scope navigation
|
||||
const mockNavigations: ScopeNavigation[] = [
|
||||
{
|
||||
@@ -284,10 +284,10 @@ describe('Dashboards list', () => {
|
||||
await renderDashboard();
|
||||
expect(locationService.getLocation().pathname).toBe('/');
|
||||
|
||||
// When scopes are set programmatically (not through UI), redirects should not happen
|
||||
await updateScopes(scopesService, ['grafana']);
|
||||
expect(locationService.getLocation().pathname).toBe('/d/dashboard1');
|
||||
// renderDashboard defaults to home dashboard
|
||||
expect(locationService.getLocation().pathname).not.toBe('/');
|
||||
// Should stay on the current page, not redirect
|
||||
expect(locationService.getLocation().pathname).toBe('/');
|
||||
expect(fetchDashboardsSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user