From c2d2001a145995f437daa52fabea1ab863211341 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Mudr=C3=A1k?= <david@moodle.com>
Date: Fri, 12 Apr 2013 03:32:35 +0200
Subject: [PATCH] MDL-39087 Improve the Plugins overview renderer

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.
---
 admin/plugins.php  | 27 ++++++++++++++++++++++-----
 admin/renderer.php |  9 ---------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/admin/plugins.php b/admin/plugins.php
index 35c882df511..73e34d4f352 100644
--- a/admin/plugins.php
+++ b/admin/plugins.php
@@ -18,6 +18,15 @@
 /**
  * UI for general plugins management
  *
+ * Supported HTTP parameters:
+ *
+ *  ?fetchremote=1      - check for available updates
+ *  ?updatesonly=1      - display plugins with available update only
+ *  ?contribonly=1      - display non-standard add-ons only
+ *  ?uninstall=foo_bar  - uninstall the given plugin
+ *  ?delete=foo_bar     - delete the plugin folder (it must not be installed)
+ *  &confirm=1          - confirm the uninstall or delete action
+ *
  * @package    core
  * @subpackage admin
  * @copyright  2011 David Mudrak <david@moodle.com>
@@ -47,16 +56,20 @@ if ($uninstall) {
     require_sesskey();
     $pluginfo = $pluginman->get_plugin_info($uninstall);
 
+    // Make sure we know the plugin.
     if (is_null($pluginfo)) {
         throw new moodle_exception('err_uninstalling_unknown_plugin', 'core_plugin', '', array('plugin' => $uninstall),
             'plugin_manager::get_plugin_info() returned null for the plugin to be uninstalled');
     }
 
-    $requiredby = $pluginman->other_plugins_that_require($pluginfo->component);
-    if (!empty($requiredby)) {
-        throw new moodle_exception('err_uninstalling_required_plugin', 'core_plugin', '',
-            array('plugin' => $pluginfo->component, 'requiredby' => implode(', ', $requiredby)),
-            'plugin_manager::other_plugins_that_require() returned non-empty array');
+    $pluginname = $pluginman->plugin_name($pluginfo->component);
+    $PAGE->set_title($pluginname);
+    $PAGE->navbar->add(get_string('uninstalling', 'core_plugin', array('name' => $pluginname)));
+
+    if (!$pluginman->can_uninstall_plugin($pluginfo->component)) {
+        throw new moodle_exception('err_cannot_uninstall_plugin', 'core_plugin', '',
+            array('plugin' => $pluginfo->component),
+            'plugin_manager::can_uninstall_plugin() returned false');
     }
 
     if (!$confirmed) {
@@ -90,6 +103,10 @@ if ($delete and $confirmed) {
             'plugin_manager::get_plugin_info() returned null for the plugin to be deleted');
     }
 
+    $pluginname = $pluginman->plugin_name($pluginfo->component);
+    $PAGE->set_title($pluginname);
+    $PAGE->navbar->add(get_string('uninstalling', 'core_plugin', array('name' => $pluginname)));
+
     // Make sure it is not installed.
     if (!is_null($pluginfo->versiondb)) {
         throw new moodle_exception('err_removing_installed_plugin', 'core_plugin', '',
diff --git a/admin/renderer.php b/admin/renderer.php
index 5a1d2e01d3e..ba3b65ac594 100644
--- a/admin/renderer.php
+++ b/admin/renderer.php
@@ -384,9 +384,6 @@ class core_admin_renderer extends plugin_renderer_base {
 
         $pluginname = $pluginman->plugin_name($pluginfo->component);
 
-        $this->page->set_title($pluginname);
-        $this->page->navbar->add(get_string('uninstalling', 'core_plugin', array('name' => $pluginname)));
-
         $output .= $this->output->header();
         $output .= $this->output->heading(get_string('uninstalling', 'core_plugin', array('name' => $pluginname)));
         $output .= $this->output->confirm(get_string('uninstallconfirm', 'core_plugin', array('name' => $pluginname)),
@@ -411,9 +408,6 @@ class core_admin_renderer extends plugin_renderer_base {
 
         $pluginname = $pluginman->plugin_name($pluginfo->component);
 
-        $this->page->set_title($pluginname);
-        $this->page->navbar->add(get_string('uninstalling', 'core_plugin', array('name' => $pluginname)));
-
         $output .= $this->output->header();
         $output .= $this->output->heading(get_string('uninstalling', 'core_plugin', array('name' => $pluginname)));
 
@@ -448,9 +442,6 @@ class core_admin_renderer extends plugin_renderer_base {
 
         $pluginname = $pluginman->plugin_name($pluginfo->component);
 
-        $this->page->set_title($pluginname);
-        $this->page->navbar->add(get_string('uninstalling', 'core_plugin', array('name' => $pluginname)));
-
         $output .= $this->output->header();
         $output .= $this->output->heading(get_string('uninstalling', 'core_plugin', array('name' => $pluginname)));