From 31a2d2aff41c8884882a0e5409706b8cdbf7b298 Mon Sep 17 00:00:00 2001 From: Alex Khomenko Date: Thu, 30 Oct 2025 09:20:41 +0200 Subject: [PATCH] Provisioning: Show last and preview branches in the branch dropdown (#113148) * Provisioning: Show configured and last used branches * Remove unused var * Add hooks * Extract branch logic * remove type assertion * fix tests * Memoize descriptions --- eslint-suppressions.json | 5 -- .../provisioning/File/FileStatusPage.tsx | 22 +++-- .../DeleteProvisionedDashboardForm.tsx | 2 + .../MoveProvisionedDashboardForm.tsx | 2 + .../SaveProvisionedDashboardForm.test.tsx | 12 +++ .../SaveProvisionedDashboardForm.tsx | 11 +-- .../Folders/DeleteProvisionedFolderForm.tsx | 2 + .../Folders/NewProvisionedFolderForm.test.tsx | 12 +++ .../Folders/NewProvisionedFolderForm.tsx | 1 + .../ResourceEditFormSharedFields.test.tsx | 12 +++ .../Shared/ResourceEditFormSharedFields.tsx | 39 +++------ .../hooks/useBranchDropdownOptions.ts | 84 +++++++++++++++++++ .../provisioning/hooks/useLastBranch.ts | 35 ++++++++ .../provisioning/hooks/usePRBranch.ts | 13 +++ .../hooks/useProvisionedRequestHandler.ts | 27 +++++- public/locales/en-US/grafana.json | 4 +- 16 files changed, 232 insertions(+), 51 deletions(-) create mode 100644 public/app/features/provisioning/hooks/useBranchDropdownOptions.ts create mode 100644 public/app/features/provisioning/hooks/useLastBranch.ts create mode 100644 public/app/features/provisioning/hooks/usePRBranch.ts diff --git a/eslint-suppressions.json b/eslint-suppressions.json index c704beaefc3..2d84e04dadf 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -3374,11 +3374,6 @@ "count": 2 } }, - "public/app/features/provisioning/File/FileStatusPage.tsx": { - "@typescript-eslint/consistent-type-assertions": { - "count": 2 - } - }, "public/app/features/provisioning/Shared/BranchValidationError.tsx": { "react/no-unescaped-entities": { "count": 26 diff --git a/public/app/features/provisioning/File/FileStatusPage.tsx b/public/app/features/provisioning/File/FileStatusPage.tsx index 5a946122e11..9b32521a5fb 100644 --- a/public/app/features/provisioning/File/FileStatusPage.tsx +++ b/public/app/features/provisioning/File/FileStatusPage.tsx @@ -18,12 +18,24 @@ import { useQueryParams } from 'app/core/hooks/useQueryParams'; import { PROVISIONING_URL } from '../constants'; import { useGetResourceRepositoryView } from '../hooks/useGetResourceRepositoryView'; +import { usePRBranch } from '../hooks/usePRBranch'; + +enum TabSelection { + File = 'file', + Existing = 'existing', + DryRun = 'dryRun', +} + +function isTabSelection(value: unknown): value is TabSelection { + return value === TabSelection.File || value === TabSelection.Existing || value === TabSelection.DryRun; +} export default function FileStatusPage() { const params = useParams(); const [queryParams] = useQueryParams(); - const ref = (queryParams['ref'] as string) ?? undefined; - const tab = (queryParams['tab'] as TabSelection) ?? TabSelection.File; + const ref = usePRBranch(); + const tabParam = queryParams['tab']; + const tab = isTabSelection(tabParam) ? tabParam : TabSelection.File; const name = params['name'] ?? ''; const path = params['*'] ?? ''; const file = useGetRepositoryFilesWithPathQuery({ name, path, ref }); @@ -52,12 +64,6 @@ export default function FileStatusPage() { ); } -enum TabSelection { - File = 'file', - Existing = 'existing', - DryRun = 'dryRun', -} - interface Props { wrap: ResourceWrapper; repo: string; diff --git a/public/app/features/provisioning/components/Dashboards/DeleteProvisionedDashboardForm.tsx b/public/app/features/provisioning/components/Dashboards/DeleteProvisionedDashboardForm.tsx index 743975947a0..87948f5f555 100644 --- a/public/app/features/provisioning/components/Dashboards/DeleteProvisionedDashboardForm.tsx +++ b/public/app/features/provisioning/components/Dashboards/DeleteProvisionedDashboardForm.tsx @@ -166,6 +166,8 @@ export function DeleteProvisionedDashboardForm({ request, workflow, resourceType: 'dashboard', + repository, + selectedBranch: ref || loadedFromRef, successMessage: t( 'dashboard-scene.delete-provisioned-dashboard-form.success-message', 'Dashboard deleted successfully' diff --git a/public/app/features/provisioning/components/Dashboards/MoveProvisionedDashboardForm.tsx b/public/app/features/provisioning/components/Dashboards/MoveProvisionedDashboardForm.tsx index 637d0861882..abdd4a74eae 100644 --- a/public/app/features/provisioning/components/Dashboards/MoveProvisionedDashboardForm.tsx +++ b/public/app/features/provisioning/components/Dashboards/MoveProvisionedDashboardForm.tsx @@ -225,6 +225,8 @@ export function MoveProvisionedDashboardForm({ request: moveRequest, workflow, resourceType: 'dashboard', + repository, + selectedBranch: ref || loadedFromRef, successMessage: t( 'dashboard-scene.move-provisioned-dashboard-form.success-message', 'Dashboard moved successfully' diff --git a/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.test.tsx b/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.test.tsx index 0f3029c51df..f74f7ddd949 100644 --- a/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.test.tsx +++ b/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.test.tsx @@ -77,6 +77,18 @@ jest.mock('app/api/clients/provisioning/v0alpha1', () => ({ useGetRepositoryRefsQuery: jest.fn().mockReturnValue({ data: { items: [] }, isLoading: false, error: null }), })); +// Mock the new hooks that depend on router context +jest.mock('../../hooks/usePRBranch', () => ({ + usePRBranch: jest.fn().mockReturnValue(undefined), +})); + +jest.mock('../../hooks/useLastBranch', () => ({ + useLastBranch: jest.fn().mockReturnValue({ + getLastBranch: jest.fn().mockReturnValue(undefined), + setLastBranch: jest.fn(), + }), +})); + jest.mock('app/features/dashboard-scene/saving/SaveDashboardForm', () => { const actual = jest.requireActual('app/features/dashboard-scene/saving/SaveDashboardForm'); return { diff --git a/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.tsx b/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.tsx index bffaab79a73..a807ada4911 100644 --- a/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.tsx +++ b/public/app/features/provisioning/components/Dashboards/SaveProvisionedDashboardForm.tsx @@ -148,6 +148,7 @@ export function SaveProvisionedDashboardForm({ workflow, resourceType: 'dashboard', repository, + selectedBranch: methods.getValues().ref, handlers: { onBranchSuccess: ({ ref, path }, info, resource) => onBranchSuccess(ref, path, info, resource), onWriteSuccess, @@ -156,15 +157,7 @@ export function SaveProvisionedDashboardForm({ }); // Submit handler for saving the form data - const handleFormSubmit = async ({ - title, - description, - repo, - path, - comment, - ref, - folder, - }: ProvisionedDashboardFormData) => { + const handleFormSubmit = async ({ title, description, repo, path, comment, ref }: ProvisionedDashboardFormData) => { // Validate required fields if (!repo || !path) { console.error('Missing required fields for saving:', { repo, path }); diff --git a/public/app/features/provisioning/components/Folders/DeleteProvisionedFolderForm.tsx b/public/app/features/provisioning/components/Folders/DeleteProvisionedFolderForm.tsx index 102cacbf7d4..6682d21af88 100644 --- a/public/app/features/provisioning/components/Folders/DeleteProvisionedFolderForm.tsx +++ b/public/app/features/provisioning/components/Folders/DeleteProvisionedFolderForm.tsx @@ -143,6 +143,8 @@ function FormContent({ initialValues, parentFolder, repository, workflowOptions, request, workflow, resourceType: 'folder', + repository, + selectedBranch: ref, successMessage: t( 'browse-dashboards.delete-provisioned-folder-form.success-message', 'Folder deleted successfully' diff --git a/public/app/features/provisioning/components/Folders/NewProvisionedFolderForm.test.tsx b/public/app/features/provisioning/components/Folders/NewProvisionedFolderForm.test.tsx index 0bd80b282a9..1aa011c708c 100644 --- a/public/app/features/provisioning/components/Folders/NewProvisionedFolderForm.test.tsx +++ b/public/app/features/provisioning/components/Folders/NewProvisionedFolderForm.test.tsx @@ -51,6 +51,18 @@ jest.mock('app/api/clients/provisioning/v0alpha1', () => { }; }); +// Mock the new hooks that depend on router context +jest.mock('../../hooks/usePRBranch', () => ({ + usePRBranch: jest.fn().mockReturnValue(undefined), +})); + +jest.mock('../../hooks/useLastBranch', () => ({ + useLastBranch: jest.fn().mockReturnValue({ + getLastBranch: jest.fn().mockReturnValue(undefined), + setLastBranch: jest.fn(), + }), +})); + jest.mock('../../hooks/useProvisionedFolderFormData', () => { return { useProvisionedFolderFormData: jest.fn(), diff --git a/public/app/features/provisioning/components/Folders/NewProvisionedFolderForm.tsx b/public/app/features/provisioning/components/Folders/NewProvisionedFolderForm.tsx index 109277d38a7..367bcd25860 100644 --- a/public/app/features/provisioning/components/Folders/NewProvisionedFolderForm.tsx +++ b/public/app/features/provisioning/components/Folders/NewProvisionedFolderForm.tsx @@ -89,6 +89,7 @@ function FormContent({ initialValues, repository, workflowOptions, folder, onDis workflow, repository, resourceType: 'folder', + selectedBranch: methods.getValues().ref, handlers: { onDismiss, onBranchSuccess, diff --git a/public/app/features/provisioning/components/Shared/ResourceEditFormSharedFields.test.tsx b/public/app/features/provisioning/components/Shared/ResourceEditFormSharedFields.test.tsx index 306571926c1..0df03fab5a4 100644 --- a/public/app/features/provisioning/components/Shared/ResourceEditFormSharedFields.test.tsx +++ b/public/app/features/provisioning/components/Shared/ResourceEditFormSharedFields.test.tsx @@ -13,6 +13,18 @@ jest.mock('app/api/clients/provisioning/v0alpha1', () => ({ useGetRepositoryRefsQuery: jest.fn().mockReturnValue({ data: { items: [] }, isLoading: false, error: null }), })); +// Mock the new hooks that depend on router context +jest.mock('../../hooks/usePRBranch', () => ({ + usePRBranch: jest.fn().mockReturnValue(undefined), +})); + +jest.mock('../../hooks/useLastBranch', () => ({ + useLastBranch: jest.fn().mockReturnValue({ + getLastBranch: jest.fn().mockReturnValue(undefined), + setLastBranch: jest.fn(), + }), +})); + const mockRepo: { github: RepositoryView; local: RepositoryView } = { github: { type: 'github', diff --git a/public/app/features/provisioning/components/Shared/ResourceEditFormSharedFields.tsx b/public/app/features/provisioning/components/Shared/ResourceEditFormSharedFields.tsx index da05af72fa2..ecad626dbba 100644 --- a/public/app/features/provisioning/components/Shared/ResourceEditFormSharedFields.tsx +++ b/public/app/features/provisioning/components/Shared/ResourceEditFormSharedFields.tsx @@ -10,6 +10,9 @@ import { WorkflowOption } from 'app/features/provisioning/types'; import { validateBranchName } from 'app/features/provisioning/utils/git'; import { isGitProvider } from 'app/features/provisioning/utils/repositoryTypes'; +import { useBranchDropdownOptions } from '../../hooks/useBranchDropdownOptions'; +import { useLastBranch } from '../../hooks/useLastBranch'; +import { usePRBranch } from '../../hooks/usePRBranch'; import { generateNewBranchName } from '../utils/newBranchName'; interface DashboardEditFormSharedFieldsProps { @@ -40,34 +43,16 @@ export const ResourceEditFormSharedFields = memo { - const options: Array<{ label: string; value: string; description?: string }> = []; + const { getLastBranch } = useLastBranch(); + const prBranch = usePRBranch(); + const lastBranch = getLastBranch(repository?.name); - const configuredBranch = repository?.branch; - const prefix = t( - 'provisioned-resource-form.save-or-delete-resource-shared-fields.suffix-configured-branch', - 'Configured branch' - ); - // Show the configured branch first in the list - if (configuredBranch) { - options.push({ - label: `${configuredBranch}`, - value: configuredBranch, - description: prefix, - }); - } - - // Create combobox options - if (branchData?.items) { - for (const ref of branchData.items) { - if (ref.name !== configuredBranch) { - options.push({ label: ref.name, value: ref.name }); - } - } - } - - return options; - }, [branchData?.items, repository?.branch]); + const branchOptions = useBranchDropdownOptions({ + repository, + prBranch, + lastBranch, + branchData, + }); const newBranchDefaultName = useMemo(() => generateNewBranchName(resourceType), [resourceType]); diff --git a/public/app/features/provisioning/hooks/useBranchDropdownOptions.ts b/public/app/features/provisioning/hooks/useBranchDropdownOptions.ts new file mode 100644 index 00000000000..4302cabf92b --- /dev/null +++ b/public/app/features/provisioning/hooks/useBranchDropdownOptions.ts @@ -0,0 +1,84 @@ +import { useMemo } from 'react'; + +import { t } from '@grafana/i18n'; +import { GetRepositoryRefsApiResponse, RepositoryView } from 'app/api/clients/provisioning/v0alpha1'; + +interface UseBranchDropdownOptionsParams { + repository?: RepositoryView; + prBranch?: string; + lastBranch?: string; + branchData?: GetRepositoryRefsApiResponse; +} + +interface BranchOption { + label: string; + value: string; + description?: string; +} + +function getBranchDescriptions() { + return { + configured: t( + 'provisioned-resource-form.save-or-delete-resource-shared-fields.suffix-configured-branch', + 'Configured branch' + ), + pr: t('provisioned-resource-form.save-or-delete-resource-shared-fields.suffix-pr-branch', 'Pull request branch'), + lastUsed: t('provisioned-resource-form.save-or-delete-resource-shared-fields.suffix-last-used', 'Last branch'), + }; +} + +/** + * Hook to generate branch dropdown options with proper ordering and deduplication. + * Order: Configured branch → PR branch → Last used branch → Other branches + */ +export const useBranchDropdownOptions = ({ + repository, + prBranch, + lastBranch, + branchData, +}: UseBranchDropdownOptionsParams): BranchOption[] => { + const descriptions = useMemo(() => getBranchDescriptions(), []); + + const options: BranchOption[] = []; + const addedBranches = new Set(); + + const configuredBranch = repository?.branch; + + if (configuredBranch) { + options.push({ + label: `${configuredBranch}`, + value: configuredBranch, + description: descriptions.configured, + }); + addedBranches.add(configuredBranch); + } + + if (prBranch && !addedBranches.has(prBranch)) { + options.push({ + label: prBranch, + value: prBranch, + description: descriptions.pr, + }); + addedBranches.add(prBranch); + } + + if (lastBranch && !addedBranches.has(lastBranch)) { + options.push({ + label: lastBranch, + value: lastBranch, + description: descriptions.lastUsed, + }); + addedBranches.add(lastBranch); + } + + if (branchData?.items) { + for (const ref of branchData.items) { + if (!addedBranches.has(ref.name)) { + options.push({ label: ref.name, value: ref.name }); + addedBranches.add(ref.name); + } + } + } + + return options; +}; diff --git a/public/app/features/provisioning/hooks/useLastBranch.ts b/public/app/features/provisioning/hooks/useLastBranch.ts new file mode 100644 index 00000000000..ca977362cc4 --- /dev/null +++ b/public/app/features/provisioning/hooks/useLastBranch.ts @@ -0,0 +1,35 @@ +import { useCallback } from 'react'; + +import { store } from '@grafana/data'; + +const LAST_BRANCH_KEY_PREFIX = 'grafana.provisioning.lastBranch'; + +/** + * Get the local storage key for a repository's last used branch + */ +const getStorageKey = (repositoryName: string) => { + return `${LAST_BRANCH_KEY_PREFIX}.${repositoryName}`; +}; + +/** + * Hook to manage the last used branch per repository in local storage + */ +export const useLastBranch = () => { + const getLastBranch = useCallback((repositoryName: string | undefined): string | undefined => { + if (!repositoryName) { + return undefined; + } + const key = getStorageKey(repositoryName); + return store.get(key) || undefined; + }, []); + + const setLastBranch = useCallback((repositoryName: string | undefined, branch: string | undefined) => { + if (!repositoryName || !branch) { + return; + } + const key = getStorageKey(repositoryName); + store.set(key, branch); + }, []); + + return { getLastBranch, setLastBranch }; +}; diff --git a/public/app/features/provisioning/hooks/usePRBranch.ts b/public/app/features/provisioning/hooks/usePRBranch.ts new file mode 100644 index 00000000000..ba31e94f132 --- /dev/null +++ b/public/app/features/provisioning/hooks/usePRBranch.ts @@ -0,0 +1,13 @@ +import { useQueryParams } from 'app/core/hooks/useQueryParams'; + +/** + * Hook to get a properly typed URL ref param + */ +export const usePRBranch = () => { + const [queryParams] = useQueryParams(); + const ref = queryParams['ref']; + if (typeof ref !== 'string') { + return undefined; + } + return ref; +}; diff --git a/public/app/features/provisioning/hooks/useProvisionedRequestHandler.ts b/public/app/features/provisioning/hooks/useProvisionedRequestHandler.ts index 188f6218950..528e3ee49be 100644 --- a/public/app/features/provisioning/hooks/useProvisionedRequestHandler.ts +++ b/public/app/features/provisioning/hooks/useProvisionedRequestHandler.ts @@ -14,6 +14,8 @@ import { refetchChildren } from 'app/features/browse-dashboards/state/actions'; import { RepoType } from 'app/features/provisioning/Wizard/types'; import { useDispatch } from 'app/types/store'; +import { useLastBranch } from './useLastBranch'; + type ResourceType = 'dashboard' | 'folder'; // Add more as needed, e.g., 'alert', etc. // Information object that gets passed to all handlers @@ -56,6 +58,7 @@ interface Props { successMessage?: string; repository?: RepositoryView; resourceType?: ResourceType; + selectedBranch?: string; // The branch selected by the user in the form } /** @@ -72,10 +75,12 @@ export function useProvisionedRequestHandler({ successMessage, repository, resourceType, + selectedBranch, }: Props) { const dispatch = useDispatch(); // useRef to ensure handlers are only called once per request const hasHandled = useRef(false); + const { setLastBranch } = useLastBranch(); useEffect(() => { const repoType = repository?.type || 'git'; @@ -97,6 +102,15 @@ export function useProvisionedRequestHandler({ // eslint-disable-next-line @typescript-eslint/consistent-type-assertions const resourceData = resource.upsert as Resource; + // Save the last used branch to local storage + if (workflow === 'branch' && ref) { + // For branch workflow, save the ref from the response + setLastBranch(repository?.name, ref); + } else if (workflow === 'write') { + // For write workflow, save the selectedBranch or fall back to repository branch + setLastBranch(repository?.name, selectedBranch || repository?.branch); + } + // Success message const message = successMessage || getContextualSuccessMessage(info); getAppEvents().publish({ @@ -121,7 +135,18 @@ export function useProvisionedRequestHandler({ handlers.onDismiss?.(); } - }, [request, workflow, handlers, successMessage, repository, resourceType, folderUID, dispatch]); + }, [ + request, + workflow, + handlers, + successMessage, + repository, + resourceType, + folderUID, + dispatch, + selectedBranch, + setLastBranch, + ]); } function getContextualSuccessMessage(info: ProvisionedOperationInfo): string { diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 4e435b87ed2..62cf6c11a10 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -11364,7 +11364,9 @@ "label-workflow": "Workflow", "placeholder-branch": "Select or enter branch name", "placeholder-new-branch": "Enter new branch name", - "suffix-configured-branch": "Configured branch" + "suffix-configured-branch": "Configured branch", + "suffix-last-used": "Last branch", + "suffix-pr-branch": "Pull request branch" } }, "provisioned-resource-preview-banner": {