course categories: Fix many bugs with category editing and permission…

…s. Clean up code. Bugs: MDL-17479, MDL-16426, MDL-16063, MDL-16013, MDL-15658, MDL-15556, MDL-15161, MDL-14925, MDL-13742, MDL-11557. * Simplify category editing permissions to just moodle/category:manage and moodle/category:seehiddencategories. * Enforce those correctly. (Note MDL 17502 is still outstanding.) * Don't screw up category sort order when you just edit name or description. * Niceties like where redirects go when you cancel or submit forms. * Make sure a global course creator can see the site admin block. * Don't allow a category to be made the child of one of its children! * General code cleanup to bring key files more in line with best pracitice. Apologies for the fact it is one big patch, rather than a series of smaller patches. However, categoryedit.php, category.php and index.php where in pretty bad shape and needed significant cleaning up. categoryedit.php, in particular, was almost completely rewritten. Merged from MOODLE_19_STABLE.
Andmat7 · Dec 4, 2008 · 8ed5dd6 · 8ed5dd6
1 parent 4faace3
commit 8ed5dd6
Show file tree

Hide file tree

Showing 18 changed files with 495 additions and 715 deletions.
diff --git a/admin/settings/courses.php b/admin/settings/courses.php
@@ -4,10 +4,11 @@
 
 if ($hassiteconfig
  or has_capability('moodle/site:backup', $systemcontext)
- or has_capability('moodle/category:update', $systemcontext)) { // speedup for non-admins, add all caps used on this page
+ or has_capability('moodle/category:manage', $systemcontext)
+ or has_capability('moodle/course:create', $systemcontext)) { // speedup for non-admins, add all caps used on this page
 
-
-    $ADMIN->add('courses', new admin_externalpage('coursemgmt', get_string('coursemgmt', 'admin'), $CFG->wwwroot . '/course/index.php?categoryedit=on','moodle/category:update'));
+    $ADMIN->add('courses', new admin_externalpage('coursemgmt', get_string('coursemgmt', 'admin'), $CFG->wwwroot . '/course/index.php?categoryedit=on',
+            array('moodle/category:manage', 'moodle/course:create')));
 
     $ADMIN->add('courses', new admin_enrolment_page());
 

diff --git a/course/category.php b/course/category.php
diff --git a/course/delete_category_form.php b/course/delete_category_form.php
@@ -11,44 +11,38 @@ function definition() {
 
         $mform    =& $this->_form;
         $category = $this->_customdata;
+        ensure_context_subobj_present($category, CONTEXT_COURSECAT);
         $this->_category = $category;
 
         $mform->addElement('header','general', get_string('categorycurrentcontents', '', format_string($category->name)));
 
         $displaylist = array();
-        $parentlist = array();
-        $children = array();
-        make_categories_list($displaylist, $parentlist);
-        unset($displaylist[$category->id]);
-        foreach ($displaylist as $catid=>$unused) {
-            // remove all children of $category
-            if (isset($parentlist[$catid]) and in_array($category->id, $parentlist[$catid])) {
-                $children[] = $catid;
-                unset($displaylist[$catid]);
-                continue;
-            }
-            if (!has_capability('moodle/course:create', get_context_instance(CONTEXT_COURSECAT, $catid))) {
-                unset($displaylist[$catid]);
-            }
-        }
+        $notused = array();
+        make_categories_list($displaylist, $notused, 'moodle/course:create', $category->id);
 
+        // Check permissions, to see if it OK to give the option to delete
+        // the contents, rather than move elsewhere.
         $candeletecontent = true;
-        foreach ($children as $catid) {
-            $context = get_context_instance(CONTEXT_COURSECAT, $catid);
-            if (!has_capability('moodle/category:delete', $context)) {
+        $tocheck = array($category);
+        while (!empty($tocheck)) {
+            $checkcat = array_pop($tocheck);
+            $tocheck = $tocheck + get_child_categories($checkcat->id);
+            if (!has_capability('moodle/category:manage', $checkcat->context)) {
                 $candeletecontent = false;
                 break;
             }
         }
 
+        // TODO check that the user is allowed to delete all the courses MDL-17502!
+
         $options = array();
 
         if ($displaylist) {
             $options[0] = get_string('move');
         }
 
         if ($candeletecontent) {
-            $options[1] =get_string('delete');
+            $options[1] = get_string('delete');
         }
 
         if (empty($options)) {

diff --git a/course/edit_form.php b/course/edit_form.php
@@ -57,16 +57,11 @@ function definition() {
 //--------------------------------------------------------------------------------
         $mform->addElement('header','general', get_string('general', 'form'));
 
-        //must have create course capability in both categories in order to move course
+        // Must have create course capability in both categories in order to move course
         if (has_capability('moodle/course:create', $categorycontext)) {
             $displaylist = array();
             $parentlist = array();
-            make_categories_list($displaylist, $parentlist);
-            foreach ($displaylist as $key=>$val) {
-                if (!has_capability('moodle/course:create', get_context_instance(CONTEXT_COURSECAT, $key))) {
-                    unset($displaylist[$key]);
-                }
-            }
+            make_categories_list($displaylist, $parentlist, 'moodle/course:create');
             $mform->addElement('select', 'category', get_string('category'), $displaylist);
         } else {
             $mform->addElement('hidden', 'category', null);