diff --git a/admin/tool/uploaduser/classes/cli_helper.php b/admin/tool/uploaduser/classes/cli_helper.php index e258e7d8ad9..da2dbe54bef 100644 --- a/admin/tool/uploaduser/classes/cli_helper.php +++ b/admin/tool/uploaduser/classes/cli_helper.php @@ -111,7 +111,10 @@ class cli_helper { $form = new \admin_uploaduser_form1(); [$elements, $defaults] = $form->get_form_for_cli(); $options += $this->prepare_form_elements_for_cli($elements, $defaults); - $form = new \admin_uploaduser_form2(null, ['columns' => ['type1'], 'data' => []]); + // Specify pseudo-column 'type1' to force the form to populate the legacy role mapping selector + // but only if user is allowed to assign roles in courses (otherwise form validation will fail). + $columns = uu_allowed_roles() ? ['type1'] : []; + $form = new \admin_uploaduser_form2(null, ['columns' => $columns, 'data' => []]); [$elements, $defaults] = $form->get_form_for_cli(); $options += $this->prepare_form_elements_for_cli($elements, $defaults); return $options; diff --git a/admin/tool/uploaduser/classes/process.php b/admin/tool/uploaduser/classes/process.php index 6ef4f854f97..8569812b878 100644 --- a/admin/tool/uploaduser/classes/process.php +++ b/admin/tool/uploaduser/classes/process.php @@ -126,7 +126,6 @@ class process { $today = make_timestamp(date('Y', $today), date('m', $today), date('d', $today), 0, 0, 0); $this->today = $today; - $this->rolecache = uu_allowed_roles_cache(); // Course roles lookup cache. $this->sysrolecache = uu_allowed_sysroles_cache(); // System roles lookup cache. $this->supportedauths = uu_supported_auths(); // Officially supported plugins that are enabled. @@ -1209,14 +1208,18 @@ class process { } } + if (!array_key_exists($courseid, $this->rolecache)) { + $this->rolecache[$courseid] = uu_allowed_roles_cache(null, (int)$courseid); + } + if ($courseid == SITEID) { // Technically frontpage does not have enrolments, but only role assignments, // let's not invent new lang strings here for this rarely used feature. if (!empty($user->{'role'.$i})) { $rolename = $user->{'role'.$i}; - if (array_key_exists($rolename, $this->rolecache)) { - $roleid = $this->rolecache[$rolename]->id; + if (array_key_exists($rolename, $this->rolecache[$courseid]) ) { + $roleid = $this->rolecache[$courseid][$rolename]->id; } else { $this->upt->track('enrolments', get_string('unknownrole', 'error', s($rolename)), 'error'); continue; @@ -1226,7 +1229,7 @@ class process { $a = new \stdClass(); $a->course = $shortname; - $a->role = $this->rolecache[$roleid]->name; + $a->role = $this->rolecache[$courseid][$roleid]->name; $this->upt->track('enrolments', get_string('enrolledincourserole', 'enrol_manual', $a), 'info'); } @@ -1236,8 +1239,8 @@ class process { $roleid = false; if (!empty($user->{'role'.$i})) { $rolename = $user->{'role'.$i}; - if (array_key_exists($rolename, $this->rolecache)) { - $roleid = $this->rolecache[$rolename]->id; + if (array_key_exists($rolename, $this->rolecache[$courseid])) { + $roleid = $this->rolecache[$courseid][$rolename]->id; } else { $this->upt->track('enrolments', get_string('unknownrole', 'error', s($rolename)), 'error'); continue; @@ -1256,7 +1259,15 @@ class process { } } else { // No role specified, use the default from manual enrol plugin. - $roleid = $this->manualcache[$courseid]->roleid; + $defaultenrolroleid = (int)$this->manualcache[$courseid]->roleid; + // Validate the current user can assign this role. + if (array_key_exists($defaultenrolroleid, $this->rolecache[$courseid]) ) { + $roleid = $defaultenrolroleid; + } else { + $role = $DB->get_record('role', ['id' => $defaultenrolroleid]); + $this->upt->track('enrolments', get_string('unknownrole', 'error', s($role->shortname)), 'error'); + continue; + } } if ($roleid) { @@ -1299,7 +1310,7 @@ class process { $a = new \stdClass(); $a->course = $shortname; - $a->role = $this->rolecache[$roleid]->name; + $a->role = $this->rolecache[$courseid][$roleid]->name; $this->upt->track('enrolments', get_string('enrolledincourserole', 'enrol_manual', $a), 'info'); } } diff --git a/admin/tool/uploaduser/locallib.php b/admin/tool/uploaduser/locallib.php index 0a62791488e..d44071e5921 100644 --- a/admin/tool/uploaduser/locallib.php +++ b/admin/tool/uploaduser/locallib.php @@ -384,23 +384,32 @@ function uu_allowed_roles() { } /** - * Returns mapping of roles using short role name as index. + * Returns assignable roles for current user using short role name and role ID as index. + * This function is no longer called without parameters. * - * @param int|null $categoryid Id of the category to get roles for. Null means all roles. + * @param int|null $categoryid Id of the category to get roles for. + * @param int|null $courseid Id of the course to get roles for. * @return array */ -function uu_allowed_roles_cache(?int $categoryid = null): array { - if (is_null($categoryid)) { - $allowedroles = get_assignable_roles(context_course::instance(SITEID), ROLENAME_SHORT); - } else { +function uu_allowed_roles_cache(?int $categoryid = null, ?int $courseid = null): array { + if (!is_null($categoryid) && !is_null($courseid)) { + return []; + } else if (is_null($categoryid) && !is_null($courseid)) { + $allowedroles = get_assignable_roles(context_course::instance($courseid), ROLENAME_SHORT); + } else if (is_null($courseid) && !is_null($categoryid)) { $allowedroles = get_assignable_roles(context_coursecat::instance($categoryid), ROLENAME_SHORT); + } else { + $allowedroles = get_assignable_roles(context_course::instance(SITEID), ROLENAME_SHORT); } + $rolecache = []; + // A role can be searched for by its ID or by its shortname. foreach ($allowedroles as $rid=>$rname) { $rolecache[$rid] = new stdClass(); $rolecache[$rid]->id = $rid; $rolecache[$rid]->name = $rname; - if (!is_numeric($rname)) { // only non-numeric shortnames are supported!!! + // Since numeric short names are allowed, to avoid replacement of another role, we only accept non-numeric values. + if (!is_numeric($rname)) { $rolecache[$rname] = new stdClass(); $rolecache[$rname]->id = $rid; $rolecache[$rname]->name = $rname; diff --git a/admin/tool/uploaduser/tests/upload_users_test.php b/admin/tool/uploaduser/tests/upload_users_test.php new file mode 100644 index 00000000000..49e204e716f --- /dev/null +++ b/admin/tool/uploaduser/tests/upload_users_test.php @@ -0,0 +1,221 @@ +. + +namespace tool_uploaduser; + +use advanced_testcase; +use context_system; +use context_course; +use context_coursecat; +use stdClass; +use tool_uploaduser\cli_helper; +use tool_uploaduser\local\text_progress_tracker; + +/** + * Class upload_users_test + * + * @package tool_uploaduser + * @copyright 2020 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class upload_users_test extends advanced_testcase { + + /** + * Load required test libraries + */ + public static function setUpBeforeClass(): void { + global $CFG; + + require_once("{$CFG->dirroot}/{$CFG->admin}/tool/uploaduser/locallib.php"); + } + + /** + * Test upload users, enrol and role assignation + * @covers \tool_uploadusers::process + */ + public function test_user_can_upload_with_course_enrolment(): void { + + $this->resetAfterTest(); + set_config('passwordpolicy', 0); + $this->setAdminUser(); + + // Create category and course. + $coursecat = $this->getDataGenerator()->create_category(); + $coursecatcontext = context_coursecat::instance($coursecat->id); + $course = $this->getDataGenerator()->create_course(['shortname' => 'course01', 'category' => $coursecat->id]); + $coursecontext = context_course::instance($course->id); + + // Create user. + $user = $this->getDataGenerator()->create_user(); + + // Create role with capability to upload CSV files, and assign this role to user. + $uploadroleid = create_role('upload role', 'uploadrole', ''); + set_role_contextlevels($uploadroleid, [CONTEXT_SYSTEM]); + $systemcontext = context_system::instance(); + assign_capability('moodle/site:uploadusers', CAP_ALLOW, $uploadroleid, $systemcontext->id); + $this->getDataGenerator()->role_assign($uploadroleid, $user->id, $systemcontext->id); + + // Create role with some of allowed capabilities to enrol users, and assign this role to user. + $enrolroleid = create_role('enrol role', 'enrolrole', ''); + set_role_contextlevels($enrolroleid, [CONTEXT_COURSECAT]); + assign_capability('enrol/manual:enrol', CAP_ALLOW, $enrolroleid, $coursecatcontext->id); + assign_capability('moodle/course:enrolreview', CAP_ALLOW, $enrolroleid, $coursecatcontext->id); + assign_capability('moodle/role:assign', CAP_ALLOW, $enrolroleid, $coursecatcontext->id); + $this->getDataGenerator()->role_assign($enrolroleid, $user->id, $coursecatcontext->id); + + // User makes assignments. + $studentarch = get_archetype_roles('student'); + $studentrole = array_shift($studentarch); + core_role_set_assign_allowed($enrolroleid, $studentrole->id); + + // Flush accesslib. + accesslib_clear_all_caches_for_unit_testing(); + + // Process CSV file as user. + $csv = <<shortname},{$studentrole->shortname} +student2,Student,Two,s2@example.com,{$course->shortname},teacher +EOF; + $this->setUser($user); + $output = $this->process_csv_upload($csv, ['--uutype=' . UU_USER_ADDNEW]); + + $this->assertStringContainsString('Enrolled in "course01" as "student"', $output); + $this->assertStringContainsString('Unknown role "teacher"', $output); + + // Check user creation, enrolment and role assignation. + $this->assertEquals(1, count_enrolled_users($coursecontext)); + + $usersasstudent = get_role_users($studentrole->id, $coursecontext); + $this->assertCount(1, $usersasstudent); + $this->assertEquals('student1', reset($usersasstudent)->username); + } + + /** + * Test upload users, enrol and assign default role from manual enrol plugin. + * @covers \tool_uploadusers::process + */ + public function test_user_can_upload_with_course_enrolment_default_role(): void { + + $this->resetAfterTest(); + set_config('passwordpolicy', 0); + $this->setAdminUser(); + + // Create category and courses. + $coursecat = $this->getDataGenerator()->create_category(); + $coursecatcontext = context_coursecat::instance($coursecat->id); + $course1 = $this->getDataGenerator()->create_course(['shortname' => 'course01', 'category' => $coursecat->id]); + $course1context = context_course::instance($course1->id); + // Change the default role to 'teacher'. + set_config('roleid', 4, 'enrol_manual'); + $course2 = $this->getDataGenerator()->create_course(['shortname' => 'course02', 'category' => $coursecat->id]); + $course2context = context_course::instance($course2->id); + + // Create user. + $user = $this->getDataGenerator()->create_user(); + + // Create role with capability to upload CSV files, and assign this role to user. + $uploadroleid = create_role('upload role', 'uploadrole', ''); + set_role_contextlevels($uploadroleid, [CONTEXT_SYSTEM]); + $systemcontext = context_system::instance(); + assign_capability('moodle/site:uploadusers', CAP_ALLOW, $uploadroleid, $systemcontext->id); + $this->getDataGenerator()->role_assign($uploadroleid, $user->id, $systemcontext->id); + + // Create role with some of allowed capabilities to enrol users, and assign this role to user. + $enrolroleid = create_role('enrol role', 'enrolrole', ''); + set_role_contextlevels($enrolroleid, [CONTEXT_COURSECAT]); + assign_capability('enrol/manual:enrol', CAP_ALLOW, $enrolroleid, $coursecatcontext->id); + assign_capability('moodle/course:enrolreview', CAP_ALLOW, $enrolroleid, $coursecatcontext->id); + assign_capability('moodle/role:assign', CAP_ALLOW, $enrolroleid, $coursecatcontext->id); + $this->getDataGenerator()->role_assign($enrolroleid, $user->id, $coursecatcontext->id); + + // User makes assignments. + $studentarch = get_archetype_roles('student'); + $studentrole = array_shift($studentarch); + core_role_set_assign_allowed($enrolroleid, $studentrole->id); + + // Flush accesslib. + accesslib_clear_all_caches_for_unit_testing(); + + // Process CSV file (no roles specified) as user. + $csv = <<shortname}, +student2,Student,Two,s2@example.com,{$course2->shortname}, +EOF; + $this->setUser($user); + $output = $this->process_csv_upload($csv, ['--uutype=' . UU_USER_ADDNEW]); + + $this->assertStringContainsString('Enrolled in "course01" as "student"', $output); + // This $user cannot assign teacher role. + $this->assertStringContainsString('Unknown role "teacher"', $output); + + // Check user creation, enrolment and role assignation. + $this->assertEquals(1, count_enrolled_users($course1context)); + // This $user cannot enrol anyone as teacher. + $this->assertEquals(0, count_enrolled_users($course2context)); + + // Test user is enrolled as default-manual-enrol-plugin role. + $manualenrolinstance = new stdClass; + $enrolinstances = enrol_get_instances($course1->id, true); + foreach ($enrolinstances as $courseenrolinstance) { + if ($courseenrolinstance->enrol === 'manual') { + $manualenrolinstance = $courseenrolinstance; + break; + } + } + $defaulroleidexpected = $manualenrolinstance->roleid ?? 0; + // The default role of course01 is student, id 5. + $this->assertEquals(5, $defaulroleidexpected); + + $usersasdefaultrole = get_role_users($defaulroleidexpected, $course1context); + $this->assertCount(1, $usersasdefaultrole); + $this->assertEquals('student1', reset($usersasdefaultrole)->username); + } + + /** + * Generate cli_helper and mock $_SERVER['argv'] + * + * @param string $filecontent + * @param array $mockargv + * @return string + */ + protected function process_csv_upload(string $filecontent, array $mockargv = []): string { + $filepath = make_request_directory() . '/upload.csv'; + file_put_contents($filepath, $filecontent); + $mockargv[] = "--file={$filepath}"; + + if (array_key_exists('argv', $_SERVER)) { + $oldservervars = $_SERVER['argv']; + } + + $_SERVER['argv'] = array_merge([''], $mockargv); + $clihelper = new cli_helper(text_progress_tracker::class); + + if (isset($oldservervars)) { + $_SERVER['argv'] = $oldservervars; + } else { + unset($_SERVER['argv']); + } + + ob_start(); + $clihelper->process(); + $output = ob_get_contents(); + ob_end_clean(); + + return $output; + } +}