MDL-61132 Questions: Question Bank amendments to cope with Top category

* Display "Top" categories in the category filter when listing questions in the question bank
* Prevent editing "Top" categories
* Prevent deleting "Top" categories

lang/en/question.php

@@ -41,6 +41,8 @@ $string['cannotdeletecate'] = 'You can\'t delete that category it is the default
 $string['cannotdeleteneededbehaviour'] = 'Cannot delete the question behaviour \'{$a}\'. There are other behaviours installed that rely on it.';
 $string['cannotdeleteqtypeinuse'] = 'You cannot delete the question type \'{$a}\'. There are questions of this type in the question bank.';
 $string['cannotdeleteqtypeneeded'] = 'You cannot delete the question type \'{$a}\'. There are other question types installed that rely on it.';
+$string['cannotdeletetopcat'] = 'Top categories can not be deleted.';
+$string['cannotedittopcat'] = 'Top categories can not be edited.';
 $string['cannotenable'] = 'Question type {$a} cannot be created directly.';
 $string['cannotenablebehaviour'] = 'Question behaviour {$a} cannot be used directly. It is for internal use only.';
 $string['cannotfindcate'] = 'Could not find category record';

lib/deprecatedlib.php

@@ -6560,4 +6560,19 @@ function question_add_tops($categories, $pcontexts) {
     }
     // Put topcats in at beginning of array - they'll be sorted into different contexts later.
     return array_merge($topcats, $categories);
-}
\ No newline at end of file
+}
+
+/**
+ * Checks if the question category is the highest-level category in the context that can be edited, and has no siblings.
+ *
+ * @param int $categoryid a category id.
+ * @return bool
+ * @deprecated since Moodle 3.5. MDL-61132
+ */
+function question_is_only_toplevel_category_in_context($categoryid) {
+    debugging('question_is_only_toplevel_category_in_context() has been deprecated. '
+            . 'Please update your code to use question_is_only_child_of_top_category_in_context() instead.',
+            DEBUG_DEVELOPER);
+
+    return question_is_only_child_of_top_category_in_context($categoryid);
+}

question/category_class.php

