MDL-39087 Simplify get_uninstall_url() interpretation
authorDavid Mudrák <david@moodle.com>
Thu, 11 Apr 2013 23:00:51 +0000 (01:00 +0200)
committerDavid Mudrák <david@moodle.com>
Thu, 11 Apr 2013 23:44:35 +0000 (01:44 +0200)
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).

admin/renderer.php
lib/editor/tinymce/adminlib.php
lib/pluginlib.php
lib/tests/fixtures/mockplugins/mod/baz/meg/one/version.php [new file with mode: 0644]
lib/tests/fixtures/mockplugins/mod/baz/version.php [new file with mode: 0644]
lib/tests/fixtures/mockplugins/mod/foo/lish/hippo/version.php [new file with mode: 0644]
lib/tests/fixtures/mockplugins/mod/foo/version.php
lib/tests/fixtures/mockplugins/mod/qux/cat/one/version.php [new file with mode: 0644]
lib/tests/fixtures/mockplugins/mod/qux/version.php [new file with mode: 0644]
lib/tests/pluginlib_test.php
lib/upgrade.txt

index 1784de9..c215a89 100644 (file)
@@ -1229,8 +1229,8 @@ class core_admin_renderer extends plugin_renderer_base {
                     $actions[] = html_writer::link($settingsurl, get_string('settings', 'core_plugin'), array('class' => 'settings'));
                 }
 
