From c6435dc5c1203c62d6cf10ff2a293324b5d3ae7a Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Fri, 7 Mar 2025 12:28:20 +0300 Subject: [PATCH] Provisioning: Redirect to the new URL after save (#101757) Co-authored-by: Alex Khomenko --- .betterer.results | 7 +-- .../SaveProvisionedDashboard.test.tsx | 15 ++++-- .../provisioned/SaveProvisionedDashboard.tsx | 49 +++++++++++++------ .../dashboard-scene/scene/DashboardScene.tsx | 15 +----- 4 files changed, 51 insertions(+), 35 deletions(-) diff --git a/.betterer.results b/.betterer.results index eb66b7d6e92..214207392aa 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3280,7 +3280,7 @@ exports[`better eslint`] = { ], "public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboard.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"], + [0, 0, 0, "Do not use any type assertions.", "1"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "4"], @@ -3292,14 +3292,15 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "10"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "11"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "12"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "13"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "13"], [0, 0, 0, "No untranslated strings. Wrap text with ", "14"], [0, 0, 0, "No untranslated strings. Wrap text with ", "15"], [0, 0, 0, "No untranslated strings. Wrap text with ", "16"], [0, 0, 0, "No untranslated strings. Wrap text with ", "17"], [0, 0, 0, "No untranslated strings. Wrap text with ", "18"], [0, 0, 0, "No untranslated strings. Wrap text with ", "19"], - [0, 0, 0, "Unexpected any. Specify a different type.", "20"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "20"], + [0, 0, 0, "Unexpected any. Specify a different type.", "21"] ], "public/app/features/dashboard-scene/saving/shared.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], diff --git a/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboard.test.tsx b/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboard.test.tsx index 3308ac620d8..2e33b40ea11 100644 --- a/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboard.test.tsx +++ b/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboard.test.tsx @@ -4,7 +4,6 @@ import userEvent from '@testing-library/user-event'; import { AppEvents } from '@grafana/data'; import { getAppEvents, locationService } from '@grafana/runtime'; import { Dashboard } from '@grafana/schema'; -import { ManagerKind } from 'app/features/apiserver/types'; import { validationSrv } from 'app/features/manage-dashboards/services/ValidationSrv'; import { useCreateOrUpdateRepositoryFile } from 'app/features/provisioning/hooks'; @@ -152,6 +151,14 @@ function setup(props: Partial = {}) { }; } +const mockRequestBase = { + isSuccess: true, + isError: false, + isLoading: false, + error: null, + data: { resource: { upsert: {} } }, +}; + describe('SaveProvisionedDashboard', () => { beforeEach(() => { jest.clearAllMocks(); @@ -188,6 +195,7 @@ describe('SaveProvisionedDashboard', () => { // Mock useCreateOrUpdateRepositoryFile const mockAction = jest.fn(); const mockRequest = { + ...mockRequestBase, isSuccess: true, isError: false, isLoading: false, @@ -256,6 +264,7 @@ describe('SaveProvisionedDashboard', () => { const mockAction = jest.fn(); const mockRequest = { + ...mockRequestBase, isSuccess: true, isError: false, isLoading: false, @@ -297,8 +306,6 @@ describe('SaveProvisionedDashboard', () => { // Check if the action was called expect(mockAction).toHaveBeenCalled(); }); - // Check if manager was set - expect(props.dashboard.setManager).toHaveBeenCalledWith(ManagerKind.Repo, 'test-repo'); // Check if success alert was published const appEvents = getAppEvents(); @@ -362,6 +369,7 @@ describe('SaveProvisionedDashboard', () => { const mockAction = jest.fn(); const mockRequest = { + ...mockRequestBase, isSuccess: true, isError: false, isLoading: false, @@ -419,6 +427,7 @@ describe('SaveProvisionedDashboard', () => { const mockAction = jest.fn(); const mockRequest = { + ...mockRequestBase, isSuccess: false, isError: true, isLoading: false, diff --git a/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboard.tsx b/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboard.tsx index 2cdae08a8c5..63ac2aaf06d 100644 --- a/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboard.tsx +++ b/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboard.tsx @@ -2,12 +2,14 @@ import { useEffect } from 'react'; import { Controller, useForm } from 'react-hook-form'; import { useNavigate } from 'react-router-dom-v5-compat'; -import { AppEvents } from '@grafana/data'; +import { AppEvents, locationUtil } from '@grafana/data'; import { getAppEvents, locationService } from '@grafana/runtime'; +import { Dashboard } from '@grafana/schema/dist/esm/raw/dashboard/x/dashboard_types.gen'; import { Alert, Button, Field, Input, RadioButtonGroup, Stack, TextArea } from '@grafana/ui'; import { FolderPicker } from 'app/core/components/Select/FolderPicker'; import { useUrlParams } from 'app/core/navigation/hooks'; -import { AnnoKeyManagerIdentity, ManagerKind } from 'app/features/apiserver/types'; +import kbn from 'app/core/utils/kbn'; +import { AnnoKeyManagerIdentity, Resource } from 'app/features/apiserver/types'; import { validationSrv } from 'app/features/manage-dashboards/services/ValidationSrv'; import { RepositoryView } from 'app/features/provisioning/api'; import { PROVISIONING_URL } from 'app/features/provisioning/constants'; @@ -16,6 +18,7 @@ import { WorkflowOption } from 'app/features/provisioning/types'; import { validateBranchName } from 'app/features/provisioning/utils/git'; import { DashboardScene } from '../../scene/DashboardScene'; +import { getDashboardUrl } from '../../utils/getDashboardUrl'; import { SaveDashboardDrawer } from '../SaveDashboardDrawer'; import { SaveDashboardFormCommonOptions } from '../SaveDashboardForm'; import { DashboardChangeInfo } from '../shared'; @@ -72,23 +75,38 @@ export function SaveProvisionedDashboard({ drawer, changeInfo, dashboard }: Prop const appEvents = getAppEvents(); if (request.isSuccess) { dashboard.setState({ isDirty: false }); - if (isNew) { - dashboard.setManager(ManagerKind.Repo, defaultValues.repo); - } if (workflow === 'branch' && ref !== '' && path !== '') { // Redirect to the provisioning preview pages navigate(`${PROVISIONING_URL}/${defaultValues.repo}/dashboard/preview/${path}?ref=${ref}`); - } else { - appEvents.publish({ - type: AppEvents.alertSuccess.name, - payload: ['Dashboard changes saved'], - }); - dashboard.closeModal(); - locationService.partial({ - viewPanel: null, - editPanel: null, - }); + return; } + + appEvents.publish({ + type: AppEvents.alertSuccess.name, + payload: ['Dashboard changes saved'], + }); + + // Load the new URL + const upsert = request.data.resource.upsert as Resource; + if (isNew && upsert?.metadata?.name) { + const url = locationUtil.assureBaseUrl( + getDashboardUrl({ + uid: upsert.metadata.name, + slug: kbn.slugifyForUrl(upsert.spec.title ?? ''), + currentQueryParams: window.location.search, + }) + ); + + navigate(url); + return; + } + + // Keep the same URL + dashboard.closeModal(); + locationService.partial({ + viewPanel: null, + editPanel: null, + }); } else if (request.isError) { appEvents.publish({ type: AppEvents.alertError.name, @@ -99,6 +117,7 @@ export function SaveProvisionedDashboard({ drawer, changeInfo, dashboard }: Prop request.isSuccess, request.isError, request.error, + request.data, dashboard, workflow, isNew, diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.tsx index 8e30df9dde0..77bdc27bc9e 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardScene.tsx @@ -42,7 +42,7 @@ import { VariablesChanged } from 'app/features/variables/types'; import { DashboardDTO, DashboardMeta, KioskMode, SaveDashboardResponseDTO } from 'app/types'; import { ShowConfirmModalEvent } from 'app/types/events'; -import { AnnoKeyManagerIdentity, AnnoKeyManagerKind, AnnoKeySourcePath, ManagerKind } from '../../apiserver/types'; +import { AnnoKeyManagerKind, AnnoKeySourcePath, ManagerKind } from '../../apiserver/types'; import { DashboardEditPane } from '../edit-pane/DashboardEditPane'; import { PanelEditor } from '../panel-edit/PanelEditor'; import { DashboardSceneChangeTracker } from '../saving/DashboardSceneChangeTracker'; @@ -777,19 +777,6 @@ export class DashboardScene extends SceneObjectBase impleme getPath() { return this.state.meta.k8s?.annotations?.[AnnoKeySourcePath]; } - - setManager(kind: ManagerKind, id: string) { - this.setState({ - meta: { - k8s: { - annotations: { - [AnnoKeyManagerKind]: kind, - [AnnoKeyManagerIdentity]: id, - }, - }, - }, - }); - } } export class DashboardVariableDependency implements SceneVariableDependencyConfigLike {