diff --git a/.betterer.results b/.betterer.results index 71a3864b644..1e8dfe8bef1 100644 --- a/.betterer.results +++ b/.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"], diff --git a/public/app/features/alerting/unified/RuleList.tsx b/public/app/features/alerting/unified/RuleList.tsx index f51f440bd14..c5e8a2dd08e 100644 --- a/public/app/features/alerting/unified/RuleList.tsx +++ b/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 {newView ? : }; }; diff --git a/public/app/features/alerting/unified/components/rules/Filter/RulesViewModeSelector.tsx b/public/app/features/alerting/unified/components/rules/Filter/RulesViewModeSelector.tsx index a0c63afa419..fe8303fdfd1 100644 --- a/public/app/features/alerting/unified/components/rules/Filter/RulesViewModeSelector.tsx +++ b/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; diff --git a/public/app/features/alerting/unified/featureToggles.test.ts b/public/app/features/alerting/unified/featureToggles.test.ts index 0665b102226..7917f449aec 100644 --- a/public/app/features/alerting/unified/featureToggles.test.ts +++ b/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(); +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); + }); + }); }); }); diff --git a/public/app/features/alerting/unified/featureToggles.ts b/public/app/features/alerting/unified/featureToggles.ts index 1d4ab503836..f40298b36d8 100644 --- a/public/app/features/alerting/unified/featureToggles.ts +++ b/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(','); -} diff --git a/public/app/features/alerting/unified/mocks.ts b/public/app/features/alerting/unified/mocks.ts index de3b06d7a08..be3484ba89f 100644 --- a/public/app/features/alerting/unified/mocks.ts +++ b/public/app/features/alerting/unified/mocks.ts @@ -814,3 +814,35 @@ export const mockThresholdExpression = (partial: Partial = {}): ...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(); +} diff --git a/public/app/features/alerting/unified/previewToggles.test.ts b/public/app/features/alerting/unified/previewToggles.test.ts new file mode 100644 index 00000000000..d665566d2c3 --- /dev/null +++ b/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); + }); + }); +}); diff --git a/public/app/features/alerting/unified/previewToggles.ts b/public/app/features/alerting/unified/previewToggles.ts new file mode 100644 index 00000000000..59f88efe099 --- /dev/null +++ b/public/app/features/alerting/unified/previewToggles.ts @@ -0,0 +1,22 @@ +import { FeatureToggles, store } from '@grafana/data'; + +type AlertingPreviewToggles = Pick; + +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(previewToggleStoreKey, {}); + + return previewToggles[previewToggleName]; +} + +export function setPreviewToggle(previewToggleName: keyof AlertingPreviewToggles, value: boolean | undefined) { + const previewToggles = store.getObject(previewToggleStoreKey, {}); + + previewToggles[previewToggleName] = value; + store.setObject(previewToggleStoreKey, previewToggles); +} diff --git a/public/app/features/alerting/unified/rule-list/RuleListPageTitle.test.tsx b/public/app/features/alerting/unified/rule-list/RuleListPageTitle.test.tsx index c4fe4479579..7a1618e26b6 100644 --- a/public/app/features/alerting/unified/rule-list/RuleListPageTitle.test.tsx +++ b/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(); - 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(); - - return { - ...view, - storage, - }; + return render(); } 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()); diff --git a/public/app/features/alerting/unified/rule-list/RuleListPageTitle.tsx b/public/app/features/alerting/unified/rule-list/RuleListPageTitle.tsx index fb7c349671c..74ff140af4e 100644 --- a/public/app/features/alerting/unified/rule-list/RuleListPageTitle.tsx +++ b/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 }) { ); } - -function useV2AlertListViewToggle() { - const listViewV2Enabled = shouldUseAlertingListViewV2(); - - const enableListViewV2 = useCallback(() => { - setLocalStorageFeatureToggle('alertingListViewV2', true); - }, []); - - const disableListViewV2 = useCallback(() => { - setLocalStorageFeatureToggle('alertingListViewV2', undefined); - }, []); - - return { - listViewV2Enabled, - enableListViewV2, - disableListViewV2, - }; -}