@@ -112,8 +112,8 @@ class question_category_list_item extends list_item {
         $item .= format_text($category->info, $category->infoformat,
                 array('context' => $this->parentlist->context, 'noclean' => true));
 
-        // don't allow delete if this is the last category in this context.
-        if (!question_is_only_toplevel_category_in_context($category->id)) {
+        // Don't allow delete if this is the top category, or the last editable category in this context.
+        if ($category->parent && !question_is_only_child_of_top_category_in_context($category->id)) {
             $deleteurl = new moodle_url($this->parentlist->pageurl, array('delete' => $this->id, 'sesskey' => sesskey()));
             $item .= html_writer::link($deleteurl,
                     $OUTPUT->pix_icon('t/delete', $str->delete),
@@ -295,17 +295,19 @@ class question_category_object {
 
     public function edit_single_category($categoryid) {
     /// Interface for adding a new category
-        global $COURSE, $DB;
+        global $DB;
         /// Interface for editing existing categories
-        if ($category = $DB->get_record("question_categories", array("id" => $categoryid))) {
-
+        $category = $DB->get_record("question_categories", array("id" => $categoryid));
+        if (empty($category)) {
+            print_error('invalidcategory', '', '', $categoryid);
+        } else if ($category->parent == 0) {
+            print_error('cannotedittopcat', 'question', '', $categoryid);
+        } else {
             $category->parent = "{$category->parent},{$category->contextid}";
             $category->submitbutton = get_string('savechanges');
             $category->categoryheader = $this->str->edit;
             $this->catform->set_data($category);
             $this->catform->display();
-        } else {
-            print_error('invalidcategory', '', '', $categoryid);
         }
     }
 
@@ -440,7 +442,7 @@ class question_category_object {
 
         // Get the record we are updating.
         $oldcat = $DB->get_record('question_categories', array('id' => $updateid));
-        $lastcategoryinthiscontext = question_is_only_toplevel_category_in_context($updateid);
+        $lastcategoryinthiscontext = question_is_only_child_of_top_category_in_context($updateid);
 
         if (!empty($newparent) && !$lastcategoryinthiscontext) {
             list($parentid, $tocontextid) = explode(',', $newparent);

question/category_form.php

@@ -38,7 +38,6 @@ require_once($CFG->libdir.'/formslib.php');
 class question_category_edit_form extends moodleform {
 
     protected function definition() {
-        global $CFG, $DB;
         $mform    = $this->_form;
 
         $contexts   = $this->_customdata['contexts'];
@@ -46,10 +45,10 @@ class question_category_edit_form extends moodleform {
 
         $mform->addElement('header', 'categoryheader', get_string('addcategory', 'question'));
 
-        $questioncategoryel = $mform->addElement('questioncategory', 'parent', get_string('parentcategory', 'question'),
-                    array('contexts'=>$contexts, 'top'=>true, 'currentcat'=>$currentcat, 'nochildrenof'=>$currentcat));
+        $mform->addElement('questioncategory', 'parent', get_string('parentcategory', 'question'),
+                array('contexts' => $contexts, 'top' => true, 'currentcat' => $currentcat, 'nochildrenof' => $currentcat));
         $mform->setType('parent', PARAM_SEQUENCE);
-        if (question_is_only_toplevel_category_in_context($currentcat)) {
+        if (question_is_only_child_of_top_category_in_context($currentcat)) {
             $mform->hardFreeze('parent');
         }
         $mform->addHelpButton('parent', 'parentcategory', 'question');

question/classes/bank/search/category_condition.php

@@ -130,12 +130,11 @@ class category_condition extends condition {
      * @param string $current 'categoryID,contextID'.
      */
     protected function display_category_form($contexts, $pageurl, $current) {
-        global $OUTPUT;
-
         echo \html_writer::start_div('choosecategory');
-        $catmenu = question_category_options($contexts, false, 0, true);
+        $catmenu = question_category_options($contexts, true, 0, true);
         echo \html_writer::label(get_string('selectacategory', 'question'), 'id_selectacategory');
-        echo \html_writer::select($catmenu, 'category', $current, array(), array('class' => 'searchoptions custom-select', 'id' => 'id_selectacategory'));
+        echo \html_writer::select($catmenu, 'category', $current, array(),
+                array('class' => 'searchoptions custom-select', 'id' => 'id_selectacategory'));
         echo \html_writer::end_div() . "\n";
     }
 

question/editlib.php

@@ -95,28 +95,42 @@ function get_questions_category( $category, $noparent=false, $recurse=true, $exp
 }
 
 /**
+ * Checks whether this is the only child of a top category in a context.
+ *
  * @param int $categoryid a category id.
- * @return bool whether this is the only top-level category in a context.
+ * @return bool
  */
-function question_is_only_toplevel_category_in_context($categoryid) {
+function question_is_only_child_of_top_category_in_context($categoryid) {
     global $DB;
     return 1 == $DB->count_records_sql("
             SELECT count(*)
-              FROM {question_categories} c1,
-                   {question_categories} c2
-             WHERE c2.id = ?
-               AND c1.contextid = c2.contextid
-               AND c1.parent = 0 AND c2.parent = 0", array($categoryid));
+              FROM {question_categories} c
+              JOIN {question_categories} p ON c.parent = p.id
+              JOIN {question_categories} s ON s.parent = c.parent
+             WHERE c.id = ? AND p.parent = 0", array($categoryid));
+}
+
+/**
+ * Checks whether the category is a "Top" category (with no parent).
+ *
+ * @param int $categoryid a category id.
+ * @return bool
+ */
+function question_is_top_category($categoryid) {
+    global $DB;
+    return 0 == $DB->get_field('question_categories', 'parent', array('id' => $categoryid));
 }
 
 /**
- * Check whether this user is allowed to delete this category.
+ * Ensures that this user is allowed to delete this category.
  *
  * @param int $todelete a category id.
  */
 function question_can_delete_cat($todelete) {
     global $DB;
-    if (question_is_only_toplevel_category_in_context($todelete)) {
+    if (question_is_top_category($todelete)) {
+        print_error('cannotdeletetopcat', 'question');
+    } else if (question_is_only_child_of_top_category_in_context($todelete)) {
         print_error('cannotdeletecate', 'question');
     } else {
         $contextid = $DB->get_field('question_categories', 'contextid', array('id' => $todelete));

question/type/edit_question_form.php

@@ -732,6 +732,12 @@ abstract class question_edit_form extends question_wizard_form {
             $errors['currentgrp'] = get_string('nopermissionmove', 'question');
         }
 
+        // Category.
+        if (empty($fromform['category'])) {
+            // User has provided an invalid category.
+            $errors['category'] = get_string('required');
+        }
+
         // Default mark.
         if (array_key_exists('defaultmark', $fromform) && $fromform['defaultmark'] < 0) {
             $errors['defaultmark'] = get_string('defaultmarkmustbepositive', 'question');