From 481cfdc3f03fe1b0865ba675a2c10182035d45f1 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Wed, 2 Mar 2022 13:16:18 +0700 Subject: [PATCH] MDL-73549 Course: My course page menu improvement - Introduce core_course_category::get_nearest_editable_subcategory() - This function will return the first creatable/manageable category for current user - With this new function, we can fix the issue that the users with course management or creation permision at category level cannot see the manage menu on My courses page --- course/classes/category.php | 58 +++++++++++++++++ course/tests/category_test.php | 105 ++++++++++++++++++++++++++++++ course/upgrade.txt | 3 +- my/courses.php | 18 +++-- my/templates/dropdown.mustache | 8 ++- my/tests/behat/my_courses.feature | 68 +++++++++++++++++++ 6 files changed, 251 insertions(+), 9 deletions(-) diff --git a/course/classes/category.php b/course/classes/category.php index 523f3f818b2fb..d054318effcd6 100644 --- a/course/classes/category.php +++ b/course/classes/category.php @@ -3100,6 +3100,36 @@ public function can_request_course() { return course_request::can_request($this->get_context()); } + /** + * Returns true if the user has all the given permissions. + * + * @param array $permissionstocheck The value can be create, manage or any specific capability. + * @return bool + */ + private function has_capabilities(array $permissionstocheck): bool { + if (empty($permissionstocheck)) { + throw new coding_exception('Invalid permissionstocheck parameter'); + } + foreach ($permissionstocheck as $permission) { + if ($permission == 'create') { + if (!$this->can_create_course()) { + return false; + } + } else if ($permission == 'manage') { + if (!$this->has_manage_capability()) { + return false; + } + } else { + // Specific capability. + if (!$this->is_uservisible() || !has_capability($permission, $this->get_context())) { + return false; + } + } + } + + return true; + } + /** * Returns true if the user can approve course requests. * @return bool @@ -3146,4 +3176,32 @@ public static function page_setup() { } } } + + /** + * Returns the core_course_category object for the first category that the current user have the permission for the course. + * + * Only returns if it exists and is creatable/manageable to the current user + * + * @param core_course_category $parentcat Parent category to check. + * @param array $permissionstocheck The value can be create, manage or any specific capability. + * @return core_course_category|null + */ + public static function get_nearest_editable_subcategory(core_course_category $parentcat, + array $permissionstocheck): ?core_course_category { + // First, check the parent category. + if ($parentcat->has_capabilities($permissionstocheck)) { + return $parentcat; + } + + // Check the child categories. + $subcategoryids = $parentcat->get_all_children_ids(); + foreach ($subcategoryids as $subcategoryid) { + $subcategory = static::get($subcategoryid); + if ($subcategory->has_capabilities($permissionstocheck)) { + return $subcategory; + } + } + + return null; + } } diff --git a/course/tests/category_test.php b/course/tests/category_test.php index 9ed5fb9c00e53..162c56446d753 100644 --- a/course/tests/category_test.php +++ b/course/tests/category_test.php @@ -1102,4 +1102,109 @@ public function test_get_courses_during_delete() { $this->assertCount(1, $courses); $this->assertArrayHasKey($othercourse->id, $courses); } + + /** + * Test get_nearest_editable_subcategory() method. + * + * @covers \core_course_category::get_nearest_editable_subcategory + */ + public function test_get_nearest_editable_subcategory(): void { + global $DB; + + $coursecreatorrole = $DB->get_record('role', ['shortname' => 'coursecreator']); + $managerrole = $DB->get_record('role', ['shortname' => 'manager']); + + // Create categories. + $category1 = core_course_category::create(['name' => 'Cat1']); + $category2 = core_course_category::create(['name' => 'Cat2']); + $category3 = core_course_category::create(['name' => 'Cat3']); + // Get the category contexts. + $category1context = $category1->get_context(); + $category2context = $category2->get_context(); + $category3context = $category3->get_context(); + // Create user. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $user3 = $this->getDataGenerator()->create_user(); + // Assign the user1 to 'Course creator' role for Cat1. + role_assign($coursecreatorrole->id, $user1->id, $category1context->id); + // Assign the user2 to 'Manager' role for Cat3. + role_assign($managerrole->id, $user2->id, $category3context->id); + + // Start scenario 1. + // user3 has no permission to create course or manage category. + $this->setUser($user3); + $coursecat = core_course_category::user_top(); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['create'])); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/course:create'])); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['manage'])); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/category:manage'])); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['create', 'manage'])); + // End scenario 1. + + // Start scenario 2. + // user1 has permission to create course but has no permission to manage category. + $this->setUser($user1); + $coursecat = core_course_category::user_top(); + $this->assertNotEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['create'])); + $this->assertNotEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/course:create'])); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['manage'])); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/category:manage'])); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['create', 'manage'])); + // The get_nearest_editable_subcategory should return Cat1. + $this->assertEquals($category1->id, core_course_category::get_nearest_editable_subcategory($coursecat, ['create'])->id); + $this->assertEquals($category1->id, + core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/course:create'])->id); + // Assign the user1 to 'Course creator' role for Cat2. + role_assign($coursecreatorrole->id, $user1->id, $category2context->id); + // The get_nearest_editable_subcategory should still return Cat1 (First creatable subcategory) for create course capability. + $this->assertEquals($category1->id, core_course_category::get_nearest_editable_subcategory($coursecat, ['create'])->id); + $this->assertEquals($category1->id, + core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/course:create'])->id); + // End scenario 2. + + // Start scenario 3. + // user2 has no permission to create course but has permission to manage category. + $this->setUser($user2); + // Remove the moodle/course:create capability for the manager role. + unassign_capability('moodle/course:create', $managerrole->id); + $coursecat = core_course_category::user_top(); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['create'])); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/course:create'])); + $this->assertNotEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['manage'])); + $this->assertNotEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/category:manage'])); + $this->assertEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['create', 'manage'])); + // The get_nearest_editable_subcategory should return Cat3. + $this->assertEquals($category3->id, core_course_category::get_nearest_editable_subcategory($coursecat, ['manage'])->id); + $this->assertEquals($category3->id, + core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/category:manage'])->id); + // End scenario 3. + + // Start scenario 4. + // user2 has both permission to create course and manage category. + // Add the moodle/course:create capability back again for the manager role. + assign_capability('moodle/course:create', CAP_ALLOW, $managerrole->id, $category3context->id); + $this->setUser($user2); + $coursecat = core_course_category::user_top(); + $this->assertNotEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['create'])); + $this->assertNotEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/course:create'])); + $this->assertNotEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['manage'])); + $this->assertNotEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['moodle/category:manage'])); + $this->assertNotEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, ['create', 'manage'])); + // The get_nearest_editable_subcategory should return Cat3. + $this->assertEquals($category3->id, + core_course_category::get_nearest_editable_subcategory($coursecat, ['create', 'manage'])->id); + $this->assertEquals($category3->id, core_course_category::get_nearest_editable_subcategory($coursecat, + ['moodle/course:create', 'moodle/category:manage'])->id); + // End scenario 4. + + // Start scenario 5. + // Exception will be thrown if $permissionstocheck is empty. + $this->setUser($user1); + $coursecat = core_course_category::user_top(); + $this->expectException('coding_exception'); + $this->expectExceptionMessage('Invalid permissionstocheck parameter'); + $this->assertNotEmpty(core_course_category::get_nearest_editable_subcategory($coursecat, [])); + // End scenario 5. + } } diff --git a/course/upgrade.txt b/course/upgrade.txt index 6d57cdd1a1eae..cdf444d1cc3cc 100644 --- a/course/upgrade.txt +++ b/course/upgrade.txt @@ -87,6 +87,8 @@ course formats don't have their own renderer. - print_course_request_buttons * New page_setup() method in the core_course_category class. This method can be used for a general page setup in the course category pages. +* New core_course_category::get_nearest_editable_subcategory(): + - Return the core_course_category object for the first subcategory that the current user has the permission on it. === 3.11 === * A new callback xxx_coursemodule_definition_after_data that allows plugins to extend activity forms after the data is set. @@ -100,7 +102,6 @@ course formats don't have their own renderer. - activity_dates_information_in_activity_should_not_exist() - Given the activity date information in "" should not exist * A user preference usemodchooser has been removed and the activities/resources (non-ajax) activity chooser has been deprecated and will be removed in the future. - === 3.10 === * The function make_categories_options() has now been deprecated. Please use \core_course_category::make_categories_list() instead. diff --git a/my/courses.php b/my/courses.php index 9b59fc84acbdd..49cb4f4cd9ef2 100644 --- a/my/courses.php +++ b/my/courses.php @@ -63,12 +63,18 @@ // Add course management if the user has the capabilities for it. $coursecat = core_course_category::user_top(); -if ($coursecat->can_create_course() || $coursecat->has_manage_capability()) { - $data = [ - 'newcourseurl' => new moodle_url('/course/edit.php', ['category' => $coursecat->id]), - 'manageurl' => new moodle_url('/course/management.php', ['categoryid' => $coursecat->id]), - ]; - $PAGE->add_header_action($OUTPUT->render_from_template('my/dropdown', $data)); +$coursemanagemenu = []; +if ($category = core_course_category::get_nearest_editable_subcategory($coursecat, ['create'])) { + // The user has the capability to create course. + $coursemanagemenu['newcourseurl'] = new moodle_url('/course/edit.php', ['category' => $category->id]); +} +if ($category = core_course_category::get_nearest_editable_subcategory($coursecat, ['manage'])) { + // The user has the capability to manage the course category. + $coursemanagemenu['manageurl'] = new moodle_url('/course/management.php', ['categoryid' => $category->id]); +} +if (!empty($coursemanagemenu)) { + // Render the course management menu. + $PAGE->add_header_action($OUTPUT->render_from_template('my/dropdown', $coursemanagemenu)); } echo $OUTPUT->header(); diff --git a/my/templates/dropdown.mustache b/my/templates/dropdown.mustache index 4784d9a1ad1f0..ada1243be9c6b 100644 --- a/my/templates/dropdown.mustache +++ b/my/templates/dropdown.mustache @@ -31,7 +31,11 @@
- {{#str}}newcourse, core{{/str}} - {{#str}}managecourses, core{{/str}} + {{#newcourseurl}} + {{#str}}newcourse, core{{/str}} + {{/newcourseurl}} + {{#manageurl}} + {{#str}}managecourses, core{{/str}} + {{/manageurl}}
diff --git a/my/tests/behat/my_courses.feature b/my/tests/behat/my_courses.feature index 8814c7e0c1e6c..1ce8b577a2d72 100644 --- a/my/tests/behat/my_courses.feature +++ b/my/tests/behat/my_courses.feature @@ -1,6 +1,20 @@ @core @core_my Feature: Run tests over my courses. + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | user1 | User | 1 | user1@example.com | + And the following "categories" exist: + | name | category | idnumber | + | CatA | 0 | cata | + And the following "roles" exist: + | shortname | name | archetype | + | role1 | Role 1 | | + And the following "system role assigns" exist: + | user | role | contextlevel | reference | + | user1 | role1 | Category | CatA | + Scenario: Admin can add new courses or manage them from my courses Given I am on the "My courses" page logged in as "admin" And I click on "Course management options" "link" @@ -11,3 +25,57 @@ Feature: Run tests over my courses. And I click on "Course management options" "link" And I click on "Manage courses" "link" And I should see "Manage course categories and courses" + + Scenario: User without creating a course and managing category permissions cannot see any link + Given I am on the "My courses" page logged in as "user1" + Then "Course management options" "link" should not exist + + @javascript + Scenario: User with creating a course permission can see the Create course link only + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/course:create | Allow | role1 | Category | cata | + When I am on the "My courses" page logged in as "user1" + Then "Course management options" "link" should exist + And I click on "Course management options" "link" + And I should see "New course" + And I should not see "Manage courses" + And I click on "New course" "link" + And I wait to be redirected + And I should see "Add a new course" + And "CatA" "autocomplete_selection" should exist + + @javascript + Scenario: User with managing a category permission can see the Manage course link only + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/category:manage | Allow | role1 | Category | cata | + When I am on the "My courses" page logged in as "user1" + Then "Course management options" "link" should exist + And I click on "Course management options" "link" + And I should not see "New course" + And I should see "Manage courses" + And I click on "Manage courses" "link" + And I wait to be redirected + And I should see "Manage course categories and courses" + + @javascript + Scenario: User with both creating a course and managing a category permission can see both links + Given the following "permission overrides" exist: + | capability | permission | role | contextlevel | reference | + | moodle/course:create | Allow | role1 | Category | cata | + | moodle/category:manage | Allow | role1 | Category | cata | + When I am on the "My courses" page logged in as "user1" + Then "Course management options" "link" should exist + And I click on "Course management options" "link" + And I should see "New course" + And I should see "Manage courses" + And I click on "New course" "link" + And I wait to be redirected + And I should see "Add a new course" + And "CatA" "autocomplete_selection" should exist + And I am on the "My courses" page + And I click on "Course management options" "link" + And I click on "Manage courses" "link" + And I wait to be redirected + And I should see "Manage course categories and courses"