From 8a1b1c328d36fdc1d061eb932d960b758f9a8aae Mon Sep 17 00:00:00 2001 From: tjhunt Date: Mon, 8 Dec 2008 07:28:19 +0000 Subject: [PATCH] course categories: MDL-17502 when deleting a category and its contents, check moodle/course:delete capability. * Note: this would never lead to problems with default role definions. * Also ended up mostly rewriting delete_category_form to simplify the messages that are displayed. * New helper function require_all_capabilities, a bit like require_any_capability. --- course/delete_category_form.php | 116 +++++++++++++++++++++++--------- course/index.php | 5 -- course/lib.php | 7 +- lang/en_utf8/error.php | 1 + lang/en_utf8/moodle.php | 6 ++ lib/accesslib.php | 24 +++++++ 6 files changed, 118 insertions(+), 41 deletions(-) diff --git a/course/delete_category_form.php b/course/delete_category_form.php index 82eb305c7c..372773beeb 100644 --- a/course/delete_category_form.php +++ b/course/delete_category_form.php @@ -1,63 +1,116 @@ libdir.'/formslib.php'); +require_once($CFG->libdir.'/questionlib.php'); class delete_category_form extends moodleform { var $_category; function definition() { - global $CFG; + global $CFG, $DB; $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(); - $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. + /// Check permissions, to see if it OK to give the option to delete + /// the contents, rather than move elsewhere. + /// Are there any subcategories of this one, can they be deleted? $candeletecontent = true; - $tocheck = array($category); + $tocheck = get_child_categories($category->id); + $containscategories = !empty($tocheck); + $categoryids = array($category->id); while (!empty($tocheck)) { $checkcat = array_pop($tocheck); + $childcategoryids[] = $checkcat->id; $tocheck = $tocheck + get_child_categories($checkcat->id); - if (!has_capability('moodle/category:manage', $checkcat->context)) { + if ($candeletecontent && !has_capability('moodle/category:manage', $checkcat->context)) { $candeletecontent = false; - break; } } - // TODO check that the user is allowed to delete all the courses MDL-17502! + /// Are there any courses in here, can they be deleted? + list($test, $params) = $DB->get_in_or_equal($categoryids); + $containedcourses = $DB->get_records_sql( + "SELECT id,1 FROM {course} c WHERE c.category $test", $params); + $containscourses = false; + if ($containedcourses) { + $containscourses = true; + foreach ($containedcourses as $courseid => $notused) { + if ($candeletecontent && !can_delete_course($courseid)) { + $candeletecontent = false; + break; + } + } + } - $options = array(); + /// Are there any questions in the question bank here? + $containsquestions = question_context_has_any_questions($category->context); - if ($displaylist) { - $options[0] = get_string('move'); + /// Get the list of categories we might be able to move to. + $testcaps = array(); + if ($containscourses) { + $testcaps[] = 'moodle/course:create'; + } + if ($containscategories || $containsquestions) { + $testcaps[] = 'moodle/category:manage'; + } + $displaylist = array(); + $notused = array(); + if (!empty($testcaps)) { + make_categories_list($displaylist, $notused, $testcaps, $category->id); } + /// Now build the options. + $options = array(); + if ($displaylist) { + $options[0] = get_string('movecontentstoanothercategory'); + } if ($candeletecontent) { - $options[1] = get_string('delete'); + $options[1] = get_string('deleteallcannotundo'); } - if (empty($options)) { - print_error('nocategorydelete', 'error', 'index.php', format_string($category->name)); - } + /// Now build the form. + $mform->addElement('header','general', get_string('categorycurrentcontents', '', format_string($category->name))); - $mform->addElement('select', 'fulldelete', get_string('categorycontents'), $options); - $mform->disabledIf('newparent', 'fulldelete', 'eq', '1'); - $mform->setDefault('newparent', 0); + if ($containscourses || $containscategories || $containsquestions) { + if (empty($options)) { + print_error('youcannotdeletecategory', 'error', 'index.php', format_string($category->name)); + } - if ($displaylist) { - $mform->addElement('select', 'newparent', get_string('movecategorycontentto'), $displaylist); - if (in_array($category->parent, $displaylist)) { - $mform->setDefault('newparent', $category->parent); + /// Describe the contents of this category. + $contents = ''; + $mform->addElement('static', 'emptymessage', get_string('thiscategorycontains'), $contents); + + /// Give the options for what to do. + $mform->addElement('select', 'fulldelete', get_string('whattodo'), $options); + if (count($options) == 1) { + $mform->hardFreeze('fulldelete'); + $mform->setConstant('fulldelete', reset(array_keys($options))); + } + + if ($displaylist) { + $mform->addElement('select', 'newparent', get_string('movecategorycontentto'), $displaylist); + if (in_array($category->parent, $displaylist)) { + $mform->setDefault('newparent', $category->parent); + } + $mform->disabledIf('newparent', 'fulldelete', 'eq', '1'); + } + } else { + $mform->addElement('hidden', 'fulldelete', 1); + $mform->addElement('static', 'emptymessage', '', get_string('deletecategoryempty')); } $mform->addElement('hidden', 'delete'); @@ -73,12 +126,9 @@ class delete_category_form extends moodleform { function validation($data, $files) { $errors = parent::validation($data, $files); - if (!empty($data['fulldelete'])) { - // already verified - } else { - if (empty($data['newparent'])) { - $errors['newparent'] = get_string('required'); - } + if (empty($data['fulldelete']) && empty($data['newparent'])) { + /// When they have chosen the move option, they must specify a destination. + $errors['newparent'] = get_string('required'); } if ($data['sure'] != md5(serialize($this->_category))) { diff --git a/course/index.php b/course/index.php index 65d6bd65d8..5f6818a8f5 100644 --- a/course/index.php +++ b/course/index.php @@ -108,11 +108,6 @@ require_once($CFG->libdir . '/questionlib.php'); print_category_edit_header(); print_heading($heading); - print_box(get_string('deletecategorycheck2'), 'generalbox boxwidthnormal boxaligncenter'); - if (question_context_has_any_questions($context)) { - print_box(get_string('deletecoursecategorywithquestions', 'question'), - 'generalbox boxwidthnormal boxaligncenter'); - } $mform->display(); admin_externalpage_print_footer(); exit(); diff --git a/course/lib.php b/course/lib.php index 7985947184..452cc77e7c 100644 --- a/course/lib.php +++ b/course/lib.php @@ -1749,8 +1749,9 @@ function get_child_categories($parentid) { * * @param array $list For output, accumulates an array categoryid => full category path name * @param array $parents For output, accumulates an array categoryid => list of parent category ids. - * @param string $requiredcapability if given, only categories where the current - * user has this capability will be added to $list. + * @param string/array $requiredcapability if given, only categories where the current + * user has this capability will be added to $list. Can also be an array of capabilities, + * in which case they are all required. * @param integer $excludeid Omit this category and its children from the lists built. * @param object $category Build the tree starting at this category - otherwise starts at the top level. * @param string $path For internal use, as part of recursive calls. @@ -1787,7 +1788,7 @@ function make_categories_list(&$list, &$parents, $requiredcapability = '', if ($requiredcapability) { ensure_context_subobj_present($category, CONTEXT_COURSECAT); } - if (!$requiredcapability || has_capability($requiredcapability, $category->context)) { + if (!$requiredcapability || has_all_capabilities($requiredcapability, $category->context)) { $list[$category->id] = $path; } } diff --git a/lang/en_utf8/error.php b/lang/en_utf8/error.php index b046643ea1..059d6771c8 100644 --- a/lang/en_utf8/error.php +++ b/lang/en_utf8/error.php @@ -467,5 +467,6 @@ $string['wrongroleid'] = 'Incorrect role ID!'; $string['wrongsourcebase'] = 'Wrong source URL base'; $string['wrongzipfilename'] = 'Wrong ZIP file name'; $string['xmldberror'] = 'XMLDB error!'; +$string['youcannotdeletecategory'] = 'You cannot delete category \'$a\' becuase you can neither delete the contents, nor move them elsewhere.'; ?> diff --git a/lang/en_utf8/moodle.php b/lang/en_utf8/moodle.php index 24ddeac9f1..17f79aa23f 100644 --- a/lang/en_utf8/moodle.php +++ b/lang/en_utf8/moodle.php @@ -398,11 +398,13 @@ $string['defaultcourseteacherdescription'] = 'Teachers can do anything within a $string['defaultcourseteachers'] = 'Teachers'; $string['delete'] = 'Delete'; $string['deleteall'] = 'Delete all'; +$string['deleteallcannotundo'] = 'Delete all - cannot be undone'; $string['deleteallcomments'] = 'Delete all comments'; $string['deleteallratings'] = 'Delete all ratings'; $string['deletecategory'] = 'Delete category: $a'; $string['deletecategorycheck'] = 'Are you absolutely sure you want to completely delete this category \'$a\'?
This will move all courses into the parent category if there is one, or into Miscellaneous.'; $string['deletecategorycheck2'] = 'If you delete this category, you need to choose what to do with the courses and subcategories it contains.'; +$string['deletecategoryempty'] = 'This category is empty.'; $string['deletecheck'] = 'Delete $a ?'; $string['deletecheckfiles'] = 'Are you absolutely sure you want to delete these files?'; $string['deletecheckfull'] = 'Are you absolutely sure you want to completely delete $a ?'; @@ -1013,6 +1015,7 @@ $string['mostrecently'] = 'most recently'; $string['move'] = 'Move'; $string['movecategoryto'] = 'Move category to:'; $string['movecategorycontentto'] = 'Move into'; +$string['movecontentstoanothercategory'] = 'Move contents to another category'; $string['movecourseto'] = 'Move course to:'; $string['movedown'] = 'Move down'; $string['movefilestohere'] = 'Move files to here'; @@ -1259,6 +1262,7 @@ $string['publicdirectory2'] = 'Publish the site name with a link'; $string['publicdirectorytitle'] = 'See the current list of sites'; $string['publicsitefileswarning'] = 'Note: files placed here can be accessed by anyone'; $string['question'] = 'Question'; +$string['questionsinthequestionbank'] = 'Questions in the question bank'; $string['readinginfofrombackup'] = 'Reading info from backup'; $string['readme'] = 'README'; $string['recentactivity'] = 'Recent Activity'; @@ -1535,6 +1539,7 @@ $string['theme'] = 'Theme'; $string['themes'] = 'Themes'; $string['themesaved'] = 'New theme saved'; $string['thereareno'] = 'There are no $a in this course'; +$string['thiscategorycontains'] = 'This category contains'; $string['thischarset'] = 'UTF-8'; $string['thisdirection'] = 'ltr'; $string['thislanguage'] = 'English'; @@ -1680,6 +1685,7 @@ within the course so that we can learn more about you: $a->profileurl'; $string['whattocallzip'] = 'What do you want to call the zip file?'; +$string['whattodo'] = 'What to do'; $string['withchosenfiles'] = 'With chosen files'; $string['withoutuserdata'] = 'without user data'; $string['withselectedusers'] = 'With selected users...'; diff --git a/lib/accesslib.php b/lib/accesslib.php index 3b64ae5966..1865f6e5a4 100755 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -37,6 +37,8 @@ * * Whether the user can do something... * - has_capability() + * - has_any_capability() + * - has_all_capabilities() * - require_capability() * - require_login() (from moodlelib) * @@ -496,6 +498,28 @@ function has_any_capability($capabilities, $context, $userid=NULL, $doanything=t return false; } +/** + * This function returns whether the current user has all of the capabilities in the + * $capabilities array. This is a simple wrapper around has_capability for convinience. + * + * There are probably tricks that could be done to improve the performance here, for example, + * check the capabilities that are already cached first. + * + * @param array $capabilities - an array of capability names. + * @param object $context - a context object (record from context table) + * @param integer $userid - a userid number, empty if current $USER + * @param bool $doanything - if false, ignore do anything + * @return bool + */ +function has_all_capabilities($capabilities, $context, $userid=NULL, $doanything=true) { + foreach ($capabilities as $capability) { + if (!has_capability($capability, $context, $userid, $doanything)) { + return false; + } + } + return true; +} + /** * Uses 1 DB query to answer whether a user is an admin at the sitelevel. * It depends on DB schema >=1.7 but does not depend on the new datastructures -- 2.39.5