diff --git a/admin/renderer.php b/admin/renderer.php index 1784de95de2..c215a898e3a 100644 --- a/admin/renderer.php +++ b/admin/renderer.php @@ -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')); } diff --git a/lib/editor/tinymce/adminlib.php b/lib/editor/tinymce/adminlib.php index 36eb494e803..6b4b23242e8 100644 --- a/lib/editor/tinymce/adminlib.php +++ b/lib/editor/tinymce/adminlib.php @@ -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())); } diff --git a/lib/pluginlib.php b/lib/pluginlib.php index 2522e225ea5..354cc4370cc 100644 --- a/lib/pluginlib.php +++ b/lib/pluginlib.php @@ -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. @@ -2709,32 +2791,42 @@ abstract class plugininfo_base { public function load_settings(part_of_admin_tree $adminroot, $parentnodename, $hassiteconfig) { } + /** + * Should there be a way to uninstall the plugin via the administration UI + * + * 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. + * + * 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 boolean + */ + public function is_uninstall_allowed() { + + if ($this->is_subplugin()) { + return $this->get_plugin_manager()->get_plugin_info($this->get_parent_plugin())->is_uninstall_allowed(); + } + + 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. 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). + * for actual uninstallation of the plugin. By default, URL to a common + * uninstalling tool is returned. * - * By default, URL to a common uninstalling handler is returned for all - * add-ons and null is returned for standard plugins. - * - * @return null|moodle_url + * @return moodle_url */ public function get_uninstall_url() { - - if ($this->is_standard()) { - return null; - } - - $pluginman = plugin_manager::instance(); - $requiredby = $pluginman->other_plugins_that_require($this->component); - if (!empty($requiredby)) { - return null; - } - 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 index 00000000000..b099a779bbf --- /dev/null +++ b/lib/tests/fixtures/mockplugins/mod/baz/meg/one/version.php @@ -0,0 +1,5 @@ +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 index 00000000000..47613f65a4c --- /dev/null +++ b/lib/tests/fixtures/mockplugins/mod/baz/version.php @@ -0,0 +1,4 @@ +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 index 00000000000..f64b02d59ff --- /dev/null +++ b/lib/tests/fixtures/mockplugins/mod/foo/lish/hippo/version.php @@ -0,0 +1,6 @@ +version = 2013041103; +$plugin->requires = 2012010100; +$plugin->component = 'foolish_hippo'; +$plugin->dependencies = array('foolish_frog' => ANY_VERSION); diff --git a/lib/tests/fixtures/mockplugins/mod/foo/version.php b/lib/tests/fixtures/mockplugins/mod/foo/version.php index 5bcd9ed2c4e..a12a76a40dc 100644 --- a/lib/tests/fixtures/mockplugins/mod/foo/version.php +++ b/lib/tests/fixtures/mockplugins/mod/foo/version.php @@ -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 index 00000000000..4c9e9781270 --- /dev/null +++ b/lib/tests/fixtures/mockplugins/mod/qux/cat/one/version.php @@ -0,0 +1,6 @@ +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 index 00000000000..9c1c5f01a28 --- /dev/null +++ b/lib/tests/fixtures/mockplugins/mod/qux/version.php @@ -0,0 +1,5 @@ +version = 2013041103; +$plugin->requires = 2013010100; +$plugin->component = 'mod_qux'; diff --git a/lib/tests/pluginlib_test.php b/lib/tests/pluginlib_test.php index 39a1db0a7f9..0556b2ed333 100644 --- a/lib/tests/pluginlib_test.php +++ b/lib/tests/pluginlib_test.php @@ -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; - } - } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index eedc6903c55..edd5f9b79df 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -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,