From 3e15abe50071d770dd223b4d0fcefe6bf3eff6ec Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Mon, 23 Sep 2019 16:37:23 +0200 Subject: [PATCH] MDL-66740 core_course: Add context to capabilities to request course --- course/classes/category.php | 6 +- course/classes/management_renderer.php | 2 +- course/lib.php | 82 +++++++++++++--- course/pending.php | 24 ++++- course/request.php | 17 +++- course/request_form.php | 2 +- course/tests/behat/course_request.feature | 109 ++++++++++++++++++++++ course/tests/courselib_test.php | 88 +++++++++++++++++ lib/db/access.php | 4 +- 9 files changed, 307 insertions(+), 27 deletions(-) create mode 100644 course/tests/behat/course_request.feature diff --git a/course/classes/category.php b/course/classes/category.php index 251c460823e..7c247c98970 100644 --- a/course/classes/category.php +++ b/course/classes/category.php @@ -2973,11 +2973,7 @@ class core_course_category implements renderable, cacheable_object, IteratorAggr * @return bool */ public function can_request_course() { - global $CFG; - if (empty($CFG->enablecourserequests) || $this->id != $CFG->defaultrequestcategory) { - return false; - } - return !$this->can_create_course() && has_capability('moodle/course:request', $this->get_context()); + return course_request::can_request($this->get_context()); } /** diff --git a/course/classes/management_renderer.php b/course/classes/management_renderer.php index 1f12b7f7146..162a68de549 100644 --- a/course/classes/management_renderer.php +++ b/course/classes/management_renderer.php @@ -664,7 +664,7 @@ class core_course_management_renderer extends plugin_renderer_base { } if ($category->can_request_course()) { // Request a new course. - $url = new moodle_url('/course/request.php', array('return' => 'management')); + $url = new moodle_url('/course/request.php', array('category' => $category->id, 'return' => 'management')); $actions[] = html_writer::link($url, get_string('requestcourse')); } if ($category->can_resort_courses()) { diff --git a/course/lib.php b/course/lib.php index fbeea7a68df..63c95ac8761 100644 --- a/course/lib.php +++ b/course/lib.php @@ -752,16 +752,21 @@ function make_categories_options() { /** * Print the buttons relating to course requests. * - * @param object $context current page context. + * @param context $context current page context. */ function print_course_request_buttons($context) { global $CFG, $DB, $OUTPUT; if (empty($CFG->enablecourserequests)) { return; } - if (!has_capability('moodle/course:create', $context) && has_capability('moodle/course:request', $context)) { - /// Print a button to request a new course - echo $OUTPUT->single_button(new moodle_url('/course/request.php'), get_string('requestcourse'), 'get'); + if (course_request::can_request($context)) { + // Print a button to request a new course. + $params = []; + if ($context instanceof context_coursecat) { + $params['category'] = $context->instanceid; + } + echo $OUTPUT->single_button(new moodle_url('/course/request.php', $params), + get_string('requestcourse'), 'get'); } /// Print a button to manage pending requests if (has_capability('moodle/site:approvecourse', $context)) { @@ -2972,6 +2977,31 @@ class course_request { return $this->properties->collision; } + /** + * Checks user capability to approve a requested course + * + * If course was requested without category for some reason (might happen if $CFG->defaultrequestcategory is + * misconfigured), we check capabilities 'moodle/site:approvecourse' and 'moodle/course:changecategory'. + * + * @return bool + */ + public function can_approve() { + global $CFG; + $category = null; + if ($this->properties->category) { + $category = core_course_category::get($this->properties->category, IGNORE_MISSING); + } else if ($CFG->defaultrequestcategory) { + $category = core_course_category::get($CFG->defaultrequestcategory, IGNORE_MISSING); + } + if ($category) { + return has_capability('moodle/site:approvecourse', $category->get_context()); + } + + // We can not determine the context where the course should be created. The approver should have + // both capabilities to approve courses and change course category in the system context. + return has_all_capabilities(['moodle/site:approvecourse', 'moodle/course:changecategory'], context_system::instance()); + } + /** * Returns the category where this course request should be created * @@ -2983,17 +3013,14 @@ class course_request { */ public function get_category() { global $CFG; - // If the category is not set, if the current user does not have the rights to change the category, or if the - // category does not exist, we set the default category to the course to be approved. - // The system level is used because the capability moodle/site:approvecourse is based on a system level. - if (empty($this->properties->category) || !has_capability('moodle/course:changecategory', context_system::instance()) || - (!$category = core_course_category::get($this->properties->category, IGNORE_MISSING, true))) { - $category = core_course_category::get($CFG->defaultrequestcategory, IGNORE_MISSING, true); + if ($this->properties->category && ($category = core_course_category::get($this->properties->category, IGNORE_MISSING))) { + return $category; + } else if ($CFG->defaultrequestcategory && + ($category = core_course_category::get($CFG->defaultrequestcategory, IGNORE_MISSING))) { + return $category; + } else { + return core_course_category::get_default(); } - if (!$category) { - $category = core_course_category::get_default(); - } - return $category; } /** @@ -3119,6 +3146,33 @@ class course_request { $eventdata->notification = 1; message_send($eventdata); } + + /** + * Checks if current user can request a course in this context + * + * @param context $context + * @return bool + */ + public static function can_request(context $context) { + global $CFG; + if (empty($CFG->enablecourserequests)) { + return false; + } + if (has_capability('moodle/course:create', $context)) { + return false; + } + + if ($context instanceof context_system) { + $defaultcontext = context_coursecat::instance($CFG->defaultrequestcategory, IGNORE_MISSING); + return $defaultcontext && + has_capability('moodle/course:request', $defaultcontext); + } else if ($context instanceof context_coursecat) { + if ($CFG->requestcategoryselection || $CFG->defaultrequestcategory == $context->instanceid) { + return has_capability('moodle/course:request', $context); + } + } + return false; + } } /** diff --git a/course/pending.php b/course/pending.php index 710ca555eb5..85a59f4d2bf 100644 --- a/course/pending.php +++ b/course/pending.php @@ -39,7 +39,20 @@ $approve = optional_param('approve', 0, PARAM_INT); $reject = optional_param('reject', 0, PARAM_INT); $baseurl = $CFG->wwwroot . '/course/pending.php'; -admin_externalpage_setup('coursespending'); +$context = context_system::instance(); +if (has_capability('moodle/site:approvecourse', $context)) { + // Similar to course management capabilities, if user has approve capability in system context + // we add the link to the admin menu. Otherwise we check if user has capability anywhere. + admin_externalpage_setup('coursespending'); +} else { + require_login(null, false); + $categories = core_course_category::make_categories_list('moodle/site:approvecourse'); + if (!$categories) { + require_capability('moodle/site:approvecourse', $context); + } + $PAGE->set_context($context); + $PAGE->set_url(new moodle_url('/course/pending.php')); +} /// Process approval of a course. if (!empty($approve) and confirm_sesskey()) { @@ -48,7 +61,11 @@ if (!empty($approve) and confirm_sesskey()) { $courseid = $course->approve(); if ($courseid !== false) { - redirect(new moodle_url('/course/edit.php', ['id' => $courseid, 'returnto' => 'pending'])); + if (has_capability('moodle/course:update', context_course::instance($courseid))) { + redirect(new moodle_url('/course/edit.php', ['id' => $courseid, 'returnto' => 'pending'])); + } else { + redirect(new moodle_url('/course/view.php', ['id' => $courseid])); + } } else { print_error('courseapprovedfailed'); } @@ -109,6 +126,9 @@ if (empty($pending)) { // Check here for shortname collisions and warn about them. $course->check_shortname_collision(); + if (!$course->can_approve()) { + continue; + } $category = $course->get_category(); $row = array(); diff --git a/course/request.php b/course/request.php index e330e1d9df3..62d32e0b57f 100644 --- a/course/request.php +++ b/course/request.php @@ -30,6 +30,7 @@ require_once($CFG->dirroot . '/course/request_form.php'); // Where we came from. Used in a number of redirects. $url = new moodle_url('/course/request.php'); $return = optional_param('return', null, PARAM_ALPHANUMEXT); +$categoryid = optional_param('category', null, PARAM_INT); if ($return === 'management') { $url->param('return', $return); $returnurl = new moodle_url('/course/management.php', array('categoryid' => $CFG->defaultrequestcategory)); @@ -47,12 +48,24 @@ if (isguestuser()) { if (empty($CFG->enablecourserequests)) { print_error('courserequestdisabled', '', $returnurl); } -$context = context_system::instance(); + +if (!$CFG->requestcategoryselection) { + // Category selection is not enabled, user will always request in the default request category. + $categoryid = null; +} else if (!$categoryid) { + // Category selection is enabled but category is not specified. + // Find a category where user has capability to request courses (preferably the default category). + $list = core_course_category::make_categories_list('moodle/course:request'); + $categoryid = array_key_exists($CFG->defaultrequestcategory, $list) ? $CFG->defaultrequestcategory : key($list); +} + +$context = context_coursecat::instance($categoryid ?: $CFG->defaultrequestcategory); $PAGE->set_context($context); require_capability('moodle/course:request', $context); // Set up the form. -$data = course_request::prepare(); +$data = $categoryid ? (object)['category' => $categoryid] : null; +$data = course_request::prepare($data); $requestform = new course_request_form($url); $requestform->set_data($data); diff --git a/course/request_form.php b/course/request_form.php index 054149db7a0..3c03797e97d 100644 --- a/course/request_form.php +++ b/course/request_form.php @@ -69,7 +69,7 @@ class course_request_form extends moodleform { $mform->setType('shortname', PARAM_TEXT); if (!empty($CFG->requestcategoryselection)) { - $displaylist = core_course_category::make_categories_list(); + $displaylist = core_course_category::make_categories_list('moodle/course:request'); $mform->addElement('select', 'category', get_string('coursecategory'), $displaylist); $mform->setDefault('category', $CFG->defaultrequestcategory); $mform->addHelpButton('category', 'coursecategory'); diff --git a/course/tests/behat/course_request.feature b/course/tests/behat/course_request.feature new file mode 100644 index 00000000000..86a00ef97cf --- /dev/null +++ b/course/tests/behat/course_request.feature @@ -0,0 +1,109 @@ +@core @core_course +Feature: Users can request and approve courses + As a moodle admin + In order to improve course creation process + I need to be able to enable course approval + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | user1 | User | 1 | user1@example.com | + | user2 | User | 2 | user2@example.com | + | user3 | User | 3 | user3@example.com | + + Scenario: Simple course request workflow + Given the following "system role assigns" exist: + | user | course | role | + | user2 | Acceptance test site | manager | + Given I log in as "admin" + And I set the following administration settings values: + | enablecourserequests | 1 | + And I log out + When I log in as "user1" + And I am on course index + And I press "Request a course" + And I set the following fields to these values: + | Course full name | My new course | + | Course short name | Mynewcourse | + | Supporting information | pretty please | + And I press "Request a course" + And I should see "Your course request has been saved successfully." + And I press "Continue" + And I am on course index + And I should not see "My new course" + And I log out + And I log in as "user2" + And I am on course index + And I press "Courses pending approval" + And I should see "Miscellaneous" in the "My new course" "table_row" + And I click on "Approve" "button" in the "My new course" "table_row" + And I press "Save and return" + And I should see "There are no courses pending approval" + And I press "Back to course listing" + And I should see "My new course" + And I log out + And I log in as "user1" + And I am on course index + And I follow "My new course" + And I navigate to course participants + And I should see "Teacher" in the "User 1" "table_row" + And I log out + + Scenario: Course request with category selection + Given the following "categories" exist: + | name | category | idnumber | + | Science category | 0 | SCI | + | English category | 0 | ENG | + | Other category | 0 | MISC | + Given the following "roles" exist: + | name | shortname | description | archetype | + | Course requestor | courserequestor | My custom role 1 | | + And the following "role assigns" exist: + | user | role | contextlevel | reference | + | user1 | courserequestor | Category | SCI | + | user1 | courserequestor | Category | ENG | + | user2 | manager | Category | SCI | + | user3 | manager | Category | ENG | + Given I log in as "admin" + And I set the following administration settings values: + | enablecourserequests | 1 | + | requestcategoryselection | 1 | + And I set the following system permissions of "Authenticated user" role: + | capability | permission | + | moodle/course:request | Prevent | + And I set the following system permissions of "Course requestor" role: + | capability | permission | + | moodle/course:request | Allow | + And I log out + And I log in as "user1" + And I am on course index + And I follow "English category" + And I press "Request a course" + And the field "Course category" matches value "English category" + And I set the following fields to these values: + | Course full name | My new course | + | Course short name | Mynewcourse | + | Supporting information | pretty please | + And I press "Request a course" + And I log out + And I log in as "user2" + And I am on course index + And I follow "English category" + And "Courses pending approval" "button" should not exist + And I am on course index + And I follow "Science category" + And I press "Courses pending approval" + And I should not see "Mynewcourse" + And I press "Back to course listing" + And I log out + And I log in as "user3" + And I am on course index + And I follow "English category" + And I press "Courses pending approval" + And I should see "English category" in the "Mynewcourse" "table_row" + And I click on "Approve" "button" in the "Mynewcourse" "table_row" + And I press "Save and return" + And I am on course index + And I follow "English category" + And I should see "My new course" + And I log out diff --git a/course/tests/courselib_test.php b/course/tests/courselib_test.php index 6779405b62f..9f2e1f24a18 100644 --- a/course/tests/courselib_test.php +++ b/course/tests/courselib_test.php @@ -6811,4 +6811,92 @@ class core_course_courselib_testcase extends advanced_testcase { course_delete_module($moduleinstances[$indextodelete]->cmid, true); // Try to delete the instance asynchronously. $this->assertEquals($expected, course_modules_pending_deletion($course->id, $gradable)); } + + /** + * Tests for the course_request::can_request + */ + public function test_can_request_course() { + global $CFG; + $this->resetAfterTest(); + + $user = $this->getDataGenerator()->create_user(); + $cat1 = $CFG->defaultrequestcategory; + $cat2 = $this->getDataGenerator()->create_category()->id; + $cat3 = $this->getDataGenerator()->create_category()->id; + $context1 = context_coursecat::instance($cat1); + $context2 = context_coursecat::instance($cat2); + $context3 = context_coursecat::instance($cat3); + $this->setUser($user); + + // By default course request is not available. + $this->assertFalse(course_request::can_request(context_system::instance())); + + // Enable course requests. Default 'user' role has capability to request courses. + $CFG->enablecourserequests = true; + $this->assertTrue(course_request::can_request(context_system::instance())); + $this->assertTrue(course_request::can_request($context1)); + $this->assertFalse(course_request::can_request($context2)); + $this->assertFalse(course_request::can_request($context3)); + + // Enable category selection. + $CFG->requestcategoryselection = 1; + $this->assertTrue(course_request::can_request(context_system::instance())); + $this->assertTrue(course_request::can_request($context1)); + $this->assertTrue(course_request::can_request($context2)); + $this->assertTrue(course_request::can_request($context3)); + + // Remove cap from cat2. + $roleid = create_role('Test role', 'testrole', 'Test role description'); + assign_capability('moodle/course:request', CAP_PROHIBIT, $roleid, + $context2->id, true); + role_assign($roleid, $user->id, $context2->id); + accesslib_clear_all_caches_for_unit_testing(); + + $this->assertTrue(course_request::can_request(context_system::instance())); + $this->assertTrue(course_request::can_request($context1)); + $this->assertFalse(course_request::can_request($context2)); + $this->assertTrue(course_request::can_request($context3)); + } + + /** + * Tests for the course_request::can_approve + */ + public function test_can_approve_course_request() { + global $CFG; + $this->resetAfterTest(); + + $requestor = $this->getDataGenerator()->create_user(); + $user = $this->getDataGenerator()->create_user(); + $cat1 = $CFG->defaultrequestcategory; + $cat2 = $this->getDataGenerator()->create_category()->id; + $cat3 = $this->getDataGenerator()->create_category()->id; + + // Enable course requests. Default 'user' role has capability to request courses. + $CFG->enablecourserequests = true; + $CFG->requestcategoryselection = 1; + $this->setUser($requestor); + $requestdata = ['summary_editor' => ['text' => '', 'format' => 0], 'name' => 'Req', 'reason' => 'test']; + $request1 = course_request::create((object)($requestdata)); + $request2 = course_request::create((object)($requestdata + ['category' => $cat2])); + $request3 = course_request::create((object)($requestdata + ['category' => $cat3])); + + $this->setUser($user); + // Add capability to approve courses. + $roleid = create_role('Test role', 'testrole', 'Test role description'); + assign_capability('moodle/site:approvecourse', CAP_ALLOW, $roleid, + context_system::instance()->id, true); + role_assign($roleid, $user->id, context_coursecat::instance($cat2)->id); + accesslib_clear_all_caches_for_unit_testing(); + + $this->assertFalse($request1->can_approve()); + $this->assertTrue($request2->can_approve()); + $this->assertFalse($request3->can_approve()); + + // Delete category where course was requested. Now only site-wide manager can approve it. + core_course_category::get($cat2, MUST_EXIST, true)->delete_full(false); + $this->assertFalse($request2->can_approve()); + + $this->setAdminUser(); + $this->assertTrue($request2->can_approve()); + } } diff --git a/lib/db/access.php b/lib/db/access.php index 4253f7dfa78..61126bc7d8c 100644 --- a/lib/db/access.php +++ b/lib/db/access.php @@ -133,7 +133,7 @@ $capabilities = array( 'riskbitmask' => RISK_XSS, 'captype' => 'write', - 'contextlevel' => CONTEXT_SYSTEM, + 'contextlevel' => CONTEXT_COURSECAT, 'archetypes' => array( 'manager' => CAP_ALLOW ) @@ -782,7 +782,7 @@ $capabilities = array( 'moodle/course:request' => array( 'captype' => 'write', - 'contextlevel' => CONTEXT_SYSTEM, + 'contextlevel' => CONTEXT_COURSECAT, 'archetypes' => array( 'user' => CAP_ALLOW, )