From ce22d8238cbed7f7405756baf045f3de4f71a727 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Fri, 14 Jun 2024 11:36:26 -0700 Subject: [PATCH] Mobile: Plugin settings: Fix plugins without settings can't be disabled without reinstall (#10579) Co-authored-by: Laurent Cozic --- .../plugins/PluginStates.installed.test.tsx | 47 ++++++++++++++++--- .../plugins/utils/usePluginCallbacks.ts | 10 ++-- .../plugins/utils/usePluginItem.ts | 6 ++- 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.installed.test.tsx b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.installed.test.tsx index 36ad5f06e5..1a4834734b 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.installed.test.tsx +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/PluginStates.installed.test.tsx @@ -52,6 +52,9 @@ const showInstalledTab = async () => { await userEvent.press(installedTab); }; +const abcPluginId = 'org.joplinapp.plugins.AbcSheetMusic'; +const backlinksPluginId = 'joplin.plugin.ambrt.backlinksToNote'; + describe('PluginStates.installed', () => { beforeEach(async () => { await setupDatabaseAndSynchronizer(0); @@ -77,9 +80,6 @@ describe('PluginStates.installed', () => { expect(shim.mobilePlatform()).toBe(platform); await mockRepositoryApiConstructor(); - const abcPluginId = 'org.joplinapp.plugins.AbcSheetMusic'; - const backlinksPluginId = 'joplin.plugin.ambrt.backlinksToNote'; - const defaultPluginSettings: PluginSettings = { [abcPluginId]: defaultPluginSetting(), [backlinksPluginId]: defaultPluginSetting(), @@ -117,7 +117,6 @@ describe('PluginStates.installed', () => { }); it('should show the current plugin version on updatable plugins', async () => { - const abcPluginId = 'org.joplinapp.plugins.AbcSheetMusic'; const defaultPluginSettings: PluginSettings = { [abcPluginId]: defaultPluginSetting() }; const outdatedVersion = '0.0.1'; @@ -173,7 +172,6 @@ describe('PluginStates.installed', () => { }); it('should support disabling plugins from the info modal', async () => { - const abcPluginId = 'org.joplinapp.plugins.AbcSheetMusic'; const defaultPluginSettings: PluginSettings = { [abcPluginId]: defaultPluginSetting() }; await loadMockPlugin(abcPluginId, 'ABC Sheet Music', '1.2.3', defaultPluginSettings); @@ -213,8 +211,6 @@ describe('PluginStates.installed', () => { it('should support updating plugins from the info modal', async () => { await mockRepositoryApiConstructor(); - const abcPluginId = 'org.joplinapp.plugins.AbcSheetMusic'; - const defaultPluginSettings: PluginSettings = { [abcPluginId]: defaultPluginSetting(), }; @@ -268,4 +264,41 @@ describe('PluginStates.installed', () => { wrapper.unmount(); }); + + it('should be possible to disable plugins, even if missing from plugins.states', async () => { + await mockRepositoryApiConstructor(); + + const defaultPluginSettings: PluginSettings = {}; + await loadMockPlugin(abcPluginId, 'ABC Sheet Music', '3.4.5', defaultPluginSettings); + expect(PluginService.instance().plugins[abcPluginId]).toBeTruthy(); + + const wrapper = render( + , + ); + await showInstalledTab(); + + // Should be shown as installed. + const card = await screen.findByText('ABC Sheet Music'); + expect(card).toBeVisible(); + + const user = userEvent.setup(); + await user.press(card); + + // Should be considered installed -- should be possible to disable: + const enabledSwitch = await screen.findByLabelText('Enabled'); + expect(enabledSwitch).toBeVisible(); + fireEvent(enabledSwitch, 'valueChange', false); + + // Disabling should add the plugin to plugins.states, if not present before. + await waitFor(() => { + expect(Setting.value('plugins.states')).toMatchObject({ + [abcPluginId]: { enabled: false }, + }); + }); + + wrapper.unmount(); + }); }); diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/utils/usePluginCallbacks.ts b/packages/app-mobile/components/screens/ConfigScreen/plugins/utils/usePluginCallbacks.ts index bb3b000e2c..77feb74904 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/utils/usePluginCallbacks.ts +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/utils/usePluginCallbacks.ts @@ -2,7 +2,7 @@ import { ItemEvent, OnPluginSettingChangeEvent } from '@joplin/lib/components/sh import useOnDeleteHandler from '@joplin/lib/components/shared/config/plugins/useOnDeleteHandler'; import useOnInstallHandler from '@joplin/lib/components/shared/config/plugins/useOnInstallHandler'; import NavService from '@joplin/lib/services/NavService'; -import { PluginSettings } from '@joplin/lib/services/plugins/PluginService'; +import { PluginSettings, defaultPluginSetting } from '@joplin/lib/services/plugins/PluginService'; import RepositoryApi from '@joplin/lib/services/plugins/RepositoryApi'; import { useCallback, useMemo, useState } from 'react'; @@ -29,14 +29,18 @@ const usePluginCallbacks = (props: Props) => { const updatePluginEnabled = useCallback((pluginId: string, enabled: boolean) => { const newSettings = { ...props.pluginSettings }; - newSettings[pluginId].enabled = enabled; + newSettings[pluginId] = { + ...defaultPluginSetting(), + ...newSettings[pluginId], + enabled, + }; props.updatePluginStates(newSettings); }, [props.pluginSettings, props.updatePluginStates]); const onToggle = useCallback((event: ItemEvent) => { const pluginId = event.item.manifest.id; - const settings = props.pluginSettings[pluginId]; + const settings = props.pluginSettings[pluginId] ?? defaultPluginSetting(); updatePluginEnabled(pluginId, !settings.enabled); }, [props.pluginSettings, updatePluginEnabled]); diff --git a/packages/app-mobile/components/screens/ConfigScreen/plugins/utils/usePluginItem.ts b/packages/app-mobile/components/screens/ConfigScreen/plugins/utils/usePluginItem.ts index b3085b976a..9c4e65de7a 100644 --- a/packages/app-mobile/components/screens/ConfigScreen/plugins/utils/usePluginItem.ts +++ b/packages/app-mobile/components/screens/ConfigScreen/plugins/utils/usePluginItem.ts @@ -21,12 +21,14 @@ const usePluginItem = (id: string, pluginSettings: PluginSettings, initialItem: if (!manifest) return null; const settings = pluginSettings[id]; + const installed = !!settings || !!plugin; + return { id, manifest, - installed: !!settings, - enabled: settings?.enabled ?? false, + installed, + enabled: settings?.enabled ?? installed, deleted: settings?.deleted ?? false, hasBeenUpdated: settings?.hasBeenUpdated ?? false, devMode: plugin?.devMode ?? false,