Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
MDL-39148 Non-installed plugins cannot be uninstalled
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.
  • Loading branch information
mudrd8mz committed Apr 19, 2013
1 parent 3a8c438 commit badf464
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 2 deletions.
6 changes: 6 additions & 0 deletions lib/pluginlib.php
Expand Up @@ -948,6 +948,12 @@ protected function common_uninstall_check(plugininfo_base $pluginfo) {
return false;
}

if ($pluginfo->get_status() === plugin_manager::PLUGIN_STATUS_NEW) {
// The plugin is not installed. It should be either installed or removed from the disk.
// Relying on this temporary state may be tricky.
return false;
}

if (is_null($pluginfo->get_uninstall_url())) {
// Backwards compatibility.
debugging('plugininfo_base subclasses should use is_uninstall_allowed() instead of returning null in get_uninstall_url()',
Expand Down
5 changes: 5 additions & 0 deletions lib/tests/fixtures/mockplugins/mod/new/version.php
@@ -0,0 +1,5 @@
<?php

$module->version = 2013041900;
$module->requires = 2012010100;
$module->component = 'mod_new';
14 changes: 12 additions & 2 deletions lib/tests/pluginlib_test.php
Expand Up @@ -53,11 +53,12 @@ public function test_get_plugins_of_type() {
$pluginman = testable_plugin_manager::instance();
$mods = $pluginman->get_plugins_of_type('mod');
$this->assertEquals('array', gettype($mods));
$this->assertEquals(4, count($mods));
$this->assertEquals(5, count($mods));
$this->assertTrue($mods['foo'] instanceof testable_plugininfo_mod);
$this->assertTrue($mods['bar'] instanceof testable_plugininfo_mod);
$this->assertTrue($mods['baz'] instanceof testable_plugininfo_mod);
$this->assertTrue($mods['qux'] instanceof testable_plugininfo_mod);
$this->assertTrue($mods['new'] instanceof testable_plugininfo_mod);
$foolishes = $pluginman->get_plugins_of_type('foolish');
$this->assertEquals(2, count($foolishes));
$this->assertTrue($foolishes['frog'] instanceof testable_pluginfo_foolish);
Expand All @@ -79,11 +80,13 @@ public function test_get_plugins() {
$this->assertTrue(isset($plugins['mod']['foo']));
$this->assertTrue(isset($plugins['mod']['bar']));
$this->assertTrue(isset($plugins['mod']['baz']));
$this->assertTrue(isset($plugins['mod']['new']));
$this->assertTrue(isset($plugins['foolish']['frog']));
$this->assertTrue(isset($plugins['foolish']['hippo']));
$this->assertTrue($plugins['mod']['foo'] instanceof testable_plugininfo_mod);
$this->assertTrue($plugins['mod']['bar'] instanceof testable_plugininfo_mod);
$this->assertTrue($plugins['mod']['baz'] instanceof testable_plugininfo_mod);
$this->assertTrue($plugins['mod']['new'] instanceof testable_plugininfo_mod);
$this->assertTrue($plugins['foolish']['frog'] instanceof testable_pluginfo_foolish);
$this->assertTrue($plugins['foolish']['hippo'] instanceof testable_pluginfo_foolish);
$this->assertTrue($plugins['bazmeg']['one'] instanceof testable_pluginfo_bazmeg);
Expand Down Expand Up @@ -217,6 +220,7 @@ public function test_get_status() {
$pluginman = testable_plugin_manager::instance();
$plugins = $pluginman->get_plugins();
$this->assertEquals(plugin_manager::PLUGIN_STATUS_UPGRADE, $plugins['mod']['foo']->get_status());
$this->assertEquals(plugin_manager::PLUGIN_STATUS_NEW, $plugins['mod']['new']->get_status());
$this->assertEquals(plugin_manager::PLUGIN_STATUS_NEW, $plugins['bazmeg']['one']->get_status());
$this->assertEquals(plugin_manager::PLUGIN_STATUS_UPTODATE, $plugins['quxcat']['one']->get_status());
}
Expand All @@ -240,6 +244,7 @@ public function test_can_uninstall_plugin() {
$this->assertFalse($pluginman->can_uninstall_plugin('mod_qux')); // Because even if no plugin (not even subplugins) declare
// dependency on it, but its subplugin can't be uninstalled.
$this->assertFalse($pluginman->can_uninstall_plugin('mod_baz')); // Because it's subplugin bazmeg_one is required by quxcat_one.
$this->assertFalse($pluginman->can_uninstall_plugin('mod_new')); // Because it is not installed.
$this->assertFalse($pluginman->can_uninstall_plugin('quxcat_one')); // Because of testable_pluginfo_quxcat::is_uninstall_allowed().
$this->assertFalse($pluginman->can_uninstall_plugin('foolish_frog')); // Because foolish_hippo requires it.
}
Expand Down Expand Up @@ -546,7 +551,9 @@ public function is_standard() {
}

public function load_db_version() {
$this->versiondb = 2012022900;
if ($this->component !== 'mod_new') {
$this->versiondb = 2012022900;
}
}

public function is_uninstall_allowed() {
Expand Down Expand Up @@ -660,6 +667,8 @@ public function get_plugins($disablecache=false) {
$dirroot.'/mod/baz', 'testable_plugininfo_mod'),
'qux' => plugininfo_default_factory::make('mod', $dirroot.'/qux', 'qux',
$dirroot.'/mod/qux', 'testable_plugininfo_mod'),
'new' => plugininfo_default_factory::make('mod', $dirroot.'/new', 'new',
$dirroot.'/mod/new', 'testable_plugininfo_mod'),
),
'foolish' => array(
'frog' => plugininfo_default_factory::make('foolish', $dirroot.'/mod/foo/lish', 'frog',
Expand All @@ -681,6 +690,7 @@ public function get_plugins($disablecache=false) {
$this->pluginsinfo['mod']['foo']->check_available_updates($checker);
$this->pluginsinfo['mod']['bar']->check_available_updates($checker);
$this->pluginsinfo['mod']['baz']->check_available_updates($checker);
$this->pluginsinfo['mod']['new']->check_available_updates($checker);
$this->pluginsinfo['bazmeg']['one']->check_available_updates($checker);
$this->pluginsinfo['quxcat']['one']->check_available_updates($checker);

Expand Down

0 comments on commit badf464

Please sign in to comment.