joplin
joplin copied to clipboard
Desktop: Install default plugins on first-app-start
Hey, This is a draft PR for installing default plugins. To implement this, I am installing plugins before the code that loads and runs the plugins, as to avoid restarting.
One issue I noticed is that, after this installation is complete, if we searched any plugin that we installed, we still see the "Install' button instead of "Installed". But after restarting the app, it displays it correctly as ''Installed". I think we need to update pluginSettings
in order to fix this. But apart from that I didn't notice any issue.
Feel free to give any suggestions you have.
@mak2002 For future reference, you can mark a PR as draft while creating it or after creating the PR
Please try to make progress on this PR. In general we prefer not to keep draft PR for too long. If you're not sure what you need to do to finish please ask us.
Sorry for making slow progress. I had some college stuff going on last week, so that's why less activity. In the upcoming week, I will add tests and some initial settings for plugins. If there is anything other than this that I should do, please let me know.
Ok no problem, take your time in that case. The thing with draft PRs is that I stumble upon them... and I can't do much because it doesn't make sense to review if it's not finished (it already takes me forever to review PRs that are supposed to be finished). Perhaps we will not allow draft PRs in the future, especially during GSoC.
Hey, Now the only thing remaining is tests for defaultPluginsSettings.
Hey, The PR is ready to be reviewed now! I have included .jpl files of some plugins for now but will remove it before merging as there will be another script handling this part.
Tests are passing now. Don't know why they were failing randomly.
Hey @laurent22, Sorry for causing trouble for you. I think I have missed some cases like adding new default plugin, duplication of plugin IDs. I will fix issues which you mentioned above asap. I will make sure all of my code is ready to merge.
Thanks for the update, the fact that most of your code is now in a utility file makes this feature easier to manage.
Does your code now handle updates correctly?
- What if a new default plugin is added? Does it get correctly added to the user's plugins?
- Does it handle correctly existing user (who already have settings) and new users (who don't)?
- What happens when a default plugin setting changes?
- Does it get correctly set for the user?
- What happens when the user has already set that setting to a different value?
- What if a new default plugin is added? Does it get correctly added to the user's plugins?
Yes. The new plugin gets installed and added correctly to user's plugins.
- Does it handle correctly existing user (who already have settings) and new users (who don't)?
Yes, It does. If the user already has some default plugins installed, then it will skip the installation and won't set default settings for them.
What happens when a default plugin setting changes?
- Does it get correctly set for the user?
- What happens when the user has already set that setting to a different value?
Do you mean if we change the default plugins settings from the code? If yes, then it won't affect users who already have default plugins installed. It will only affect the users who will be getting default plugins for first time.
Thanks for clarifying @mak2002, let's merge!
Hey @laurent22 , Can we revert this commit so that I can remove some plugin.jpl
files that I had kept for demo purposes? Sorry for the inconvenience.
@mak2002 for now please just make a PR removing the files. That way Laurent can decide how he wants to proceed when he sees this.