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.

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

Show an error message if installing a plugin fails #9218

Merged
merged 1 commit into from Nov 18, 2015
Merged

Show an error message if installing a plugin fails #9218

merged 1 commit into from Nov 18, 2015

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Nov 16, 2015

fixes #9160

Error message will look like this:

image

Initial error message suggested to just add the plugin to the PluginsInstalled list to hide the message but this is wrong since most likely the plugin is not installed yet. Many users won't know whether the plugin is already installed or not. Therefore we are rather recommending to disable the plugin.

I was thinking about using a translation there but as such an error occurs very early in the bootstrapping. The translations won't be loaded yet and we cannot even load plugin translations safely as there might be problems with plugins etc. to load correct language. Some day it would be nice to make this error message understandable in a way that someone doesn't have to understand the language. Eg with a Github-like diff with red and green colors and an arrow etc :) Such an error should occur rarely though so it might be ok like this for now.

Initially I wanted to show a notification when the plugin fails to install but this is not trivial as such an error occurs early in the bootstrapping. Notifications require a session and at this point the sessions are not yet loaded and we don't even know if the system is supposed to or even can start a session. Plugins are installed during FrontController::init and we don't know whether we are in a CLI mode, API call etc. Safest way is to let the error handler take care of it which makes sure to format the error message in a correct way (eg in an API call it will format the API error message).
Even saving all plugins installed error messages temporarily and outputting them later in dispatch is not safe since we don't know whether this method will be called and there might be nested dispatch calls which can lead to errors etc.

@tsteur tsteur added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Nov 16, 2015
@tsteur tsteur added this to the 2.15.1 milestone Nov 16, 2015
@mattab
Copy link
Member

mattab commented Nov 18, 2015

LGTM

mattab pushed a commit that referenced this pull request Nov 18, 2015
Show an error message if installing a plugin fails
@mattab mattab merged commit eca17c0 into master Nov 18, 2015
@mattab mattab deleted the 9160 branch November 18, 2015 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants