diff --git a/classes/local/course_category_manager.php b/classes/local/course_category_manager.php index 3eee967..96c781f 100644 --- a/classes/local/course_category_manager.php +++ b/classes/local/course_category_manager.php @@ -40,10 +40,16 @@ public static function create_category_user_can_create_courses_in(string $userna throw new moodle_exception('category_already_exists', 'local_adler'); } + // get role id and check if role is assignable to a course category + $role_id = $moodle_core_repository->get_role_id_by_shortname($role); + if (!in_array(CONTEXT_COURSECAT, moodle_core::get_role_contextlevels($role_id))) { + throw new invalid_parameter_exception('role_not_assignable_to_course_category'); + } + // create category and assign user to role $category_id = $category_path->create(); - self::assign_user_to_role_in_category($username, $role, $category_id); + self::assign_user_to_role_in_category($username, $role_id, $category_id); return $category_id; } @@ -52,11 +58,10 @@ public static function create_category_user_can_create_courses_in(string $userna * @throws coding_exception * @throws dml_exception */ - private static function assign_user_to_role_in_category(string $username, string $role_shortname, int $category_id): void { + private static function assign_user_to_role_in_category(string $username, int $role_id, int $category_id): void { $moodle_core_repository = new moodle_core_repository(); $user_id = $moodle_core_repository->get_user_id_by_username($username); - $role_id = $moodle_core_repository->get_role_id_by_shortname($role_shortname); $context = moodle_core::context_coursecat_instance($category_id); moodle_core::role_assign($role_id, $user_id, $context->id); } diff --git a/classes/moodle_core.php b/classes/moodle_core.php index 8301378..da184ba 100644 --- a/classes/moodle_core.php +++ b/classes/moodle_core.php @@ -16,8 +16,13 @@ public static function role_assign(...$args): int { return role_assign(...$args); } - /** alias for context_coursecat::instance */ + /** alias for context_coursecat::instance() */ public static function context_coursecat_instance(...$args): object { return context_coursecat::instance(...$args); } + + /** alias for get_role_contextlevels() */ + public static function get_role_contextlevels(...$args): array { + return get_role_contextlevels(...$args); + } } diff --git a/cli/create_course_cat_and_assign_user_role.php b/cli/create_course_cat_and_assign_user_role.php index d68281c..f2e8803 100644 --- a/cli/create_course_cat_and_assign_user_role.php +++ b/cli/create_course_cat_and_assign_user_role.php @@ -9,7 +9,7 @@ require_once(__DIR__ . '/../../../config.php'); global $CFG; -require_once("{$CFG->libdir}/clilib.php"); +require_once($CFG->libdir . '/clilib.php'); $help = "Create a new course category and grant the user permission to create adler courses in it. diff --git a/tests/cli/create_course_cat_and_assign_user_role_test.php b/tests/cli/create_course_cat_and_assign_user_role_test.php index e9be6e6..3c5b521 100644 --- a/tests/cli/create_course_cat_and_assign_user_role_test.php +++ b/tests/cli/create_course_cat_and_assign_user_role_test.php @@ -176,4 +176,29 @@ public function test_script_without_category_path() { // default creates two categories, "adler" and "adler / " $this->assertEquals($category_count_before + 2, count(core_course_category::make_categories_list())); } -} \ No newline at end of file + + public function test_with_role_that_cannot_be_assigned_to_course_category() { + global $CFG, $DB; + + // Arrange + $user = $this->getDataGenerator()->create_user(); + $role_id = $this->getDataGenerator()->create_role(); + $role = $DB->get_record('role', ['id' => $role_id]); + $contextlevels = [ + CONTEXT_COURSE, + ]; + set_role_contextlevels($role_id, $contextlevels); + + $_SERVER['argv'] = [ + 'create_course_cat_and_assign_user_role.php', + '--username=' . $user->username, + '--role=' . $role->shortname, + ]; + + $this->expectException(exit_exception::class); + $this->expectExceptionCode(1); + + // Act + require $CFG->dirroot . '/local/adler/cli/create_course_cat_and_assign_user_role.php'; + } +} diff --git a/tests/local/course_category_manager_test.php b/tests/local/course_category_manager_test.php index 7c09f53..908ab41 100644 --- a/tests/local/course_category_manager_test.php +++ b/tests/local/course_category_manager_test.php @@ -3,6 +3,7 @@ namespace local_adler\local; use context_coursecat; +use invalid_parameter_exception; use local_adler\lib\adler_testcase; use local_adler\local\db\moodle_core_repository; use local_adler\moodle_core; @@ -17,9 +18,11 @@ */ class course_category_manager_test extends adler_testcase { private $mockRepo; + private $moodle_core_mock; public function setUp(): void { $this->mockRepo = Mockery::mock('overload:' . moodle_core_repository::class); + $this->moodle_core_mock = Mockery::mock('alias:' . moodle_core::class); } public function test_username_doesnt_exist() { @@ -58,7 +61,7 @@ public function test_category_already_exists() { } /** - * @dataProvider providerTestValidUsernameRoleAndCategoryPath + * @dataProvider provide_test_valid_username_role_and_category_path_data */ public function test_valid_username_role_and_category_path($category_path, $expected_result) { // Arrange @@ -69,11 +72,15 @@ public function test_valid_username_role_and_category_path($category_path, $expe $mockPath->shouldReceive('exists')->andReturn(false); $mockPath->shouldReceive('create')->andReturn(42); - $moodle_core_mock = Mockery::mock('alias:' . moodle_core::class) + $this->moodle_core_mock + ->shouldReceive('get_role_contextlevels') + ->andReturn([CONTEXT_COURSECAT]); + + $this->moodle_core_mock ->shouldReceive('context_coursecat_instance') ->andReturn(Mockery::mock(context_coursecat::class)); - $moodle_core_mock + $this->moodle_core_mock ->shouldReceive('role_assign') ->withArgs([1, 1, null]) ->andReturn(true); @@ -85,11 +92,29 @@ public function test_valid_username_role_and_category_path($category_path, $expe $this->assertEquals($expected_result, $result); } - public function providerTestValidUsernameRoleAndCategoryPath() { + public function provide_test_valid_username_role_and_category_path_data() { return [ 'null category path' => [null, 42], 'empty category path' => ['', 42], 'non-existing category path' => ['non_existing_category_path', 42] ]; } + + public function test_with_role_that_cannot_be_assigned_to_course_category() { + // Arrange + $this->mockRepo->shouldReceive('get_user_id_by_username')->andReturn(1); // return a valid user ID + $this->mockRepo->shouldReceive('get_role_id_by_shortname')->andReturn(1); // return a valid role ID + + $mockPath = Mockery::mock('overload:' . course_category_path::class); + $mockPath->shouldReceive('exists')->andReturn(false); + $mockPath->shouldReceive('create')->andReturn(42); + + $this->moodle_core_mock + ->shouldReceive('get_role_contextlevels') + ->andReturn([CONTEXT_SYSTEM]); + + // Act and Assert + $this->expectException(invalid_parameter_exception::class); + course_category_manager::create_category_user_can_create_courses_in('valid_username', 'valid_role', 'category_path'); + } } \ No newline at end of file