This is much better API than using the array passed by reference. At the
moment, it is pretty hacky as it abuses text_progress_trace to output
raw HTML echoed by uninstall_plugin() but that will be improved later
while moving the logic out of that function into the plugin_manager.
As suggested by Tim Hunt during the peer-review, rendering methods
should not set properties of the page they are producing HTML code for.
Additionally, the page now uses correct check that the uninstalling can
happen.
This patch returns the layout of the Uninstall | Settings links to two
columns. There is no space saved on the screen by using the single
column and two columns align better. The reasoning for using single
column was that there would be multiple links in the 'Actions' column
but that does not seem to happen anytime soon.
The get_uninstall_url() method of all subclasses of plugininfo_base
class is now expected to always return moodle_url. Subclasses can use
the new method is_uninstall_allowed() to control the availability of the
'Uninstall' link at the Plugins overview page (previously they would do
it by get_uninstall_url() returning null). By default, URL to a new
general plugin uninstall tool is returned. Unless the plugin type needs
extra steps that can't be handled by plugininfo_xxx::uninstall() method
or xmldb_xxx_uninstall() function, this default URL should satisfy all
plugin types.
The overall logic is implemented in plugin_manager::can_install_plugin()
that respects the plugininfo class decision and vetoes it in certain
cases (typically when plugin or its subplugin is required by some other
plugin).
The plugin_manager::is_plugin_folder_removable() method should do just
one thing and do it well. Also, as was raised during the peer-review,
there should not be technical differences between standard plugins and
add-ons.
This patch improves and adds unit tests for the plugin_manager class.
These unit tests cover the existing functionalities. Tests for the
new features related directly with MDL-38259 will be added in a separate
commit (to make it clear what's related to it).
This is not directly related to the issue. However, it turned out that
if this method was called on plugin_manager without loaded plugins, it
would throw an error. This new implementation uses cleaner access to the
plugininfo subclass.
This is not actually related to MDL-38259 but it was discovered while
running unit tests for file_storage. When running the tests for this
class separately, the cronlib.php was not included (it is included
when this method is normally called during cron execution).
This is necessary now as many plugins management related features
started to use MUC intensively recently. During the development of this
issue, we realized that the plugin was still considered as installed if
caches were not purged.
Plugins may use this general tool for uninstallation and eventually
removal of the deployed source code. At the moment, this is implemented
as a wrapper for the core function uninstall_plugin() with an extra hook
in the relevant plugin info subclass.
For non-standard add-ons, the tool can remove the deployed plugin source
code as well, if the web server has required write permissions. Ideally,
all add-ons installed via the new tool_installaddon should be removable
via the web interface as well.
The method now returns null if there should be no 'Uninstall' link at
the Plugins management screen. For non-standard add-ons the method now
returns URL to a general uninstall tool.
Plugin info subclasses can still override the method to provide URL to
their own UI for uninstalling. If the plugin type wants to use the
general uninstall tool also for standard plugins, it should override
this method and explicitly return $this->get_default_uninstall_url().
Otherwise, the 'Uninstall' link will be provided for add-ons only.
