Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Desktop: Install default plugins on first-app-start #6585

Merged
merged 34 commits into from
Sep 1, 2022
Merged

Desktop: Install default plugins on first-app-start #6585

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
bd677c4
initial commit
mak2002 Jun 16, 2022
eb67977
don't load default plugins immediately after installing
mak2002 Jun 16, 2022
302b380
Merge branch 'dev' into install-default-plugins
laurent22 Jun 18, 2022
d8d86f2
fix installed state of default plugins
mak2002 Jun 21, 2022
75d2601
Merge branch 'install-default-plugins' of https://github.com/mak2002/…
mak2002 Jun 21, 2022
4491cf0
organize the code for installing default plugins
mak2002 Jun 23, 2022
84c02c9
improve types of functions
mak2002 Jun 24, 2022
8d61611
add function for default Plugins settings
mak2002 Jun 25, 2022
3c14807
add _built_in field in plugins manifest
mak2002 Jun 25, 2022
191c8a5
added test
mak2002 Jun 28, 2022
32c003b
implement initial settings for plugins
mak2002 Jun 29, 2022
3536393
removed code for mkdir command
mak2002 Jun 29, 2022
4aa26bc
remove unnecessary files
mak2002 Jun 29, 2022
78c2ac7
added tests for initial settings for default plugins
mak2002 Jul 2, 2022
46007c5
remove async keyword from app.ts
mak2002 Jul 2, 2022
ebde62b
Merge branch 'laurent22:dev' into install-default-plugins
mak2002 Jul 6, 2022
4d05817
Merge branch 'laurent22:dev' into install-default-plugins
mak2002 Jul 10, 2022
360de71
Merge branch 'laurent22:dev' into install-default-plugins
mak2002 Jul 13, 2022
25820d3
Merge branch 'dev' into install-default-plugins
mak2002 Jul 22, 2022
b80c487
refactor code
mak2002 Jul 25, 2022
228aa96
small fix
mak2002 Jul 25, 2022
aa3a5fa
Merge branch 'install-default-plugins' of https://github.com/mak2002/…
mak2002 Jul 25, 2022
343c4de
remove old plugins
mak2002 Jul 25, 2022
b68fd07
refactor code
mak2002 Jul 28, 2022
fbc1793
Merge branch 'laurent22:dev' into install-default-plugins
mak2002 Jul 30, 2022
d1db3ec
move default plugins functions into separate folder | update tests
mak2002 Jul 31, 2022
53ddafb
remove test plugins
mak2002 Jul 31, 2022
4cc80d1
small fix
mak2002 Jul 31, 2022
90dfd0d
add comment for checkArrayAndUpdate method
mak2002 Jul 31, 2022
a4fcf8e
make function for initial settings to support profileDir
mak2002 Jul 31, 2022
6ef7c3d
refactor plugins info object
mak2002 Aug 10, 2022
5f6671d
fix import
mak2002 Aug 10, 2022
d90dee1
update import in ConfigScreen
mak2002 Aug 11, 2022
263d33b
Merge branch 'laurent22:dev' into install-default-plugins
mak2002 Aug 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .eslintignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,9 @@ packages/app-desktop/gui/utils/convertToScreenCoordinates.js.map
packages/app-desktop/gui/utils/loadScript.d.ts
packages/app-desktop/gui/utils/loadScript.js
packages/app-desktop/gui/utils/loadScript.js.map
packages/app-desktop/installDefaultPlugins.d.ts
packages/app-desktop/installDefaultPlugins.js
packages/app-desktop/installDefaultPlugins.js.map
packages/app-desktop/loadResources.testEnv.d.ts
packages/app-desktop/loadResources.testEnv.js
packages/app-desktop/loadResources.testEnv.js.map
Expand Down Expand Up @@ -1573,6 +1576,9 @@ packages/lib/services/plugins/api/JoplinWorkspace.js.map
packages/lib/services/plugins/api/types.d.ts
packages/lib/services/plugins/api/types.js
packages/lib/services/plugins/api/types.js.map
packages/lib/services/plugins/defaultPlugins.d.ts
packages/lib/services/plugins/defaultPlugins.js
packages/lib/services/plugins/defaultPlugins.js.map
packages/lib/services/plugins/reducer.d.ts
packages/lib/services/plugins/reducer.js
packages/lib/services/plugins/reducer.js.map
Expand Down
6 changes: 6 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,9 @@ packages/app-desktop/gui/utils/convertToScreenCoordinates.js.map
packages/app-desktop/gui/utils/loadScript.d.ts
packages/app-desktop/gui/utils/loadScript.js
packages/app-desktop/gui/utils/loadScript.js.map
packages/app-desktop/installDefaultPlugins.d.ts
packages/app-desktop/installDefaultPlugins.js
packages/app-desktop/installDefaultPlugins.js.map
packages/app-desktop/loadResources.testEnv.d.ts
packages/app-desktop/loadResources.testEnv.js
packages/app-desktop/loadResources.testEnv.js.map
Expand Down Expand Up @@ -1563,6 +1566,9 @@ packages/lib/services/plugins/api/JoplinWorkspace.js.map
packages/lib/services/plugins/api/types.d.ts
packages/lib/services/plugins/api/types.js
packages/lib/services/plugins/api/types.js.map
packages/lib/services/plugins/defaultPlugins.d.ts
packages/lib/services/plugins/defaultPlugins.js
packages/lib/services/plugins/defaultPlugins.js.map
packages/lib/services/plugins/reducer.d.ts
packages/lib/services/plugins/reducer.js
packages/lib/services/plugins/reducer.js.map
Expand Down
4 changes: 3 additions & 1 deletion packages/app-desktop/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import checkForUpdates from './checkForUpdates';
import { AppState } from './app.reducer';
import syncDebugLog from '@joplin/lib/services/synchronizer/syncDebugLog';
import eventManager from '@joplin/lib/eventManager';
import installDefaultPlugins from './installDefaultPlugins';
// import { runIntegrationTests } from '@joplin/lib/services/e2ee/ppkTestUtils';

