@tsteur opened this Pull Request on February 26th 2021 Member

Description:

Notices this earlier on demo2. We had removed various plugins from the disk but the config still had the plugins marked as installed. Then I wanted to reinstall the plugins but the marketplace wouldn't let me install it because it was thinking the plugin is already installed. That's because it was only looking at PluginsInstalled[] entries in the config but not if the plugin actually still existed in the file system.

This change now makes plugins installable again in such a case. I noticed few other usages of isPluginInstalled() but not always would we actually want to check if the plugin is in the filesystem. Either from a performance point of view (eg in the component update checker) or because the logic doesn't need/want it.

For now just wanted to fix this specific case. Seems there are no tests so far for the plugin manager but can add one if needed.

Review

  • [ ] Functional review done
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@diosmosis commented on March 2nd 2021 Member

@tsteur is this something we'd want covered by a test? (I can add one if needed)

@tsteur commented on March 2nd 2021 Member

@diosmosis added test for the plugins manager 👍 I reckon it might not easy to add a test for the marketplace where it thinks the plugin is installed but actually missing from the files since the marketplace would also need to show that plugin (meaning it would need to exist on the marketplace). I'm thinking a test wouldn't be needed

This Pull Request was closed on March 7th 2021
Powered by GitHub Issue Mirror