add check whether role can be assigned in the context of a course cat…

…egory. Without this additional check it would look as everything worked, although it didn't

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);
     }

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);
+    }
 }

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.

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 / <username>"
         $this->assertEquals($category_count_before + 2, count(core_course_category::make_categories_list()));
     }
-}
+
+    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';
+    }
+}

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');
+    }
 }