From 99ae0ec21a79da731d6567334ac4030af665bdc6 Mon Sep 17 00:00:00 2001 From: Vincent T Date: Thu, 21 Nov 2024 13:33:36 -0500 Subject: [PATCH] frontend: PluginSettings: Rework plugin settings local storage usage Signed-off-by: Vincent T --- .../PluginSettings/PluginSettings.stories.tsx | 19 +++ .../App/PluginSettings/PluginSettings.tsx | 148 +++++++++++------- .../PluginSettings/PluginSettingsDetails.tsx | 2 +- ...ttings.DefaultSaveEnable.stories.storyshot | 14 -- frontend/src/plugin/Plugins.tsx | 8 +- frontend/src/plugin/filterSources.test.ts | 27 +++- frontend/src/plugin/index.ts | 8 +- frontend/src/plugin/pluginSlice.test.tsx | 11 +- frontend/src/plugin/pluginsSlice.ts | 34 ++-- 9 files changed, 172 insertions(+), 99 deletions(-) diff --git a/frontend/src/components/App/PluginSettings/PluginSettings.stories.tsx b/frontend/src/components/App/PluginSettings/PluginSettings.stories.tsx index 3ed2ae0f04..0ac401ad0a 100644 --- a/frontend/src/components/App/PluginSettings/PluginSettings.stories.tsx +++ b/frontend/src/components/App/PluginSettings/PluginSettings.stories.tsx @@ -1,4 +1,5 @@ import { Meta, StoryFn } from '@storybook/react'; +import { PluginInfo } from '../../../plugin/pluginsSlice'; import { TestContext } from '../../../test'; import { PluginSettingsPure, PluginSettingsPureProps } from './PluginSettings'; @@ -40,6 +41,18 @@ function createDemoData(arrSize: number, useHomepage?: boolean) { return pluginArr; } +/** + * createDemoEnabledList function will create a list of plugin objects with a boolean value to enable/disable the plugin. + * The function will return an object of plugin names with a boolean value. + */ +function createDemoEnabledList(arr: PluginInfo[]): Record { + const enabledList = arr.reduce((acc, p) => { + acc[p.name] = !!p.isEnabled; + return acc; + }, {} as Record); + return enabledList; +} + /** * Creation of data arrays ranging from 0 to 50 to demo state of empty, few, many, and large numbers of data objects. * NOTE: The numbers used are up to the users preference. @@ -55,6 +68,7 @@ const demoEmpty = createDemoData(0); export const FewItems = Template.bind({}); FewItems.args = { plugins: demoFew, + pluginsEnabledMap: createDemoEnabledList(demoFew), onSave: plugins => { console.log('demo few', plugins); }, @@ -63,12 +77,14 @@ FewItems.args = { export const Empty = Template.bind({}); Empty.args = { plugins: demoEmpty, + pluginsEnabledMap: createDemoEnabledList(demoEmpty), }; /** NOTE: The save button will load by default on plugin page regardless of data */ export const DefaultSaveEnable = Template.bind({}); DefaultSaveEnable.args = { plugins: demoFewSaveEnable, + pluginsEnabledMap: createDemoEnabledList(demoFewSaveEnable), onSave: plugins => { console.log('demo few', plugins); }, @@ -78,6 +94,7 @@ DefaultSaveEnable.args = { export const ManyItems = Template.bind({}); ManyItems.args = { plugins: demoMany, + pluginsEnabledMap: createDemoEnabledList(demoMany), onSave: plugins => { console.log('demo many', plugins); }, @@ -86,6 +103,7 @@ ManyItems.args = { export const MoreItems = Template.bind({}); MoreItems.args = { plugins: demoMore, + pluginsEnabledMap: createDemoEnabledList(demoMore), onSave: plugins => { console.log('demo more', plugins); }, @@ -94,6 +112,7 @@ MoreItems.args = { export const EmptyHomepageItems = Template.bind({}); EmptyHomepageItems.args = { plugins: demoHomepageEmpty, + pluginsEnabledMap: createDemoEnabledList(demoHomepageEmpty), onSave: (plugins: any) => { console.log('Empty Homepage', plugins); }, diff --git a/frontend/src/components/App/PluginSettings/PluginSettings.tsx b/frontend/src/components/App/PluginSettings/PluginSettings.tsx index c55060b610..f7a01d5c2f 100644 --- a/frontend/src/components/App/PluginSettings/PluginSettings.tsx +++ b/frontend/src/components/App/PluginSettings/PluginSettings.tsx @@ -3,12 +3,12 @@ import Box from '@mui/material/Box'; import Button from '@mui/material/Button'; import Link from '@mui/material/Link'; import { MRT_Row } from 'material-react-table'; -import { useEffect, useState } from 'react'; +import { useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { useDispatch } from 'react-redux'; import helpers from '../../../helpers'; import { useFilterFunc } from '../../../lib/util'; -import { PluginInfo, reloadPage, setPluginSettings } from '../../../plugin/pluginsSlice'; +import { PluginInfo, reloadPage, setEnablePlugin } from '../../../plugin/pluginsSlice'; import { useTypedSelector } from '../../../redux/reducers/reducers'; import { Link as HeadlampLink, SectionBox, Table } from '../../common'; import SectionFilterHeader from '../../common/SectionFilterHeader'; @@ -24,6 +24,7 @@ import SectionFilterHeader from '../../common/SectionFilterHeader'; */ export interface PluginSettingsPureProps { plugins: PluginInfo[]; + pluginsEnabledMap: Record; onSave: (plugins: PluginInfo[]) => void; saveAlwaysEnable?: boolean; } @@ -92,20 +93,15 @@ const EnableSwitch = (props: SwitchProps) => { /** PluginSettingsPure is the main component to where we render the plugin data. */ export function PluginSettingsPure(props: PluginSettingsPureProps) { - const { t } = useTranslation(['translation']); - - /** Plugin arr to be rendered to the page from prop data */ - const pluginArr: any = props.plugins ? props.plugins : []; + const dispatch = useDispatch(); - /** enableSave state enables the save button when changes are made to the plugin list */ - const [enableSave, setEnableSave] = useState(false); + const { t } = useTranslation(['translation']); /** - * pluginChanges state is the array of plugin data and any current changes made by the user to a plugin's "Enable" field via toggler. - * The name and origin fields are split for consistency. + * Copy of plugins with pending changes */ - const [pluginChanges, setPluginChanges] = useState(() => - pluginArr.map((plugin: PluginInfo) => { + const [pluginChanges, setPluginChanges] = useState( + props.plugins.map((plugin: PluginInfo) => { const [author, name] = plugin.name.includes('@') ? plugin.name.split(/\/(.+)/) : [null, plugin.name]; @@ -119,42 +115,27 @@ export function PluginSettingsPure(props: PluginSettingsPureProps) { ); /** - * useEffect to control the rendering of the save button. - * By default, the enableSave is set to false. - * If props.plugins matches pluginChanges enableSave is set to false, disabling the save button. + * pendingPluginsEnabled is either from the local storage or the prop data */ - useEffect(() => { - /** This matcher function compares the fields of name and isEnabled of each object in props.plugins to each object in pluginChanges */ - function matcher(objA: PluginInfo, objB: PluginInfo) { - return objA.name === objB.name && objA.isEnabled === objB.isEnabled; - } - - /** - * arrayComp returns true if each object in both arrays are identical by name and isEnabled. - * If both arrays are identical in this scope, then no changes need to be saved. - * If they do not match, there are changes in the pluginChanges array that can be saved and thus enableSave should be enabled. - */ - const arrayComp = props.plugins.every((val, key) => matcher(val, pluginChanges[key])); + const [pendingPluginsEnabled, setPendingPluginsEnabled] = useState>( + props.pluginsEnabledMap + ); - /** For storybook usage, determines if the save button should be enabled by default */ - if (props.saveAlwaysEnable) { - setEnableSave(true); - } else { - if (arrayComp) { - setEnableSave(false); - } - if (!arrayComp) { - setEnableSave(true); - } - } - }, [pluginChanges]); + const enableSave = useMemo(() => { + return !pluginChanges.every((plugin: { name: string }) => { + return ( + Boolean(props.pluginsEnabledMap[plugin.name]) === + Boolean(pendingPluginsEnabled[plugin.name]) + ); + }); + }, [pendingPluginsEnabled, pluginChanges, props.pluginsEnabledMap]); /** * onSaveButton function to be called once the user clicks the Save button. - * This function then takes the current state of the pluginChanges array and inputs it to the onSave prop function. */ function onSaveButtonHandler() { - props.onSave(pluginChanges); + dispatch(setEnablePlugin(pendingPluginsEnabled)); + dispatch(reloadPage()); } /** @@ -162,17 +143,25 @@ export function PluginSettingsPure(props: PluginSettingsPureProps) { * This function is called on every plugin toggle action and recreates the state for pluginChanges. * Once the user clicks a toggle, the Save button is also rendered via setEnableSave. */ - function switchChangeHanlder(plug: { name: any }) { + function switchChangeHandler(plug: { name: any }) { const plugName = plug.name; - setPluginChanges((currentInfo: any[]) => - currentInfo.map((p: { name: any; isEnabled: any }) => { + setPluginChanges([ + ...pluginChanges.map(p => { if (p.name === plugName) { - return { ...p, isEnabled: !p.isEnabled }; + return { + ...p, + isEnabled: !p.isEnabled, + }; } return p; - }) - ); + }), + ]); + + setPendingPluginsEnabled({ + ...pendingPluginsEnabled, + [plugName]: !pendingPluginsEnabled[plugName], + }); } return ( @@ -235,7 +224,9 @@ export function PluginSettingsPure(props: PluginSettingsPureProps) { if (plugin.isCompatible === false) { return t('translation|Incompatible'); } - return plugin.isEnabled ? t('translation|Enabled') : t('translation|Disabled'); + return pendingPluginsEnabled[plugin.name] + ? t('translation|Enabled') + : t('translation|Disabled'); }, }, { @@ -247,8 +238,8 @@ export function PluginSettingsPure(props: PluginSettingsPureProps) { return ( switchChangeHanlder(plugin)} + checked={pendingPluginsEnabled[plugin.name]} + onChange={() => switchChangeHandler(plugin)} color="primary" name={plugin.name} /> @@ -284,15 +275,60 @@ export function PluginSettingsPure(props: PluginSettingsPureProps) { export default function PluginSettings() { const dispatch = useDispatch(); - const pluginSettings = useTypedSelector(state => state.plugins.pluginSettings); + const pluginData = useTypedSelector(state => state.plugins.pluginData); + + const oldLocalEnabledList = localStorage.getItem('headlampPluginSettings'); + const localEnabledList = localStorage.getItem('enabledPluginsList'); + + let pluginsEnabledMap; + + /** + * Note: this is a temporary fix to handle the migration of the old plugin settings. + */ + if (oldLocalEnabledList) { + const oldSettings = JSON.parse(oldLocalEnabledList); + + pluginsEnabledMap = oldSettings.reduce( + (acc: Record, p: { name: string; isEnabled: boolean }) => { + acc[p.name] = !!p.isEnabled; + return acc; + }, + {} as Record + ); + + localStorage.setItem('enabledPluginsList', JSON.stringify(pluginsEnabledMap)); + localStorage.removeItem('headlampPluginSettings'); + + dispatch(setEnablePlugin(pluginsEnabledMap)); + dispatch(reloadPage()); + } else { + /** + * NOTE: For compatibility with old settings, we need to check if the `localEnabledList` exists. + * If `localEnabledList` exists, parse it and assign it to `pluginsEnabledMap`. + * This indicates that previous plugin settings have been saved and can be used. + * + * If `localEnabledList` does not exist, it means the settings are not initialized + * and no previous plugin settings have been saved. In this case, default the plugins + * to being disabled to allow users to turn on their desired plugins. + */ + if (localEnabledList) { + pluginsEnabledMap = JSON.parse(localEnabledList) as Record; + } else { + pluginsEnabledMap = pluginData.reduce((acc, p) => { + acc[p.name] = !!p.isEnabled; + return acc; + }, {} as Record); + + dispatch(setEnablePlugin(pluginsEnabledMap)); + dispatch(reloadPage()); + } + } return ( { - dispatch(setPluginSettings(plugins)); - dispatch(reloadPage()); - }} + plugins={pluginData} + pluginsEnabledMap={pluginsEnabledMap} + onSave={() => {}} /> ); } diff --git a/frontend/src/components/App/PluginSettings/PluginSettingsDetails.tsx b/frontend/src/components/App/PluginSettings/PluginSettingsDetails.tsx index 776f916498..f1d2cc9883 100644 --- a/frontend/src/components/App/PluginSettings/PluginSettingsDetails.tsx +++ b/frontend/src/components/App/PluginSettings/PluginSettingsDetails.tsx @@ -54,7 +54,7 @@ const PluginSettingsDetailsInitializer = (props: { plugin: PluginInfo }) => { }; export default function PluginSettingsDetails() { - const pluginSettings = useTypedSelector(state => state.plugins.pluginSettings); + const pluginSettings = useTypedSelector(state => state.plugins.pluginData); const { name } = useParams<{ name: string }>(); const plugin = useMemo(() => { diff --git a/frontend/src/components/App/PluginSettings/__snapshots__/PluginSettings.DefaultSaveEnable.stories.storyshot b/frontend/src/components/App/PluginSettings/__snapshots__/PluginSettings.DefaultSaveEnable.stories.storyshot index 7702abea12..b4a87c5b74 100644 --- a/frontend/src/components/App/PluginSettings/__snapshots__/PluginSettings.DefaultSaveEnable.stories.storyshot +++ b/frontend/src/components/App/PluginSettings/__snapshots__/PluginSettings.DefaultSaveEnable.stories.storyshot @@ -579,19 +579,5 @@ -
- -
\ No newline at end of file diff --git a/frontend/src/plugin/Plugins.tsx b/frontend/src/plugin/Plugins.tsx index 46521ec7b5..8a28888f72 100644 --- a/frontend/src/plugin/Plugins.tsx +++ b/frontend/src/plugin/Plugins.tsx @@ -8,7 +8,7 @@ import helpers from '../helpers'; import { UI_INITIALIZE_PLUGIN_VIEWS } from '../redux/actions/actions'; import { useTypedSelector } from '../redux/reducers/reducers'; import { fetchAndExecutePlugins } from './index'; -import { pluginsLoaded, setPluginSettings } from './pluginsSlice'; +import { pluginsLoaded, setPluginData } from './pluginsSlice'; /** * For discovering and executing plugins. @@ -26,7 +26,8 @@ export default function Plugins() { const history = useHistory(); const { t } = useTranslation(); - const settingsPlugins = useTypedSelector(state => state.plugins.pluginSettings); + const settingsPlugins = useTypedSelector(state => state.plugins.pluginData); + const enabledPlugins = useTypedSelector(state => state.plugins.enabledPlugins); // only run on first load useEffect(() => { @@ -34,8 +35,9 @@ export default function Plugins() { fetchAndExecutePlugins( settingsPlugins, + enabledPlugins, updatedSettingsPackages => { - dispatch(setPluginSettings(updatedSettingsPackages)); + dispatch(setPluginData(updatedSettingsPackages)); }, incompatiblePlugins => { const pluginList = Object.values(incompatiblePlugins) diff --git a/frontend/src/plugin/filterSources.test.ts b/frontend/src/plugin/filterSources.test.ts index 3bc645fd91..609f9a0d41 100644 --- a/frontend/src/plugin/filterSources.test.ts +++ b/frontend/src/plugin/filterSources.test.ts @@ -5,15 +5,15 @@ describe('filterSources', () => { test('when sources is empty, it also returns an empty array', () => { const sources: string[] = []; const packageInfos: PluginInfo[] = []; - const settingsPackages = undefined; + const enabledPlugins: Record = {}; const appMode = false; const { sourcesToExecute } = filterSources( sources, packageInfos, + enabledPlugins, appMode, - '>=0.8.0-alpha.3', - settingsPackages + '>=0.8.0-alpha.3' ); expect(sourcesToExecute.length).toBe(0); }); @@ -32,15 +32,15 @@ describe('filterSources', () => { }, }, ]; - const settingsPackages = undefined; + const enabledPlugins: Record = {}; const appMode = false; const { sourcesToExecute, incompatiblePlugins } = filterSources( sources, packageInfos, + enabledPlugins, appMode, - '>=0.8.0-alpha.3', - settingsPackages + '>=0.8.0-alpha.3' ); expect(Object.keys(incompatiblePlugins).length).toBe(0); expect(sourcesToExecute[0]).toBe('source1'); @@ -67,10 +67,14 @@ describe('filterSources', () => { isEnabled: false, }, ]; + const enabledPlugins: Record = { + ourplugin1: false, + }; const appMode = true; const { sourcesToExecute } = filterSources( sources, packageInfos, + enabledPlugins, appMode, '>=0.8.0-alpha.3', settingsPackages @@ -121,10 +125,15 @@ describe('filterSources', () => { isEnabled: false, }, ]; + const enabledPlugins: Record = { + ourplugin1: true, + ourplugin2: false, + }; const appMode = true; const { sourcesToExecute } = filterSources( sources, packageInfos, + enabledPlugins, appMode, '>=0.8.0-alpha.3', settingsPackages @@ -176,10 +185,15 @@ describe('filterSources', () => { isEnabled: true, }, ]; + const enabledPlugins: Record = { + ourplugin1: true, + ourplugin2: true, + }; const appMode = true; const { sourcesToExecute, incompatiblePlugins } = filterSources( sources, packageInfos, + enabledPlugins, appMode, '>=0.8.0-alpha.3', settingsPackages @@ -193,6 +207,7 @@ describe('filterSources', () => { const disabledCompatCheck = filterSources( sources, packageInfos, + enabledPlugins, appMode, '', // empty string disables compatibility check settingsPackages diff --git a/frontend/src/plugin/index.ts b/frontend/src/plugin/index.ts index d42a63f672..98678aa7a1 100644 --- a/frontend/src/plugin/index.ts +++ b/frontend/src/plugin/index.ts @@ -137,6 +137,7 @@ export async function initializePlugins() { export function filterSources( sources: string[], packageInfos: PluginInfo[], + enabledPlugins: Record, appMode: boolean, compatibleVersion: string, settingsPackages?: PluginInfo[] @@ -161,9 +162,8 @@ export function filterSources( // settingsPackages might have a different order or length than packageInfos // If it's not in the settings don't enable the plugin. - const enabledInSettings = - settingsPackages[settingsPackages.findIndex(x => x.name === packageInfo.name)]?.isEnabled === - true; + + const enabledInSettings = enabledPlugins[packageInfo.name]; return enabledInSettings; }); @@ -242,6 +242,7 @@ export function updateSettingsPackages( */ export async function fetchAndExecutePlugins( settingsPackages: PluginInfo[], + enabledPlugins: Record, onSettingsChange: (plugins: PluginInfo[]) => void, onIncompatible: (plugins: Record) => void ) { @@ -298,6 +299,7 @@ export async function fetchAndExecutePlugins( const { sourcesToExecute, incompatiblePlugins } = filterSources( sources, packageInfos, + enabledPlugins, helpers.isElectron(), compatibleHeadlampPluginVersion, updatedSettingsPackages diff --git a/frontend/src/plugin/pluginSlice.test.tsx b/frontend/src/plugin/pluginSlice.test.tsx index 805f6f31e3..3ba719ece1 100644 --- a/frontend/src/plugin/pluginSlice.test.tsx +++ b/frontend/src/plugin/pluginSlice.test.tsx @@ -11,7 +11,8 @@ const initialState: PluginsState = { /** Once the plugins have been fetched and executed. */ loaded: false, /** If plugin settings are saved use those. */ - pluginSettings: JSON.parse(localStorage.getItem('headlampPluginSettings') || '[]'), + enabledPlugins: JSON.parse(localStorage.getItem('headlampPluginSettings') || '{}'), + pluginData: [], }; // Mock React component for testing @@ -24,7 +25,7 @@ describe('pluginsSlice reducers', () => { const existingPluginName = 'test-plugin'; const initialStateWithPlugin: PluginsState = { ...initialState, - pluginSettings: [ + pluginData: [ { name: existingPluginName, settingsComponent: undefined, @@ -41,15 +42,15 @@ describe('pluginsSlice reducers', () => { const newState = pluginsSlice.reducer(initialStateWithPlugin, action); - expect(newState.pluginSettings[0].settingsComponent).toBeDefined(); - expect(newState.pluginSettings[0].displaySettingsComponentWithSaveButton).toBe(true); + expect(newState.pluginData[0].settingsComponent).toBeDefined(); + expect(newState.pluginData[0].displaySettingsComponentWithSaveButton).toBe(true); }); test('should not modify state when plugin name does not match any existing plugin', () => { const nonExistingPluginName = 'non-existing-plugin'; const initialStateWithPlugin: PluginsState = { ...initialState, - pluginSettings: [ + pluginData: [ { name: 'existing-plugin', settingsComponent: undefined, diff --git a/frontend/src/plugin/pluginsSlice.ts b/frontend/src/plugin/pluginsSlice.ts index 37e3277ac0..868259e174 100644 --- a/frontend/src/plugin/pluginsSlice.ts +++ b/frontend/src/plugin/pluginsSlice.ts @@ -90,14 +90,17 @@ export type PluginInfo = { export interface PluginsState { /** Have plugins finished executing? */ loaded: boolean; + /** Map where key is plugin's name and value is whether it is enabled */ + enabledPlugins: Record; /** Information stored by settings about plugins. */ - pluginSettings: PluginInfo[]; + pluginData: PluginInfo[]; } const initialState: PluginsState = { /** Once the plugins have been fetched and executed. */ loaded: false, /** If plugin settings are saved use those. */ - pluginSettings: JSON.parse(localStorage.getItem('headlampPluginSettings') || '[]'), + enabledPlugins: JSON.parse(localStorage.getItem('enabledPluginsList') || '[]'), + pluginData: [], }; export const pluginsSlice = createSlice({ @@ -107,12 +110,16 @@ export const pluginsSlice = createSlice({ pluginsLoaded(state) { state.loaded = true; }, - /** - * Save the plugin settings. To both the store, and localStorage. - */ - setPluginSettings(state, action: PayloadAction) { - state.pluginSettings = action.payload; - localStorage.setItem('headlampPluginSettings', JSON.stringify(action.payload)); + + /** Updates the local storage for plugin enable settings */ + setEnablePlugin(state, action: PayloadAction>) { + state.enabledPlugins = action.payload; + localStorage.setItem('enabledPluginsList', JSON.stringify(action.payload)); + }, + + /** Sets the plugin data */ + setPluginData(state, action: PayloadAction) { + state.pluginData = action.payload; }, /** Reloads the browser page */ reloadPage() { @@ -130,7 +137,7 @@ export const pluginsSlice = createSlice({ }> ) { const { name, component, displaySaveButton } = action.payload; - state.pluginSettings = state.pluginSettings.map(plugin => { + state.pluginData = state.pluginData.map(plugin => { if (plugin.name === name) { return { ...plugin, @@ -144,7 +151,12 @@ export const pluginsSlice = createSlice({ }, }); -export const { pluginsLoaded, setPluginSettings, setPluginSettingsComponent, reloadPage } = - pluginsSlice.actions; +export const { + pluginsLoaded, + setEnablePlugin, + setPluginData, + setPluginSettingsComponent, + reloadPage, +} = pluginsSlice.actions; export default pluginsSlice.reducer;