Provisioning: Fix missing migration (#106005)

* Provisioning: Fix migration * Add tests * Fix queries * Betterer * Betterer * Fix showing history checkbox
2025-05-28 06:46:37 +02:00
parent d4c446f42c
commit 09882bb07b
5 changed files with 296 additions and 24 deletions
@@ -2734,19 +2734,12 @@ exports[`better eslint`] = {
      [0, 0, 0, "Add noMargin prop to Card components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "2"],
      [0, 0, 0, "Add noMargin prop to Card components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "3"]
    ],
-    "public/app/features/provisioning/Wizard/BootstrapStep.tsx:5381": [
-      [0, 0, 0, "Add noMargin prop to Card components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"],
-      [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "1"]
-    ],
    "public/app/features/provisioning/Wizard/FinishStep.tsx:5381": [
      [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"],
      [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "1"],
      [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "2"],
      [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "3"]
    ],
-    "public/app/features/provisioning/Wizard/SynchronizeStep.tsx:5381": [
-      [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"]
-    ],
    "public/app/features/provisioning/types.ts:5381": [
      [0, 0, 0, "Unexpected any. Specify a different type.", "0"]
    ],
@@ -0,0 +1,273 @@
+import { render, screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { ReactNode } from 'react';
+import { useForm, FormProvider } from 'react-hook-form';
+
+import { useGetRepositoryFilesQuery, useGetResourceStatsQuery } from 'app/api/clients/provisioning';
+
+import { BootstrapStep, Props } from './BootstrapStep';
+import { getResourceStats, useModeOptions } from './actions';
+import { WizardFormData } from './types';
+
+jest.mock('app/api/clients/provisioning', () => ({
+  useGetRepositoryFilesQuery: jest.fn(),
+  useGetResourceStatsQuery: jest.fn(),
+}));
+
+jest.mock('./actions', () => ({
+  getResourceStats: jest.fn(),
+  useModeOptions: jest.fn(),
+}));
+
+// Wrapper component to provide form context
+function FormWrapper({ children, defaultValues }: { children: ReactNode; defaultValues?: Partial<WizardFormData> }) {
+  const methods = useForm<WizardFormData>({
+    defaultValues: {
+      repository: {
+        type: 'github',
+        url: 'https://github.com/test/repo',
+        title: '',
+        sync: {
+          target: 'instance',
+          enabled: true,
+        },
+        branch: 'main',
+        path: '',
+        readOnly: false,
+        prWorkflow: false,
+        ...defaultValues?.repository,
+      },
+      ...defaultValues,
+    },
+  });
+
+  return <FormProvider {...methods}>{children}</FormProvider>;
+}
+
+function setup(props: Partial<Props> = {}, formDefaultValues?: Partial<WizardFormData>) {
+  const user = userEvent.setup();
+
+  const defaultProps: Props = {
+    onOptionSelect: jest.fn(),
+    onStepStatusUpdate: jest.fn(),
+    repoName: 'test-repo',
+    settingsData: undefined,
+    ...props,
+  };
+
+  const utils = render(
+    <FormWrapper defaultValues={formDefaultValues}>
+      <BootstrapStep {...defaultProps} />
+    </FormWrapper>
+  );
+
+  return {
+    user,
+    props: defaultProps,
+    ...utils,
+  };
+}
+
+describe('BootstrapStep', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+
+    (useGetRepositoryFilesQuery as jest.Mock).mockReturnValue({
+      data: { items: [] },
+      isLoading: false,
+    });
+
+    (useGetResourceStatsQuery as jest.Mock).mockReturnValue({
+      data: { instance: [] },
+      isLoading: false,
+    });
+
+    (getResourceStats as jest.Mock).mockReturnValue({
+      fileCount: 0,
+      resourceCount: 0,
+      resourceCountString: '',
+    });
+
+    (useModeOptions as jest.Mock).mockReturnValue([
+      {
+        target: 'instance',
+        label: 'Sync all resources with external storage',
+        description: 'Resources will be synced with external storage',
+        subtitle: 'Use this option if you want to sync your entire instance',
+      },
+      {
+        target: 'folder',
+        label: 'Sync external storage to a new Grafana folder',
+        description: 'A new Grafana folder will be created',
+        subtitle: 'Use this option to sync into a new folder',
+      },
+    ]);
+  });
+
+  describe('rendering', () => {
+    it('should render loading state when data is loading', () => {
+      (useGetRepositoryFilesQuery as jest.Mock).mockReturnValue({
+        data: undefined,
+        isLoading: true,
+      });
+
+      const { props } = setup();
+
+      expect(screen.getByText('Loading resource information...')).toBeInTheDocument();
+      expect(props.onStepStatusUpdate).toHaveBeenCalledWith({ status: 'running' });
+    });
+
+    it('should render correct info for GitHub repository type', async () => {
+      const { props } = setup();
+
+      expect(await screen.findByText('Grafana instance')).toBeInTheDocument();
+      expect(screen.getByText('External storage')).toBeInTheDocument();
+      expect(screen.getAllByText('Empty')).toHaveLength(2); // Both should show empty
+
+      expect(props.onStepStatusUpdate).toHaveBeenCalledWith({ status: 'idle' });
+    });
+
+    it('should render correct info for local file repository type', async () => {
+      (useGetRepositoryFilesQuery as jest.Mock).mockReturnValue({
+        data: {
+          items: [
+            { path: 'dashboard1.json' },
+            { path: 'dashboard2.yaml' },
+            { path: 'README.md' }, // Should not be counted
+          ],
+        },
+        isLoading: false,
+      });
+
+      (getResourceStats as jest.Mock).mockReturnValue({
+        fileCount: 2,
+        resourceCount: 0,
+        resourceCountString: '',
+      });
+
+      setup();
+
+      expect(await screen.findByText('2 files')).toBeInTheDocument();
+    });
+
+    it('should display resource counts when resources exist', async () => {
+      (useGetResourceStatsQuery as jest.Mock).mockReturnValue({
+        data: {
+          instance: [
+            { group: 'dashboard.grafana.app', count: 5 },
+            { group: 'folders', count: 2 },
+          ],
+        },
+        isLoading: false,
+      });
+
+      (getResourceStats as jest.Mock).mockReturnValue({
+        fileCount: 0,
+        resourceCount: 7,
+        resourceCountString: '7 resources',
+      });
+
+      setup();
+
+      expect(await screen.findByText('7 resources')).toBeInTheDocument();
+    });
+  });
+
+  describe('option selection', () => {
+    it('should call onOptionSelect with correct argument when no migration needed', async () => {
+      const { props } = setup();
+
+      await waitFor(() => {
+        expect(props.onOptionSelect).toHaveBeenCalledWith(false);
+      });
+    });
+
+    it('should call onOptionSelect with true when legacy storage exists', async () => {
+      const { props } = setup({
+        settingsData: {
+          legacyStorage: true,
+          items: [],
+        },
+      });
+
+      await waitFor(() => {
+        expect(props.onOptionSelect).toHaveBeenCalledWith(true);
+      });
+    });
+
+    it('should call onOptionSelect with true when resources exist', async () => {
+      (useGetResourceStatsQuery as jest.Mock).mockReturnValue({
+        data: {
+          instance: [{ group: 'dashboard.grafana.app', count: 1 }],
+        },
+        isLoading: false,
+      });
+
+      (getResourceStats as jest.Mock).mockReturnValue({
+        fileCount: 0,
+        resourceCount: 1,
+        resourceCountString: '1 resource',
+      });
+
+      const { props } = setup();
+
+      await waitFor(() => {
+        expect(props.onOptionSelect).toHaveBeenCalledWith(true);
+      });
+    });
+  });
+
+  describe('sync target options', () => {
+    it('should display both instance and folder options by default', async () => {
+      setup();
+
+      expect(await screen.findByText('Sync all resources with external storage')).toBeInTheDocument();
+      expect(await screen.findByText('Sync external storage to a new Grafana folder')).toBeInTheDocument();
+    });
+
+    it('should only display instance option when legacy storage exists', async () => {
+      (useModeOptions as jest.Mock).mockReturnValue([
+        {
+          target: 'instance',
+          label: 'Sync all resources with external storage',
+          description: 'Resources will be synced with external storage',
+          subtitle: 'Use this option if you want to sync your entire instance',
+        },
+      ]);
+
+      setup({
+        settingsData: {
+          legacyStorage: true,
+          items: [],
+        },
+      });
+
+      expect(await screen.findByText('Sync all resources with external storage')).toBeInTheDocument();
+      expect(screen.queryByText('Sync external storage to a new Grafana folder')).not.toBeInTheDocument();
+    });
+
+    it('should allow selecting different sync targets', async () => {
+      const { user } = setup();
+
+      const folderOption = await screen.findByText('Sync external storage to a new Grafana folder');
+      await user.click(folderOption);
+
+      // Check that the folder option is now selected by looking for the title field
+      expect(await screen.findByRole('textbox', { name: /display name/i })).toBeInTheDocument();
+    });
+  });
+
+  describe('title field visibility', () => {
+    it('should show title field only for folder sync target', async () => {
+      const { user } = setup();
+
+      // Initially should not show title field (default is instance)
+      expect(screen.queryByRole('textbox', { name: /display name/i })).not.toBeInTheDocument();
+
+      const folderOption = await screen.findByText('Sync external storage to a new Grafana folder');
+      await user.click(folderOption);
+
+      expect(await screen.findByRole('textbox', { name: /display name/i })).toBeInTheDocument();
+    });
+  });
+});
@@ -8,7 +8,7 @@ import { RepositoryViewList, useGetRepositoryFilesQuery, useGetResourceStatsQuer
 import { getResourceStats, useModeOptions } from './actions';
 import { StepStatusInfo, WizardFormData } from './types';

-interface Props {
+export interface Props {
  onOptionSelect: (requiresMigration: boolean) => void;
  onStepStatusUpdate: (info: StepStatusInfo) => void;
  settingsData?: RepositoryViewList;
@@ -29,10 +29,14 @@ export function BootstrapStep({ onOptionSelect, settingsData, repoName, onStepSt
  const filesQuery = useGetRepositoryFilesQuery({ name: repoName });
  const selectedTarget = watch('repository.sync.target');
  const options = useModeOptions(repoName, settingsData);
+  const { target } = options[0];
  const { resourceCount, resourceCountString, fileCount } = useMemo(
    () => getResourceStats(filesQuery.data, resourceStats.data),
    [filesQuery.data, resourceStats.data]
  );
+  const requiresMigration = settingsData?.legacyStorage || resourceCount > 0;
+  const isLoading = resourceStats.isLoading || filesQuery.isLoading;
+  const { t } = useTranslate();

  useEffect(() => {
    // Pick a name nice name based on type+settings
@@ -49,22 +53,15 @@ export function BootstrapStep({ onOptionSelect, settingsData, repoName, onStepSt
  }, [getValues, setValue]);

  useEffect(() => {
-    const isLoading = resourceStats.isLoading || filesQuery.isLoading;
    onStepStatusUpdate({ status: isLoading ? 'running' : 'idle' });
-  }, [filesQuery.isLoading, onStepStatusUpdate, resourceStats.isLoading]);
+  }, [isLoading, onStepStatusUpdate]);

-  // Auto select the first option on mount
  useEffect(() => {
-    const { target } = options[0];
    setValue('repository.sync.target', target);
-    onOptionSelect(settingsData?.legacyStorage || resourceCount > 0);
-    // Only run this effect on mount
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+    onOptionSelect(requiresMigration);
+  }, [target, setValue, onOptionSelect, requiresMigration]);

-  const { t } = useTranslate();
-
-  if (resourceStats.isLoading || filesQuery.isLoading) {
+  if (isLoading) {
    return (
      <Box padding={4}>
        <LoadingPlaceholder
@@ -107,13 +104,14 @@ export function BootstrapStep({ onOptionSelect, settingsData, repoName, onStepSt
          control={control}
          render={({ field: { ref, onChange, ...field } }) => (
            <>
-              {options.map((action, index) => (
+              {options.map((action) => (
                <Card
                  key={action.target}
                  isSelected={action.target === selectedTarget}
                  onClick={() => {
                    onChange(action.target);
                  }}
+                  noMargin
                  {...field}
                >
                  <Card.Heading>{action.label}</Card.Heading>
@@ -140,8 +138,10 @@ export function BootstrapStep({ onOptionSelect, settingsData, repoName, onStepSt
            error={errors.repository?.title?.message}
            invalid={!!errors.repository?.title}
            required
+            noMargin
          >
            <Input
+              id="repository-title"
              {...register('repository.title', {
                required: t('provisioning.bootstrap-step.error-field-required', 'This field is required.'),
              })}
@@ -258,7 +258,11 @@ export function ProvisioningWizard({ type }: { type: RepoType }) {
                />
              )}
              {activeStep === 'synchronize' && (
-                <SynchronizeStep onStepStatusUpdate={setStepStatusInfo} requiresMigration={requiresMigration} />
+                <SynchronizeStep
+                  onStepStatusUpdate={setStepStatusInfo}
+                  requiresMigration={requiresMigration}
+                  isLegacyStorage={Boolean(settingsQuery.data?.legacyStorage)}
+                />
              )}
              {activeStep === 'finish' && <FinishStep />}
            </div>
@@ -12,13 +12,14 @@ import { StepStatusInfo, WizardFormData } from './types';
 export interface SynchronizeStepProps {
  onStepStatusUpdate: (info: StepStatusInfo) => void;
  requiresMigration: boolean;
+  isLegacyStorage?: boolean;
 }

-export function SynchronizeStep({ onStepStatusUpdate, requiresMigration }: SynchronizeStepProps) {
+export function SynchronizeStep({ onStepStatusUpdate, requiresMigration, isLegacyStorage }: SynchronizeStepProps) {
  const [createJob] = useCreateRepositoryJobsMutation();
  const { getValues, register, watch } = useFormContext<WizardFormData>();
  const repoType = watch('repository.type');
-  const supportsHistory = requiresMigration && repoType === 'github';
+  const supportsHistory = repoType === 'github' && isLegacyStorage;
  const [job, setJob] = useState<Job>();
  const { t } = useTranslate();
  const startSynchronization = async () => {
@@ -117,9 +118,10 @@ export function SynchronizeStep({ onStepStatusUpdate, requiresMigration }: Synch
          <Text element="h3">
            <Trans i18nKey="provisioning.synchronize-step.synchronization-options">Synchronization options</Trans>
          </Text>
-          <Field>
+          <Field noMargin>
            <Checkbox
              {...register('migrate.history')}
+              id="migrate-history"
              label={t('provisioning.wizard.sync-option-history', 'History')}
              description={
                <Trans i18nKey="provisioning.synchronize-step.synchronization-description">