const pluginClasses = [
Expand Down Expand Up @@ -261,7 +262,7 @@ class Application extends BaseApplication {
service.initialize(packageInfo.version, PlatformImplementation.instance(), pluginRunner, this.store());
service.isSafeMode = Setting.value('isSafeMode');

const pluginSettings = service.unserializePluginSettings(Setting.value('plugins.states'));
let pluginSettings = service.unserializePluginSettings(Setting.value('plugins.states'));

{
// Users can add and remove plugins from the config screen at any
Expand All @@ -273,6 +274,7 @@ class Application extends BaseApplication {
}

try {
pluginSettings = await installDefaultPlugins(pluginSettings, service);
if (await shim.fsDriver().exists(Setting.value('pluginDir'))) {
await service.loadAndRunPlugins(Setting.value('pluginDir'), pluginSettings);
}
Expand Down
Binary file added packages/app-desktop/build/defaultPlugins/io.github.jackgruber.backup.jpl
View file
Open in desktop
Binary file not shown.
Binary file added packages/app-desktop/build/defaultPlugins/plugin.calebjohn.rich-markdown.jpl
View file
Open in desktop
Binary file not shown.
15 changes: 15 additions & 0 deletions packages/app-desktop/gui/ConfigScreen/ConfigScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import SyncTargetRegistry from '@joplin/lib/SyncTargetRegistry';
const shared = require('@joplin/lib/components/shared/config-shared.js');
import ClipperConfigScreen from '../ClipperConfigScreen';
import restart from '../../services/restart';
import getDefaultPluginSettings from '@joplin/lib/services/plugins/defaultPlugins';
const { KeymapConfigScreen } = require('../KeymapConfig/KeymapConfigScreen');

const settingKeyToControl: any = {
Expand Down Expand Up @@ -60,12 +61,26 @@ class ConfigScreenComponent extends React.Component<any, any> {
this.setState({ settings: this.props.settings });
}

updatePluginsStates(value: any) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a descriptive parameter name value should be newPluginStates. And please add typing.

const key = 'plugins.states';
const md = Setting.settingMetadata(key);
shared.updateSettingValue(this, key, value);

if (md.autoSave) {
shared.scheduleSaveSettings(this);
}
}

CalebJohn marked this conversation as resolved.
Show resolved Hide resolved
componentDidMount() {
if (this.props.defaultSection) {
this.setState({ selectedSectionName: this.props.defaultSection }, () => {
this.switchSection(this.props.defaultSection);
});
}
if (!Setting.value('updatedDefaultPluginsInstallStates')) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't it mean the plugin settings will be set only once? How about when we add a new default plugin?

Copy link
Contributor Author

@mak2002 mak2002 Jul 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now checking in installedDefaultPlugins array to see if new default plugin is available.

this.updatePluginsStates(getDefaultPluginSettings());
Setting.setValue('updatedDefaultPluginsInstallStates', true);
}
}

private async handleSettingButton(key: string) {
Expand Down
20 changes: 20 additions & 0 deletions packages/app-desktop/installDefaultPlugins.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import shim from '@joplin/lib/shim';
import PluginService, { defaultPluginSetting, PluginSettings } from '@joplin/lib/services/plugins/PluginService';
import path = require('path');
import { filename } from '@joplin/lib/path-utils';
import produce from 'immer';
import Setting from '@joplin/lib/models/Setting';

export default async function installDefaultPlugins(pluginSettings: PluginSettings, service: PluginService): Promise<PluginSettings> {
if (!Setting.value('firstStart')) return pluginSettings;
const defaultPlugins = await shim.fsDriver().readDirStats(path.join(__dirname, '..', 'app-desktop/build/defaultPlugins/'));
for (const plugin of defaultPlugins) {
const defaultPluginPath: string = path.join(__dirname, '..', `app-desktop/build/defaultPlugins/${plugin.path}`);
await service.installPlugin(defaultPluginPath, false);

pluginSettings = produce(pluginSettings, (draft: PluginSettings) => {
draft[filename(plugin.path)] = defaultPluginSetting();
});
}
return pluginSettings;
}
8 changes: 8 additions & 0 deletions packages/lib/models/Setting.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1504,6 +1504,14 @@ class Setting extends BaseModel {
storage: SettingStorage.Database,
},

updatedDefaultPluginsInstallStates: {
value: false,
type: SettingItemType.Bool,
public: false,
appTypes: [AppType.Desktop],
storage: SettingStorage.File,
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this won't cause a dependency loop. Please correct me if I am wrong.


lastSettingDefaultMigration: {
value: -1,
type: SettingItemType.Int,
Expand Down
10 changes: 6 additions & 4 deletions packages/lib/services/plugins/PluginService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ export default class PluginService extends BaseService {
return this.installPluginFromRepo(repoApi, pluginId);
}

public async installPlugin(jplPath: string): Promise<Plugin> {
public async installPlugin(jplPath: string, loadPlugin: boolean = true): Promise<Plugin | null> {
logger.info(`Installing plugin: "${jplPath}"`);

// Before moving the plugin to the profile directory, we load it
Expand All @@ -429,9 +429,11 @@ export default class PluginService extends BaseService {
await shim.fsDriver().copy(jplPath, destPath);

// Now load it from the profile directory
const plugin = await this.loadPluginFromPath(destPath);
if (!this.plugins_[plugin.id]) this.setPluginAt(plugin.id, plugin);
return plugin;
if (loadPlugin) {
const plugin = await this.loadPluginFromPath(destPath);
if (!this.plugins_[plugin.id]) this.setPluginAt(plugin.id, plugin);
return plugin;
} else { return null; }
}

private async pluginPath(pluginId: string) {
Expand Down
11 changes: 11 additions & 0 deletions packages/lib/services/plugins/defaultPlugins.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { defaultPluginSetting, PluginSettings } from './PluginService';

const defaultPluginsId = ['io.github.jackgruber.backup', 'plugin.calebjohn.rich-markdown'];

export default function getDefaultPluginSettings(): PluginSettings {
const settings: PluginSettings = {};
defaultPluginsId.map((pluginId: string) => {
settings[pluginId] = defaultPluginSetting();
});
return settings;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this function doesn't do anything useful. Also you are duplicating the list of default plugin IDs here and in the place where settings are defined. That list should only appear once in the codebase.