I have accidentally spotted this typo while working on this branch. It
has never caused any harm as both parameters callerurl and returnurl are
always passed. Blame copy/pasting of code blocks.
The method available_update_deployer::make_execution_widget() used to
have hard-coded return URL. Now it accepts the return URL as the second
parameter and passes it to the mdeploy.php utility.
The callerurl parameter is now correctly passed and used.
If the plugin has been only deployed to the disk without installing into
the database, do not allow going through the uninstallation procedure.
Not only it does not have much sense. But it can also lead to some
tricky situation due to dependencies. Better to block it and wait till
the plugin is either fully installed or removed from the disk.
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.
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.
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.
This has exactly the same behaviour as the existing
plugintype_name_plural() method, but returns the singular form of the
plugin type (such as 'Activity module').
This new method just takes the result of the core get_plugin_types() and
re-orders the plugin types in the same way as they are displayed at the
Plugins overview screen.
For statslib tests was enough to require cronlib.php as
far as all the tests there are using ob-capture / output
expectations, so the new function does not bork anything.
For pluginlib tests finally the use was deleted because
it's a part of cron not interesting (should be always "cheap")
and to keep it there we should be adding a bunch of output
expectations under some cases, for practically nothing.
Data cached in these caches change only at well defined places (during
need for upgrade checks, at the plugin management screen etc). So it
makes sense to use proper application caches instead of request caches.
This saves couple of database queries at almost every page in Moodle.
Where the static variable was not really needed (as in case of arrays
defined by the hard-coded list of items), non-static variable is used as
I believe that there is no real performance gain.
There was a problem experienced after 2.4.0 release because the version
of the 2.4.0 release was the same as 2.5dev release. So the version
value matched twice in the loop and the line was repeated in the email.
Before this patch, the available_update_checker::cron_notifications()
method accessed the availableupdates property. But for plugins, that property
basically contains the most recent version available. So we were missing
the check against the actual version installed.
The fix was simple - obtain available updates via the available_updates()
method that performs the check.
This patch makes Moodle call HTTP HEAD method via cURL to see if the ZIP
is expected to be downloadable by mdeploy.php. This is mainly intended
for SSL certificates check.
From now on, Moodle verifies the available updates provider server. To
make it work, either there must be a valid CA certificate available in
the operating system, or the administrator has to upload the valid CA
certificate to moodledata/moodleorgca.crt (PEM format) file manually.
The correct policy is that users with moodle/question:config can set the
default settings for particular qtypes. However, it requires
moodle/site:config in order to do manage qbehaviours or manage qtypes.
It turned out that this well-meant debugging message just caused a lot
of false alarm with no good reason. It seems to be better just ignore it
at the moment.
