From 7c6cc6f37929895e93a82d1ceff0ce77f034f970 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Fri, 14 Mar 2025 14:28:16 +0100 Subject: [PATCH 01/18] chore: proper default handling removing necessity for most of the nulls Signed-off-by: Dominikus Hellgartner --- src/ElectronBackend/main/user-settings.ts | 13 +++++++----- .../AuditingOptions/AuditingOptions.util.tsx | 2 +- .../AttributionPanels/AttributionPanels.tsx | 6 ++---- .../state/variables/use-panel-sizes.ts | 9 +------- src/shared/shared-constants.ts | 21 +++++++++++++++++++ src/shared/shared-types.ts | 12 +++++------ 6 files changed, 39 insertions(+), 24 deletions(-) create mode 100644 src/shared/shared-constants.ts diff --git a/src/ElectronBackend/main/user-settings.ts b/src/ElectronBackend/main/user-settings.ts index 04a743294..5ef076b2b 100644 --- a/src/ElectronBackend/main/user-settings.ts +++ b/src/ElectronBackend/main/user-settings.ts @@ -7,17 +7,20 @@ import log from 'electron-log'; import settings from 'electron-settings'; import { AllowedFrontendChannels } from '../../shared/ipc-channels'; +import { DEFAULT_USER_SETTINGS } from '../../shared/shared-constants'; import { UserSettings as IUserSettings } from '../../shared/shared-types'; -const DEFAULT_USER_SETTINGS = { - showClassifications: true, -}; - export class UserSettings { public static async init() { if (process.argv.includes('--reset') || process.env.RESET) { log.info('Resetting user settings'); - await settings.set(DEFAULT_USER_SETTINGS); + await settings.set(DEFAULT_USER_SETTINGS as unknown as never); + } else { + const currentSettings = await settings.get(); + await settings.set({ + ...DEFAULT_USER_SETTINGS, + ...currentSettings, + }); } } diff --git a/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx b/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx index a2dc02d36..a3a72ca3b 100644 --- a/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx +++ b/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx @@ -112,7 +112,7 @@ export function useAuditingOptions({ preferred: false, }), ), - interactive: isPreferenceFeatureEnabled && !!qaMode && isEditable, + interactive: isPreferenceFeatureEnabled && qaMode && isEditable, }, { id: 'was-preferred', diff --git a/src/Frontend/Components/AttributionPanels/AttributionPanels.tsx b/src/Frontend/Components/AttributionPanels/AttributionPanels.tsx index 3b8ee4365..1563368ac 100644 --- a/src/Frontend/Components/AttributionPanels/AttributionPanels.tsx +++ b/src/Frontend/Components/AttributionPanels/AttributionPanels.tsx @@ -5,15 +5,13 @@ import { useCallback } from 'react'; import { AllowedFrontendChannels } from '../../../shared/ipc-channels'; +import { DEFAULT_PANEL_SIZES } from '../../../shared/shared-constants'; import { text } from '../../../shared/text'; import { useFilteredAttributions, useFilteredSignals, } from '../../state/variables/use-filtered-data'; -import { - DEFAULT_PANEL_SIZES, - usePanelSizes, -} from '../../state/variables/use-panel-sizes'; +import { usePanelSizes } from '../../state/variables/use-panel-sizes'; import { ResizePanels } from '../ResizePanels/ResizePanels'; import { Container } from './AttributionPanels.style'; import { AttributionsPanel } from './AttributionsPanel/AttributionsPanel'; diff --git a/src/Frontend/state/variables/use-panel-sizes.ts b/src/Frontend/state/variables/use-panel-sizes.ts index 83bebf189..0a24f6035 100644 --- a/src/Frontend/state/variables/use-panel-sizes.ts +++ b/src/Frontend/state/variables/use-panel-sizes.ts @@ -2,16 +2,9 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 -import { UserSettings } from '../../../shared/shared-types'; +import { DEFAULT_PANEL_SIZES } from '../../../shared/shared-constants'; import { useUserSetting } from './use-user-setting'; -export const DEFAULT_PANEL_SIZES: NonNullable = { - resourceBrowserWidth: 340, - packageListsWidth: 340, - linkedResourcesPanelHeight: null, - signalsPanelHeight: null, -}; - export const usePanelSizes = () => { return useUserSetting({ defaultValue: DEFAULT_PANEL_SIZES, diff --git a/src/shared/shared-constants.ts b/src/shared/shared-constants.ts new file mode 100644 index 000000000..bb08ad0eb --- /dev/null +++ b/src/shared/shared-constants.ts @@ -0,0 +1,21 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import { UserSettings } from './shared-types'; + +export const DEFAULT_PANEL_SIZES: NonNullable = { + resourceBrowserWidth: 340, + packageListsWidth: 340, + linkedResourcesPanelHeight: null, + signalsPanelHeight: null, +}; + +export const DEFAULT_USER_SETTINGS: UserSettings = { + qaMode: false, + showProjectStatistics: true, + areHiddenSignalsVisible: false, + showCriticality: true, + showClassifications: true, + panelSizes: DEFAULT_PANEL_SIZES, +}; diff --git a/src/shared/shared-types.ts b/src/shared/shared-types.ts index 05ec060db..e309b5a9c 100644 --- a/src/shared/shared-types.ts +++ b/src/shared/shared-types.ts @@ -318,15 +318,15 @@ export interface Log { } export interface UserSettings { - qaMode: boolean | null; - showProjectStatistics: boolean | null; - areHiddenSignalsVisible: boolean | null; - showCriticality: boolean | null; - showClassifications: boolean | null; + qaMode: boolean; + showProjectStatistics: boolean; + areHiddenSignalsVisible: boolean; + showCriticality: boolean; + showClassifications: boolean; panelSizes: { resourceBrowserWidth: number; packageListsWidth: number; linkedResourcesPanelHeight: number | null; signalsPanelHeight: number | null; - } | null; + }; } From b16240a46c799c0c629b1922b405a955b79ecd35 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Fri, 14 Mar 2025 16:06:15 +0100 Subject: [PATCH 02/18] test: ad test for properly setting default values Signed-off-by: Dominikus Hellgartner --- .../main/__tests__/user-settings.test.ts | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 src/ElectronBackend/main/__tests__/user-settings.test.ts diff --git a/src/ElectronBackend/main/__tests__/user-settings.test.ts b/src/ElectronBackend/main/__tests__/user-settings.test.ts new file mode 100644 index 000000000..f2a96d51b --- /dev/null +++ b/src/ElectronBackend/main/__tests__/user-settings.test.ts @@ -0,0 +1,58 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import settings from 'electron-settings'; +import { rmSync } from 'node:fs'; +import { mkdtemp } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { DEFAULT_USER_SETTINGS } from '../../../shared/shared-constants'; +import { UserSettings } from '../user-settings'; + +describe('UserSettings', () => { + let temporaryDir: string | undefined = undefined; + beforeEach(async () => { + temporaryDir = await mkdtemp(join(tmpdir(), 'OpossumUiTesting-')); + settings.configure({ dir: temporaryDir }); + }); + + afterEach(() => { + if (temporaryDir) { + rmSync(temporaryDir, { recursive: true, force: true }); + } + }); + + it('sets up the default values if empty', async () => { + await UserSettings.init(); + + const result = await settings.get(); + + expect(result).toEqual(DEFAULT_USER_SETTINGS); + }); + + it('overwrites only the non set values if there are already values set', async () => { + await settings.set('qaMode', true); + + await UserSettings.init(); + + const result = await settings.get(); + + expect(result).toEqual({ ...DEFAULT_USER_SETTINGS, qaMode: true }); + }); + + it('resets everything if reset is requested', async () => { + const oldEnvironment = process.env; + process.env = { ...oldEnvironment, RESET: 'TRUE' }; + + await settings.set('qaMode', true); + + await UserSettings.init(); + + const result = await settings.get(); + + expect(result).toEqual({ ...DEFAULT_USER_SETTINGS }); + process.env = oldEnvironment; + }); +}); From 194a1eec2a709fe1efc8e5ae9e2bad2235948149 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Tue, 18 Mar 2025 07:53:56 +0100 Subject: [PATCH 03/18] chore: add reducer for user config Signed-off-by: Dominikus Hellgartner --- src/Frontend/Components/App/App.tsx | 6 ----- .../actions/user-settings-actions/types.ts | 14 +++++++++++ .../user-settings-actions.ts | 13 ++++++++++ src/Frontend/state/reducer.ts | 2 ++ .../state/reducers/user-settings-reducer.ts | 25 +++++++++++++++++++ 5 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 src/Frontend/state/actions/user-settings-actions/types.ts create mode 100644 src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts create mode 100644 src/Frontend/state/reducers/user-settings-reducer.ts diff --git a/src/Frontend/Components/App/App.tsx b/src/Frontend/Components/App/App.tsx index 78a65b953..56ddac95b 100644 --- a/src/Frontend/Components/App/App.tsx +++ b/src/Frontend/Components/App/App.tsx @@ -10,9 +10,6 @@ import { View } from '../../enums/enums'; import { useAppSelector } from '../../state/hooks'; import { getResources } from '../../state/selectors/resource-selectors'; import { getSelectedView } from '../../state/selectors/view-selector'; -import { usePanelSizes } from '../../state/variables/use-panel-sizes'; -import { useShowClassifications } from '../../state/variables/use-show-classifications'; -import { useShowCriticality } from '../../state/variables/use-show-criticality'; import { useSignalsWorker } from '../../web-workers/use-signals-worker'; import { AuditView } from '../AuditView/AuditView'; import { ErrorFallback } from '../ErrorFallback/ErrorFallback'; @@ -33,9 +30,6 @@ export function App() { useSignalsWorker(); //pre-hydrate values - usePanelSizes(); - useShowClassifications(); - useShowCriticality(); return ( diff --git a/src/Frontend/state/actions/user-settings-actions/types.ts b/src/Frontend/state/actions/user-settings-actions/types.ts new file mode 100644 index 000000000..4af46603d --- /dev/null +++ b/src/Frontend/state/actions/user-settings-actions/types.ts @@ -0,0 +1,14 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import { UserSettings } from '../../../../shared/shared-types'; + +export interface SetUserSetting { + type: typeof ACTION_SET_USER_SETTING; + payload: Partial; +} + +export const ACTION_SET_USER_SETTING = 'ACTION_SET_USER_SETTING'; + +export type UserSettingsAction = SetUserSetting; diff --git a/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts b/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts new file mode 100644 index 000000000..c68f458eb --- /dev/null +++ b/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts @@ -0,0 +1,13 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import { UserSettings } from '../../../../shared/shared-types'; +import { ACTION_SET_USER_SETTING, SetUserSetting } from './types'; + +export function setUserSetting(setting: Partial): SetUserSetting { + return { + type: ACTION_SET_USER_SETTING, + payload: setting, + }; +} diff --git a/src/Frontend/state/reducer.ts b/src/Frontend/state/reducer.ts index 1b1e54156..c472c86b0 100644 --- a/src/Frontend/state/reducer.ts +++ b/src/Frontend/state/reducer.ts @@ -5,6 +5,7 @@ import { combineReducers } from '@reduxjs/toolkit'; import { resourceState } from './reducers/resource-reducer'; +import { userSettingsState } from './reducers/user-settings-reducer'; import { variablesState } from './reducers/variables-reducer'; import { viewState } from './reducers/view-reducer'; @@ -12,4 +13,5 @@ export const reducer = combineReducers({ viewState, resourceState, variablesState, + userSettingsState, }); diff --git a/src/Frontend/state/reducers/user-settings-reducer.ts b/src/Frontend/state/reducers/user-settings-reducer.ts new file mode 100644 index 000000000..00e77fc32 --- /dev/null +++ b/src/Frontend/state/reducers/user-settings-reducer.ts @@ -0,0 +1,25 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import { UserSettings } from '../../../ElectronBackend/main/user-settings'; +import { DEFAULT_USER_SETTINGS } from '../../../shared/shared-constants'; +import { + ACTION_SET_USER_SETTING, + UserSettingsAction, +} from '../actions/user-settings-actions/types'; + +export function userSettingsState( + state: UserSettings = DEFAULT_USER_SETTINGS, + action: UserSettingsAction, +): UserSettings { + switch (action.type) { + case ACTION_SET_USER_SETTING: + return { + ...state, + ...action.payload, + }; + default: + return state; + } +} From b725fc85edd6905672647c0994342bade7d6aa49 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Tue, 18 Mar 2025 09:15:56 +0100 Subject: [PATCH 04/18] chore: sync user settings on app start Signed-off-by: Dominikus Hellgartner --- src/ElectronBackend/main/main.ts | 1 + src/ElectronBackend/main/user-settings.ts | 11 +++++++++-- src/ElectronBackend/preload.ts | 1 + src/Frontend/Components/App/App.tsx | 2 ++ .../user-settings-actions/user-settings-actions.ts | 8 ++++++++ src/Frontend/state/variables/use-user-setting.ts | 9 +++++++++ src/shared/ipc-channels.ts | 1 + src/shared/shared-types.ts | 1 + src/testing/setup-tests.ts | 2 ++ 9 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/ElectronBackend/main/main.ts b/src/ElectronBackend/main/main.ts index 13e1c471d..3248f0c89 100644 --- a/src/ElectronBackend/main/main.ts +++ b/src/ElectronBackend/main/main.ts @@ -95,6 +95,7 @@ export async function main(): Promise { ipcMain.handle(IpcChannel.GetUserSettings, (_, key) => UserSettings.get(key), ); + ipcMain.handle(IpcChannel.GetFullUserSettings, () => UserSettings.get()); ipcMain.handle(IpcChannel.SetUserSettings, (_, { key, value }) => UserSettings.set(key, value, { skipNotification: true }), ); diff --git a/src/ElectronBackend/main/user-settings.ts b/src/ElectronBackend/main/user-settings.ts index 5ef076b2b..66e7eb6e0 100644 --- a/src/ElectronBackend/main/user-settings.ts +++ b/src/ElectronBackend/main/user-settings.ts @@ -26,8 +26,15 @@ export class UserSettings { public static get( path: T, - ): Promise { - return settings.get(path) as Promise; + ): Promise; + public static get(): Promise; + public static get( + path?: T, + ): Promise | Promise { + if (path) { + return settings.get(path) as Promise; + } + return settings.get() as unknown as Promise; } public static async set( diff --git a/src/ElectronBackend/preload.ts b/src/ElectronBackend/preload.ts index 0b0e2e58e..2a62b801a 100644 --- a/src/ElectronBackend/preload.ts +++ b/src/ElectronBackend/preload.ts @@ -35,6 +35,7 @@ const electronAPI: ElectronAPI = { return () => ipcRenderer.removeListener(channel, listener); }, getUserSetting: (key) => ipcRenderer.invoke(IpcChannel.GetUserSettings, key), + getFullUserSettings: () => ipcRenderer.invoke(IpcChannel.GetFullUserSettings), setUserSetting: (key, value) => ipcRenderer.invoke(IpcChannel.SetUserSettings, { key, value }), }; diff --git a/src/Frontend/Components/App/App.tsx b/src/Frontend/Components/App/App.tsx index 56ddac95b..283f9fb7c 100644 --- a/src/Frontend/Components/App/App.tsx +++ b/src/Frontend/Components/App/App.tsx @@ -10,6 +10,7 @@ import { View } from '../../enums/enums'; import { useAppSelector } from '../../state/hooks'; import { getResources } from '../../state/selectors/resource-selectors'; import { getSelectedView } from '../../state/selectors/view-selector'; +import { useInitialSyncUserSettings } from '../../state/variables/use-user-setting'; import { useSignalsWorker } from '../../web-workers/use-signals-worker'; import { AuditView } from '../AuditView/AuditView'; import { ErrorFallback } from '../ErrorFallback/ErrorFallback'; @@ -30,6 +31,7 @@ export function App() { useSignalsWorker(); //pre-hydrate values + useInitialSyncUserSettings(); return ( diff --git a/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts b/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts index c68f458eb..782448629 100644 --- a/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts +++ b/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts @@ -3,6 +3,7 @@ // // SPDX-License-Identifier: Apache-2.0 import { UserSettings } from '../../../../shared/shared-types'; +import { AppThunkAction } from '../../types'; import { ACTION_SET_USER_SETTING, SetUserSetting } from './types'; export function setUserSetting(setting: Partial): SetUserSetting { @@ -11,3 +12,10 @@ export function setUserSetting(setting: Partial): SetUserSetting { payload: setting, }; } + +export function fetchUserSettings(): AppThunkAction { + return async (dispatch) => { + const userSettings = await window.electronAPI.getFullUserSettings(); + dispatch(setUserSetting(userSettings)); + }; +} diff --git a/src/Frontend/state/variables/use-user-setting.ts b/src/Frontend/state/variables/use-user-setting.ts index 1037377f3..295bb0e95 100644 --- a/src/Frontend/state/variables/use-user-setting.ts +++ b/src/Frontend/state/variables/use-user-setting.ts @@ -8,6 +8,8 @@ import { DependencyList, useCallback, useEffect } from 'react'; import { AllowedFrontendChannels } from '../../../shared/ipc-channels'; import { UserSettings } from '../../../shared/shared-types'; import { useIpcRenderer } from '../../util/use-ipc-renderer'; +import { fetchUserSettings } from '../actions/user-settings-actions/user-settings-actions'; +import { useAppDispatch } from '../hooks'; import { useVariable } from './use-variable'; /** @@ -91,3 +93,10 @@ export function useUserSetting( return [storedValue, setStoredValue, hydrated]; } + +export function useInitialSyncUserSettings() { + const dispatch = useAppDispatch(); + useEffect(() => { + dispatch(fetchUserSettings()); + }, [dispatch]); +} diff --git a/src/shared/ipc-channels.ts b/src/shared/ipc-channels.ts index 61e04fe97..f258f0fde 100644 --- a/src/shared/ipc-channels.ts +++ b/src/shared/ipc-channels.ts @@ -18,6 +18,7 @@ export enum IpcChannel { */ StopLoading = 'stop-loading', GetUserSettings = 'get-user-settings', + GetFullUserSettings = 'get-full-user-settings', SetUserSettings = 'set-user-settings', Quit = 'quit', Relaunch = 'relaunch', diff --git a/src/shared/shared-types.ts b/src/shared/shared-types.ts index e309b5a9c..06b804196 100644 --- a/src/shared/shared-types.ts +++ b/src/shared/shared-types.ts @@ -299,6 +299,7 @@ export interface ElectronAPI { getUserSetting: ( key: T, ) => Promise; + getFullUserSettings: () => Promise; setUserSetting: ( key: T, value: UserSettings[T], diff --git a/src/testing/setup-tests.ts b/src/testing/setup-tests.ts index e5e68d8e1..1f30cec0c 100644 --- a/src/testing/setup-tests.ts +++ b/src/testing/setup-tests.ts @@ -6,6 +6,7 @@ import '@testing-library/jest-dom'; import { noop } from 'lodash'; +import { DEFAULT_USER_SETTINGS } from '../shared/shared-constants'; import { ElectronAPI } from '../shared/shared-types'; import { faker } from './Faker'; @@ -45,6 +46,7 @@ global.window.electronAPI = { stopLoading: jest.fn(), on: jest.fn().mockReturnValue(jest.fn()), getUserSetting: jest.fn().mockReturnValue(undefined), + getFullUserSettings: jest.fn().mockReturnValue(DEFAULT_USER_SETTINGS), setUserSetting: jest.fn(), } satisfies ElectronAPI; From bc4dee447f48662133058dde6c70908f9faadc0c Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Tue, 18 Mar 2025 09:48:04 +0100 Subject: [PATCH 05/18] chore: keep state in sync with backend Signed-off-by: Dominikus Hellgartner --- .../state/variables/use-user-setting.ts | 18 ++++++++++++++++-- src/Frontend/util/use-ipc-renderer.ts | 9 ++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/Frontend/state/variables/use-user-setting.ts b/src/Frontend/state/variables/use-user-setting.ts index 295bb0e95..edc46d84b 100644 --- a/src/Frontend/state/variables/use-user-setting.ts +++ b/src/Frontend/state/variables/use-user-setting.ts @@ -7,8 +7,14 @@ import { DependencyList, useCallback, useEffect } from 'react'; import { AllowedFrontendChannels } from '../../../shared/ipc-channels'; import { UserSettings } from '../../../shared/shared-types'; -import { useIpcRenderer } from '../../util/use-ipc-renderer'; -import { fetchUserSettings } from '../actions/user-settings-actions/user-settings-actions'; +import { + useIpcRenderer, + UserSettingsChangedListener, +} from '../../util/use-ipc-renderer'; +import { + fetchUserSettings, + setUserSetting, +} from '../actions/user-settings-actions/user-settings-actions'; import { useAppDispatch } from '../hooks'; import { useVariable } from './use-variable'; @@ -94,9 +100,17 @@ export function useUserSetting( return [storedValue, setStoredValue, hydrated]; } +// should only be called once export function useInitialSyncUserSettings() { const dispatch = useAppDispatch(); useEffect(() => { dispatch(fetchUserSettings()); }, [dispatch]); + + useIpcRenderer( + AllowedFrontendChannels.UserSettingsChanged, + (_, updatedSettings: Partial) => + dispatch(setUserSetting(updatedSettings)), + [dispatch], + ); } diff --git a/src/Frontend/util/use-ipc-renderer.ts b/src/Frontend/util/use-ipc-renderer.ts index 46e9bb2ef..1e57d4d3a 100644 --- a/src/Frontend/util/use-ipc-renderer.ts +++ b/src/Frontend/util/use-ipc-renderer.ts @@ -14,6 +14,7 @@ import { IsLoadingArgs, Log, ParsedFileContent, + UserSettings, } from '../../shared/shared-types'; export type ResetStateListener = ( @@ -53,6 +54,11 @@ export type ShowMergeDialogListener = ( fileFormat: FileFormatInfo, ) => void; +export type UserSettingsChangedListener = ( + event: IpcRendererEvent, + payload: Partial, +) => void; + export type Listener = | ResetStateListener | SetStateListener @@ -61,7 +67,8 @@ export type Listener = | SetBaseURLForRootListener | IsLoadingListener | ShowImportDialogListener - | ShowMergeDialogListener; + | ShowMergeDialogListener + | UserSettingsChangedListener; export function useIpcRenderer( channel: AllowedFrontendChannels, From 93def01bce8eb631fb7ae9e59737cb9b44721153 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Tue, 18 Mar 2025 15:04:52 +0100 Subject: [PATCH 06/18] chore: move show classifications and show criticality to new approach Signed-off-by: Dominikus Hellgartner --- src/ElectronBackend/main/__tests__/menu.test.ts | 4 ++-- .../main/__tests__/user-settings.test.ts | 8 ++++---- src/ElectronBackend/main/main.ts | 12 +++++++----- src/ElectronBackend/main/menu/viewMenu.ts | 17 ++++++++++------- ...er-settings.ts => user-settings-provider.ts} | 11 ++++++----- .../AuditingOptions/AuditingOptions.util.tsx | 4 ++-- .../__tests__/AttributionForm.test.tsx | 9 ++++----- .../Components/PackageCard/PackageCard.tsx | 4 ++-- .../PackageCard/__tests__/PackageCard.test.tsx | 7 +++---- .../ProjectStatisticsPopup.tsx | 4 ++-- .../__tests__/ProjectStatisticsPopup.test.tsx | 8 +++----- .../ResourcesTreeNode/ResourcesTreeNode.tsx | 4 ++-- .../Components/SortButton/useSortingOptions.tsx | 4 ++-- .../SwitchableProcessBar.tsx | 4 ++-- .../__tests__/SwitchableProcessBar.test.tsx | 11 +++++------ .../state/reducers/user-settings-reducer.ts | 2 +- .../state/selectors/user-settings-selector.ts | 13 +++++++++++++ .../state/variables/use-show-classifications.ts | 8 +++----- .../state/variables/use-show-criticality.ts | 10 +++------- .../state/variables/use-user-setting.ts | 10 ---------- .../test-helpers/user-settings-helpers.ts | 16 ---------------- src/Frontend/types/types.ts | 2 ++ 22 files changed, 78 insertions(+), 94 deletions(-) rename src/ElectronBackend/main/{user-settings.ts => user-settings-provider.ts} (87%) create mode 100644 src/Frontend/state/selectors/user-settings-selector.ts delete mode 100644 src/Frontend/test-helpers/user-settings-helpers.ts diff --git a/src/ElectronBackend/main/__tests__/menu.test.ts b/src/ElectronBackend/main/__tests__/menu.test.ts index 3c49f4314..48845f956 100644 --- a/src/ElectronBackend/main/__tests__/menu.test.ts +++ b/src/ElectronBackend/main/__tests__/menu.test.ts @@ -5,7 +5,7 @@ import electron, { BrowserWindow, MenuItemConstructorOptions } from 'electron'; import { createMenu } from '../menu'; -import { UserSettings } from '../user-settings'; +import { UserSettingsProvider } from '../user-settings-provider'; jest.mock('electron', () => ({ BrowserWindow: class BrowserWindowMock {}, @@ -48,7 +48,7 @@ describe('create menu', () => { ]; testCases.forEach((testCase) => { it(`evaluates ${testCase.darkMode ? 'dark' : 'light'} mode properly`, async () => { - await UserSettings.init(); + await UserSettingsProvider.init(); const mainWindow = new BrowserWindow(); // Important to set this up only here and not in the mock setup diff --git a/src/ElectronBackend/main/__tests__/user-settings.test.ts b/src/ElectronBackend/main/__tests__/user-settings.test.ts index f2a96d51b..c64dcd97c 100644 --- a/src/ElectronBackend/main/__tests__/user-settings.test.ts +++ b/src/ElectronBackend/main/__tests__/user-settings.test.ts @@ -9,7 +9,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { DEFAULT_USER_SETTINGS } from '../../../shared/shared-constants'; -import { UserSettings } from '../user-settings'; +import { UserSettingsProvider } from '../user-settings-provider'; describe('UserSettings', () => { let temporaryDir: string | undefined = undefined; @@ -25,7 +25,7 @@ describe('UserSettings', () => { }); it('sets up the default values if empty', async () => { - await UserSettings.init(); + await UserSettingsProvider.init(); const result = await settings.get(); @@ -35,7 +35,7 @@ describe('UserSettings', () => { it('overwrites only the non set values if there are already values set', async () => { await settings.set('qaMode', true); - await UserSettings.init(); + await UserSettingsProvider.init(); const result = await settings.get(); @@ -48,7 +48,7 @@ describe('UserSettings', () => { await settings.set('qaMode', true); - await UserSettings.init(); + await UserSettingsProvider.init(); const result = await settings.get(); diff --git a/src/ElectronBackend/main/main.ts b/src/ElectronBackend/main/main.ts index 3248f0c89..606a64341 100644 --- a/src/ElectronBackend/main/main.ts +++ b/src/ElectronBackend/main/main.ts @@ -21,7 +21,7 @@ import { import { createMenu } from './menu'; import { DisabledMenuItemHandler } from './menu/DisabledMenuItemHandler'; import { openFileFromCliOrEnvVariableIfProvided } from './openFileFromCliOrEnvVariableIfProvided'; -import { UserSettings } from './user-settings'; +import { UserSettingsProvider } from './user-settings-provider'; export async function main(): Promise { try { @@ -35,7 +35,7 @@ export async function main(): Promise { const mainWindow = createWindow(); - await UserSettings.init(); + await UserSettingsProvider.init(); Menu.setApplicationMenu(await createMenu(mainWindow)); mainWindow.webContents.session.webRequest.onBeforeSendHeaders( @@ -93,11 +93,13 @@ export async function main(): Promise { ); ipcMain.handle(IpcChannel.OpenLink, openLinkListener); ipcMain.handle(IpcChannel.GetUserSettings, (_, key) => - UserSettings.get(key), + UserSettingsProvider.get(key), + ); + ipcMain.handle(IpcChannel.GetFullUserSettings, () => + UserSettingsProvider.get(), ); - ipcMain.handle(IpcChannel.GetFullUserSettings, () => UserSettings.get()); ipcMain.handle(IpcChannel.SetUserSettings, (_, { key, value }) => - UserSettings.set(key, value, { skipNotification: true }), + UserSettingsProvider.set(key, value, { skipNotification: true }), ); await loadWebApp(mainWindow); diff --git a/src/ElectronBackend/main/menu/viewMenu.ts b/src/ElectronBackend/main/menu/viewMenu.ts index 85bed3611..676edaaad 100644 --- a/src/ElectronBackend/main/menu/viewMenu.ts +++ b/src/ElectronBackend/main/menu/viewMenu.ts @@ -7,7 +7,7 @@ import { MenuItemConstructorOptions } from 'electron'; import { text } from '../../../shared/text'; import { getIconBasedOnTheme } from '../iconHelpers'; -import { UserSettings } from '../user-settings'; +import { UserSettingsProvider } from '../user-settings-provider'; import { switchableMenuItem } from './switchableMenuItem'; function getShowDevTools(): MenuItemConstructorOptions { @@ -61,7 +61,7 @@ function getShowClassifications( id: 'show-classifications', label: text.menu.viewSubmenu.showClassifications, onToggle: (newState: boolean) => - UserSettings.set('showClassifications', newState), + UserSettingsProvider.set('showClassifications', newState), }); } @@ -72,7 +72,7 @@ function getShowCriticality( id: 'show-criticality', label: text.menu.viewSubmenu.showCriticality, onToggle: (newState: boolean) => - UserSettings.set('showCriticality', newState), + UserSettingsProvider.set('showCriticality', newState), }); } @@ -80,14 +80,17 @@ function getQaMode(qaMode: boolean | null): Array { return switchableMenuItem(qaMode, { id: 'qa-mode', label: text.menu.viewSubmenu.qaMode, - onToggle: (newState: boolean) => UserSettings.set('qaMode', newState), + onToggle: (newState: boolean) => + UserSettingsProvider.set('qaMode', newState), }); } export async function getViewMenu(): Promise { - const qaMode = await UserSettings.get('qaMode'); - const showCriticality = await UserSettings.get('showCriticality'); - const showClassifications = await UserSettings.get('showClassifications'); + const qaMode = await UserSettingsProvider.get('qaMode'); + const showCriticality = await UserSettingsProvider.get('showCriticality'); + const showClassifications = await UserSettingsProvider.get( + 'showClassifications', + ); return { label: text.menu.view, submenu: [ diff --git a/src/ElectronBackend/main/user-settings.ts b/src/ElectronBackend/main/user-settings-provider.ts similarity index 87% rename from src/ElectronBackend/main/user-settings.ts rename to src/ElectronBackend/main/user-settings-provider.ts index 66e7eb6e0..bb0b7b0ac 100644 --- a/src/ElectronBackend/main/user-settings.ts +++ b/src/ElectronBackend/main/user-settings-provider.ts @@ -10,7 +10,7 @@ import { AllowedFrontendChannels } from '../../shared/ipc-channels'; import { DEFAULT_USER_SETTINGS } from '../../shared/shared-constants'; import { UserSettings as IUserSettings } from '../../shared/shared-types'; -export class UserSettings { +export class UserSettingsProvider { public static async init() { if (process.argv.includes('--reset') || process.env.RESET) { log.info('Resetting user settings'); @@ -45,11 +45,12 @@ export class UserSettings { await settings.set(path, value); if (!skipNotification) { + const partialSettingsToUpdate = Object.fromEntries([[path, value]]); BrowserWindow.getAllWindows().forEach((window) => { - window.webContents.send(AllowedFrontendChannels.UserSettingsChanged, { - path, - value, - }); + window.webContents.send( + AllowedFrontendChannels.UserSettingsChanged, + partialSettingsToUpdate, + ); }); } } diff --git a/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx b/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx index a3a72ca3b..cfc1f7d09 100644 --- a/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx +++ b/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx @@ -64,8 +64,8 @@ export function useAuditingOptions({ getIsPreferenceFeatureEnabled, ); const classifications = useAppSelector(getClassifications); - const [showClassifications] = useShowClassifications(); - const [showCriticality] = useShowCriticality(); + const showClassifications = useShowClassifications(); + const showCriticality = useShowCriticality(); const source = useMemo(() => { const sourceName = diff --git a/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx b/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx index d6981baa6..9c4777369 100644 --- a/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx +++ b/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx @@ -17,10 +17,9 @@ import { setConfig, setFrequentLicenses, } from '../../../state/actions/resource-actions/all-views-simple-actions'; +import { setUserSetting } from '../../../state/actions/user-settings-actions/user-settings-actions'; import { getTemporaryDisplayPackageInfo } from '../../../state/selectors/resource-selectors'; -import { SHOW_CLASSIFICATIONS_KEY } from '../../../state/variables/use-show-classifications'; import { renderComponent } from '../../../test-helpers/render'; -import { setUserSetting } from '../../../test-helpers/user-settings-helpers'; import { generatePurl } from '../../../util/handle-purl'; import { AttributionForm } from '../AttributionForm'; @@ -241,7 +240,7 @@ describe('AttributionForm', () => { }); renderComponent(, { - actions: [setUserSetting('showCriticality', false)], + actions: [setUserSetting({ showCriticality: false })], }); const criticalityChip = screen.queryByTestId( @@ -258,7 +257,7 @@ describe('AttributionForm', () => { }); renderComponent(, { - actions: [setUserSetting(SHOW_CLASSIFICATIONS_KEY, true)], + actions: [setUserSetting({ showClassifications: true })], }); const classificationChip = screen.getByTestId( @@ -273,7 +272,7 @@ describe('AttributionForm', () => { }); renderComponent(, { - actions: [setUserSetting(SHOW_CLASSIFICATIONS_KEY, false)], + actions: [setUserSetting({ showClassifications: false })], }); const classificationChip = screen.queryByTestId( diff --git a/src/Frontend/Components/PackageCard/PackageCard.tsx b/src/Frontend/Components/PackageCard/PackageCard.tsx index 5698d648d..121434e80 100644 --- a/src/Frontend/Components/PackageCard/PackageCard.tsx +++ b/src/Frontend/Components/PackageCard/PackageCard.tsx @@ -143,8 +143,8 @@ export const PackageCard = memo( }), [cardConfig, packageInfo, classification_mapping], ); - const [showClassifications] = useShowClassifications(); - const [showCriticality] = useShowCriticality(); + const showClassifications = useShowClassifications(); + const showCriticality = useShowCriticality(); const rightIcons = useMemo( () => getRightIcons( diff --git a/src/Frontend/Components/PackageCard/__tests__/PackageCard.test.tsx b/src/Frontend/Components/PackageCard/__tests__/PackageCard.test.tsx index b3b0778d4..dd01d15d6 100644 --- a/src/Frontend/Components/PackageCard/__tests__/PackageCard.test.tsx +++ b/src/Frontend/Components/PackageCard/__tests__/PackageCard.test.tsx @@ -7,9 +7,8 @@ import { screen } from '@testing-library/react'; import { Criticality, RawCriticality } from '../../../../shared/shared-types'; import { faker } from '../../../../testing/Faker'; import { setConfig } from '../../../state/actions/resource-actions/all-views-simple-actions'; -import { SHOW_CLASSIFICATIONS_KEY } from '../../../state/variables/use-show-classifications'; +import { setUserSetting } from '../../../state/actions/user-settings-actions/user-settings-actions'; import { renderComponent } from '../../../test-helpers/render'; -import { setUserSetting } from '../../../test-helpers/user-settings-helpers'; import { PackageCard } from '../PackageCard'; describe('The PackageCard', () => { @@ -148,7 +147,7 @@ describe('The PackageCard', () => { 1: faker.opossum.classificationEntry(), }, }), - setUserSetting(SHOW_CLASSIFICATIONS_KEY, false), + setUserSetting({ showClassifications: false }), ], }, ); @@ -194,7 +193,7 @@ describe('The PackageCard', () => { renderComponent( , - { actions: [setUserSetting('showCriticality', false)] }, + { actions: [setUserSetting({ showCriticality: false })] }, ); const criticalityIcon = screen.queryByLabelText('Criticality icon'); diff --git a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx index fcaad0708..144e93769 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx +++ b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx @@ -94,8 +94,8 @@ export const ProjectStatisticsPopup: React.FC = () => { const [selectedTab, setSelectedTab] = useState(0); - const [showClassifications] = useShowClassifications(); - const [showCriticality] = useShowCriticality(); + const showClassifications = useShowClassifications(); + const showCriticality = useShowCriticality(); return ( { it('does not show the classification statistics if it has been disabled', () => { renderComponent(, { actions: [ - setUserSetting(SHOW_CLASSIFICATIONS_KEY, false), + setUserSetting({ showClassifications: false }), loadFromFile(getParsedInputFileEnrichedWithTestData(fileSetup)), ], }); @@ -323,7 +321,7 @@ describe('The ProjectStatisticsPopup', () => { it('does not show the criticality statistics if it has been disabled', () => { renderComponent(, { actions: [ - setUserSetting(SHOW_CRITICALITY_KEY, false), + setUserSetting({ showCriticality: false }), loadFromFile(getParsedInputFileEnrichedWithTestData(fileSetup)), ], }); diff --git a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx index e2c84daab..fc8a75c69 100644 --- a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx +++ b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx @@ -55,8 +55,8 @@ export function ResourcesTreeNode({ node, nodeId, nodeName }: TreeNode) { const canHaveChildren = node !== 1; const classification_mapping = useAppSelector(getClassifications); - const [showClassifications] = useShowClassifications(); - const [showCriticality] = useShowCriticality(); + const showClassifications = useShowClassifications(); + const showCriticality = useShowCriticality(); return ( ; export function useSortConfiguration(): SortConfiguration { - const [showClassifications] = useShowClassifications(); - const [showCriticality] = useShowCriticality(); + const showClassifications = useShowClassifications(); + const showCriticality = useShowCriticality(); return useMemo(() => { return { diff --git a/src/Frontend/Components/SwitchableProcessBar/SwitchableProcessBar.tsx b/src/Frontend/Components/SwitchableProcessBar/SwitchableProcessBar.tsx index 37fd08218..d661e75fd 100644 --- a/src/Frontend/Components/SwitchableProcessBar/SwitchableProcessBar.tsx +++ b/src/Frontend/Components/SwitchableProcessBar/SwitchableProcessBar.tsx @@ -43,8 +43,8 @@ interface ProgressBarSwitchConfiguration { } export const SwitchableProcessBar: React.FC = () => { - const [showClassifications] = useShowClassifications(); - const [showCriticality] = useShowCriticality(); + const showClassifications = useShowClassifications(); + const showCriticality = useShowCriticality(); const switchableProgressBarConfiguration: Record< SelectedProgressBar, diff --git a/src/Frontend/Components/SwitchableProcessBar/__tests__/SwitchableProcessBar.test.tsx b/src/Frontend/Components/SwitchableProcessBar/__tests__/SwitchableProcessBar.test.tsx index bed9605eb..79d17ea40 100644 --- a/src/Frontend/Components/SwitchableProcessBar/__tests__/SwitchableProcessBar.test.tsx +++ b/src/Frontend/Components/SwitchableProcessBar/__tests__/SwitchableProcessBar.test.tsx @@ -6,10 +6,10 @@ import { fireEvent, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { text } from '../../../../shared/text'; +import { setUserSetting } from '../../../state/actions/user-settings-actions/user-settings-actions'; import { setVariable } from '../../../state/actions/variables-actions/variables-actions'; import { PROGRESS_DATA } from '../../../state/variables/use-progress-data'; import { renderComponent } from '../../../test-helpers/render'; -import { setUserSetting } from '../../../test-helpers/user-settings-helpers'; import { ProgressBarData } from '../../../types/types'; import { SwitchableProcessBar } from '../SwitchableProcessBar'; @@ -114,7 +114,7 @@ describe('SwitchableProcessBar', () => { renderComponent(, { actions: [ setVariable(PROGRESS_DATA, PROGRESS_BAR_DATA), - setUserSetting('showClassifications', true), + setUserSetting({ showClassifications: true }), ], }); @@ -136,7 +136,7 @@ describe('SwitchableProcessBar', () => { renderComponent(, { actions: [ setVariable(PROGRESS_DATA, PROGRESS_BAR_DATA), - setUserSetting('showClassifications', false), + setUserSetting({ showClassifications: false }), ], }); @@ -154,7 +154,7 @@ describe('SwitchableProcessBar', () => { renderComponent(, { actions: [ setVariable(PROGRESS_DATA, PROGRESS_BAR_DATA), - setUserSetting('showCriticality', false), + setUserSetting({ showCriticality: false }), ], }); @@ -172,8 +172,7 @@ describe('SwitchableProcessBar', () => { renderComponent(, { actions: [ setVariable(PROGRESS_DATA, PROGRESS_BAR_DATA), - setUserSetting('showCriticality', false), - setUserSetting('showClassifications', false), + setUserSetting({ showCriticality: false, showClassifications: false }), ], }); diff --git a/src/Frontend/state/reducers/user-settings-reducer.ts b/src/Frontend/state/reducers/user-settings-reducer.ts index 00e77fc32..368d6c02a 100644 --- a/src/Frontend/state/reducers/user-settings-reducer.ts +++ b/src/Frontend/state/reducers/user-settings-reducer.ts @@ -2,8 +2,8 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 -import { UserSettings } from '../../../ElectronBackend/main/user-settings'; import { DEFAULT_USER_SETTINGS } from '../../../shared/shared-constants'; +import { UserSettings } from '../../../shared/shared-types'; import { ACTION_SET_USER_SETTING, UserSettingsAction, diff --git a/src/Frontend/state/selectors/user-settings-selector.ts b/src/Frontend/state/selectors/user-settings-selector.ts new file mode 100644 index 000000000..517971089 --- /dev/null +++ b/src/Frontend/state/selectors/user-settings-selector.ts @@ -0,0 +1,13 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import { State } from '../../types/types'; + +export function getShowCriticality(state: State): boolean { + return state.userSettingsState.showCriticality; +} + +export function getShowClassifications(state: State): boolean { + return state.userSettingsState.showClassifications; +} diff --git a/src/Frontend/state/variables/use-show-classifications.ts b/src/Frontend/state/variables/use-show-classifications.ts index 355da75bc..9eaa92c7d 100644 --- a/src/Frontend/state/variables/use-show-classifications.ts +++ b/src/Frontend/state/variables/use-show-classifications.ts @@ -2,13 +2,11 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 -import { useUserSetting } from './use-user-setting'; +import { useAppSelector } from '../hooks'; +import { getShowClassifications } from '../selectors/user-settings-selector'; export const SHOW_CLASSIFICATIONS_KEY = 'showClassifications'; export function useShowClassifications() { - return useUserSetting({ - key: SHOW_CLASSIFICATIONS_KEY, - defaultValue: true, - }); + return useAppSelector(getShowClassifications); } diff --git a/src/Frontend/state/variables/use-show-criticality.ts b/src/Frontend/state/variables/use-show-criticality.ts index fd4f75e3f..2120ac074 100644 --- a/src/Frontend/state/variables/use-show-criticality.ts +++ b/src/Frontend/state/variables/use-show-criticality.ts @@ -2,13 +2,9 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 -import { useUserSetting } from './use-user-setting'; - -export const SHOW_CRITICALITY_KEY = 'showCriticality'; +import { useAppSelector } from '../hooks'; +import { getShowCriticality } from '../selectors/user-settings-selector'; export function useShowCriticality() { - return useUserSetting({ - key: SHOW_CRITICALITY_KEY, - defaultValue: true, - }); + return useAppSelector(getShowCriticality); } diff --git a/src/Frontend/state/variables/use-user-setting.ts b/src/Frontend/state/variables/use-user-setting.ts index edc46d84b..20268c83a 100644 --- a/src/Frontend/state/variables/use-user-setting.ts +++ b/src/Frontend/state/variables/use-user-setting.ts @@ -87,16 +87,6 @@ export function useUserSetting( })(); }, [readStoredValue, setStoredValue]); - useIpcRenderer( - AllowedFrontendChannels.UserSettingsChanged, - async () => - setVariable({ - hydrated: true, - storedValue: await readStoredValue(), - }), - [readStoredValue], - ); - return [storedValue, setStoredValue, hydrated]; } diff --git a/src/Frontend/test-helpers/user-settings-helpers.ts b/src/Frontend/test-helpers/user-settings-helpers.ts deleted file mode 100644 index b23e65552..000000000 --- a/src/Frontend/test-helpers/user-settings-helpers.ts +++ /dev/null @@ -1,16 +0,0 @@ -// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates -// SPDX-FileCopyrightText: TNG Technology Consulting GmbH -// -// SPDX-License-Identifier: Apache-2.0 -import { UserSettings } from '../../shared/shared-types'; -import { setVariable } from '../state/actions/variables-actions/variables-actions'; - -export function setUserSetting( - key: T, - value: NonNullable, -) { - return setVariable(key, { - hydrated: true, - storedValue: value, - }); -} diff --git a/src/Frontend/types/types.ts b/src/Frontend/types/types.ts index d4528191d..68fa54dd1 100644 --- a/src/Frontend/types/types.ts +++ b/src/Frontend/types/types.ts @@ -7,6 +7,7 @@ import { Classification, Criticality, FileFormatInfo, + UserSettings, } from '../../shared/shared-types'; import { PopupType } from '../enums/enums'; import { ResourceState } from '../state/reducers/resource-reducer'; @@ -17,6 +18,7 @@ export type State = { resourceState: ResourceState; viewState: ViewState; variablesState: VariablesState; + userSettingsState: UserSettings; }; export type SelectedProgressBar = From 769c62331618f4721bc1fcb41a178cf4e563ed27 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Tue, 18 Mar 2025 16:53:24 +0100 Subject: [PATCH 07/18] chore: move the remaining usage of use user settings to the new way Signed-off-by: Dominikus Hellgartner --- src/ElectronBackend/main/main.ts | 7 +- .../main/user-settings-provider.ts | 18 +++- src/ElectronBackend/preload.ts | 6 +- .../AuditingOptions/AuditingOptions.util.tsx | 4 +- .../AttributionPanels/AttributionPanels.tsx | 12 ++- .../IncludeExcludeButton.tsx | 10 ++- .../RestoreButton/RestoreButton.tsx | 2 +- .../ProjectStatisticsPopup.tsx | 15 ++-- .../ResourceBrowser/ResourceBrowser.tsx | 15 ++-- .../user-settings-actions.ts | 46 +++++++++- .../state/selectors/user-settings-selector.ts | 17 ++++ .../__tests__/use-user-setting.test.ts | 86 ------------------- .../use-are-hidden-signals-visible.ts | 10 +-- .../state/variables/use-panel-sizes.ts | 20 +++-- .../state/variables/use-user-setting.ts | 76 +--------------- .../web-workers/use-signals-worker.ts | 2 +- src/shared/shared-types.ts | 7 +- src/testing/setup-tests.ts | 2 +- 18 files changed, 139 insertions(+), 216 deletions(-) delete mode 100644 src/Frontend/state/variables/__tests__/use-user-setting.test.ts diff --git a/src/ElectronBackend/main/main.ts b/src/ElectronBackend/main/main.ts index 606a64341..91d643a25 100644 --- a/src/ElectronBackend/main/main.ts +++ b/src/ElectronBackend/main/main.ts @@ -6,6 +6,7 @@ import { dialog, ipcMain, Menu, systemPreferences } from 'electron'; import os from 'os'; import { AllowedFrontendChannels, IpcChannel } from '../../shared/ipc-channels'; +import { UserSettings } from '../../shared/shared-types'; import { getMessageBoxContentForErrorsWrapper } from '../errorHandling/errorHandling'; import { createWindow, loadWebApp } from './createWindow'; import { @@ -98,8 +99,10 @@ export async function main(): Promise { ipcMain.handle(IpcChannel.GetFullUserSettings, () => UserSettingsProvider.get(), ); - ipcMain.handle(IpcChannel.SetUserSettings, (_, { key, value }) => - UserSettingsProvider.set(key, value, { skipNotification: true }), + ipcMain.handle( + IpcChannel.SetUserSettings, + (_, userSettings: Partial) => + UserSettingsProvider.update(userSettings, true), ); await loadWebApp(mainWindow); diff --git a/src/ElectronBackend/main/user-settings-provider.ts b/src/ElectronBackend/main/user-settings-provider.ts index bb0b7b0ac..0f7779149 100644 --- a/src/ElectronBackend/main/user-settings-provider.ts +++ b/src/ElectronBackend/main/user-settings-provider.ts @@ -8,7 +8,10 @@ import settings from 'electron-settings'; import { AllowedFrontendChannels } from '../../shared/ipc-channels'; import { DEFAULT_USER_SETTINGS } from '../../shared/shared-constants'; -import { UserSettings as IUserSettings } from '../../shared/shared-types'; +import { + UserSettings as IUserSettings, + UserSettings, +} from '../../shared/shared-types'; export class UserSettingsProvider { public static async init() { @@ -37,6 +40,19 @@ export class UserSettingsProvider { return settings.get() as unknown as Promise; } + public static async update( + userSettings: Partial, + skipNotification: boolean, + ): Promise { + await Promise.all( + Object.entries(userSettings).map(async ([key, value]) => { + await UserSettingsProvider.set(key as keyof UserSettings, value, { + skipNotification, + }); + }), + ); + } + public static async set( path: T, value: IUserSettings[T], diff --git a/src/ElectronBackend/preload.ts b/src/ElectronBackend/preload.ts index 2a62b801a..f9a6f90e8 100644 --- a/src/ElectronBackend/preload.ts +++ b/src/ElectronBackend/preload.ts @@ -6,7 +6,7 @@ import { contextBridge, ipcRenderer } from 'electron'; import { IpcChannel } from '../shared/ipc-channels'; -import { ElectronAPI } from '../shared/shared-types'; +import { ElectronAPI, UserSettings } from '../shared/shared-types'; const electronAPI: ElectronAPI = { quit: () => ipcRenderer.invoke(IpcChannel.Quit), @@ -36,8 +36,8 @@ const electronAPI: ElectronAPI = { }, getUserSetting: (key) => ipcRenderer.invoke(IpcChannel.GetUserSettings, key), getFullUserSettings: () => ipcRenderer.invoke(IpcChannel.GetFullUserSettings), - setUserSetting: (key, value) => - ipcRenderer.invoke(IpcChannel.SetUserSettings, { key, value }), + setUserSettings: (userSettings: Partial) => + ipcRenderer.invoke(IpcChannel.SetUserSettings, userSettings), }; // This exposes an API to communicate from the window in the frontend with the backend diff --git a/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx b/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx index cfc1f7d09..6493a7d05 100644 --- a/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx +++ b/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx @@ -26,9 +26,9 @@ import { getIsPreferenceFeatureEnabled, getTemporaryDisplayPackageInfo, } from '../../../state/selectors/resource-selectors'; +import { getQaMode } from '../../../state/selectors/user-settings-selector'; import { useShowClassifications } from '../../../state/variables/use-show-classifications'; import { useShowCriticality } from '../../../state/variables/use-show-criticality'; -import { useUserSetting } from '../../../state/variables/use-user-setting'; import { prettifySource } from '../../../util/prettify-source'; import { ClassificationIcon, @@ -58,7 +58,7 @@ export function useAuditingOptions({ }) { const dispatch = useAppDispatch(); const store = useAppStore(); - const [qaMode] = useUserSetting({ key: 'qaMode' }); + const qaMode = useAppSelector(getQaMode); const attributionSources = useAppSelector(getExternalAttributionSources); const isPreferenceFeatureEnabled = useAppSelector( getIsPreferenceFeatureEnabled, diff --git a/src/Frontend/Components/AttributionPanels/AttributionPanels.tsx b/src/Frontend/Components/AttributionPanels/AttributionPanels.tsx index 1563368ac..4741d4a78 100644 --- a/src/Frontend/Components/AttributionPanels/AttributionPanels.tsx +++ b/src/Frontend/Components/AttributionPanels/AttributionPanels.tsx @@ -21,18 +21,16 @@ export function AttributionPanels() { const [{ search: attributionSearch }, setFilteredAttributions] = useFilteredAttributions(); const [{ search: signalSearch }, setFilteredSignals] = useFilteredSignals(); - const [{ packageListsWidth, signalsPanelHeight }, setPanelSizes] = - usePanelSizes(); + const { panelSizes, setPanelSizes } = usePanelSizes(); const setWidth = useCallback( - (width: number) => - setPanelSizes((prev) => ({ ...prev, packageListsWidth: width })), + (width: number) => setPanelSizes({ packageListsWidth: width }), [setPanelSizes], ); const setHeight = useCallback( (height: number) => { - setPanelSizes((prev) => ({ ...prev, signalsPanelHeight: height })); + setPanelSizes({ signalsPanelHeight: height }); }, [setPanelSizes], ); @@ -42,8 +40,8 @@ export function AttributionPanels() { { - const [areHiddenSignalsVisible, setAreHiddenSignalsVisible] = - useAreHiddenSignalsVisible(); + const dispatch = useAppDispatch(); + const areHiddenSignalsVisible = useAreHiddenSignalsVisible(); const label = areHiddenSignalsVisible ? text.packageLists.hideDeleted : text.packageLists.showDeleted; @@ -22,7 +24,9 @@ export const IncludeExcludeButton: React.FC = () => { setAreHiddenSignalsVisible((prev) => !prev)} + onClick={() => { + dispatch(toggleAreHiddenSignalsVisible); + }} > diff --git a/src/Frontend/Components/AttributionPanels/SignalsPanel/RestoreButton/RestoreButton.tsx b/src/Frontend/Components/AttributionPanels/SignalsPanel/RestoreButton/RestoreButton.tsx index 7dae8636a..d32281623 100644 --- a/src/Frontend/Components/AttributionPanels/SignalsPanel/RestoreButton/RestoreButton.tsx +++ b/src/Frontend/Components/AttributionPanels/SignalsPanel/RestoreButton/RestoreButton.tsx @@ -21,7 +21,7 @@ export const RestoreButton: React.FC = ({ const resolvedExternalAttributionIds = useAppSelector( getResolvedExternalAttributions, ); - const [areHiddenSignalsVisible] = useAreHiddenSignalsVisible(); + const areHiddenSignalsVisible = useAreHiddenSignalsVisible(); const someSelectedAttributionsAreHidden = useMemo( () => !!selectedAttributionIds.length && diff --git a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx index 144e93769..f23ee93dd 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx +++ b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx @@ -13,6 +13,7 @@ import { PropsWithChildren, useState } from 'react'; import { Criticality } from '../../../shared/shared-types'; import { text } from '../../../shared/text'; import { criticalityColor } from '../../shared-styles'; +import { updateUserSettings } from '../../state/actions/user-settings-actions/user-settings-actions'; import { closePopup } from '../../state/actions/view-actions/view-actions'; import { useAppDispatch, useAppSelector } from '../../state/hooks'; import { @@ -21,9 +22,9 @@ import { getManualAttributions, getUnresolvedExternalAttributions, } from '../../state/selectors/resource-selectors'; +import { getShowProjectStatistics } from '../../state/selectors/user-settings-selector'; import { useShowClassifications } from '../../state/variables/use-show-classifications'; import { useShowCriticality } from '../../state/variables/use-show-criticality'; -import { useUserSetting } from '../../state/variables/use-user-setting'; import { AttributionCountPerSourcePerLicenseTable } from '../AttributionCountPerSourcePerLicenseTable/AttributionCountPerSourcePerLicenseTable'; import { BarChart } from '../BarChart/BarChart'; import { Checkbox } from '../Checkbox/Checkbox'; @@ -89,8 +90,7 @@ export const ProjectStatisticsPopup: React.FC = () => { dispatch(closePopup()); } - const [showProjectStatistics, setShowProjectStatistics, hydrated] = - useUserSetting({ defaultValue: true, key: 'showProjectStatistics' }); + const showProjectStatistics = useAppSelector(getShowProjectStatistics); const [selectedTab, setSelectedTab] = useState(0); @@ -206,8 +206,13 @@ export const ProjectStatisticsPopup: React.FC = () => { customAction={ setShowProjectStatistics(event.target.checked)} - disabled={!hydrated} + onChange={(event) => + dispatch( + updateUserSettings({ + showProjectStatistics: event.target.checked, + }), + ) + } label={text.projectStatisticsPopup.toggleStartupCheckbox} /> } diff --git a/src/Frontend/Components/ResourceBrowser/ResourceBrowser.tsx b/src/Frontend/Components/ResourceBrowser/ResourceBrowser.tsx index 12c2edfb7..073fbe315 100644 --- a/src/Frontend/Components/ResourceBrowser/ResourceBrowser.tsx +++ b/src/Frontend/Components/ResourceBrowser/ResourceBrowser.tsx @@ -37,8 +37,7 @@ export function ResourceBrowser() { ); const debouncedSearchAll = useDebouncedInput(searchAll); const debouncedSearchLinked = useDebouncedInput(searchLinked); - const [{ resourceBrowserWidth, linkedResourcesPanelHeight }, setPanelSizes] = - usePanelSizes(); + const { panelSizes, setPanelSizes } = usePanelSizes(); const allResourcesFiltered = useMemo( () => @@ -62,17 +61,15 @@ export function ResourceBrowser() { ); const setWidth = useCallback( - (width: number) => - setPanelSizes((prev) => ({ ...prev, resourceBrowserWidth: width })), + (width: number) => setPanelSizes({ resourceBrowserWidth: width }), [setPanelSizes], ); const setHeight = useCallback( (height: number) => { - setPanelSizes((prev) => ({ - ...prev, + setPanelSizes({ linkedResourcesPanelHeight: height, - })); + }); }, [setPanelSizes], ); @@ -84,8 +81,8 @@ export function ResourceBrowser() { return ( // // SPDX-License-Identifier: Apache-2.0 -import { UserSettings } from '../../../../shared/shared-types'; -import { AppThunkAction } from '../../types'; +import { PanelSizes, UserSettings } from '../../../../shared/shared-types'; +import { State } from '../../../types/types'; +import { + getAreHiddenSignalsVisible, + getPanelSizes, +} from '../../selectors/user-settings-selector'; +import { AppThunkAction, AppThunkDispatch } from '../../types'; import { ACTION_SET_USER_SETTING, SetUserSetting } from './types'; export function setUserSetting(setting: Partial): SetUserSetting { @@ -19,3 +24,40 @@ export function fetchUserSettings(): AppThunkAction { dispatch(setUserSetting(userSettings)); }; } + +export function updateUserSettings( + userSettings: Partial, +): AppThunkAction { + return async (dispatch) => { + await window.electronAPI.setUserSettings(userSettings); + dispatch(setUserSetting(userSettings)); + }; +} + +export function updatePanelSizes( + panelSizes: Partial, +): AppThunkAction { + return (dispatch, getState): void => { + const currentState = getState(); + const currentPanelSizes = getPanelSizes(currentState); + dispatch( + updateUserSettings({ + panelSizes: { ...currentPanelSizes, ...panelSizes }, + }), + ); + }; +} + +export function toggleAreHiddenSignalsVisible( + dispatch: AppThunkDispatch, + getState: () => State, +): void { + const currentState = getState(); + const currentAreHiddenSignalsVisible = + getAreHiddenSignalsVisible(currentState); + dispatch( + updateUserSettings({ + areHiddenSignalsVisible: !currentAreHiddenSignalsVisible, + }), + ); +} diff --git a/src/Frontend/state/selectors/user-settings-selector.ts b/src/Frontend/state/selectors/user-settings-selector.ts index 517971089..d4294e878 100644 --- a/src/Frontend/state/selectors/user-settings-selector.ts +++ b/src/Frontend/state/selectors/user-settings-selector.ts @@ -2,6 +2,7 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 +import { UserSettings } from '../../../shared/shared-types'; import { State } from '../../types/types'; export function getShowCriticality(state: State): boolean { @@ -11,3 +12,19 @@ export function getShowCriticality(state: State): boolean { export function getShowClassifications(state: State): boolean { return state.userSettingsState.showClassifications; } + +export function getQaMode(state: State): boolean { + return state.userSettingsState.qaMode; +} + +export function getShowProjectStatistics(state: State): boolean { + return state.userSettingsState.showProjectStatistics; +} + +export function getAreHiddenSignalsVisible(state: State): boolean { + return state.userSettingsState.areHiddenSignalsVisible; +} + +export function getPanelSizes(state: State): UserSettings['panelSizes'] { + return state.userSettingsState.panelSizes; +} diff --git a/src/Frontend/state/variables/__tests__/use-user-setting.test.ts b/src/Frontend/state/variables/__tests__/use-user-setting.test.ts deleted file mode 100644 index 6f0bf566f..000000000 --- a/src/Frontend/state/variables/__tests__/use-user-setting.test.ts +++ /dev/null @@ -1,86 +0,0 @@ -// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates -// SPDX-FileCopyrightText: TNG Technology Consulting GmbH -// -// SPDX-License-Identifier: Apache-2.0 -import { act, waitFor } from '@testing-library/react'; - -import { ElectronAPI } from '../../../../shared/shared-types'; -import { faker } from '../../../../testing/Faker'; -import { renderHook } from '../../../test-helpers/render'; -import { useUserSetting } from '../use-user-setting'; - -const mockGetUserSetting = jest.fn(); -const mockSetUserSetting = jest.fn(); - -const electronAPI: Pick< - ElectronAPI, - 'setUserSetting' | 'getUserSetting' | 'on' -> = { - getUserSetting: mockGetUserSetting, - setUserSetting: mockSetUserSetting, - on: jest.fn().mockReturnValue(jest.fn()), -}; - -describe('useUserSetting', () => { - beforeEach(() => { - window.electronAPI = electronAPI as ElectronAPI; - }); - - it('returns default value when no stored value exist', async () => { - const defaultValue = faker.datatype.boolean(); - const { result } = renderHook(() => - useUserSetting({ key: 'showProjectStatistics', defaultValue }), - ); - - await waitFor(() => expect(result.current[2]).toBe(true)); - - expect(result.current[0]).toBe(defaultValue); - expect(mockGetUserSetting).toHaveBeenCalledTimes(1); - expect(mockSetUserSetting).toHaveBeenCalledTimes(1); - }); - - it('returns default value as long as not hydrated', () => { - const defaultValue = faker.datatype.boolean(); - const storedValue = !defaultValue; - mockGetUserSetting.mockReturnValue(storedValue); - const { result } = renderHook(() => - useUserSetting({ key: 'showProjectStatistics', defaultValue }), - ); - - expect(result.current[0]).toBe(defaultValue); - expect(mockGetUserSetting).toHaveBeenCalledTimes(1); - expect(mockSetUserSetting).not.toHaveBeenCalled(); - }); - - it('returns stored value', async () => { - const defaultValue = faker.datatype.boolean(); - const storedValue = !defaultValue; - mockGetUserSetting.mockReturnValue(storedValue); - const { result } = renderHook(() => - useUserSetting({ key: 'showProjectStatistics', defaultValue }), - ); - - await waitFor(() => expect(result.current[2]).toBe(true)); - - expect(result.current[0]).toBe(storedValue); - expect(mockGetUserSetting).toHaveBeenCalledTimes(1); - expect(mockSetUserSetting).toHaveBeenCalledTimes(1); - }); - - it('updates user setting', async () => { - const defaultValue = faker.datatype.boolean(); - const newValue = !defaultValue; - const { result } = renderHook(() => - useUserSetting({ key: 'showProjectStatistics', defaultValue }), - ); - - await waitFor(() => expect(result.current[2]).toBe(true)); - act(() => { - result.current[1](newValue); - }); - - expect(result.current[0]).toBe(newValue); - expect(mockGetUserSetting).toHaveBeenCalledTimes(1); - expect(mockSetUserSetting).toHaveBeenCalledTimes(2); - }); -}); diff --git a/src/Frontend/state/variables/use-are-hidden-signals-visible.ts b/src/Frontend/state/variables/use-are-hidden-signals-visible.ts index ed7df2ba4..a12ec66f4 100644 --- a/src/Frontend/state/variables/use-are-hidden-signals-visible.ts +++ b/src/Frontend/state/variables/use-are-hidden-signals-visible.ts @@ -2,13 +2,9 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 -import { useUserSetting } from './use-user-setting'; - -export const ARE_HIDDEN_SIGNALS_VISIBLE = 'are-hidden-signals-visible'; +import { useAppSelector } from '../hooks'; +import { getAreHiddenSignalsVisible } from '../selectors/user-settings-selector'; export function useAreHiddenSignalsVisible() { - return useUserSetting({ - defaultValue: false, - key: 'areHiddenSignalsVisible', - }); + return useAppSelector(getAreHiddenSignalsVisible); } diff --git a/src/Frontend/state/variables/use-panel-sizes.ts b/src/Frontend/state/variables/use-panel-sizes.ts index 0a24f6035..56daa8677 100644 --- a/src/Frontend/state/variables/use-panel-sizes.ts +++ b/src/Frontend/state/variables/use-panel-sizes.ts @@ -2,12 +2,18 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 -import { DEFAULT_PANEL_SIZES } from '../../../shared/shared-constants'; -import { useUserSetting } from './use-user-setting'; +import { PanelSizes } from '../../../shared/shared-types'; +import { updatePanelSizes } from '../actions/user-settings-actions/user-settings-actions'; +import { useAppDispatch, useAppSelector } from '../hooks'; +import { getPanelSizes } from '../selectors/user-settings-selector'; -export const usePanelSizes = () => { - return useUserSetting({ - defaultValue: DEFAULT_PANEL_SIZES, - key: 'panelSizes', - }); +export const usePanelSizes = (): { + panelSizes: PanelSizes; + setPanelSizes: (panelsSizes: Partial) => void; +} => { + const panelSizes = useAppSelector(getPanelSizes); + const dispatch = useAppDispatch(); + const setPanelSizes = (panelSizes: Partial) => + dispatch(updatePanelSizes(panelSizes)); + return { panelSizes, setPanelSizes }; }; diff --git a/src/Frontend/state/variables/use-user-setting.ts b/src/Frontend/state/variables/use-user-setting.ts index 20268c83a..f8c2b7bb9 100644 --- a/src/Frontend/state/variables/use-user-setting.ts +++ b/src/Frontend/state/variables/use-user-setting.ts @@ -2,8 +2,7 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 -import { isFunction } from 'lodash'; -import { DependencyList, useCallback, useEffect } from 'react'; +import { useEffect } from 'react'; import { AllowedFrontendChannels } from '../../../shared/ipc-channels'; import { UserSettings } from '../../../shared/shared-types'; @@ -16,79 +15,6 @@ import { setUserSetting, } from '../actions/user-settings-actions/user-settings-actions'; import { useAppDispatch } from '../hooks'; -import { useVariable } from './use-variable'; - -/** - * Use this hook to get and set app-wide user settings. - * @param props Specify the user setting key and its default value while hydrating. - * @param deps Dependency array of the hook. - * @returns A tuple containing the current value, a setter function and a boolean indicating whether the value has been hydrated. - */ -export function useUserSetting( - { defaultValue, key }: { defaultValue?: never; key: T }, - deps?: DependencyList, -): [UserSettings[T], (newValue: UserSettings[T]) => void, boolean]; -export function useUserSetting( - { defaultValue, key }: { defaultValue: NonNullable; key: T }, - deps?: DependencyList, -): [ - NonNullable, - ( - newValue: - | UserSettings[T] - | ((prev: NonNullable) => UserSettings[T]), - ) => void, - boolean, -]; -export function useUserSetting( - { defaultValue, key }: { defaultValue?: UserSettings[T]; key: T }, - deps: DependencyList = [], -): [UserSettings[T] | undefined, (newValue: UserSettings[T]) => void, boolean] { - const [{ hydrated, storedValue }, setVariable] = useVariable(key, { - hydrated: false, - storedValue: defaultValue, - }); - - const setStoredValue = useCallback( - ( - newValue: - | UserSettings[T] - | ((prev: UserSettings[T] | undefined) => UserSettings[T]), - ): void => { - setVariable((prev) => { - const effectiveNewValue = isFunction(newValue) - ? newValue(prev.storedValue) - : newValue; - void window.electronAPI.setUserSetting(key, effectiveNewValue); - return { - hydrated: true, - storedValue: effectiveNewValue ?? defaultValue, - }; - }); - }, - // eslint-disable-next-line react-hooks/exhaustive-deps - deps, - ); - - const readStoredValue = useCallback( - async (): Promise => { - const value = await window.electronAPI.getUserSetting(key); - - return value ?? defaultValue; - }, - // eslint-disable-next-line react-hooks/exhaustive-deps - deps, - ); - - useEffect(() => { - void (async (): Promise => { - const storedValue = await readStoredValue(); - storedValue !== undefined && setStoredValue(storedValue); - })(); - }, [readStoredValue, setStoredValue]); - - return [storedValue, setStoredValue, hydrated]; -} // should only be called once export function useInitialSyncUserSettings() { diff --git a/src/Frontend/web-workers/use-signals-worker.ts b/src/Frontend/web-workers/use-signals-worker.ts index d65a992b7..531d2d1cd 100644 --- a/src/Frontend/web-workers/use-signals-worker.ts +++ b/src/Frontend/web-workers/use-signals-worker.ts @@ -70,7 +70,7 @@ export function useSignalsWorker() { ] = useFilteredAttributionsInReportView(); const debouncedSignalSearch = useDebouncedInput(signalSearch); const debouncedAttributionSearch = useDebouncedInput(attributionSearch); - const [areHiddenSignalsVisible] = useAreHiddenSignalsVisible(); + const areHiddenSignalsVisible = useAreHiddenSignalsVisible(); const [, setProgressData] = useProgressData(); useEffect(() => { diff --git a/src/shared/shared-types.ts b/src/shared/shared-types.ts index 06b804196..92bcfc07c 100644 --- a/src/shared/shared-types.ts +++ b/src/shared/shared-types.ts @@ -300,10 +300,7 @@ export interface ElectronAPI { key: T, ) => Promise; getFullUserSettings: () => Promise; - setUserSetting: ( - key: T, - value: UserSettings[T], - ) => Promise; + setUserSettings: (userSettings: Partial) => Promise; } declare global { @@ -331,3 +328,5 @@ export interface UserSettings { signalsPanelHeight: number | null; }; } + +export type PanelSizes = UserSettings['panelSizes']; diff --git a/src/testing/setup-tests.ts b/src/testing/setup-tests.ts index 1f30cec0c..a31f5b557 100644 --- a/src/testing/setup-tests.ts +++ b/src/testing/setup-tests.ts @@ -47,7 +47,7 @@ global.window.electronAPI = { on: jest.fn().mockReturnValue(jest.fn()), getUserSetting: jest.fn().mockReturnValue(undefined), getFullUserSettings: jest.fn().mockReturnValue(DEFAULT_USER_SETTINGS), - setUserSetting: jest.fn(), + setUserSettings: jest.fn(), } satisfies ElectronAPI; window.ResizeObserver = ResizeObserver; From 0f3855f421a61fa9095dd42cbf2893aa6d71ac85 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Wed, 19 Mar 2025 08:50:02 +0100 Subject: [PATCH 08/18] test: fix e2e tests Signed-off-by: Dominikus Hellgartner --- .../__tests__/ProjectStatisticsPopup.test.tsx | 4 +++- src/e2e-tests/utils/fixtures.ts | 5 +++-- src/shared/shared-constants.ts | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx b/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx index dede31622..a6e86f29c 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx +++ b/src/Frontend/Components/ProjectStatisticsPopup/__tests__/ProjectStatisticsPopup.test.tsx @@ -334,7 +334,9 @@ describe('The ProjectStatisticsPopup', () => { }); it('allows toggling of show-on-startup checkbox', async () => { - renderComponent(); + renderComponent(, { + actions: [setUserSetting({ showProjectStatistics: true })], + }); expect( screen.getByLabelText(text.projectStatisticsPopup.toggleStartupCheckbox), diff --git a/src/e2e-tests/utils/fixtures.ts b/src/e2e-tests/utils/fixtures.ts index 60f4dcb2c..04f320d33 100644 --- a/src/e2e-tests/utils/fixtures.ts +++ b/src/e2e-tests/utils/fixtures.ts @@ -102,8 +102,9 @@ export const test = base.extend<{ width: 1920, height: 1080, }); - // eslint-disable-next-line playwright/no-networkidle - await window.waitForLoadState('networkidle', { timeout: LOAD_TIMEOUT }); + await window.waitForLoadState('domcontentloaded', { + timeout: LOAD_TIMEOUT, + }); await window .context() .tracing.start({ screenshots: true, snapshots: true }); diff --git a/src/shared/shared-constants.ts b/src/shared/shared-constants.ts index bb08ad0eb..ef04c47ce 100644 --- a/src/shared/shared-constants.ts +++ b/src/shared/shared-constants.ts @@ -13,7 +13,7 @@ export const DEFAULT_PANEL_SIZES: NonNullable = { export const DEFAULT_USER_SETTINGS: UserSettings = { qaMode: false, - showProjectStatistics: true, + showProjectStatistics: false, areHiddenSignalsVisible: false, showCriticality: true, showClassifications: true, From 2e3347169d62f246b656895ab721175ec4531e3c Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Wed, 19 Mar 2025 10:48:31 +0100 Subject: [PATCH 09/18] fix: fix updating multiple values tests showed: electron-settings do not reliably update if multiple updates are pending at the same time Signed-off-by: Dominikus Hellgartner --- .../__tests__/user-settings-provider.test.ts | 149 ++++++++++++++++++ .../main/__tests__/user-settings.test.ts | 58 ------- .../main/user-settings-provider.ts | 13 +- 3 files changed, 156 insertions(+), 64 deletions(-) create mode 100644 src/ElectronBackend/main/__tests__/user-settings-provider.test.ts delete mode 100644 src/ElectronBackend/main/__tests__/user-settings.test.ts diff --git a/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts b/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts new file mode 100644 index 000000000..58f3a4c1c --- /dev/null +++ b/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts @@ -0,0 +1,149 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import { BrowserWindow } from 'electron'; +import settings from 'electron-settings'; +import { rmSync } from 'node:fs'; +import { mkdtemp } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { AllowedFrontendChannels } from '../../../shared/ipc-channels'; +import { + DEFAULT_PANEL_SIZES, + DEFAULT_USER_SETTINGS, +} from '../../../shared/shared-constants'; +import { UserSettingsProvider } from '../user-settings-provider'; + +type MockedBrowserWindow = BrowserWindow & { + sendFunction: (channel: string, ...args: Array) => void; +}; + +jest.mock('electron', () => { + const sendFunction = jest.fn(); + return { + BrowserWindow: { + getAllWindows: () => { + return [ + { + webContents: { + send: sendFunction, + }, + }, + ]; + }, + sendFunction, + }, + }; +}); +describe('UserSettings', () => { + let temporaryDir: string | undefined = undefined; + beforeEach(async () => { + temporaryDir = await mkdtemp(join(tmpdir(), 'OpossumUiTesting-')); + settings.configure({ dir: temporaryDir, atomicSave: true }); + }); + + afterEach(() => { + if (temporaryDir) { + rmSync(temporaryDir, { recursive: true, force: true }); + } + }); + + describe('init', () => { + it('sets up the default values if empty', async () => { + await UserSettingsProvider.init(); + + const result = await settings.get(); + + expect(result).toEqual(DEFAULT_USER_SETTINGS); + }); + + it('overwrites only the non set values if there are already values set', async () => { + await settings.set('qaMode', true); + + await UserSettingsProvider.init(); + + const result = await settings.get(); + + expect(result).toEqual({ ...DEFAULT_USER_SETTINGS, qaMode: true }); + }); + + it('resets everything if reset is requested', async () => { + const oldEnvironment = process.env; + process.env = { ...oldEnvironment, RESET: 'TRUE' }; + + await settings.set('qaMode', true); + + await UserSettingsProvider.init(); + + const result = await settings.get(); + + expect(result).toEqual({ ...DEFAULT_USER_SETTINGS }); + process.env = oldEnvironment; + }); + }); + describe('get', () => { + it('gets a user setting from a predescribed path', async () => { + await UserSettingsProvider.init(); + + const panelSizes = await UserSettingsProvider.get('panelSizes'); + + expect(panelSizes).toEqual(DEFAULT_PANEL_SIZES); + }); + + it('gets the full user settings', async () => { + await UserSettingsProvider.init(); + + const userSettings = await UserSettingsProvider.get(); + + expect(userSettings).toEqual(DEFAULT_USER_SETTINGS); + }); + }); + + describe('write operations', () => { + it('sets a value and communicates to the frontend', async () => { + await UserSettingsProvider.set('qaMode', true); + + const qaMode = await UserSettingsProvider.get('qaMode'); + expect(qaMode).toBe(true); + expect( + (BrowserWindow as unknown as MockedBrowserWindow).sendFunction, + ).toHaveBeenCalledWith(AllowedFrontendChannels.UserSettingsChanged, { + qaMode: true, + }); + }); + + it('sets a value and does not communicates to the frontend if disabled', async () => { + await UserSettingsProvider.set('qaMode', true, { + skipNotification: true, + }); + + const qaMode = await UserSettingsProvider.get('qaMode'); + expect(qaMode).toBe(true); + expect( + (BrowserWindow as unknown as MockedBrowserWindow).sendFunction, + ).not.toHaveBeenCalled(); + }); + + it('allows to update multiple values at once', async () => { + await UserSettingsProvider.init(); + + await UserSettingsProvider.update({ + qaMode: true, + showClassifications: false, + }); + + const userSettings = await UserSettingsProvider.get(); + + expect( + (BrowserWindow as unknown as MockedBrowserWindow).sendFunction, + ).toHaveBeenCalledTimes(2); + expect(userSettings).toEqual({ + ...DEFAULT_USER_SETTINGS, + qaMode: true, + showClassifications: false, + }); + }); + }); +}); diff --git a/src/ElectronBackend/main/__tests__/user-settings.test.ts b/src/ElectronBackend/main/__tests__/user-settings.test.ts deleted file mode 100644 index c64dcd97c..000000000 --- a/src/ElectronBackend/main/__tests__/user-settings.test.ts +++ /dev/null @@ -1,58 +0,0 @@ -// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates -// SPDX-FileCopyrightText: TNG Technology Consulting GmbH -// -// SPDX-License-Identifier: Apache-2.0 -import settings from 'electron-settings'; -import { rmSync } from 'node:fs'; -import { mkdtemp } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; - -import { DEFAULT_USER_SETTINGS } from '../../../shared/shared-constants'; -import { UserSettingsProvider } from '../user-settings-provider'; - -describe('UserSettings', () => { - let temporaryDir: string | undefined = undefined; - beforeEach(async () => { - temporaryDir = await mkdtemp(join(tmpdir(), 'OpossumUiTesting-')); - settings.configure({ dir: temporaryDir }); - }); - - afterEach(() => { - if (temporaryDir) { - rmSync(temporaryDir, { recursive: true, force: true }); - } - }); - - it('sets up the default values if empty', async () => { - await UserSettingsProvider.init(); - - const result = await settings.get(); - - expect(result).toEqual(DEFAULT_USER_SETTINGS); - }); - - it('overwrites only the non set values if there are already values set', async () => { - await settings.set('qaMode', true); - - await UserSettingsProvider.init(); - - const result = await settings.get(); - - expect(result).toEqual({ ...DEFAULT_USER_SETTINGS, qaMode: true }); - }); - - it('resets everything if reset is requested', async () => { - const oldEnvironment = process.env; - process.env = { ...oldEnvironment, RESET: 'TRUE' }; - - await settings.set('qaMode', true); - - await UserSettingsProvider.init(); - - const result = await settings.get(); - - expect(result).toEqual({ ...DEFAULT_USER_SETTINGS }); - process.env = oldEnvironment; - }); -}); diff --git a/src/ElectronBackend/main/user-settings-provider.ts b/src/ElectronBackend/main/user-settings-provider.ts index 0f7779149..93bc1988d 100644 --- a/src/ElectronBackend/main/user-settings-provider.ts +++ b/src/ElectronBackend/main/user-settings-provider.ts @@ -42,15 +42,16 @@ export class UserSettingsProvider { public static async update( userSettings: Partial, - skipNotification: boolean, + skipNotification: boolean = false, ): Promise { - await Promise.all( - Object.entries(userSettings).map(async ([key, value]) => { - await UserSettingsProvider.set(key as keyof UserSettings, value, { + for (const key of Object.keys(userSettings)) { + const properKey = key as keyof UserSettings; + if (userSettings[properKey] !== undefined) { + await UserSettingsProvider.set(properKey, userSettings[properKey], { skipNotification, }); - }), - ); + } + } } public static async set( From 86a4850fc00c1d96c619273ddd66135d71e80b19 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Wed, 19 Mar 2025 11:40:34 +0100 Subject: [PATCH 10/18] test: add tests for user settings actions Signed-off-by: Dominikus Hellgartner --- .../__tests__/user-settings-actions.test.ts | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts diff --git a/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts b/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts new file mode 100644 index 000000000..a3a719981 --- /dev/null +++ b/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts @@ -0,0 +1,106 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import { waitFor } from '@testing-library/react'; + +import { DEFAULT_USER_SETTINGS } from '../../../../../shared/shared-constants'; +import { faker } from '../../../../../testing/Faker'; +import { createAppStore } from '../../../configure-store'; +import { + getAreHiddenSignalsVisible, + getPanelSizes, + getQaMode, + getShowClassifications, +} from '../../../selectors/user-settings-selector'; +import { + fetchUserSettings, + setUserSetting, + toggleAreHiddenSignalsVisible, + updatePanelSizes, + updateUserSettings, +} from '../user-settings-actions'; + +describe('user-settings-actions', () => { + it('sets the users setting', () => { + const store = createAppStore(); + + store.dispatch( + setUserSetting({ qaMode: true, showClassifications: false }), + ); + + expect(getQaMode(store.getState())).toBe(true); + expect(getShowClassifications(store.getState())).toBe(false); + + store.dispatch(setUserSetting({ qaMode: false })); + expect(getQaMode(store.getState())).toBe(false); + expect(getShowClassifications(store.getState())).toBe(false); + }); + + it('loads the user settings from the backend', async () => { + const store = createAppStore(); + const backendCall = jest.mocked(window.electronAPI.getFullUserSettings); + backendCall.mockReturnValue( + Promise.resolve({ + ...DEFAULT_USER_SETTINGS, + qaMode: true, + }), + ); + + store.dispatch(fetchUserSettings()); + + await waitFor(() => { + expect(getQaMode(store.getState())).toBe(true); + }); + }); + + it('updates user settings and communicates to the backend', async () => { + const store = createAppStore(); + const backendCall = jest.mocked(window.electronAPI.setUserSettings); + + const userSettings = { showClassifications: false }; + store.dispatch(updateUserSettings(userSettings)); + + await waitFor(() => { + expect(getShowClassifications(store.getState())).toBe(false); + }); + expect(backendCall).toHaveBeenCalledWith(userSettings); + }); + + it('updates panel sizes', async () => { + const store = createAppStore(); + const backendCall = jest.mocked(window.electronAPI.setUserSettings); + + const signalsPanelHeight = faker.number.int({ min: 1 }); + store.dispatch(updatePanelSizes({ signalsPanelHeight })); + + await waitFor(() => { + const panelSizes = getPanelSizes(store.getState()); + expect(panelSizes.signalsPanelHeight).toBe(signalsPanelHeight); + }); + expect(backendCall).toHaveBeenCalled(); + }); + + it('toggles hidden signals', async () => { + const store = createAppStore(); + const hiddenSignalsVisibleAtStart = getAreHiddenSignalsVisible( + store.getState(), + ); + + toggleAreHiddenSignalsVisible(store.dispatch, store.getState); + + await waitFor(() => + expect(getAreHiddenSignalsVisible(store.getState())).toBe( + !hiddenSignalsVisibleAtStart, + ), + ); + + toggleAreHiddenSignalsVisible(store.dispatch, store.getState); + + await waitFor(() => + expect(getAreHiddenSignalsVisible(store.getState())).toBe( + hiddenSignalsVisibleAtStart, + ), + ); + }); +}); From 25861efe045cdeecd5a906abe30dc61a95f855e7 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Wed, 19 Mar 2025 13:39:26 +0100 Subject: [PATCH 11/18] test: add e2e test Signed-off-by: Dominikus Hellgartner --- ...showing-classification-criticality.test.ts | 67 +++++++++++++++++++ src/e2e-tests/page-objects/MenuBar.ts | 8 +++ .../page-objects/ProjectStatisticsPopup.ts | 10 +++ 3 files changed, 85 insertions(+) create mode 100644 src/e2e-tests/__tests__/toggle-showing-classification-criticality.test.ts diff --git a/src/e2e-tests/__tests__/toggle-showing-classification-criticality.test.ts b/src/e2e-tests/__tests__/toggle-showing-classification-criticality.test.ts new file mode 100644 index 000000000..4fc51a14b --- /dev/null +++ b/src/e2e-tests/__tests__/toggle-showing-classification-criticality.test.ts @@ -0,0 +1,67 @@ +// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates +// SPDX-FileCopyrightText: TNG Technology Consulting GmbH +// +// SPDX-License-Identifier: Apache-2.0 +import { faker } from '../../testing/Faker'; +import { test } from '../utils'; + +const [resourceName1, resourceName2, resourceName3, resourceName4] = + faker.opossum.resourceNames({ count: 4 }); +const [attributionId1, packageInfo1] = faker.opossum.rawAttribution({ + packageName: 'a', + classification: 0, + criticality: 'medium', +}); +const [attributionId2, packageInfo2] = faker.opossum.rawAttribution({ + packageName: 'b', + classification: 1, + criticality: 'high', +}); + +test.use({ + data: { + inputData: faker.opossum.inputData({ + resources: faker.opossum.resources({ + [resourceName1]: { + [resourceName2]: { + [resourceName3]: 1, + }, + }, + [resourceName4]: 1, + }), + externalAttributions: faker.opossum.rawAttributions({ + [attributionId1]: packageInfo1, + [attributionId2]: packageInfo2, + }), + resourcesToAttributions: faker.opossum.resourcesToAttributions({ + [faker.opossum.filePath(resourceName1, resourceName2, resourceName3)]: [ + attributionId1, + ], + [faker.opossum.filePath(resourceName4)]: [attributionId2], + }), + }), + }, +}); + +test('shows classification and criticality in statistics popup only if selected', async ({ + menuBar, + projectStatisticsPopup, +}) => { + await menuBar.openProjectStatistics(); + await projectStatisticsPopup.assert.titleIsVisible(); + // hover on title to avoid getting tooltips that mess up locators + await projectStatisticsPopup.title.hover(); + + await projectStatisticsPopup.assert.signalsByCriticalityIsVisible(); + await projectStatisticsPopup.assert.signalsByClassificationIsShown(); + + await menuBar.toggleShowClassificationOff(); + + await projectStatisticsPopup.assert.signalsByCriticalityIsVisible(); + await projectStatisticsPopup.assert.signalsByClassificationIsNotShown(); + + await menuBar.toggleShowCriticalityOff(); + + await projectStatisticsPopup.assert.signalsByCriticalityIsNotVisible(); + await projectStatisticsPopup.assert.signalsByClassificationIsNotShown(); +}); diff --git a/src/e2e-tests/page-objects/MenuBar.ts b/src/e2e-tests/page-objects/MenuBar.ts index 00981dcf5..ed83457d3 100644 --- a/src/e2e-tests/page-objects/MenuBar.ts +++ b/src/e2e-tests/page-objects/MenuBar.ts @@ -95,6 +95,14 @@ export class MenuBar { await clickMenuItem(this.window.app, 'label', 'QA Mode'); } + async toggleShowClassificationOff(): Promise { + await clickMenuItemById(this.window.app, 'enabled-show-classifications'); + } + + async toggleShowCriticalityOff(): Promise { + await clickMenuItemById(this.window.app, 'enabled-show-criticality'); + } + async saveChanges(): Promise { await clickMenuItem(this.window.app, 'label', 'Save'); } diff --git a/src/e2e-tests/page-objects/ProjectStatisticsPopup.ts b/src/e2e-tests/page-objects/ProjectStatisticsPopup.ts index 7f40c9b6b..ddcea9781 100644 --- a/src/e2e-tests/page-objects/ProjectStatisticsPopup.ts +++ b/src/e2e-tests/page-objects/ProjectStatisticsPopup.ts @@ -78,12 +78,22 @@ export class ProjectStatisticsPopup { .mediumCritical, ); }, + signalsByCriticalityIsNotVisible: async (): Promise => { + await expect(this.signalsByCriticalityChart).toBeHidden(); + }, + signalsByClassificationIsVisible: async (): Promise => { await expect(this.signalsByClassificationChart).toContainText( text.projectStatisticsPopup.charts.signalCountByClassificationPieChart .noClassification, ); }, + signalsByClassificationIsShown: async (): Promise => { + await expect(this.signalsByClassificationChart).toBeVisible(); + }, + signalsByClassificationIsNotShown: async (): Promise => { + await expect(this.signalsByClassificationChart).toBeHidden(); + }, incompleteAttributionsIsVisible: async (): Promise => { await expect(this.incompleteAttributionsChart).toContainText( text.projectStatisticsPopup.charts.incompleteAttributionsPieChart From 9c3af153c9ac1ac6a1cb268a3e320cec0f37ea2c Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Thu, 20 Mar 2025 09:30:05 +0100 Subject: [PATCH 12/18] fix: review-comment: add a few empty lines to improve readability Signed-off-by: Dominikus Hellgartner --- .../main/__tests__/user-settings-provider.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts b/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts index 58f3a4c1c..0b62e908a 100644 --- a/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts +++ b/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts @@ -37,6 +37,7 @@ jest.mock('electron', () => { }, }; }); + describe('UserSettings', () => { let temporaryDir: string | undefined = undefined; beforeEach(async () => { @@ -83,6 +84,7 @@ describe('UserSettings', () => { process.env = oldEnvironment; }); }); + describe('get', () => { it('gets a user setting from a predescribed path', async () => { await UserSettingsProvider.init(); From 5393b424af85278f5e63d551447f228b2a8d9ac6 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Thu, 20 Mar 2025 13:06:02 +0100 Subject: [PATCH 13/18] fix: review-comment: Move showing the project popup to the new mechanism Note: Now the old signal for fetching a single backend information should be unused Signed-off-by: Dominikus Hellgartner --- .../BackendCommunication/BackendCommunication.tsx | 13 ++++++++----- .../state/actions/view-actions/view-actions.ts | 12 ++++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/Frontend/Components/BackendCommunication/BackendCommunication.tsx b/src/Frontend/Components/BackendCommunication/BackendCommunication.tsx index 6a0e5a774..c7e0fcc7f 100644 --- a/src/Frontend/Components/BackendCommunication/BackendCommunication.tsx +++ b/src/Frontend/Components/BackendCommunication/BackendCommunication.tsx @@ -24,7 +24,10 @@ import { setBaseUrlsForSources, } from '../../state/actions/resource-actions/all-views-simple-actions'; import { loadFromFile } from '../../state/actions/resource-actions/load-actions'; -import { openPopup } from '../../state/actions/view-actions/view-actions'; +import { + openPopup, + openStatisticsPopupAfterFileLoadIfEnabled, +} from '../../state/actions/view-actions/view-actions'; import { useAppDispatch, useAppSelector } from '../../state/hooks'; import { getBaseUrlsForSources } from '../../state/selectors/resource-selectors'; import { @@ -39,13 +42,12 @@ export const BackendCommunication: React.FC = () => { const baseUrlsForSources = useAppSelector(getBaseUrlsForSources); const dispatch = useAppDispatch(); - async function fileLoadedListener( + function fileLoadedListener( _: IpcRendererEvent, parsedFileContent: ParsedFileContent, - ): Promise { + ): void { dispatch(loadFromFile(parsedFileContent)); - (await window.electronAPI.getUserSetting('showProjectStatistics')) && - dispatch(openPopup(PopupType.ProjectStatisticsPopup)); + dispatch(openStatisticsPopupAfterFileLoadIfEnabled); } function resetLoadedFileListener( @@ -97,6 +99,7 @@ export const BackendCommunication: React.FC = () => { ); } } + useIpcRenderer(AllowedFrontendChannels.FileLoaded, fileLoadedListener, [ dispatch, ]); diff --git a/src/Frontend/state/actions/view-actions/view-actions.ts b/src/Frontend/state/actions/view-actions/view-actions.ts index 958cd0383..a0b9cca1f 100644 --- a/src/Frontend/state/actions/view-actions/view-actions.ts +++ b/src/Frontend/state/actions/view-actions/view-actions.ts @@ -7,6 +7,7 @@ import { PopupType, View } from '../../../enums/enums'; import { EMPTY_DISPLAY_PACKAGE_INFO } from '../../../shared-constants'; import { State } from '../../../types/types'; import { getPackageInfoOfSelectedAttribution } from '../../selectors/resource-selectors'; +import { getShowProjectStatistics } from '../../selectors/user-settings-selector'; import { getSelectedView } from '../../selectors/view-selector'; import { AppThunkAction, AppThunkDispatch } from '../../types'; import { setTemporaryDisplayPackageInfo } from '../resource-actions/all-views-simple-actions'; @@ -53,6 +54,17 @@ export function navigateToView(view: View): AppThunkAction { }; } +export function openStatisticsPopupAfterFileLoadIfEnabled( + dispatch: AppThunkDispatch, + getState: () => State, +): void { + const state = getState(); + const showStatisticsPopup = getShowProjectStatistics(state); + if (showStatisticsPopup) { + dispatch(openPopup(PopupType.ProjectStatisticsPopup)); + } +} + export function setView(view: View): SetView { return { type: ACTION_SET_VIEW, From 27149ef88dc70891d810a7ffe6f890eb9888510b Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Thu, 20 Mar 2025 13:13:48 +0100 Subject: [PATCH 14/18] fix: review-comment: Simplify IPC interface only allow one channel to get the full user-settings information from the backend Signed-off-by: Dominikus Hellgartner --- src/ElectronBackend/main/main.ts | 5 +---- src/ElectronBackend/preload.ts | 3 +-- .../__tests__/user-settings-actions.test.ts | 2 +- .../actions/user-settings-actions/user-settings-actions.ts | 2 +- src/shared/ipc-channels.ts | 1 - src/shared/shared-types.ts | 5 +---- src/testing/setup-tests.ts | 3 +-- 7 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/ElectronBackend/main/main.ts b/src/ElectronBackend/main/main.ts index 91d643a25..4ac844c8d 100644 --- a/src/ElectronBackend/main/main.ts +++ b/src/ElectronBackend/main/main.ts @@ -93,10 +93,7 @@ export async function main(): Promise { }), ); ipcMain.handle(IpcChannel.OpenLink, openLinkListener); - ipcMain.handle(IpcChannel.GetUserSettings, (_, key) => - UserSettingsProvider.get(key), - ); - ipcMain.handle(IpcChannel.GetFullUserSettings, () => + ipcMain.handle(IpcChannel.GetUserSettings, () => UserSettingsProvider.get(), ); ipcMain.handle( diff --git a/src/ElectronBackend/preload.ts b/src/ElectronBackend/preload.ts index f9a6f90e8..59be66937 100644 --- a/src/ElectronBackend/preload.ts +++ b/src/ElectronBackend/preload.ts @@ -34,8 +34,7 @@ const electronAPI: ElectronAPI = { ipcRenderer.on(channel, listener); return () => ipcRenderer.removeListener(channel, listener); }, - getUserSetting: (key) => ipcRenderer.invoke(IpcChannel.GetUserSettings, key), - getFullUserSettings: () => ipcRenderer.invoke(IpcChannel.GetFullUserSettings), + getUserSettings: () => ipcRenderer.invoke(IpcChannel.GetUserSettings), setUserSettings: (userSettings: Partial) => ipcRenderer.invoke(IpcChannel.SetUserSettings, userSettings), }; diff --git a/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts b/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts index a3a719981..ac13fe0b1 100644 --- a/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts +++ b/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts @@ -39,7 +39,7 @@ describe('user-settings-actions', () => { it('loads the user settings from the backend', async () => { const store = createAppStore(); - const backendCall = jest.mocked(window.electronAPI.getFullUserSettings); + const backendCall = jest.mocked(window.electronAPI.getUserSettings); backendCall.mockReturnValue( Promise.resolve({ ...DEFAULT_USER_SETTINGS, diff --git a/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts b/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts index 53980f93a..06bce22f5 100644 --- a/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts +++ b/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts @@ -20,7 +20,7 @@ export function setUserSetting(setting: Partial): SetUserSetting { export function fetchUserSettings(): AppThunkAction { return async (dispatch) => { - const userSettings = await window.electronAPI.getFullUserSettings(); + const userSettings = await window.electronAPI.getUserSettings(); dispatch(setUserSetting(userSettings)); }; } diff --git a/src/shared/ipc-channels.ts b/src/shared/ipc-channels.ts index f258f0fde..61e04fe97 100644 --- a/src/shared/ipc-channels.ts +++ b/src/shared/ipc-channels.ts @@ -18,7 +18,6 @@ export enum IpcChannel { */ StopLoading = 'stop-loading', GetUserSettings = 'get-user-settings', - GetFullUserSettings = 'get-full-user-settings', SetUserSettings = 'set-user-settings', Quit = 'quit', Relaunch = 'relaunch', diff --git a/src/shared/shared-types.ts b/src/shared/shared-types.ts index 92bcfc07c..bd6e98ac1 100644 --- a/src/shared/shared-types.ts +++ b/src/shared/shared-types.ts @@ -296,10 +296,7 @@ export interface ElectronAPI { */ stopLoading: () => void; on: (channel: AllowedFrontendChannels, listener: Listener) => () => void; - getUserSetting: ( - key: T, - ) => Promise; - getFullUserSettings: () => Promise; + getUserSettings: () => Promise; setUserSettings: (userSettings: Partial) => Promise; } diff --git a/src/testing/setup-tests.ts b/src/testing/setup-tests.ts index a31f5b557..66267399b 100644 --- a/src/testing/setup-tests.ts +++ b/src/testing/setup-tests.ts @@ -45,8 +45,7 @@ global.window.electronAPI = { saveFile: jest.fn(), stopLoading: jest.fn(), on: jest.fn().mockReturnValue(jest.fn()), - getUserSetting: jest.fn().mockReturnValue(undefined), - getFullUserSettings: jest.fn().mockReturnValue(DEFAULT_USER_SETTINGS), + getUserSettings: jest.fn().mockReturnValue(DEFAULT_USER_SETTINGS), setUserSettings: jest.fn(), } satisfies ElectronAPI; From f03015d90fbed55360260c657f2386c239332a54 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Thu, 20 Mar 2025 15:56:09 +0100 Subject: [PATCH 15/18] refactor: review-column: rename UserSettingsProvider in order to avoid clashes with react naming where a provider has a fixed meaning Signed-off-by: Dominikus Hellgartner --- .../main/__tests__/menu.test.ts | 4 +-- .../__tests__/user-settings-provider.test.ts | 30 +++++++++---------- src/ElectronBackend/main/listeners.ts | 6 ++-- src/ElectronBackend/main/main.ts | 10 +++---- src/ElectronBackend/main/menu/fileMenu.ts | 6 ++-- .../main/menu/switchableMenuItem.ts | 6 ++-- ...s-provider.ts => user-settings-service.ts} | 4 +-- 7 files changed, 32 insertions(+), 34 deletions(-) rename src/ElectronBackend/main/{user-settings-provider.ts => user-settings-service.ts} (95%) diff --git a/src/ElectronBackend/main/__tests__/menu.test.ts b/src/ElectronBackend/main/__tests__/menu.test.ts index a2fde124c..aac06c89a 100644 --- a/src/ElectronBackend/main/__tests__/menu.test.ts +++ b/src/ElectronBackend/main/__tests__/menu.test.ts @@ -5,7 +5,7 @@ import electron, { BrowserWindow, MenuItemConstructorOptions } from 'electron'; import { createMenu } from '../menu'; -import { UserSettingsProvider } from '../user-settings-provider'; +import { UserSettingsService } from '../user-settings-service'; jest.mock('electron', () => ({ BrowserWindow: class BrowserWindowMock {}, @@ -49,7 +49,7 @@ describe('create menu', () => { ]; testCases.forEach((testCase) => { it(`evaluates ${testCase.darkMode ? 'dark' : 'light'} mode properly`, async () => { - await UserSettingsProvider.init(); + await UserSettingsService.init(); const mainWindow = new BrowserWindow(); // Important to set this up only here and not in the mock setup diff --git a/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts b/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts index 0b62e908a..a1c6e2727 100644 --- a/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts +++ b/src/ElectronBackend/main/__tests__/user-settings-provider.test.ts @@ -14,7 +14,7 @@ import { DEFAULT_PANEL_SIZES, DEFAULT_USER_SETTINGS, } from '../../../shared/shared-constants'; -import { UserSettingsProvider } from '../user-settings-provider'; +import { UserSettingsService } from '../user-settings-service'; type MockedBrowserWindow = BrowserWindow & { sendFunction: (channel: string, ...args: Array) => void; @@ -53,7 +53,7 @@ describe('UserSettings', () => { describe('init', () => { it('sets up the default values if empty', async () => { - await UserSettingsProvider.init(); + await UserSettingsService.init(); const result = await settings.get(); @@ -63,7 +63,7 @@ describe('UserSettings', () => { it('overwrites only the non set values if there are already values set', async () => { await settings.set('qaMode', true); - await UserSettingsProvider.init(); + await UserSettingsService.init(); const result = await settings.get(); @@ -76,7 +76,7 @@ describe('UserSettings', () => { await settings.set('qaMode', true); - await UserSettingsProvider.init(); + await UserSettingsService.init(); const result = await settings.get(); @@ -87,17 +87,17 @@ describe('UserSettings', () => { describe('get', () => { it('gets a user setting from a predescribed path', async () => { - await UserSettingsProvider.init(); + await UserSettingsService.init(); - const panelSizes = await UserSettingsProvider.get('panelSizes'); + const panelSizes = await UserSettingsService.get('panelSizes'); expect(panelSizes).toEqual(DEFAULT_PANEL_SIZES); }); it('gets the full user settings', async () => { - await UserSettingsProvider.init(); + await UserSettingsService.init(); - const userSettings = await UserSettingsProvider.get(); + const userSettings = await UserSettingsService.get(); expect(userSettings).toEqual(DEFAULT_USER_SETTINGS); }); @@ -105,9 +105,9 @@ describe('UserSettings', () => { describe('write operations', () => { it('sets a value and communicates to the frontend', async () => { - await UserSettingsProvider.set('qaMode', true); + await UserSettingsService.set('qaMode', true); - const qaMode = await UserSettingsProvider.get('qaMode'); + const qaMode = await UserSettingsService.get('qaMode'); expect(qaMode).toBe(true); expect( (BrowserWindow as unknown as MockedBrowserWindow).sendFunction, @@ -117,11 +117,11 @@ describe('UserSettings', () => { }); it('sets a value and does not communicates to the frontend if disabled', async () => { - await UserSettingsProvider.set('qaMode', true, { + await UserSettingsService.set('qaMode', true, { skipNotification: true, }); - const qaMode = await UserSettingsProvider.get('qaMode'); + const qaMode = await UserSettingsService.get('qaMode'); expect(qaMode).toBe(true); expect( (BrowserWindow as unknown as MockedBrowserWindow).sendFunction, @@ -129,14 +129,14 @@ describe('UserSettings', () => { }); it('allows to update multiple values at once', async () => { - await UserSettingsProvider.init(); + await UserSettingsService.init(); - await UserSettingsProvider.update({ + await UserSettingsService.update({ qaMode: true, showClassifications: false, }); - const userSettings = await UserSettingsProvider.get(); + const userSettings = await UserSettingsService.get(); expect( (BrowserWindow as unknown as MockedBrowserWindow).sendFunction, diff --git a/src/ElectronBackend/main/listeners.ts b/src/ElectronBackend/main/listeners.ts index 339207f21..d1985b5bc 100644 --- a/src/ElectronBackend/main/listeners.ts +++ b/src/ElectronBackend/main/listeners.ts @@ -55,7 +55,7 @@ import { } from './globalBackendState'; import logger from './logger'; import { createMenu } from './menu'; -import { UserSettingsProvider } from './user-settings-provider'; +import { UserSettingsService } from './user-settings-service'; const MAX_NUMBER_OF_RECENTLY_OPENED_PATHS = 10; @@ -361,10 +361,10 @@ export async function openFile( } async function updateRecentlyOpenedPaths(filePath: string): Promise { - const recentlyOpenedPaths = await UserSettingsProvider.get( + const recentlyOpenedPaths = await UserSettingsService.get( 'recentlyOpenedPaths', ); - await UserSettingsProvider.set( + await UserSettingsService.set( 'recentlyOpenedPaths', uniq([filePath, ...(recentlyOpenedPaths ?? [])]).slice( 0, diff --git a/src/ElectronBackend/main/main.ts b/src/ElectronBackend/main/main.ts index 74c62d507..e41aaa301 100644 --- a/src/ElectronBackend/main/main.ts +++ b/src/ElectronBackend/main/main.ts @@ -22,7 +22,7 @@ import { import { createMenu } from './menu'; import { DisabledMenuItemHandler } from './menu/DisabledMenuItemHandler'; import { openFileFromCliOrEnvVariableIfProvided } from './openFileFromCliOrEnvVariableIfProvided'; -import { UserSettingsProvider } from './user-settings-provider'; +import { UserSettingsService } from './user-settings-service'; export async function main(): Promise { try { @@ -36,7 +36,7 @@ export async function main(): Promise { const mainWindow = createWindow(); - await UserSettingsProvider.init(); + await UserSettingsService.init(); await createMenu(mainWindow); mainWindow.webContents.session.webRequest.onBeforeSendHeaders( @@ -93,13 +93,11 @@ export async function main(): Promise { }), ); ipcMain.handle(IpcChannel.OpenLink, openLinkListener); - ipcMain.handle(IpcChannel.GetUserSettings, () => - UserSettingsProvider.get(), - ); + ipcMain.handle(IpcChannel.GetUserSettings, () => UserSettingsService.get()); ipcMain.handle( IpcChannel.SetUserSettings, (_, userSettings: Partial) => - UserSettingsProvider.update(userSettings, true), + UserSettingsService.update(userSettings, true), ); await loadWebApp(mainWindow); diff --git a/src/ElectronBackend/main/menu/fileMenu.ts b/src/ElectronBackend/main/menu/fileMenu.ts index 8a1a7d4e7..803a12a78 100644 --- a/src/ElectronBackend/main/menu/fileMenu.ts +++ b/src/ElectronBackend/main/menu/fileMenu.ts @@ -31,7 +31,7 @@ import { } from '../listeners'; import logger from '../logger'; import { createMenu } from '../menu'; -import { UserSettingsProvider } from '../user-settings-provider'; +import { UserSettingsService } from '../user-settings-service'; import { DisabledMenuItemHandler } from './DisabledMenuItemHandler'; export const importFileFormats: Array = [ @@ -67,7 +67,7 @@ function getOpenFile(mainWindow: BrowserWindow): MenuItemConstructorOptions { async function getOpenRecent( mainWindow: BrowserWindow, ): Promise { - const recentlyOpenedPaths = await UserSettingsProvider.get( + const recentlyOpenedPaths = await UserSettingsService.get( 'recentlyOpenedPaths', ); @@ -103,7 +103,7 @@ function getOpenRecentSubmenu( id: 'clear-recent', label: text.menu.fileSubmenu.clearRecent, click: async () => { - await UserSettingsProvider.set('recentlyOpenedPaths', []); + await UserSettingsService.set('recentlyOpenedPaths', []); await createMenu(mainWindow); }, }, diff --git a/src/ElectronBackend/main/menu/switchableMenuItem.ts b/src/ElectronBackend/main/menu/switchableMenuItem.ts index c583ffd11..30d64efe6 100644 --- a/src/ElectronBackend/main/menu/switchableMenuItem.ts +++ b/src/ElectronBackend/main/menu/switchableMenuItem.ts @@ -7,7 +7,7 @@ import { BrowserWindow, MenuItemConstructorOptions } from 'electron'; import { UserSettings as IUserSettings } from '../../../shared/shared-types'; import { getCheckboxBasedOnThemeAndCheckState } from '../iconHelpers'; import { createMenu } from '../menu'; -import { UserSettingsProvider } from '../user-settings-provider'; +import { UserSettingsService } from '../user-settings-service'; export interface SwitchableItemOptions { id: string; @@ -19,13 +19,13 @@ export async function switchableMenuItem( mainWindow: BrowserWindow, options: SwitchableItemOptions, ): Promise { - const state = !!(await UserSettingsProvider.get(options.userSettingsKey)); + const state = !!(await UserSettingsService.get(options.userSettingsKey)); return { icon: getCheckboxBasedOnThemeAndCheckState(state), id: state ? `enabled-${options.id}` : `disabled-${options.id}`, label: options.label, click: async () => { - await UserSettingsProvider.set(options.userSettingsKey, !state); + await UserSettingsService.set(options.userSettingsKey, !state); await createMenu(mainWindow); }, }; diff --git a/src/ElectronBackend/main/user-settings-provider.ts b/src/ElectronBackend/main/user-settings-service.ts similarity index 95% rename from src/ElectronBackend/main/user-settings-provider.ts rename to src/ElectronBackend/main/user-settings-service.ts index 93bc1988d..4ba5cc606 100644 --- a/src/ElectronBackend/main/user-settings-provider.ts +++ b/src/ElectronBackend/main/user-settings-service.ts @@ -13,7 +13,7 @@ import { UserSettings, } from '../../shared/shared-types'; -export class UserSettingsProvider { +export class UserSettingsService { public static async init() { if (process.argv.includes('--reset') || process.env.RESET) { log.info('Resetting user settings'); @@ -47,7 +47,7 @@ export class UserSettingsProvider { for (const key of Object.keys(userSettings)) { const properKey = key as keyof UserSettings; if (userSettings[properKey] !== undefined) { - await UserSettingsProvider.set(properKey, userSettings[properKey], { + await UserSettingsService.set(properKey, userSettings[properKey], { skipNotification, }); } From 8fd911b816d7f9b0b0b92134c8c08af71348e292 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Thu, 20 Mar 2025 16:16:28 +0100 Subject: [PATCH 16/18] refactor: review-comment: make update user settings accept a update function Signed-off-by: Dominikus Hellgartner --- .../__tests__/user-settings-actions.test.ts | 4 +- .../user-settings-actions.ts | 49 +++++++++++-------- .../state/selectors/user-settings-selector.ts | 4 ++ 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts b/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts index ac13fe0b1..790327574 100644 --- a/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts +++ b/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts @@ -87,7 +87,7 @@ describe('user-settings-actions', () => { store.getState(), ); - toggleAreHiddenSignalsVisible(store.dispatch, store.getState); + toggleAreHiddenSignalsVisible(store.dispatch); await waitFor(() => expect(getAreHiddenSignalsVisible(store.getState())).toBe( @@ -95,7 +95,7 @@ describe('user-settings-actions', () => { ), ); - toggleAreHiddenSignalsVisible(store.dispatch, store.getState); + toggleAreHiddenSignalsVisible(store.dispatch); await waitFor(() => expect(getAreHiddenSignalsVisible(store.getState())).toBe( diff --git a/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts b/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts index 06bce22f5..636bf58b7 100644 --- a/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts +++ b/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts @@ -4,10 +4,7 @@ // SPDX-License-Identifier: Apache-2.0 import { PanelSizes, UserSettings } from '../../../../shared/shared-types'; import { State } from '../../../types/types'; -import { - getAreHiddenSignalsVisible, - getPanelSizes, -} from '../../selectors/user-settings-selector'; +import { getUserSettings } from '../../selectors/user-settings-selector'; import { AppThunkAction, AppThunkDispatch } from '../../types'; import { ACTION_SET_USER_SETTING, SetUserSetting } from './types'; @@ -25,39 +22,49 @@ export function fetchUserSettings(): AppThunkAction { }; } +function getUserSettingsToSet( + userSettings: + | Partial + | ((currentSettings: UserSettings) => Partial), + getState: () => State, +): Partial { + if (typeof userSettings === 'function') { + const currentUserSettings: UserSettings = getUserSettings(getState()); + return userSettings(currentUserSettings); + } + return userSettings; +} + export function updateUserSettings( - userSettings: Partial, + userSettings: + | Partial + | ((currentSettings: UserSettings) => Partial), ): AppThunkAction { - return async (dispatch) => { - await window.electronAPI.setUserSettings(userSettings); - dispatch(setUserSetting(userSettings)); + return async (dispatch: AppThunkDispatch, getState: () => State) => { + const userSettingsToSet = getUserSettingsToSet(userSettings, getState); + await window.electronAPI.setUserSettings(userSettingsToSet); + dispatch(setUserSetting(userSettingsToSet)); }; } export function updatePanelSizes( panelSizes: Partial, ): AppThunkAction { - return (dispatch, getState): void => { - const currentState = getState(); - const currentPanelSizes = getPanelSizes(currentState); + return (dispatch): void => { dispatch( - updateUserSettings({ - panelSizes: { ...currentPanelSizes, ...panelSizes }, - }), + updateUserSettings((currentSettings: UserSettings) => ({ + panelSizes: { ...currentSettings.panelSizes, ...panelSizes }, + })), ); }; } export function toggleAreHiddenSignalsVisible( dispatch: AppThunkDispatch, - getState: () => State, ): void { - const currentState = getState(); - const currentAreHiddenSignalsVisible = - getAreHiddenSignalsVisible(currentState); dispatch( - updateUserSettings({ - areHiddenSignalsVisible: !currentAreHiddenSignalsVisible, - }), + updateUserSettings((currentSettings: UserSettings) => ({ + areHiddenSignalsVisible: !currentSettings.areHiddenSignalsVisible, + })), ); } diff --git a/src/Frontend/state/selectors/user-settings-selector.ts b/src/Frontend/state/selectors/user-settings-selector.ts index d4294e878..685f3b92e 100644 --- a/src/Frontend/state/selectors/user-settings-selector.ts +++ b/src/Frontend/state/selectors/user-settings-selector.ts @@ -5,6 +5,10 @@ import { UserSettings } from '../../../shared/shared-types'; import { State } from '../../types/types'; +export function getUserSettings(state: State): UserSettings { + return state.userSettingsState; +} + export function getShowCriticality(state: State): boolean { return state.userSettingsState.showCriticality; } From 8314a57d895896b1b20e450872c3a9fb9f37c7ea Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Thu, 20 Mar 2025 16:23:36 +0100 Subject: [PATCH 17/18] refactor: review-comment: user the new capability to inline the toggle functions Signed-off-by: Dominikus Hellgartner --- .../IncludeExcludeButton.tsx | 9 ++- .../__tests__/user-settings-actions.test.ts | 58 ------------------- .../user-settings-actions.ts | 24 +------- .../state/variables/use-panel-sizes.ts | 10 +++- 4 files changed, 15 insertions(+), 86 deletions(-) diff --git a/src/Frontend/Components/AttributionPanels/SignalsPanel/IncludeExcludeButton/IncludeExcludeButton.tsx b/src/Frontend/Components/AttributionPanels/SignalsPanel/IncludeExcludeButton/IncludeExcludeButton.tsx index 9ea0c77bf..605083fc6 100644 --- a/src/Frontend/Components/AttributionPanels/SignalsPanel/IncludeExcludeButton/IncludeExcludeButton.tsx +++ b/src/Frontend/Components/AttributionPanels/SignalsPanel/IncludeExcludeButton/IncludeExcludeButton.tsx @@ -8,8 +8,9 @@ import MuiIconButton from '@mui/material/IconButton'; import MuiTooltip from '@mui/material/Tooltip'; import MuiBox from '@mui/system/Box'; +import { UserSettings } from '../../../../../shared/shared-types'; import { text } from '../../../../../shared/text'; -import { toggleAreHiddenSignalsVisible } from '../../../../state/actions/user-settings-actions/user-settings-actions'; +import { updateUserSettings } from '../../../../state/actions/user-settings-actions/user-settings-actions'; import { useAppDispatch } from '../../../../state/hooks'; import { useAreHiddenSignalsVisible } from '../../../../state/variables/use-are-hidden-signals-visible'; @@ -25,7 +26,11 @@ export const IncludeExcludeButton: React.FC = () => { aria-label={label} size={'small'} onClick={() => { - dispatch(toggleAreHiddenSignalsVisible); + dispatch( + updateUserSettings((currentSettings: UserSettings) => ({ + areHiddenSignalsVisible: !currentSettings.areHiddenSignalsVisible, + })), + ); }} > diff --git a/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts b/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts index 790327574..a15586f95 100644 --- a/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts +++ b/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts @@ -5,38 +5,17 @@ import { waitFor } from '@testing-library/react'; import { DEFAULT_USER_SETTINGS } from '../../../../../shared/shared-constants'; -import { faker } from '../../../../../testing/Faker'; import { createAppStore } from '../../../configure-store'; import { - getAreHiddenSignalsVisible, - getPanelSizes, getQaMode, getShowClassifications, } from '../../../selectors/user-settings-selector'; import { fetchUserSettings, - setUserSetting, - toggleAreHiddenSignalsVisible, - updatePanelSizes, updateUserSettings, } from '../user-settings-actions'; describe('user-settings-actions', () => { - it('sets the users setting', () => { - const store = createAppStore(); - - store.dispatch( - setUserSetting({ qaMode: true, showClassifications: false }), - ); - - expect(getQaMode(store.getState())).toBe(true); - expect(getShowClassifications(store.getState())).toBe(false); - - store.dispatch(setUserSetting({ qaMode: false })); - expect(getQaMode(store.getState())).toBe(false); - expect(getShowClassifications(store.getState())).toBe(false); - }); - it('loads the user settings from the backend', async () => { const store = createAppStore(); const backendCall = jest.mocked(window.electronAPI.getUserSettings); @@ -66,41 +45,4 @@ describe('user-settings-actions', () => { }); expect(backendCall).toHaveBeenCalledWith(userSettings); }); - - it('updates panel sizes', async () => { - const store = createAppStore(); - const backendCall = jest.mocked(window.electronAPI.setUserSettings); - - const signalsPanelHeight = faker.number.int({ min: 1 }); - store.dispatch(updatePanelSizes({ signalsPanelHeight })); - - await waitFor(() => { - const panelSizes = getPanelSizes(store.getState()); - expect(panelSizes.signalsPanelHeight).toBe(signalsPanelHeight); - }); - expect(backendCall).toHaveBeenCalled(); - }); - - it('toggles hidden signals', async () => { - const store = createAppStore(); - const hiddenSignalsVisibleAtStart = getAreHiddenSignalsVisible( - store.getState(), - ); - - toggleAreHiddenSignalsVisible(store.dispatch); - - await waitFor(() => - expect(getAreHiddenSignalsVisible(store.getState())).toBe( - !hiddenSignalsVisibleAtStart, - ), - ); - - toggleAreHiddenSignalsVisible(store.dispatch); - - await waitFor(() => - expect(getAreHiddenSignalsVisible(store.getState())).toBe( - hiddenSignalsVisibleAtStart, - ), - ); - }); }); diff --git a/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts b/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts index 636bf58b7..b68fa6365 100644 --- a/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts +++ b/src/Frontend/state/actions/user-settings-actions/user-settings-actions.ts @@ -2,7 +2,7 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 -import { PanelSizes, UserSettings } from '../../../../shared/shared-types'; +import { UserSettings } from '../../../../shared/shared-types'; import { State } from '../../../types/types'; import { getUserSettings } from '../../selectors/user-settings-selector'; import { AppThunkAction, AppThunkDispatch } from '../../types'; @@ -46,25 +46,3 @@ export function updateUserSettings( dispatch(setUserSetting(userSettingsToSet)); }; } - -export function updatePanelSizes( - panelSizes: Partial, -): AppThunkAction { - return (dispatch): void => { - dispatch( - updateUserSettings((currentSettings: UserSettings) => ({ - panelSizes: { ...currentSettings.panelSizes, ...panelSizes }, - })), - ); - }; -} - -export function toggleAreHiddenSignalsVisible( - dispatch: AppThunkDispatch, -): void { - dispatch( - updateUserSettings((currentSettings: UserSettings) => ({ - areHiddenSignalsVisible: !currentSettings.areHiddenSignalsVisible, - })), - ); -} diff --git a/src/Frontend/state/variables/use-panel-sizes.ts b/src/Frontend/state/variables/use-panel-sizes.ts index 56daa8677..5b8d18e67 100644 --- a/src/Frontend/state/variables/use-panel-sizes.ts +++ b/src/Frontend/state/variables/use-panel-sizes.ts @@ -2,8 +2,8 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 -import { PanelSizes } from '../../../shared/shared-types'; -import { updatePanelSizes } from '../actions/user-settings-actions/user-settings-actions'; +import { PanelSizes, UserSettings } from '../../../shared/shared-types'; +import { updateUserSettings } from '../actions/user-settings-actions/user-settings-actions'; import { useAppDispatch, useAppSelector } from '../hooks'; import { getPanelSizes } from '../selectors/user-settings-selector'; @@ -14,6 +14,10 @@ export const usePanelSizes = (): { const panelSizes = useAppSelector(getPanelSizes); const dispatch = useAppDispatch(); const setPanelSizes = (panelSizes: Partial) => - dispatch(updatePanelSizes(panelSizes)); + dispatch( + updateUserSettings((currentSettings: UserSettings) => ({ + panelSizes: { ...currentSettings.panelSizes, ...panelSizes }, + })), + ); return { panelSizes, setPanelSizes }; }; From 3645445289c3a74ef0190a5c56084f511d6fc6c0 Mon Sep 17 00:00:00 2001 From: Dominikus Hellgartner Date: Mon, 24 Mar 2025 08:13:55 +0100 Subject: [PATCH 18/18] fix: review-comment: move to one unified useUserSettings instead of multiple selectors Signed-off-by: Dominikus Hellgartner --- .../AuditingOptions/AuditingOptions.util.tsx | 11 ++++----- .../IncludeExcludeButton.tsx | 16 +++++-------- .../RestoreButton/RestoreButton.tsx | 5 ++-- .../Components/PackageCard/PackageCard.tsx | 8 +++---- .../ProjectStatisticsPopup.tsx | 22 +++++++---------- .../ResourcesTreeNode/ResourcesTreeNode.tsx | 8 +++---- .../SortButton/useSortingOptions.tsx | 8 +++---- .../SwitchableProcessBar.tsx | 8 +++---- .../__tests__/user-settings-actions.test.ts | 10 ++++---- .../actions/view-actions/view-actions.ts | 4 ++-- .../state/selectors/user-settings-selector.ts | 24 ------------------- .../use-are-hidden-signals-visible.ts | 10 -------- .../state/variables/use-panel-sizes.ts | 16 +++++-------- .../variables/use-show-classifications.ts | 12 ---------- .../state/variables/use-show-criticality.ts | 10 -------- .../state/variables/use-user-setting.ts | 21 +++++++++++++++- .../web-workers/use-signals-worker.ts | 5 ++-- 17 files changed, 74 insertions(+), 124 deletions(-) delete mode 100644 src/Frontend/state/variables/use-are-hidden-signals-visible.ts delete mode 100644 src/Frontend/state/variables/use-show-classifications.ts delete mode 100644 src/Frontend/state/variables/use-show-criticality.ts diff --git a/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx b/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx index 6493a7d05..fbf556c04 100644 --- a/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx +++ b/src/Frontend/Components/AttributionForm/AuditingOptions/AuditingOptions.util.tsx @@ -26,9 +26,7 @@ import { getIsPreferenceFeatureEnabled, getTemporaryDisplayPackageInfo, } from '../../../state/selectors/resource-selectors'; -import { getQaMode } from '../../../state/selectors/user-settings-selector'; -import { useShowClassifications } from '../../../state/variables/use-show-classifications'; -import { useShowCriticality } from '../../../state/variables/use-show-criticality'; +import { useUserSettings } from '../../../state/variables/use-user-setting'; import { prettifySource } from '../../../util/prettify-source'; import { ClassificationIcon, @@ -58,14 +56,15 @@ export function useAuditingOptions({ }) { const dispatch = useAppDispatch(); const store = useAppStore(); - const qaMode = useAppSelector(getQaMode); const attributionSources = useAppSelector(getExternalAttributionSources); const isPreferenceFeatureEnabled = useAppSelector( getIsPreferenceFeatureEnabled, ); const classifications = useAppSelector(getClassifications); - const showClassifications = useShowClassifications(); - const showCriticality = useShowCriticality(); + const [userSettings] = useUserSettings(); + const qaMode = userSettings.qaMode; + const showClassifications = userSettings.showClassifications; + const showCriticality = userSettings.showCriticality; const source = useMemo(() => { const sourceName = diff --git a/src/Frontend/Components/AttributionPanels/SignalsPanel/IncludeExcludeButton/IncludeExcludeButton.tsx b/src/Frontend/Components/AttributionPanels/SignalsPanel/IncludeExcludeButton/IncludeExcludeButton.tsx index 605083fc6..4fcb2f22c 100644 --- a/src/Frontend/Components/AttributionPanels/SignalsPanel/IncludeExcludeButton/IncludeExcludeButton.tsx +++ b/src/Frontend/Components/AttributionPanels/SignalsPanel/IncludeExcludeButton/IncludeExcludeButton.tsx @@ -10,13 +10,11 @@ import MuiBox from '@mui/system/Box'; import { UserSettings } from '../../../../../shared/shared-types'; import { text } from '../../../../../shared/text'; -import { updateUserSettings } from '../../../../state/actions/user-settings-actions/user-settings-actions'; -import { useAppDispatch } from '../../../../state/hooks'; -import { useAreHiddenSignalsVisible } from '../../../../state/variables/use-are-hidden-signals-visible'; +import { useUserSettings } from '../../../../state/variables/use-user-setting'; export const IncludeExcludeButton: React.FC = () => { - const dispatch = useAppDispatch(); - const areHiddenSignalsVisible = useAreHiddenSignalsVisible(); + const [userSettings, updateUserSettings] = useUserSettings(); + const areHiddenSignalsVisible = userSettings.areHiddenSignalsVisible; const label = areHiddenSignalsVisible ? text.packageLists.hideDeleted : text.packageLists.showDeleted; @@ -26,11 +24,9 @@ export const IncludeExcludeButton: React.FC = () => { aria-label={label} size={'small'} onClick={() => { - dispatch( - updateUserSettings((currentSettings: UserSettings) => ({ - areHiddenSignalsVisible: !currentSettings.areHiddenSignalsVisible, - })), - ); + updateUserSettings((currentSettings: UserSettings) => ({ + areHiddenSignalsVisible: !currentSettings.areHiddenSignalsVisible, + })); }} > diff --git a/src/Frontend/Components/AttributionPanels/SignalsPanel/RestoreButton/RestoreButton.tsx b/src/Frontend/Components/AttributionPanels/SignalsPanel/RestoreButton/RestoreButton.tsx index d32281623..e2eab15a5 100644 --- a/src/Frontend/Components/AttributionPanels/SignalsPanel/RestoreButton/RestoreButton.tsx +++ b/src/Frontend/Components/AttributionPanels/SignalsPanel/RestoreButton/RestoreButton.tsx @@ -11,7 +11,7 @@ import { text } from '../../../../../shared/text'; import { removeResolvedExternalAttributionAndSave } from '../../../../state/actions/resource-actions/save-actions'; import { useAppDispatch, useAppSelector } from '../../../../state/hooks'; import { getResolvedExternalAttributions } from '../../../../state/selectors/resource-selectors'; -import { useAreHiddenSignalsVisible } from '../../../../state/variables/use-are-hidden-signals-visible'; +import { useUserSettings } from '../../../../state/variables/use-user-setting'; import { PackagesPanelChildrenProps } from '../../PackagesPanel/PackagesPanel'; export const RestoreButton: React.FC = ({ @@ -21,7 +21,8 @@ export const RestoreButton: React.FC = ({ const resolvedExternalAttributionIds = useAppSelector( getResolvedExternalAttributions, ); - const areHiddenSignalsVisible = useAreHiddenSignalsVisible(); + const [userSettings] = useUserSettings(); + const areHiddenSignalsVisible = userSettings.areHiddenSignalsVisible; const someSelectedAttributionsAreHidden = useMemo( () => !!selectedAttributionIds.length && diff --git a/src/Frontend/Components/PackageCard/PackageCard.tsx b/src/Frontend/Components/PackageCard/PackageCard.tsx index 121434e80..af5957c5a 100644 --- a/src/Frontend/Components/PackageCard/PackageCard.tsx +++ b/src/Frontend/Components/PackageCard/PackageCard.tsx @@ -18,8 +18,7 @@ import { text } from '../../../shared/text'; import { OpossumColors } from '../../shared-styles'; import { useAppSelector } from '../../state/hooks'; import { getClassifications } from '../../state/selectors/resource-selectors'; -import { useShowClassifications } from '../../state/variables/use-show-classifications'; -import { useShowCriticality } from '../../state/variables/use-show-criticality'; +import { useUserSettings } from '../../state/variables/use-user-setting'; import { getCardLabels } from '../../util/get-card-labels'; import { maybePluralize } from '../../util/maybe-pluralize'; import { Checkbox } from '../Checkbox/Checkbox'; @@ -143,8 +142,9 @@ export const PackageCard = memo( }), [cardConfig, packageInfo, classification_mapping], ); - const showClassifications = useShowClassifications(); - const showCriticality = useShowCriticality(); + const [userSettings, _] = useUserSettings(); + const showClassifications = userSettings.showClassifications; + const showCriticality = userSettings.showCriticality; const rightIcons = useMemo( () => getRightIcons( diff --git a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx index fe82fce25..97689798e 100644 --- a/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx +++ b/src/Frontend/Components/ProjectStatisticsPopup/ProjectStatisticsPopup.tsx @@ -13,7 +13,6 @@ import { PropsWithChildren, useState } from 'react'; import { Criticality } from '../../../shared/shared-types'; import { text } from '../../../shared/text'; import { criticalityColor } from '../../shared-styles'; -import { updateUserSettings } from '../../state/actions/user-settings-actions/user-settings-actions'; import { closePopup } from '../../state/actions/view-actions/view-actions'; import { useAppDispatch, useAppSelector } from '../../state/hooks'; import { @@ -22,9 +21,7 @@ import { getManualAttributions, getUnresolvedExternalAttributions, } from '../../state/selectors/resource-selectors'; -import { getShowProjectStatistics } from '../../state/selectors/user-settings-selector'; -import { useShowClassifications } from '../../state/variables/use-show-classifications'; -import { useShowCriticality } from '../../state/variables/use-show-criticality'; +import { useUserSettings } from '../../state/variables/use-user-setting'; import { AttributionCountPerSourcePerLicenseTable } from '../AttributionCountPerSourcePerLicenseTable/AttributionCountPerSourcePerLicenseTable'; import { BarChart } from '../BarChart/BarChart'; import { Checkbox } from '../Checkbox/Checkbox'; @@ -91,12 +88,13 @@ export const ProjectStatisticsPopup: React.FC = () => { dispatch(closePopup()); } - const showProjectStatistics = useAppSelector(getShowProjectStatistics); + const [userSettings, updateUserSettings] = useUserSettings(); - const [selectedTab, setSelectedTab] = useState(0); + const showProjectStatistics = userSettings.showProjectStatistics; + const showClassifications = userSettings.showClassifications; + const showCriticality = userSettings.showCriticality; - const showClassifications = useShowClassifications(); - const showCriticality = useShowCriticality(); + const [selectedTab, setSelectedTab] = useState(0); return ( { - dispatch( - updateUserSettings({ - showProjectStatistics: event.target.checked, - }), - ) + updateUserSettings({ + showProjectStatistics: event.target.checked, + }) } label={text.projectStatisticsPopup.toggleStartupCheckbox} /> diff --git a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx index fc8a75c69..74ffa5ae8 100644 --- a/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx +++ b/src/Frontend/Components/ResourceBrowser/ResourcesTree/ResourcesTreeNode/ResourcesTreeNode.tsx @@ -15,8 +15,7 @@ import { getResourcesWithExternalAttributedChildren, getResourcesWithManualAttributedChildren, } from '../../../../state/selectors/resource-selectors'; -import { useShowClassifications } from '../../../../state/variables/use-show-classifications'; -import { useShowCriticality } from '../../../../state/variables/use-show-criticality'; +import { useUserSettings } from '../../../../state/variables/use-user-setting'; import { TreeNode } from '../../../VirtualizedTree/VirtualizedTreeNode/VirtualizedTreeNode'; import { containsExternalAttribution, @@ -55,8 +54,9 @@ export function ResourcesTreeNode({ node, nodeId, nodeName }: TreeNode) { const canHaveChildren = node !== 1; const classification_mapping = useAppSelector(getClassifications); - const showClassifications = useShowClassifications(); - const showCriticality = useShowCriticality(); + const [userSettings] = useUserSettings(); + const showClassifications = userSettings.showClassifications; + const showCriticality = userSettings.showCriticality; return ( ; export function useSortConfiguration(): SortConfiguration { - const showClassifications = useShowClassifications(); - const showCriticality = useShowCriticality(); + const [userSettings] = useUserSettings(); + const showClassifications = userSettings.showClassifications; + const showCriticality = userSettings.showCriticality; return useMemo(() => { return { diff --git a/src/Frontend/Components/SwitchableProcessBar/SwitchableProcessBar.tsx b/src/Frontend/Components/SwitchableProcessBar/SwitchableProcessBar.tsx index d661e75fd..4037f6af1 100644 --- a/src/Frontend/Components/SwitchableProcessBar/SwitchableProcessBar.tsx +++ b/src/Frontend/Components/SwitchableProcessBar/SwitchableProcessBar.tsx @@ -10,8 +10,7 @@ import React, { useState } from 'react'; import { text as fullText } from '../../../shared/text'; import { OpossumColors } from '../../shared-styles'; import { useProgressData } from '../../state/variables/use-progress-data'; -import { useShowClassifications } from '../../state/variables/use-show-classifications'; -import { useShowCriticality } from '../../state/variables/use-show-criticality'; +import { useUserSettings } from '../../state/variables/use-user-setting'; import { SelectedProgressBar } from '../../types/types'; import { ProgressBar } from '../ProgressBar/ProgressBar'; @@ -43,8 +42,9 @@ interface ProgressBarSwitchConfiguration { } export const SwitchableProcessBar: React.FC = () => { - const showClassifications = useShowClassifications(); - const showCriticality = useShowCriticality(); + const [userSettings] = useUserSettings(); + const showClassifications = userSettings.showClassifications; + const showCriticality = userSettings.showCriticality; const switchableProgressBarConfiguration: Record< SelectedProgressBar, diff --git a/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts b/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts index a15586f95..e2333b249 100644 --- a/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts +++ b/src/Frontend/state/actions/user-settings-actions/__tests__/user-settings-actions.test.ts @@ -6,10 +6,6 @@ import { waitFor } from '@testing-library/react'; import { DEFAULT_USER_SETTINGS } from '../../../../../shared/shared-constants'; import { createAppStore } from '../../../configure-store'; -import { - getQaMode, - getShowClassifications, -} from '../../../selectors/user-settings-selector'; import { fetchUserSettings, updateUserSettings, @@ -29,7 +25,7 @@ describe('user-settings-actions', () => { store.dispatch(fetchUserSettings()); await waitFor(() => { - expect(getQaMode(store.getState())).toBe(true); + expect(store.getState().userSettingsState?.qaMode).toBe(true); }); }); @@ -41,7 +37,9 @@ describe('user-settings-actions', () => { store.dispatch(updateUserSettings(userSettings)); await waitFor(() => { - expect(getShowClassifications(store.getState())).toBe(false); + expect(store.getState().userSettingsState?.showClassifications).toBe( + false, + ); }); expect(backendCall).toHaveBeenCalledWith(userSettings); }); diff --git a/src/Frontend/state/actions/view-actions/view-actions.ts b/src/Frontend/state/actions/view-actions/view-actions.ts index a0b9cca1f..ed4d2aa15 100644 --- a/src/Frontend/state/actions/view-actions/view-actions.ts +++ b/src/Frontend/state/actions/view-actions/view-actions.ts @@ -7,7 +7,7 @@ import { PopupType, View } from '../../../enums/enums'; import { EMPTY_DISPLAY_PACKAGE_INFO } from '../../../shared-constants'; import { State } from '../../../types/types'; import { getPackageInfoOfSelectedAttribution } from '../../selectors/resource-selectors'; -import { getShowProjectStatistics } from '../../selectors/user-settings-selector'; +import { getUserSettings } from '../../selectors/user-settings-selector'; import { getSelectedView } from '../../selectors/view-selector'; import { AppThunkAction, AppThunkDispatch } from '../../types'; import { setTemporaryDisplayPackageInfo } from '../resource-actions/all-views-simple-actions'; @@ -59,7 +59,7 @@ export function openStatisticsPopupAfterFileLoadIfEnabled( getState: () => State, ): void { const state = getState(); - const showStatisticsPopup = getShowProjectStatistics(state); + const showStatisticsPopup = getUserSettings(state).showProjectStatistics; if (showStatisticsPopup) { dispatch(openPopup(PopupType.ProjectStatisticsPopup)); } diff --git a/src/Frontend/state/selectors/user-settings-selector.ts b/src/Frontend/state/selectors/user-settings-selector.ts index 685f3b92e..5a29d8260 100644 --- a/src/Frontend/state/selectors/user-settings-selector.ts +++ b/src/Frontend/state/selectors/user-settings-selector.ts @@ -8,27 +8,3 @@ import { State } from '../../types/types'; export function getUserSettings(state: State): UserSettings { return state.userSettingsState; } - -export function getShowCriticality(state: State): boolean { - return state.userSettingsState.showCriticality; -} - -export function getShowClassifications(state: State): boolean { - return state.userSettingsState.showClassifications; -} - -export function getQaMode(state: State): boolean { - return state.userSettingsState.qaMode; -} - -export function getShowProjectStatistics(state: State): boolean { - return state.userSettingsState.showProjectStatistics; -} - -export function getAreHiddenSignalsVisible(state: State): boolean { - return state.userSettingsState.areHiddenSignalsVisible; -} - -export function getPanelSizes(state: State): UserSettings['panelSizes'] { - return state.userSettingsState.panelSizes; -} diff --git a/src/Frontend/state/variables/use-are-hidden-signals-visible.ts b/src/Frontend/state/variables/use-are-hidden-signals-visible.ts deleted file mode 100644 index a12ec66f4..000000000 --- a/src/Frontend/state/variables/use-are-hidden-signals-visible.ts +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates -// SPDX-FileCopyrightText: TNG Technology Consulting GmbH -// -// SPDX-License-Identifier: Apache-2.0 -import { useAppSelector } from '../hooks'; -import { getAreHiddenSignalsVisible } from '../selectors/user-settings-selector'; - -export function useAreHiddenSignalsVisible() { - return useAppSelector(getAreHiddenSignalsVisible); -} diff --git a/src/Frontend/state/variables/use-panel-sizes.ts b/src/Frontend/state/variables/use-panel-sizes.ts index 5b8d18e67..86934f91a 100644 --- a/src/Frontend/state/variables/use-panel-sizes.ts +++ b/src/Frontend/state/variables/use-panel-sizes.ts @@ -3,21 +3,17 @@ // // SPDX-License-Identifier: Apache-2.0 import { PanelSizes, UserSettings } from '../../../shared/shared-types'; -import { updateUserSettings } from '../actions/user-settings-actions/user-settings-actions'; -import { useAppDispatch, useAppSelector } from '../hooks'; -import { getPanelSizes } from '../selectors/user-settings-selector'; +import { useUserSettings } from './use-user-setting'; export const usePanelSizes = (): { panelSizes: PanelSizes; setPanelSizes: (panelsSizes: Partial) => void; } => { - const panelSizes = useAppSelector(getPanelSizes); - const dispatch = useAppDispatch(); + const [userSettings, updateUserSettings] = useUserSettings(); + const panelSizes = userSettings.panelSizes; const setPanelSizes = (panelSizes: Partial) => - dispatch( - updateUserSettings((currentSettings: UserSettings) => ({ - panelSizes: { ...currentSettings.panelSizes, ...panelSizes }, - })), - ); + updateUserSettings((currentSettings: UserSettings) => ({ + panelSizes: { ...currentSettings.panelSizes, ...panelSizes }, + })); return { panelSizes, setPanelSizes }; }; diff --git a/src/Frontend/state/variables/use-show-classifications.ts b/src/Frontend/state/variables/use-show-classifications.ts deleted file mode 100644 index 9eaa92c7d..000000000 --- a/src/Frontend/state/variables/use-show-classifications.ts +++ /dev/null @@ -1,12 +0,0 @@ -// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates -// SPDX-FileCopyrightText: TNG Technology Consulting GmbH -// -// SPDX-License-Identifier: Apache-2.0 -import { useAppSelector } from '../hooks'; -import { getShowClassifications } from '../selectors/user-settings-selector'; - -export const SHOW_CLASSIFICATIONS_KEY = 'showClassifications'; - -export function useShowClassifications() { - return useAppSelector(getShowClassifications); -} diff --git a/src/Frontend/state/variables/use-show-criticality.ts b/src/Frontend/state/variables/use-show-criticality.ts deleted file mode 100644 index 2120ac074..000000000 --- a/src/Frontend/state/variables/use-show-criticality.ts +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates -// SPDX-FileCopyrightText: TNG Technology Consulting GmbH -// -// SPDX-License-Identifier: Apache-2.0 -import { useAppSelector } from '../hooks'; -import { getShowCriticality } from '../selectors/user-settings-selector'; - -export function useShowCriticality() { - return useAppSelector(getShowCriticality); -} diff --git a/src/Frontend/state/variables/use-user-setting.ts b/src/Frontend/state/variables/use-user-setting.ts index f8c2b7bb9..a90008f83 100644 --- a/src/Frontend/state/variables/use-user-setting.ts +++ b/src/Frontend/state/variables/use-user-setting.ts @@ -13,8 +13,10 @@ import { import { fetchUserSettings, setUserSetting, + updateUserSettings as updateUserSettingsThunkAction, } from '../actions/user-settings-actions/user-settings-actions'; -import { useAppDispatch } from '../hooks'; +import { useAppDispatch, useAppSelector } from '../hooks'; +import { getUserSettings } from '../selectors/user-settings-selector'; // should only be called once export function useInitialSyncUserSettings() { @@ -30,3 +32,20 @@ export function useInitialSyncUserSettings() { [dispatch], ); } + +type PartialUserSettings = + | Partial + | ((settings: UserSettings) => Partial); + +export function useUserSettings(): [ + UserSettings, + (userSettings: PartialUserSettings) => void, +] { + const userSettings = useAppSelector(getUserSettings); + const dispatch = useAppDispatch(); + + const updateUserSettings = (userSettings: PartialUserSettings): void => { + dispatch(updateUserSettingsThunkAction(userSettings)); + }; + return [userSettings, updateUserSettings]; +} diff --git a/src/Frontend/web-workers/use-signals-worker.ts b/src/Frontend/web-workers/use-signals-worker.ts index 531d2d1cd..7c5204430 100644 --- a/src/Frontend/web-workers/use-signals-worker.ts +++ b/src/Frontend/web-workers/use-signals-worker.ts @@ -18,13 +18,13 @@ import { getResources, getSelectedResourceId, } from '../state/selectors/resource-selectors'; -import { useAreHiddenSignalsVisible } from '../state/variables/use-are-hidden-signals-visible'; import { useFilteredAttributions, useFilteredAttributionsInReportView, useFilteredSignals, } from '../state/variables/use-filtered-data'; import { useProgressData } from '../state/variables/use-progress-data'; +import { useUserSettings } from '../state/variables/use-user-setting'; import { useDebouncedInput } from '../util/use-debounced-input'; import { SignalsWorkerInput, SignalsWorkerOutput } from './signals-worker'; @@ -70,7 +70,8 @@ export function useSignalsWorker() { ] = useFilteredAttributionsInReportView(); const debouncedSignalSearch = useDebouncedInput(signalSearch); const debouncedAttributionSearch = useDebouncedInput(attributionSearch); - const areHiddenSignalsVisible = useAreHiddenSignalsVisible(); + const [userSettings] = useUserSettings(); + const areHiddenSignalsVisible = userSettings.areHiddenSignalsVisible; const [, setProgressData] = useProgressData(); useEffect(() => {