From cbb828202ad34b0bfb3ae6ebd39afdfad54bba69 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Tue, 10 Jun 2025 16:01:48 +0200 Subject: [PATCH] FolderPicker: Remove old folder picker (#105374) * Remove newFolderPicker toggle * Remove usage of the old folder picker * fix some tests * remove old test --- .../src/types/featureToggles.gen.ts | 4 - pkg/services/featuremgmt/registry.go | 18 +-- pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 - pkg/services/featuremgmt/toggles_gen.json | 3 +- .../core/components/Select/FolderPicker.tsx | 71 +---------- .../BrowseActions/MoveModal.test.tsx | 114 ++++++++++++------ .../saving/SaveDashboardAsForm.tsx | 6 +- .../SaveProvisionedDashboardForm.tsx | 1 - .../settings/GeneralSettingsEditView.tsx | 10 +- .../DashboardSettings/GeneralSettings.tsx | 11 +- .../forms/SaveDashboardAsForm.test.tsx | 2 +- .../forms/SaveDashboardAsForm.tsx | 4 - .../AddLibraryPanelModal.tsx | 6 +- 14 files changed, 94 insertions(+), 161 deletions(-) diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index d33a2277c20..5b3213e4932 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -456,10 +456,6 @@ export interface FeatureToggles { */ alertingQueryOptimization?: boolean; /** - * Enables the nested folder picker without having nested folders enabled - */ - newFolderPicker?: boolean; - /** * Distributes alert rule evaluations more evenly over time, including spreading out rules within the same group. Disables sequential evaluation if enabled. */ jitterAlertRulesWithinGroups?: boolean; diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 1fd0e86b835..b79febda1c3 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1,10 +1,9 @@ -// To change feature flags, edit: -// pkg/services/featuremgmt/registry.go -// Then run tests in: -// pkg/services/featuremgmt/toggles_gen_test.go -// twice to generate and validate the feature flag files +// To change feature flags edit this file and run: +// make gen-feature-toggles // -// Alternatively, use `make gen-feature-toggles` +// Alternatively run tests in: +// pkg/services/featuremgmt/toggles_gen_test.go +// twice to generate and validate the feature flag files package featuremgmt @@ -766,13 +765,6 @@ var ( AllowSelfServe: false, Expression: "false", }, - { - Name: "newFolderPicker", - Description: "Enables the nested folder picker without having nested folders enabled", - Stage: FeatureStageExperimental, - Owner: grafanaFrontendPlatformSquad, - FrontendOnly: true, - }, { Name: "jitterAlertRulesWithinGroups", Description: "Distributes alert rule evaluations more evenly over time, including spreading out rules within the same group. Disables sequential evaluation if enabled.", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index da0fd2a226c..5d5375d1532 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -100,7 +100,6 @@ lokiQueryHints,GA,@grafana/observability-logs,false,false,true kubernetesFeatureToggles,experimental,@grafana/grafana-operator-experience-squad,false,false,true cloudRBACRoles,preview,@grafana/identity-access-team,false,true,false alertingQueryOptimization,GA,@grafana/alerting-squad,false,false,false -newFolderPicker,experimental,@grafana/grafana-frontend-platform,false,false,true jitterAlertRulesWithinGroups,preview,@grafana/alerting-squad,false,true,false onPremToCloudMigrations,GA,@grafana/grafana-operator-experience-squad,false,false,false secretsManagementAppPlatform,experimental,@grafana/grafana-operator-experience-squad,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 210d4c99888..37e9cd6fa94 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -411,10 +411,6 @@ const ( // Optimizes eligible queries in order to reduce load on datasources FlagAlertingQueryOptimization = "alertingQueryOptimization" - // FlagNewFolderPicker - // Enables the nested folder picker without having nested folders enabled - FlagNewFolderPicker = "newFolderPicker" - // FlagJitterAlertRulesWithinGroups // Distributes alert rule evaluations more evenly over time, including spreading out rules within the same group. Disables sequential evaluation if enabled. FlagJitterAlertRulesWithinGroups = "jitterAlertRulesWithinGroups" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index aeee92b6b67..1b38f0befe7 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -2287,7 +2287,8 @@ "metadata": { "name": "newFolderPicker", "resourceVersion": "1743693517832", - "creationTimestamp": "2024-01-15T11:43:19Z" + "creationTimestamp": "2024-01-15T11:43:19Z", + "deletionTimestamp": "2025-05-14T09:11:28Z" }, "spec": { "description": "Enables the nested folder picker without having nested folders enabled", diff --git a/public/app/core/components/Select/FolderPicker.tsx b/public/app/core/components/Select/FolderPicker.tsx index e445d150615..d8390db373f 100644 --- a/public/app/core/components/Select/FolderPicker.tsx +++ b/public/app/core/components/Select/FolderPicker.tsx @@ -1,70 +1,3 @@ -import { useCallback } from 'react'; +import { NestedFolderPicker } from '../NestedFolderPicker/NestedFolderPicker'; -import { config } from '@grafana/runtime'; - -import { NestedFolderPicker, NestedFolderPickerProps } from '../NestedFolderPicker/NestedFolderPicker'; - -import { OldFolderPicker } from './OldFolderPicker'; - -interface FolderPickerProps extends NestedFolderPickerProps { - // These props are only used by the old folder picker, and should be removed when old picker is removed - - /** @deprecated */ - initialTitle?: string; - - /** @deprecated */ - inputId?: string; - - /** @deprecated */ - dashboardId?: number | string; - - /** @deprecated */ - enableCreateNew?: boolean; - - /** @deprecated */ - skipInitialLoad?: boolean; -} - -// Temporary wrapper component to switch between the NestedFolderPicker and the old flat -// FolderPicker depending on feature flags -export function FolderPicker(props: FolderPickerProps) { - const nestedEnabled = config.featureToggles.newFolderPicker || config.featureToggles.nestedFolders; - const { initialTitle, dashboardId, enableCreateNew, ...newFolderPickerProps } = props; - - return nestedEnabled ? : ; -} - -// Converts new NestedFolderPicker props to old non-nested folder picker props -// Seperate component so the hooks aren't created if not used -function OldFolderPickerWrapper({ - value, - showRootFolder, - onChange, - initialTitle, - dashboardId, - enableCreateNew, - inputId, - skipInitialLoad, -}: FolderPickerProps) { - const handleOnChange = useCallback( - (newFolder: { title: string; uid: string }) => { - if (onChange) { - onChange(newFolder.uid, newFolder.title); - } - }, - [onChange] - ); - - return ( - - ); -} +export const FolderPicker = NestedFolderPicker; diff --git a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx index d6a9172a347..fb1a160d8b1 100644 --- a/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx +++ b/public/app/features/browse-dashboards/components/BrowseActions/MoveModal.test.tsx @@ -1,35 +1,79 @@ -import { render as rtlRender, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { TestProvider } from 'test/helpers/TestProvider'; -import { selectOptionInTest } from 'test/helpers/selectOptionInTest'; +import { HttpResponse, http } from 'msw'; +import { SetupServer, setupServer } from 'msw/node'; +import { render, screen } from 'test/test-utils'; -import { selectors } from '@grafana/e2e-selectors'; -import { setBackendSrv } from '@grafana/runtime'; -import { backendSrv } from 'app/core/services/__mocks__/backend_srv'; -import * as api from 'app/features/manage-dashboards/state/actions'; -import { DashboardSearchHit } from 'app/features/search/types'; +import { backendSrv } from 'app/core/services/backend_srv'; + +import { treeViewersCanEdit, wellFormedTree } from '../../fixtures/dashboardsTreeItem.fixture'; import { MoveModal, Props } from './MoveModal'; -function render(...[ui, options]: Parameters) { - rtlRender({ui}, options); -} +const [mockTree, { folderA }] = wellFormedTree(); +const [mockTreeThatViewersCanEdit /* shares folders with wellFormedTree */] = treeViewersCanEdit(); + +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + getBackendSrv: () => backendSrv, +})); describe('browse-dashboards MoveModal', () => { const mockOnDismiss = jest.fn(); const mockOnConfirm = jest.fn(); - const mockFolders = [ - { title: 'Dashboards', uid: '' } as DashboardSearchHit, - { title: 'Folder 1', uid: 'wfTJJL5Wz' } as DashboardSearchHit, - ]; let props: Props; + let server: SetupServer; + window.HTMLElement.prototype.scrollIntoView = () => {}; beforeAll(() => { - setBackendSrv(backendSrv); - jest.spyOn(backendSrv, 'get').mockResolvedValue({ - dashboard: 0, - folder: 0, - }); + server = setupServer( + http.get('/api/folders/:uid', () => { + return HttpResponse.json({ + title: folderA.item.title, + uid: folderA.item.uid, + }); + }), + + http.get('/api/folders/:uid/counts', () => { + return HttpResponse.json({ + folder: 1, + dashboard: 2, + librarypanel: 3, + alertrule: 4, + }); + }), + + http.get('/apis/provisioning.grafana.app/v0alpha1/namespaces/default/settings', () => { + return HttpResponse.json({ + items: [], + }); + }), + + http.get('/api/folders', ({ request }) => { + const url = new URL(request.url); + const parentUid = url.searchParams.get('parentUid') ?? undefined; + const permission = url.searchParams.get('permission'); + + const limit = parseInt(url.searchParams.get('limit') ?? '1000', 10); + const page = parseInt(url.searchParams.get('page') ?? '1', 10); + + const tree = permission === 'Edit' ? mockTreeThatViewersCanEdit : mockTree; + + // reconstruct a folder API response from the flat tree fixture + const folders = tree + .filter((v) => v.item.kind === 'folder' && v.item.parentUID === parentUid) + .map((folder) => { + return { + uid: folder.item.uid, + title: folder.item.kind === 'folder' ? folder.item.title : "invalid - this shouldn't happen", + }; + }) + .slice(limit * (page - 1), limit * page); + + return HttpResponse.json(folders); + }) + ); + + server.listen(); }); beforeEach(() => { @@ -44,9 +88,10 @@ describe('browse-dashboards MoveModal', () => { panel: {}, }, }; + }); - // mock the searchFolders api call so the folder picker has some folders in it - jest.spyOn(api, 'searchFolders').mockResolvedValue(mockFolders); + afterAll(() => { + server.close(); }); it('renders a dialog with the correct title', async () => { @@ -70,7 +115,7 @@ describe('browse-dashboards MoveModal', () => { it('displays a folder picker', async () => { render(); - expect(await screen.findByTestId(selectors.components.FolderPicker.input)).toBeInTheDocument(); + expect(await screen.findByRole('button', { name: 'Select folder' })).toBeInTheDocument(); }); it('displays a warning about permissions if a folder is selected', async () => { @@ -84,23 +129,24 @@ describe('browse-dashboards MoveModal', () => { ).toBeInTheDocument(); }); - it('only enables the `Move` button if a folder is selected', async () => { + it('enables the `Move` button once a folder is selected', async () => { render(); expect(await screen.findByRole('button', { name: 'Move' })).toBeDisabled(); - const folderPicker = await screen.findByTestId(selectors.components.FolderPicker.input); - await selectOptionInTest(folderPicker, mockFolders[1].title); - expect(await screen.findByRole('button', { name: 'Move' })).toBeEnabled(); - }); + // Open the picker and wait for children to load + const folderPicker = await screen.findByRole('button', { name: 'Select folder' }); + await userEvent.click(folderPicker); + await screen.findByLabelText(folderA.item.title); - it('calls onConfirm when clicking the `Move` button', async () => { - render(); - const folderPicker = await screen.findByTestId(selectors.components.FolderPicker.input); + // Select the folder + await userEvent.click(screen.getByLabelText(folderA.item.title)); + + const moveButton = await screen.findByRole('button', { name: 'Move' }); + expect(moveButton).toBeEnabled(); - await selectOptionInTest(folderPicker, mockFolders[1].title); - await userEvent.click(await screen.findByRole('button', { name: 'Move' })); - expect(mockOnConfirm).toHaveBeenCalledWith(mockFolders[1].uid); + await userEvent.click(moveButton); + expect(mockOnConfirm).toHaveBeenCalledWith(folderA.item.uid); }); it('calls onDismiss when clicking the `Cancel` button', async () => { diff --git a/public/app/features/dashboard-scene/saving/SaveDashboardAsForm.tsx b/public/app/features/dashboard-scene/saving/SaveDashboardAsForm.tsx index 152fcea1298..00df6d7aee0 100644 --- a/public/app/features/dashboard-scene/saving/SaveDashboardAsForm.tsx +++ b/public/app/features/dashboard-scene/saving/SaveDashboardAsForm.tsx @@ -43,7 +43,7 @@ export function SaveDashboardAsForm({ dashboard, changeInfo }: Props) { }, }); - const { errors, isValid, defaultValues } = formState; + const { errors, isValid } = formState; const formValues = watch(); const { state, onSaveDashboard } = useSaveDashboard(false); @@ -158,11 +158,7 @@ export function SaveDashboardAsForm({ dashboard, changeInfo }: Props) { }, }); }} - // Old folder picker fields value={formValues.folder?.uid} - initialTitle={defaultValues!.folder!.title} - dashboardId={dashboard.state.id ?? undefined} - enableCreateNew /> {!changeInfo.isNew && ( diff --git a/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboardForm.tsx b/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboardForm.tsx index a177964f7e2..341b3c517d9 100644 --- a/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboardForm.tsx +++ b/public/app/features/dashboard-scene/saving/provisioned/SaveProvisionedDashboardForm.tsx @@ -214,7 +214,6 @@ export function SaveProvisionedDashboardForm({ render={({ field: { ref, value, onChange, ...field } }) => { return ( { onChange({ uid, title }); // Update folderUid URL param diff --git a/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx b/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx index 17fe8d97f3f..0315e5e875e 100644 --- a/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx +++ b/public/app/features/dashboard-scene/settings/GeneralSettingsEditView.tsx @@ -227,15 +227,7 @@ export class GeneralSettingsEditView {dashboard.isManagedRepository() ? ( ) : ( - + )} diff --git a/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx index fbbbfb768de..1a008f650d1 100644 --- a/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx +++ b/public/app/features/dashboard/components/DashboardSettings/GeneralSettings.tsx @@ -165,16 +165,7 @@ export function GeneralSettingsUnconnected({ - + field.onChange({ uid, title })} value={field.value?.uid} - // Old folder picker fields - initialTitle={dashboard.meta.folderTitle} - dashboardId={dashboard.id} - enableCreateNew /> )} control={control} diff --git a/public/app/features/library-panels/components/AddLibraryPanelModal/AddLibraryPanelModal.tsx b/public/app/features/library-panels/components/AddLibraryPanelModal/AddLibraryPanelModal.tsx index 2f5f75be3ef..baf75e02cf7 100644 --- a/public/app/features/library-panels/components/AddLibraryPanelModal/AddLibraryPanelModal.tsx +++ b/public/app/features/library-panels/components/AddLibraryPanelModal/AddLibraryPanelModal.tsx @@ -86,11 +86,7 @@ export const AddLibraryPanelContents = ({ 'Library panel permissions are derived from the folder permissions' )} > - setFolderUid(uid)} - value={folderUid} - inputId="share-panel-library-panel-folder-picker" - /> + setFolderUid(uid)} value={folderUid} /> {config.featureToggles.newDashboardSharingComponent ? (