Ensure component updated event is triggered even if there is no update file #10807
Conversation
|
I'm not quite sure if this is the right place for it. Looking at the callers it might be called several times eg in |
|
Sounds good to move the events to the |
| */ | ||
| public function markComponentSuccessfullyUpdated($name, $version) | ||
| public function markComponentSuccessfullyUpdated($name, $version, $isNew = false) |
There was a problem hiding this comment.
Needed a new parameter to indicate if a new plugin was installed or updated, as that method is also used when installing new plugins
|
@tsteur would you have another look, if it's better that way? |
sgiehl
force-pushed
the
componentupdatedevent
branch
from
45c54d7
to
d849e9f
Compare
| * `PluginManager.pluginInstalled` triggered after a plugin was installed | ||
| * `PluginManager.pluginUninstalled` triggered after a plugin was uninstalled | ||
| * `Updater.componentInstalled` triggered after a plugin was installed | ||
| * `Updater.componentUninstalled` triggered after a plugin was uninstalled |
There was a problem hiding this comment.
We are using those events in CustomPiwikJS and it needs to be updated there if we rename it. Not sure if we should actually rename it as it is now inconsistent with PluginManager.pluginActivated and PluginManager.pluginDeactivated. Maybe we could have just both events as there might be different use cases where you only want to listen to pluginActivated but don't care so much when a dimension component was updated etc? Also it would be maybe more clear for developers to have both events as it would not be so much obvious that you need to listen to an Updater event when a plugin was installed. Any thoughts on this @sgiehl @mattab ?
There was a problem hiding this comment.
I'm not sure how to handle that best. Tried to make the event name consistent with Updater.componentUpdated. We could for sure have both events. Which would mean installing a plugin would trigger PluginManager.pluginInstalled and Updater.componentInstalled
There was a problem hiding this comment.
I think we should trigger both. That would be valuable and a bit more intuitive
sgiehl
force-pushed
the
componentupdatedevent
branch
from
d849e9f
to
b18cab6
Compare
|
As discussed now both events are triggered up on un-/install |
| * $mail->send(); | ||
| * }); | ||
| * | ||
| * @param string $componentName 'core', or plugin name |
There was a problem hiding this comment.
I think it could be also the name of a dimension etc. as they are basically components as well
| @@ -106,6 +144,13 @@ public function markComponentSuccessfullyUninstalled($name) | |||
| } catch (\Exception $e) { | |||
| // case when the option table is not yet created (before 0.2.10) | |||
| } | |||
|
|
|||
| /** | |||
| * Event triggered after a plugin has been uninstalled. | |||
There was a problem hiding this comment.
after a "component" has been uninstalled
| /** | ||
| * Event triggered after a component has been updated. | ||
| * | ||
| * Can be used to handle stuff that should be done after a component was updated |
There was a problem hiding this comment.
Can we maybe replace the word "stuff" with something else? :) Maybe with "logic" or so
|
I have mentioned small notes re the docs. Feel free to merge afterwards 👍 |
…e file (#10807) * ensure component updated event is triggered even if there is no update file * improve events triggered during component changes * improve wordings
Component update event has only be triggered if an update file was processed.
Shall we backport that for 2.x?