Alerting: Fix alert list V2 preview toggle (#105062)

* Use custom storage for previewing alerting features configuration

* Fix tests, add localStorage mock

* Use store singleton

.betterer.results

@@ -1078,12 +1078,6 @@ exports[`better eslint`] = {
     "public/app/features/alerting/unified/components/silences/SilencesFilter.tsx:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"]
     ],
-    "public/app/features/alerting/unified/featureToggles.ts:5381": [
-      [0, 0, 0, "Direct usage of localStorage is not allowed. Use \`Store\` from @grafana/data instead.", "0"],
-      [0, 0, 0, "Direct usage of localStorage is not allowed. Use \`Store\` from @grafana/data instead.", "1"],
-      [0, 0, 0, "Direct usage of localStorage is not allowed. Use \`Store\` from @grafana/data instead.", "2"],
-      [0, 0, 0, "Direct usage of localStorage is not allowed. Use \`Store\` from @grafana/data instead.", "3"]
-    ],
     "public/app/features/alerting/unified/hooks/useAlertmanagerConfig.ts:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"]
     ],
@@ -1093,6 +1087,9 @@ exports[`better eslint`] = {
     "public/app/features/alerting/unified/insights/InsightsMenuButton.tsx:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"]
     ],
+    "public/app/features/alerting/unified/mocks.ts:5381": [
+      [0, 0, 0, "Unexpected any. Specify a different type.", "0"]
+    ],
     "public/app/features/alerting/unified/rule-editor/formDefaults.ts:5381": [
       [0, 0, 0, "Direct usage of localStorage is not allowed. Use \`Store\` from @grafana/data instead.", "0"],
       [0, 0, 0, "Direct usage of localStorage is not allowed. Use \`Store\` from @grafana/data instead.", "1"],

public/app/features/alerting/unified/RuleList.tsx

@@ -1,13 +1,13 @@
 import { Suspense, lazy } from 'react';
 
-import { config } from '@grafana/runtime';
-
+import { shouldUseAlertingListViewV2 } from './featureToggles';
 import RuleListV1 from './rule-list/RuleList.v1';
 import { withPageErrorBoundary } from './withPageErrorBoundary';
+
 const RuleListV2 = lazy(() => import('./rule-list/RuleList.v2'));
 
 const RuleList = () => {
-  const newView = config.featureToggles.alertingListViewV2;
+  const newView = shouldUseAlertingListViewV2();
 
   return <Suspense>{newView ? <RuleListV2 /> : <RuleListV1 />}</Suspense>;
 };

public/app/features/alerting/unified/components/rules/Filter/RulesViewModeSelector.tsx

@@ -1,8 +1,8 @@
 import { SelectableValue } from '@grafana/data';
-import { config } from '@grafana/runtime';
 import { RadioButtonGroup } from '@grafana/ui';
 
 import { trackRulesListViewChange } from '../../../Analytics';
+import { shouldUseAlertingListViewV2 } from '../../../featureToggles';
 import { useRulesFilter } from '../../../hooks/useFilteredRules';
 import { useURLSearchParams } from '../../../hooks/useURLSearchParams';
 
@@ -66,6 +66,4 @@ function viewParamToLegacyView(viewParam: string | null): LegacySupportedView {
   return 'grouped';
 }
 
-export const RulesViewModeSelector = config.featureToggles.alertingListViewV2
-  ? RulesViewModeSelectorV2
-  : RulesViewModeSelectorV1;
+export const RulesViewModeSelector = shouldUseAlertingListViewV2() ? RulesViewModeSelectorV2 : RulesViewModeSelectorV1;

public/app/features/alerting/unified/featureToggles.test.ts

@@ -1,90 +1,81 @@
-import { setLocalStorageFeatureToggle } from './featureToggles';
+import { config } from '@grafana/runtime';
 
-const featureTogglesKey = 'grafana.featureToggles';
-const storage = new Map<string, string>();
+import { shouldUseAlertingListViewV2 } from './featureToggles';
+import { mockLocalStorage } from './mocks';
+import { setPreviewToggle } from './previewToggles';
 
-const mockLocalStorage = {
-  getItem: (key: string) => storage.get(key) ?? null,
-  setItem: (key: string, value: string) => storage.set(key, value),
-  clear: () => storage.clear(),
-};
+const localStorageMock = mockLocalStorage();
 
 Object.defineProperty(window, 'localStorage', {
-  value: mockLocalStorage,
+  value: localStorageMock,
   writable: true,
 });
 
-describe('setLocalStorageFeatureToggle', () => {
+describe('featureToggles', () => {
   beforeEach(() => {
-    storage.clear();
+    jest.clearAllMocks();
+    localStorageMock.clear();
+    config.featureToggles = {};
   });
 
-  it('should set feature toggle to true', () => {
-    setLocalStorageFeatureToggle('alertingListViewV2', true);
-    expect(storage.get(featureTogglesKey)).toBe('alertingListViewV2=true');
-  });
+  describe('shouldUseAlertingListViewV2', () => {
+    describe('when alertingListViewV2 toggle is disabled', () => {
+      beforeEach(() => {
+        config.featureToggles.alertingListViewV2 = false;
+      });
 
-  it('should set feature toggle to false', () => {
-    setLocalStorageFeatureToggle('alertingListViewV2', false);
-    expect(storage.get(featureTogglesKey)).toBe('alertingListViewV2=false');
-  });
+      it('should return false when preview toggle is disabled', () => {
+        config.featureToggles.alertingListViewV2PreviewToggle = false;
 
-  it('should remove feature toggle when set to undefined', () => {
-    storage.set(
-      featureTogglesKey,
-      'alertingListViewV2=true,alertingPrometheusRulesPrimary=true,alertingCentralAlertHistory=true'
-    );
+        expect(shouldUseAlertingListViewV2()).toBe(false);
+      });
 
-    setLocalStorageFeatureToggle('alertingPrometheusRulesPrimary', undefined);
-    expect(storage.get(featureTogglesKey)).toBe('alertingListViewV2=true,alertingCentralAlertHistory=true');
-  });
+      it('should return true when preview toggle is enabled and preview value is true', () => {
+        config.featureToggles.alertingListViewV2PreviewToggle = true;
+        setPreviewToggle('alertingListViewV2', true);
 
-  it('should not set undefined when no feature toggles are set', () => {
-    storage.set(featureTogglesKey, '');
+        expect(shouldUseAlertingListViewV2()).toBe(true);
+      });
 
-    setLocalStorageFeatureToggle('alertingPrometheusRulesPrimary', undefined);
-    expect(storage.get(featureTogglesKey)).toBe('');
-  });
+      it('should return false when preview toggle is enabled but preview value is false', () => {
+        config.featureToggles.alertingListViewV2PreviewToggle = true;
+        setPreviewToggle('alertingListViewV2', false);
 
-  it('should update only one feature toggle when multiple feature toggles are set', () => {
-    storage.set(
-      featureTogglesKey,
-      'alertingListViewV2=true,alertingPrometheusRulesPrimary=true,alertingCentralAlertHistory=true'
-    );
+        expect(shouldUseAlertingListViewV2()).toBe(false);
+      });
+    });
 
-    setLocalStorageFeatureToggle('alertingPrometheusRulesPrimary', false);
-    expect(storage.get(featureTogglesKey)).toBe(
-      'alertingListViewV2=true,alertingPrometheusRulesPrimary=false,alertingCentralAlertHistory=true'
-    );
-  });
+    describe('when alertingListViewV2 toggle is enabled', () => {
+      beforeEach(() => {
+        config.featureToggles.alertingListViewV2 = true;
+      });
 
-  it('should not rewrite other feature toggles when updating one', () => {
-    storage.set(
-      featureTogglesKey,
-      'alertingListViewV2=true,alertingPrometheusRulesPrimary=1,alertingCentralAlertHistory=false'
-    );
+      it('should return true when preview toggle is disabled', () => {
+        config.featureToggles.alertingListViewV2PreviewToggle = false;
 
-    setLocalStorageFeatureToggle('alertingListViewV2', false);
-    expect(storage.get(featureTogglesKey)).toBe(
-      'alertingListViewV2=false,alertingPrometheusRulesPrimary=1,alertingCentralAlertHistory=false'
-    );
-  });
+        expect(shouldUseAlertingListViewV2()).toBe(true);
+      });
 
-  it('should add a new toggle when others exist', () => {
-    storage.set(featureTogglesKey, 'alertingListViewV2=true');
-    setLocalStorageFeatureToggle('alertingCentralAlertHistory', true);
-    expect(storage.get(featureTogglesKey)).toBe('alertingListViewV2=true,alertingCentralAlertHistory=true');
-  });
+      it('should return true when preview toggle is disabled even if preview value is true', () => {
+        config.featureToggles.alertingListViewV2PreviewToggle = false;
+        setPreviewToggle('alertingListViewV2', true);
 
-  it('should remove the only existing toggle', () => {
-    storage.set(featureTogglesKey, 'alertingListViewV2=true');
-    setLocalStorageFeatureToggle('alertingListViewV2', undefined);
-    expect(storage.get(featureTogglesKey)).toBe('');
-  });
+        expect(shouldUseAlertingListViewV2()).toBe(true);
+      });
+
+      it('should return false when preview toggle is enabled and preview value is false', () => {
+        config.featureToggles.alertingListViewV2PreviewToggle = true;
+        setPreviewToggle('alertingListViewV2', false);
+
+        expect(shouldUseAlertingListViewV2()).toBe(false);
+      });
+
+      it('should return true when preview toggle is enabled and preview value is true', () => {
+        config.featureToggles.alertingListViewV2PreviewToggle = true;
+        setPreviewToggle('alertingListViewV2', true);
 
-  it('should not change localStorage when attempting to remove a non-existent toggle', () => {
-    storage.set(featureTogglesKey, 'alertingListViewV2=true');
-    setLocalStorageFeatureToggle('alertingCentralAlertHistory', undefined);
-    expect(storage.get(featureTogglesKey)).toBe('alertingListViewV2=true');
+        expect(shouldUseAlertingListViewV2()).toBe(true);
+      });
+    });
   });
 });

public/app/features/alerting/unified/featureToggles.ts

@@ -1,11 +1,20 @@
-import { FeatureToggles } from '@grafana/data';
 import { config } from '@grafana/runtime';
 
+import { getPreviewToggle } from './previewToggles';
 import { isAdmin } from './utils/misc';
 
 export const shouldUsePrometheusRulesPrimary = () => config.featureToggles.alertingPrometheusRulesPrimary ?? false;
 
-export const shouldUseAlertingListViewV2 = () => config.featureToggles.alertingListViewV2 ?? false;
+export const shouldUseAlertingListViewV2 = () => {
+  const previewToggleValue = getPreviewToggle('alertingListViewV2');
+
+  // If the preview toggle is enabled and has configured value it should take precedence over the feature toggle
+  if (config.featureToggles.alertingListViewV2PreviewToggle && previewToggleValue !== undefined) {
+    return previewToggleValue;
+  }
+
+  return config.featureToggles.alertingListViewV2;
+};
 
 export const useGrafanaManagedRecordingRulesSupport = () =>
   config.unifiedAlerting.recordingRulesEnabled && config.featureToggles.grafanaManagedRecordingRules;
@@ -15,42 +24,3 @@ export const shouldAllowRecoveringDeletedRules = () =>
 
 export const shouldAllowPermanentlyDeletingRules = () =>
   (shouldAllowRecoveringDeletedRules() && config.featureToggles.alertingRulePermanentlyDelete) ?? false;
-
-export function setLocalStorageFeatureToggle(featureName: keyof FeatureToggles, value: boolean | undefined) {
-  const featureToggles = localStorage.getItem('grafana.featureToggles') ?? '';
-
-  const newToggles = updateFeatureToggle(featureToggles, featureName, value);
-  localStorage.setItem('grafana.featureToggles', newToggles);
-}
-
-function updateFeatureToggle(
-  featureToggles: string | undefined,
-  featureName: string,
-  value: boolean | undefined
-): string {
-  if (!featureToggles) {
-    if (value !== undefined) {
-      return `${featureName}=${value}`;
-    }
-    return '';
-  }
-
-  const parts = featureToggles.split(',');
-  const featurePrefix = `${featureName}=`;
-  const featureIndex = parts.findIndex((part) => part.startsWith(featurePrefix));
-
-  if (featureIndex !== -1) {
-    if (value === undefined) {
-      // Remove the feature
-      parts.splice(featureIndex, 1);
-    } else {
-      // Update the feature value
-      parts[featureIndex] = `${featureName}=${value}`;
-    }
-  } else if (value !== undefined) {
-    // Add new feature
-    parts.push(`${featureName}=${value}`);
-  }
-
-  return parts.join(',');
-}

public/app/features/alerting/unified/mocks.ts

@@ -814,3 +814,35 @@ export const mockThresholdExpression = (partial: Partial<ExpressionQuery> = {}):
     ...partial,
   },
 });
+
+class LocalStorageMock implements Storage {
+  [key: string]: any;
+
+  getItem(key: string) {
+    return this[key] ?? null;
+  }
+
+  setItem(key: string, value: string) {
+    this[key] = value;
+  }
+
+  clear() {
+    Object.keys(this).forEach((key) => delete this[key]);
+  }
+
+  removeItem(key: string) {
+    delete this[key];
+  }
+
+  key(index: number) {
+    return Object.keys(this)[index] ?? null;
+  }
+
+  get length() {
+    return Object.keys(this).length;
+  }
+}
+
+export function mockLocalStorage(): Storage {
+  return new LocalStorageMock();
+}

public/app/features/alerting/unified/previewToggles.test.ts

@@ -0,0 +1,50 @@
+import { mockLocalStorage } from './mocks';
+import { getPreviewToggle, setPreviewToggle } from './previewToggles';
+
+const localStorageMock = mockLocalStorage();
+Object.defineProperty(window, 'localStorage', {
+  value: localStorageMock,
+  writable: true,
+});
+
+describe('previewToggles', () => {
+  beforeEach(() => {
+    localStorageMock.clear();
+  });
+
+  describe('getPreviewToggle', () => {
+    it('should return undefined by default for a toggle', () => {
+      const result = getPreviewToggle('alertingListViewV2');
+      expect(result).toBe(undefined);
+    });
+
+    it.each([true, false])('should return the stored value for a toggle: %s', (value) => {
+      // Set the toggle value first
+      setPreviewToggle('alertingListViewV2', value);
+
+      // Then verify it returns the correct value
+      const result = getPreviewToggle('alertingListViewV2');
+      expect(result).toBe(value);
+    });
+  });
+
+  describe('setPreviewToggle', () => {
+    it('should set a toggle value', () => {
+      setPreviewToggle('alertingListViewV2', true);
+
+      const result = getPreviewToggle('alertingListViewV2');
+      expect(result).toBe(true);
+    });
+
+    it('should override previous toggle value', () => {
+      // Set initial value
+      setPreviewToggle('alertingListViewV2', true);
+
+      // Override with new value
+      setPreviewToggle('alertingListViewV2', false);
+
+      const result = getPreviewToggle('alertingListViewV2');
+      expect(result).toBe(false);
+    });
+  });
+});

public/app/features/alerting/unified/previewToggles.ts

@@ -0,0 +1,22 @@
+import { FeatureToggles, store } from '@grafana/data';
+
+type AlertingPreviewToggles = Pick<FeatureToggles, 'alertingListViewV2'>;
+
+const previewToggleStoreKey = 'grafana.alerting.previewToggles';
+
+/**
+ * Get the preview toggle value for the given toggle name.
+ * @returns The value of the preview toggle or undefined if it is not set.
+ */
+export function getPreviewToggle(previewToggleName: keyof AlertingPreviewToggles): boolean | undefined {
+  const previewToggles = store.getObject<AlertingPreviewToggles>(previewToggleStoreKey, {});
+
+  return previewToggles[previewToggleName];
+}
+
+export function setPreviewToggle(previewToggleName: keyof AlertingPreviewToggles, value: boolean | undefined) {
+  const previewToggles = store.getObject<AlertingPreviewToggles>(previewToggleStoreKey, {});
+
+  previewToggles[previewToggleName] = value;
+  store.setObject(previewToggleStoreKey, previewToggles);
+}

public/app/features/alerting/unified/rule-list/RuleListPageTitle.test.tsx

@@ -3,14 +3,12 @@ import { byRole } from 'testing-library-selector';
 
 import { reportInteraction } from '@grafana/runtime';
 
+import { mockLocalStorage } from '../mocks';
+import { getPreviewToggle, setPreviewToggle } from '../previewToggles';
 import { testWithFeatureToggles } from '../test/test-utils';
 
 import { RuleListPageTitle } from './RuleListPageTitle';
 
-// Constants
-const featureTogglesKey = 'grafana.featureToggles';
-const toggleName = 'alertingListViewV2';
-
 jest.mock('@grafana/runtime', () => ({
   ...jest.requireActual('@grafana/runtime'),
   reportInteraction: jest.fn(),
@@ -29,32 +27,21 @@ const ui = {
   disableV2Button: byRole('button', { name: 'Go back to the old look' }),
 };
 
+const localStorageMock = mockLocalStorage();
+Object.defineProperty(window, 'localStorage', {
+  value: localStorageMock,
+  writable: true,
+});
+
 // Helper function for rendering the component
 function renderRuleListPageTitle() {
-  // Mock localStorage
-  const storage = new Map<string, string>();
-  const mockLocalStorage = {
-    getItem: (key: string) => storage.get(key) ?? null,
-    setItem: (key: string, value: string) => storage.set(key, value),
-    clear: () => storage.clear(),
-  };
-
-  Object.defineProperty(window, 'localStorage', {
-    value: mockLocalStorage,
-    writable: true,
-  });
-
-  const view = render(<RuleListPageTitle title="Alert rules" />);
-
-  return {
-    ...view,
-    storage,
-  };
+  return render(<RuleListPageTitle title="Alert rules" />);
 }
 
 describe('RuleListPageTitle', () => {
   beforeEach(() => {
     jest.clearAllMocks();
+    localStorageMock.clear();
   });
 
   it('should render the title', () => {
@@ -79,11 +66,12 @@ describe('RuleListPageTitle', () => {
     });
 
     it('should enable v2 and reload page when clicked on "Try out the new look!" button', async () => {
-      const { user, storage } = renderRuleListPageTitle();
+      const { user } = renderRuleListPageTitle();
 
       await user.click(ui.enableV2Button.get());
 
-      expect(storage.get(featureTogglesKey)).toBe(`${toggleName}=true`);
+      const previewToggle = getPreviewToggle('alertingListViewV2');
+      expect(previewToggle).toBe(true);
       expect(mockReload).toHaveBeenCalled();
     });
 
@@ -107,19 +95,18 @@ describe('RuleListPageTitle', () => {
     });
 
     it('should disable v2 and reload page when clicked on "Go back to the old look" button', async () => {
-      const { user, storage } = renderRuleListPageTitle();
-      storage.set(featureTogglesKey, `${toggleName}=true`);
+      setPreviewToggle('alertingListViewV2', true);
+      const { user } = renderRuleListPageTitle();
 
       await user.click(ui.disableV2Button.get());
 
-      // When the toggle is set to undefined, it should be removed from localStorage
-      expect(storage.get(featureTogglesKey)).toBe('');
+      expect(getPreviewToggle('alertingListViewV2')).toBe(false);
       expect(mockReload).toHaveBeenCalled();
     });
 
     it('should report interaction when disabling v2', async () => {
-      const { user, storage } = renderRuleListPageTitle();
-      storage.set(featureTogglesKey, `${toggleName}=true`);
+      setPreviewToggle('alertingListViewV2', true);
+      const { user } = renderRuleListPageTitle();
 
       await user.click(ui.disableV2Button.get());
 

public/app/features/alerting/unified/rule-list/RuleListPageTitle.tsx

@@ -1,22 +1,21 @@
-import { useCallback } from 'react';
-
 import { config, reportInteraction } from '@grafana/runtime';
 import { Button, ButtonProps, Stack } from '@grafana/ui';
 import { t } from 'app/core/internationalization';
 
-import { setLocalStorageFeatureToggle, shouldUseAlertingListViewV2 } from '../featureToggles';
+import { shouldUseAlertingListViewV2 } from '../featureToggles';
+import { setPreviewToggle } from '../previewToggles';
 
 export function RuleListPageTitle({ title }: { title: string }) {
   const shouldShowV2Toggle = config.featureToggles.alertingListViewV2PreviewToggle ?? false;
 
-  const { listViewV2Enabled, enableListViewV2, disableListViewV2 } = useV2AlertListViewToggle();
+  const listViewV2Enabled = shouldUseAlertingListViewV2();
 
   const toggleListView = () => {
     if (listViewV2Enabled) {
-      disableListViewV2();
+      setPreviewToggle('alertingListViewV2', false);
       reportInteraction('alerting.list_view.v2.disabled');
     } else {
-      enableListViewV2();
+      setPreviewToggle('alertingListViewV2', true);
       reportInteraction('alerting.list_view.v2.enabled');
     }
     window.location.reload();
@@ -49,21 +48,3 @@ export function RuleListPageTitle({ title }: { title: string }) {
     </Stack>
   );
 }
-
-function useV2AlertListViewToggle() {
-  const listViewV2Enabled = shouldUseAlertingListViewV2();
-
-  const enableListViewV2 = useCallback(() => {
-    setLocalStorageFeatureToggle('alertingListViewV2', true);
-  }, []);
-
-  const disableListViewV2 = useCallback(() => {
-    setLocalStorageFeatureToggle('alertingListViewV2', undefined);
-  }, []);
-
-  return {
-    listViewV2Enabled,
-    enableListViewV2,
-    disableListViewV2,
-  };
-}