-                $uninstallurl = $plugin->get_uninstall_url();
-                if (!is_null($uninstallurl)) {
+                if ($pluginman->can_uninstall_plugin($plugin->component)) {
+                    $uninstallurl = $plugin->get_uninstall_url();
                     $actions[] = html_writer::link($uninstallurl, get_string('uninstall', 'core_plugin'), array('class' => 'uninstall'));
                 }
 
index 36eb494..6b4b232 100644 (file)
@@ -35,6 +35,11 @@ require_once("$CFG->libdir/pluginlib.php");
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 class plugininfo_tinymce extends plugininfo_base {
+
+    public function is_uninstall_allowed() {
+        return true;
+    }
+
     public function get_uninstall_url() {
         return new moodle_url('/lib/editor/tinymce/subplugins.php', array('delete' => $this->name, 'sesskey' => sesskey()));
     }
index 2522e22..354cc43 100644 (file)
@@ -468,6 +468,70 @@ class plugin_manager {
         return $return;
     }
 
+    /**
+     * Is it possible to uninstall the given plugin?
+     *
+     * False is returned if the plugininfo subclass declares the uninstall should
+     * not be allowed via {@link plugininfo_base::is_uninstall_allowed()} or if the
+     * core vetoes it (e.g. becase the plugin or some of its subplugins is required
+     * by some other installed plugin).
+     *
+     * @param string $component full frankenstyle name, e.g. mod_foobar
+     * @return bool
+     */
+    public function can_uninstall_plugin($component) {
+
+        $pluginfo = $this->get_plugin_info($component);
+
+        if (is_null($pluginfo)) {
+            return false;
+        }
+
+        // Backwards compatibility check.
+        if (is_null($pluginfo->get_uninstall_url())) {
+            debugging('plugininfo_base subclasses should use is_uninstall_allowed() instead of returning null in get_uninstall_url()',
+                DEBUG_DEVELOPER);
+            return false;
+        }
+
+        // In case the $component has subplugins, get their list.
+        $mysubplugins = $this->get_subplugins_of_plugin($pluginfo->component);
+
+        // In case the $component is a subplugin, get all subplugins of its parent (i.e. siblings).
+        $myparent = $this->get_parent_of_subplugin($pluginfo->type);
+        if ($myparent === false) {
+            $mysiblings = array();
+        } else {
+            $mysiblings = $this->get_subplugins_of_plugin($myparent);
+        }
+
+        // If the plugin has subplugins, check we can uninstall them first.
+        foreach ($mysubplugins as $subpluginfo) {
+            if (!$this->can_uninstall_plugin($subpluginfo->component)) {
+                return false;
+            }
+        }
+
+        // Check there are no other plugins (but eventual subplugins or siblings) that
+        // require us. Subplugins would be uninstalled together with the parent plugin
+        // without the need to uninstall each of them individually.
+        foreach ($this->other_plugins_that_require($pluginfo->component) as $requiresme) {
+            $ismyparent = ($myparent === $requiresme);
+            $ismysubplugin = in_array($requiresme, array_keys($mysubplugins));
+            $ismysibling = in_array($requiresme, array_keys($mysiblings));
+            if (!$ismyparent and !$ismysubplugin and !$ismysibling) {
+                return false;
+            }
+        }
+
+        // Finally give the plugin plugininfo subclass a chance to prevent uninstallation.
+        if (!$pluginfo->is_uninstall_allowed()) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * Uninstall the given plugin.
      *
@@ -2500,6 +2564,24 @@ abstract class plugininfo_base {
         return $this->dependencies;
     }
 
+    /**
+     * Is this is a subplugin?
+     *
+     * @return boolean
+     */
+    public function is_subplugin() {
+        return ($this->get_parent_plugin() !== false);
+    }
+
+    /**
+     * If I am a subplugin, return the name of my parent plugin.
+     *
+     * @return string|bool false if not a subplugin, name of the parent otherwise
+     */
+    public function get_parent_plugin() {
+        return $this->get_plugin_manager()->get_parent_of_subplugin($this->type);
+    }
+
     /**
      * Sets {@link $versiondb} property to a numerical value representing the
      * currently installed version of the plugin.
@@ -2710,31 +2792,41 @@ abstract class plugininfo_base {
     }
 
     /**
-     * Returns the URL of the screen where this plugin can be uninstalled
+     * Should there be a way to uninstall the plugin via the administration UI
      *
-     * Visiting that URL must be safe, that is a manual confirmation is needed
-     * for actual uninstallation of the plugin. Null value means that the
-     * plugin cannot be uninstalled (such as due to dependencies), or it does
-     * not support uninstallation, or the location of the screen is not
-     * available (shortly, the 'Uninstall' link should not be displayed).
+     * By default, uninstallation is allowed for all non-standard add-ons. Subclasses
+     * may want to override this to allow uninstallation of all plugins (simply by
+     * returning true unconditionally). Subplugins follow their parent plugin's
+     * decision by default.
      *
-     * By default, URL to a common uninstalling handler is returned for all
-     * add-ons and null is returned for standard plugins.
+     * Note that even if true is returned, the core may still prohibit the uninstallation,
+     * e.g. in case there are other plugins that depend on this one.
      *
-     * @return null|moodle_url
+     * @return boolean
      */
-    public function get_uninstall_url() {
+    public function is_uninstall_allowed() {
 
-        if ($this->is_standard()) {
-            return null;
+        if ($this->is_subplugin()) {
+            return $this->get_plugin_manager()->get_plugin_info($this->get_parent_plugin())->is_uninstall_allowed();
         }
 
-        $pluginman = plugin_manager::instance();
-        $requiredby = $pluginman->other_plugins_that_require($this->component);
-        if (!empty($requiredby)) {
-            return null;
+        if ($this->is_standard()) {
+            return false;
         }
 
+        return true;
+    }
+
+    /**
+     * Returns the URL of the screen where this plugin can be uninstalled
+     *
+     * Visiting that URL must be safe, that is a manual confirmation is needed
+     * for actual uninstallation of the plugin. By default, URL to a common
+     * uninstalling tool is returned.
+     *
+     * @return moodle_url
+     */
+    public function get_uninstall_url() {
         return $this->get_default_uninstall_url();
     }
 
@@ -2771,7 +2863,7 @@ abstract class plugininfo_base {
      *
      * @return moodle_url
      */
-    protected function get_default_uninstall_url() {
+    protected final function get_default_uninstall_url() {
         return new moodle_url('/admin/plugins.php', array(
             'sesskey' => sesskey(),
             'uninstall' => $this->component,
@@ -2809,6 +2901,15 @@ abstract class plugininfo_base {
             return false;
         }
     }
+
+    /**
+     * Provides access to the plugin_manager singleton.
+     *
+     * @return plugin_manmager
+     */
+    protected function get_plugin_manager() {
+        return plugin_manager::instance();
+    }
 }
 
 
@@ -2935,8 +3036,11 @@ class plugininfo_block extends plugininfo_base {
         }
     }
 
-    public function get_uninstall_url() {
+    public function is_uninstall_allowed() {
+        return true;
+    }
 
+    public function get_uninstall_url() {
         $blocksinfo = self::get_blocks_info();
         return new moodle_url('/admin/blocks.php', array('delete' => $blocksinfo[$this->name]->id, 'sesskey' => sesskey()));
     }
@@ -3071,6 +3175,10 @@ class plugininfo_filter extends plugininfo_base {
         }
     }
 
+    public function is_uninstall_allowed() {
+        return true;
+    }
+
     public function get_uninstall_url() {
         return new moodle_url('/admin/filters.php', array('sesskey' => sesskey(), 'filterpath' => $this->name, 'action' => 'delete'));
     }
@@ -3254,15 +3362,24 @@ class plugininfo_mod extends plugininfo_base {
         }
     }
 
-    public function get_uninstall_url() {
+    /**
+     * Allow all activity modules but Forum to be uninstalled.
 
-        if ($this->name !== 'forum') {
-            return new moodle_url('/admin/modules.php', array('delete' => $this->name, 'sesskey' => sesskey()));
+     * This exception for the Forum has been hard-coded in Moodle since ages,
+     * we may want to re-think it one day.
+     */
+    public function is_uninstall_allowed() {
+        if ($this->name === 'forum') {
+            return false;
         } else {
-            return null;
+            return true;
         }
     }
 
+    public function get_uninstall_url() {
+        return new moodle_url('/admin/modules.php', array('delete' => $this->name, 'sesskey' => sesskey()));
+    }
+
     /**
      * Provides access to the records in {modules} table
      *
@@ -3296,6 +3413,10 @@ class plugininfo_mod extends plugininfo_base {
  */
 class plugininfo_qbehaviour extends plugininfo_base {
 
+    public function is_uninstall_allowed() {
+        return true;
+    }
+
     public function get_uninstall_url() {
         return new moodle_url('/admin/qbehaviours.php',
                 array('delete' => $this->name, 'sesskey' => sesskey()));
@@ -3308,6 +3429,10 @@ class plugininfo_qbehaviour extends plugininfo_base {
  */
 class plugininfo_qtype extends plugininfo_base {
 
+    public function is_uninstall_allowed() {
+        return true;
+    }
+
     public function get_uninstall_url() {
         return new moodle_url('/admin/qtypes.php',
                 array('delete' => $this->name, 'sesskey' => sesskey()));
@@ -3432,6 +3557,10 @@ class plugininfo_enrol extends plugininfo_base {
         }
     }
 
+    public function is_uninstall_allowed() {
+        return true;
+    }
+
     public function get_uninstall_url() {
         return new moodle_url('/admin/enrol.php', array('action' => 'uninstall', 'enrol' => $this->name, 'sesskey' => sesskey()));
     }
@@ -3482,16 +3611,21 @@ class plugininfo_message extends plugininfo_base {
         }
     }
 
+    public function is_uninstall_allowed() {
+        $processors = get_message_processors();
+        if (isset($processors[$this->name])) {
+            return true;
+        } else {
+            return false;
+        }
+    }
+
     /**
      * @see plugintype_interface::get_uninstall_url()
      */
     public function get_uninstall_url() {
         $processors = get_message_processors();
-        if (isset($processors[$this->name])) {
-            return new moodle_url('/admin/message.php', array('uninstall' => $processors[$this->name]->id, 'sesskey' => sesskey()));
-        } else {
-            return null;
-        }
+        return new moodle_url('/admin/message.php', array('uninstall' => $processors[$this->name]->id, 'sesskey' => sesskey()));
     }
 }
 
@@ -3636,6 +3770,10 @@ class plugininfo_mnetservice extends plugininfo_base {
  */
 class plugininfo_tool extends plugininfo_base {
 
+    public function is_uninstall_allowed() {
+        return true;
+    }
+
     public function get_uninstall_url() {
         return new moodle_url('/admin/tools.php', array('delete' => $this->name, 'sesskey' => sesskey()));
     }
@@ -3647,6 +3785,10 @@ class plugininfo_tool extends plugininfo_base {
  */
 class plugininfo_report extends plugininfo_base {
 
+    public function is_uninstall_allowed() {
+        return true;
+    }
+
     public function get_uninstall_url() {
         return new moodle_url('/admin/reports.php', array('delete' => $this->name, 'sesskey' => sesskey()));
     }
@@ -3770,6 +3912,10 @@ class plugininfo_webservice extends plugininfo_base {
         return false;
     }
 
+    public function is_uninstall_allowed() {
+        return true;
+    }
+
     public function get_uninstall_url() {
         return new moodle_url('/admin/webservice/protocols.php',
                 array('sesskey' => sesskey(), 'action' => 'uninstall', 'webservice' => $this->name));
@@ -3825,11 +3971,16 @@ class plugininfo_format extends plugininfo_base {
         return !get_config($this->component, 'disabled');
     }
 
-    public function get_uninstall_url() {
+    public function is_uninstall_allowed() {
         if ($this->name !== get_config('moodlecourse', 'format') && $this->name !== 'site') {
-            return new moodle_url('/admin/courseformats.php',
-                    array('sesskey' => sesskey(), 'action' => 'uninstall', 'format' => $this->name));
+            return true;
+        } else {
+            return false;
         }
-        return null;
+    }
+
+    public function get_uninstall_url() {
+        return new moodle_url('/admin/courseformats.php',
+                array('sesskey' => sesskey(), 'action' => 'uninstall', 'format' => $this->name));
     }
 }
diff --git a/lib/tests/fixtures/mockplugins/mod/baz/meg/one/version.php b/lib/tests/fixtures/mockplugins/mod/baz/meg/one/version.php
new file mode 100644 (file)
index 0000000..b099a77
--- /dev/null
@@ -0,0 +1,5 @@
+<?php
+
+$plugin->version = 2013041103;
+$plugin->requires = 2013010100;
+$plugin->component = 'bazmeg_one';
diff --git a/lib/tests/fixtures/mockplugins/mod/baz/version.php b/lib/tests/fixtures/mockplugins/mod/baz/version.php
new file mode 100644 (file)
index 0000000..47613f6
--- /dev/null
@@ -0,0 +1,4 @@
+<?php
+
+$module->version = 2012030500;
+$module->requires = 2012010100;
diff --git a/lib/tests/fixtures/mockplugins/mod/foo/lish/hippo/version.php b/lib/tests/fixtures/mockplugins/mod/foo/lish/hippo/version.php
new file mode 100644 (file)
index 0000000..f64b02d
--- /dev/null
@@ -0,0 +1,6 @@
+<?php
+
+$plugin->version = 2013041103;
+$plugin->requires = 2012010100;
+$plugin->component = 'foolish_hippo';
+$plugin->dependencies = array('foolish_frog' => ANY_VERSION);
index 5bcd9ed..a12a76a 100644 (file)
@@ -6,4 +6,5 @@ $module->component = 'mod_foo';
 $module->dependencies = array(
     'mod_bar' => 2012030500,
     'mod_missing' => ANY_VERSION,
+    'foolish_frog' => ANY_VERSION,
 );
diff --git a/lib/tests/fixtures/mockplugins/mod/qux/cat/one/version.php b/lib/tests/fixtures/mockplugins/mod/qux/cat/one/version.php
new file mode 100644 (file)
index 0000000..4c9e978
--- /dev/null
@@ -0,0 +1,6 @@
+<?php
+
+$plugin->version = 2013041103;
+$plugin->requires = 2013010100;
+$plugin->component = 'quxcat_one';
+$plugin->dependencies = array('bazmeg_one' => 2013010100);
diff --git a/lib/tests/fixtures/mockplugins/mod/qux/version.php b/lib/tests/fixtures/mockplugins/mod/qux/version.php
new file mode 100644 (file)
index 0000000..9c1c5f0
--- /dev/null
@@ -0,0 +1,5 @@
+<?php
+
+$plugin->version = 2013041103;
+$plugin->requires = 2013010100;
+$plugin->component = 'mod_qux';
index 39a1db0..0556b2e 100644 (file)
@@ -53,12 +53,21 @@ class plugin_manager_test extends advanced_testcase {
         $pluginman = testable_plugin_manager::instance();
         $mods = $pluginman->get_plugins_of_type('mod');
         $this->assertEquals('array', gettype($mods));
-        $this->assertEquals(2, count($mods));
+        $this->assertEquals(4, 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);
         $foolishes = $pluginman->get_plugins_of_type('foolish');
-        $this->assertEquals(1, count($foolishes));
+        $this->assertEquals(2, count($foolishes));
         $this->assertTrue($foolishes['frog'] instanceof testable_pluginfo_foolish);
+        $this->assertTrue($foolishes['hippo'] instanceof testable_pluginfo_foolish);
+        $bazmegs = $pluginman->get_plugins_of_type('bazmeg');
+        $this->assertEquals(1, count($bazmegs));
+        $this->assertTrue($bazmegs['one'] instanceof testable_pluginfo_bazmeg);
+        $quxcats = $pluginman->get_plugins_of_type('quxcat');
+        $this->assertEquals(1, count($quxcats));
+        $this->assertTrue($quxcats['one'] instanceof testable_pluginfo_quxcat);
         $unknown = $pluginman->get_plugins_of_type('muhehe');
         $this->assertSame(array(), $unknown);
     }
@@ -69,10 +78,16 @@ class plugin_manager_test extends advanced_testcase {
         $this->assertEquals('array', gettype($plugins));
         $this->assertTrue(isset($plugins['mod']['foo']));
         $this->assertTrue(isset($plugins['mod']['bar']));
+        $this->assertTrue(isset($plugins['mod']['baz']));
         $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['foolish']['frog'] instanceof testable_pluginfo_foolish);
+        $this->assertTrue($plugins['foolish']['hippo'] instanceof testable_pluginfo_foolish);
+        $this->assertTrue($plugins['bazmeg']['one'] instanceof testable_pluginfo_bazmeg);
+        $this->assertTrue($plugins['quxcat']['one'] instanceof testable_pluginfo_quxcat);
     }
 
     public function test_get_subplugins_of_plugin() {
@@ -81,21 +96,39 @@ class plugin_manager_test extends advanced_testcase {
         $this->assertSame(array(), $pluginman->get_subplugins_of_plugin('mod_bar'));
         $foosubs = $pluginman->get_subplugins_of_plugin('mod_foo');
         $this->assertEquals('array', gettype($foosubs));
-        $this->assertEquals(1, count($foosubs));
+        $this->assertEquals(2, count($foosubs));
         $this->assertTrue($foosubs['foolish_frog'] instanceof testable_pluginfo_foolish);
+        $this->assertTrue($foosubs['foolish_hippo'] instanceof testable_pluginfo_foolish);
+        $bazsubs = $pluginman->get_subplugins_of_plugin('mod_baz');
+        $this->assertEquals('array', gettype($bazsubs));
+        $this->assertEquals(1, count($bazsubs));
+        $this->assertTrue($bazsubs['bazmeg_one'] instanceof testable_pluginfo_bazmeg);
+        $quxsubs = $pluginman->get_subplugins_of_plugin('mod_qux');
+        $this->assertEquals('array', gettype($quxsubs));
+        $this->assertEquals(1, count($quxsubs));
+        $this->assertTrue($quxsubs['quxcat_one'] instanceof testable_pluginfo_quxcat);
     }
 
     public function test_get_subplugins() {
         $pluginman = testable_plugin_manager::instance();
         $subplugins = $pluginman->get_subplugins();
         $this->assertTrue(isset($subplugins['mod_foo']['foolish']));
+        $this->assertTrue(isset($subplugins['mod_baz']['bazmeg']));
+        $this->assertTrue(isset($subplugins['mod_qux']['quxcat']));
     }
 
     public function test_get_parent_of_subplugin() {
         $pluginman = testable_plugin_manager::instance();
         $this->assertEquals('mod_foo', $pluginman->get_parent_of_subplugin('foolish'));
+        $this->assertEquals('mod_baz', $pluginman->get_parent_of_subplugin('bazmeg'));
+        $this->assertEquals('mod_qux', $pluginman->get_parent_of_subplugin('quxcat'));
         $this->assertSame(false, $pluginman->get_parent_of_subplugin('mod'));
         $this->assertSame(false, $pluginman->get_parent_of_subplugin('unknown'));
+        $plugins = $pluginman->get_plugins();
+        $this->assertFalse($plugins['mod']['foo']->is_subplugin());
+        $this->assertSame(false, $plugins['mod']['foo']->get_parent_plugin());
+        $this->assertTrue($plugins['foolish']['frog']->is_subplugin());
+        $this->assertEquals('mod_foo', $plugins['foolish']['frog']->get_parent_plugin());
     }
 
     public function test_plugin_name() {
@@ -103,6 +136,9 @@ class plugin_manager_test extends advanced_testcase {
         $this->assertEquals('Foo', $pluginman->plugin_name('mod_foo'));
         $this->assertEquals('Bar', $pluginman->plugin_name('mod_bar'));
         $this->assertEquals('Frog', $pluginman->plugin_name('foolish_frog'));
+        $this->assertEquals('Hippo', $pluginman->plugin_name('foolish_hippo'));
+        $this->assertEquals('One', $pluginman->plugin_name('bazmeg_one'));
+        $this->assertEquals('One', $pluginman->plugin_name('quxcat_one'));
     }
 
     public function test_get_plugin_info() {
@@ -114,9 +150,13 @@ class plugin_manager_test extends advanced_testcase {
     public function test_other_plugins_that_require() {
         $pluginman = testable_plugin_manager::instance();
         $this->assertEquals(array('foolish_frog'), $pluginman->other_plugins_that_require('mod_foo'));
-        $this->assertEquals(array(), $pluginman->other_plugins_that_require('foolish_frog'));
+        $this->assertEquals(2, count($pluginman->other_plugins_that_require('foolish_frog')));
+        $this->assertTrue(in_array('foolish_hippo', $pluginman->other_plugins_that_require('foolish_frog')));
+        $this->assertTrue(in_array('mod_foo', $pluginman->other_plugins_that_require('foolish_frog')));
+        $this->assertEquals(array(), $pluginman->other_plugins_that_require('foolish_hippo'));
         $this->assertEquals(array('mod_foo'), $pluginman->other_plugins_that_require('mod_bar'));
         $this->assertEquals(array('mod_foo'), $pluginman->other_plugins_that_require('mod_missing'));
+        $this->assertEquals(array('quxcat_one'), $pluginman->other_plugins_that_require('bazmeg_one'));
     }
 
     public function test_are_dependencies_satisfied() {
@@ -144,18 +184,21 @@ class plugin_manager_test extends advanced_testcase {
         $this->assertTrue(in_array('mod_foo', $failedplugins)); // Requires mod_missing
         $this->assertFalse(in_array('mod_bar', $failedplugins));
         $this->assertFalse(in_array('foolish_frog', $failedplugins));
+        $this->assertFalse(in_array('foolish_hippo', $failedplugins));
 
         $failedplugins = array();
         $this->assertFalse($pluginman->all_plugins_ok(2012010100, $failedplugins));
         $this->assertTrue(in_array('mod_foo', $failedplugins)); // Requires mod_missing
         $this->assertFalse(in_array('mod_bar', $failedplugins));
         $this->assertTrue(in_array('foolish_frog', $failedplugins)); // Requires Moodle 2013010100
+        $this->assertFalse(in_array('foolish_hippo', $failedplugins));
 
         $failedplugins = array();
         $this->assertFalse($pluginman->all_plugins_ok(2011010100, $failedplugins));
         $this->assertTrue(in_array('mod_foo', $failedplugins)); // Requires mod_missing and Moodle 2012010100
         $this->assertTrue(in_array('mod_bar', $failedplugins)); // Requires Moodle 2012010100
         $this->assertTrue(in_array('foolish_frog', $failedplugins)); // Requires Moodle 2013010100
+        $this->assertTrue(in_array('foolish_hippo', $failedplugins)); // Requires Moodle 2012010100
     }
 
     public function test_some_plugins_updatable() {
@@ -173,8 +216,9 @@ class plugin_manager_test extends advanced_testcase {
     public function test_get_status() {
         $pluginman = testable_plugin_manager::instance();
         $plugins = $pluginman->get_plugins();
-        $modfoo = $plugins['mod']['foo'];
-        $this->assertEquals($modfoo->get_status(), plugin_manager::PLUGIN_STATUS_UPGRADE);
+        $this->assertEquals(plugin_manager::PLUGIN_STATUS_UPGRADE, $plugins['mod']['foo']->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());
     }
 
     public function test_available_update() {
@@ -186,6 +230,27 @@ class plugin_manager_test extends advanced_testcase {
             $this->assertInstanceOf('available_update_info', $availableupdate);
         }
     }
+
+    public function test_can_uninstall_plugin() {
+        $pluginman = testable_plugin_manager::instance();
+        $this->assertFalse($pluginman->can_uninstall_plugin('mod_missing'));
+        $this->assertTrue($pluginman->can_uninstall_plugin('mod_foo')); // Because mod_foo is required by foolish_frog only
+                                                                        // and foolish_frog is required by mod_foo and foolish_hippo only.
+        $this->assertFalse($pluginman->can_uninstall_plugin('mod_bar')); // Because mod_bar is required by mod_foo.
+        $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('quxcat_one')); // Because of testable_pluginfo_quxcat::is_uninstall_allowed().
+    }
+
+    public function test_get_uninstall_url() {
+        $pluginman = testable_plugin_manager::instance();
+        foreach ($pluginman->get_plugins() as $plugintype => $plugininfos) {
+            foreach ($plugininfos as $plugininfo) {
+                $this->assertTrue($plugininfo->get_uninstall_url() instanceof moodle_url);
+            }
+        }
+    }
 }
 
 
@@ -451,6 +516,17 @@ class available_update_checker_test extends advanced_testcase {
 }
 
 
+/**
+ * Base class for testable plugininfo classes.
+ */
+class testable_plugininfo_base extends plugininfo_base {
+
+    protected function get_plugin_manager() {
+        return testable_plugin_manager::instance();
+    }
+}
+
+
 /**
  * Modified {@link plugininfo_mod} suitable for testing purposes
  */
@@ -471,13 +547,21 @@ class testable_plugininfo_mod extends plugininfo_mod {
     public function load_db_version() {
         $this->versiondb = 2012022900;
     }
+
+    public function is_uninstall_allowed() {
+        return true; // Allow uninstall for standard plugins too.
+    }
+
+    protected function get_plugin_manager() {
+        return testable_plugin_manager::instance();
+    }
 }
 
 
 /**
  * Testable class representing subplugins of testable mod_foo
  */
-class testable_pluginfo_foolish extends plugininfo_base {
+class testable_pluginfo_foolish extends testable_plugininfo_base {
 
     public function init_display_name() {
         $this->displayname = ucfirst($this->name);
@@ -493,6 +577,48 @@ class testable_pluginfo_foolish extends plugininfo_base {
 }
 
 
+/**
+ * Testable class representing subplugins of testable mod_baz
+ */
+class testable_pluginfo_bazmeg extends testable_plugininfo_base {
+
+    public function init_display_name() {
+        $this->displayname = ucfirst($this->name);
+    }
+
+    public function is_standard() {
+        return false;
+    }
+
+    public function load_db_version() {
+        $this->versiondb = null;
+    }
+}
+
+
+/**
+ * Testable class representing subplugins of testable mod_qux
+ */
+class testable_pluginfo_quxcat extends testable_plugininfo_base {
+
+    public function init_display_name() {
+        $this->displayname = ucfirst($this->name);
+    }
+
+    public function is_standard() {
+        return false;
+    }
+
+    public function load_db_version() {
+        $this->versiondb = 2013041103;
+    }
+
+    public function is_uninstall_allowed() {
+        return false;
+    }
+}
+
+
 /**
  * Modified {@link plugin_manager} suitable for testing purposes
  */
@@ -529,16 +655,33 @@ class testable_plugin_manager extends plugin_manager {
                     $dirroot.'/mod/foo', 'testable_plugininfo_mod'),
                 'bar' => plugininfo_default_factory::make('mod', $dirroot.'/bar', 'bar',
                     $dirroot.'/mod/bar', 'testable_plugininfo_mod'),
+                'baz' => plugininfo_default_factory::make('mod', $dirroot.'/baz', 'baz',
+                    $dirroot.'/mod/baz', 'testable_plugininfo_mod'),
+                'qux' => plugininfo_default_factory::make('mod', $dirroot.'/qux', 'qux',
+                    $dirroot.'/mod/qux', 'testable_plugininfo_mod'),
             ),
             'foolish' => array(
-                'frog' => plugininfo_default_factory::make('foolish', $dirroot.'/mod/foo/foolish', 'frog',
+                'frog' => plugininfo_default_factory::make('foolish', $dirroot.'/mod/foo/lish', 'frog',
                     $dirroot.'/mod/foo/lish/frog', 'testable_pluginfo_foolish'),
+                'hippo' => plugininfo_default_factory::make('foolish', $dirroot.'/mod/foo/lish', 'hippo',
+                    $dirroot.'/mod/foo/lish/hippo', 'testable_pluginfo_foolish'),
+            ),
+            'bazmeg' => array(
+                'one' => plugininfo_default_factory::make('bazmeg', $dirroot.'/mod/baz/meg', 'one',
+                    $dirroot.'/mod/baz/meg/one', 'testable_pluginfo_bazmeg'),
+            ),
+            'quxcat' => array(
+                'one' => plugininfo_default_factory::make('quxcat', $dirroot.'/mod/qux/cat', 'one',
+                    $dirroot.'/mod/qux/cat/one', 'testable_pluginfo_quxcat'),
             ),
         );
 
         $checker = testable_available_update_checker::instance();
         $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['bazmeg']['one']->check_available_updates($checker);
+        $this->pluginsinfo['quxcat']['one']->check_available_updates($checker);
 
         return $this->pluginsinfo;
     }
@@ -547,20 +690,36 @@ class testable_plugin_manager extends plugin_manager {
      * Testable version of {@link plugin_manager::get_subplugins()} that works with
      * the simulated environment.
      *
-     * In this case, the mod_foo fake module provides subplugins of type 'foolish'.
+     * In this case, the mod_foo fake module provides subplugins of type 'foolish',
+     * mod_baz provides subplugins of type 'bazmeg' and mod_qux has 'quxcat'.
      *
      * @param bool $disablecache ignored in this class
      * @return array
      */
     public function get_subplugins($disablecache=false) {
-        return array(
+
+        $this->subpluginsinfo = array(
             'mod_foo' => array(
                 'foolish' => (object)array(
                     'type' => 'foolish',
                     'typerootdir' => 'mod/foo/lish',
                 ),
             ),
+            'mod_baz' => array(
+                'bazmeg' => (object)array(
+                    'type' => 'bazmeg',
+                    'typerootdir' => 'mod/baz/meg',
+                ),
+            ),
+            'mod_qux' => array(
+                'quxcat' => (object)array(
+                    'type' => 'quxcat',
+                    'typerootdir' => 'mod/qux/cat',
+                ),
+            ),
         );
+
+        return $this->subpluginsinfo;
     }
 
     /**
@@ -569,7 +728,7 @@ class testable_plugin_manager extends plugin_manager {
     protected function normalize_component($component) {
 
         // List of mock plugin types used in these unit tests.
-        $faketypes = array('foolish');
+        $faketypes = array('foolish', 'bazmeg', 'quxcat');
 
         foreach ($faketypes as $faketype) {
             if (strpos($component, $faketype.'_') === 0) {
@@ -594,20 +753,6 @@ class testable_plugin_manager extends plugin_manager {
         }
         return false;
     }
-
-    public static function standard_plugins_list($type) {
-        $standard_plugins = array(
-            'mod' => array(
-                'bar',
-            ),
-        );
-
-        if (isset($standard_plugins[$type])) {
-            return $standard_plugins[$type];
-        } else {
-            return false;
-        }
-    }
 }
 
 
index eedc690..edd5f9b 100644 (file)
@@ -53,6 +53,13 @@ information provided here is intended especially for developers.
   helper functions extract_suspended_users, get_suspended_userids to extract suspended user information.
 * The plugin_manager class now provides two new helper methods for getting information
   about known plugins: get_plugins_of_type() and get_subplugins_of_plugin().
+* 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.
 
 Database (DML) layer:
 * $DB->sql_empty() is deprecated, you have to use sql parameters with empty values instead,