Skip to content

Commit

Permalink
MDL-73981 tool_uploadcourse: Validate enrolment role from csv.
Browse files Browse the repository at this point in the history
  • Loading branch information
@ilyatregubov
ilyatregubov committed May 5, 2022
1 parent 1a74403 commit 82e3a9c
Show file tree
Hide file tree
Showing 7 changed files with 234 additions and 37 deletions.
144 changes: 107 additions & 37 deletions admin/tool/uploadcourse/classes/course.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ class tool_uploadcourse_course {
/** Outcome of the process: deleting the course */
const DO_DELETE = 3;

/** @var array assignable roles. */
protected $assignableroles = [];

/** @var array Roles context levels. */
protected $contextlevels = [];

/** @var array final import data. */
protected $data = array();

Expand Down Expand Up @@ -794,16 +800,15 @@ public function prepare() {

// Get enrolment data. Where the course already exists, we can also perform validation.
$this->enrolmentdata = tool_uploadcourse_helper::get_enrolment_data($this->rawdata);
if ($exists) {
$errors = $this->validate_enrolment_data($coursedata['id'], $this->enrolmentdata);
$courseid = $coursedata['id'] ?? 0;
$errors = $this->validate_enrolment_data($courseid, $this->enrolmentdata);

if (!empty($errors)) {
foreach ($errors as $key => $message) {
$this->error($key, $message);
}

return false;
if (!empty($errors)) {
foreach ($errors as $key => $message) {
$this->error($key, $message);
}

return false;
}

if (isset($this->rawdata['tags']) && strval($this->rawdata['tags']) !== '') {
Expand Down Expand Up @@ -913,6 +918,8 @@ public function proceed() {
* @return lang_string[] Errors keyed on error code
*/
protected function validate_enrolment_data(int $courseid, array $enrolmentdata): array {
global $DB;

// Nothing to validate.
if (empty($enrolmentdata)) {
return [];
Expand All @@ -924,45 +931,66 @@ protected function validate_enrolment_data(int $courseid, array $enrolmentdata):
$instances = enrol_get_instances($courseid, false);

foreach ($enrolmentdata as $method => $options) {
$plugin = $enrolmentplugins[$method];

// Find matching instances by enrolment method.
$methodinstances = array_filter($instances, static function(stdClass $instance) use ($method) {
return (strcmp($instance->enrol, $method) == 0);
});

if (!empty($options['delete'])) {
// Ensure user is able to delete the instances.
foreach ($methodinstances as $methodinstance) {
if (!$plugin->can_delete_instance($methodinstance)) {
$errors['errorcannotdeleteenrolment'] = new lang_string('errorcannotdeleteenrolment', 'tool_uploadcourse',
$plugin->get_instance_name($methodinstance));
if (isset($options['role'])) {
$role = $options['role'];
if ($courseid) {
if (!$this->validate_role_context($courseid, $role)) {
$errors['contextrolenotallowed'] = new lang_string('contextrolenotallowed', 'core_role', $role);

break;
}
}
} else if (!empty($options['disable'])) {
// Ensure user is able to toggle instance statuses.
foreach ($methodinstances as $methodinstance) {
if (!$plugin->can_hide_show_instance($methodinstance)) {
$errors['errorcannotdisableenrolment'] =
new lang_string('errorcannotdisableenrolment', 'tool_uploadcourse',
$plugin->get_instance_name($methodinstance));
} else {
// We can at least check that context level is correct while actual context not exist.
$roleid = $DB->get_field('role', 'id', ['shortname' => $role], MUST_EXIST);
if (!$this->validate_role_context_level($roleid)) {
$errors['contextrolenotallowed'] = new lang_string('contextrolenotallowed', 'core_role', $role);

break;
}
}
} else {
// Ensure user is able to create/update instance.
$methodinstance = empty($methodinstances) ? null : reset($methodinstances);
if ((empty($methodinstance) && !$plugin->can_add_instance($courseid)) ||
}

if ($courseid) {
$plugin = $enrolmentplugins[$method];

// Find matching instances by enrolment method.
$methodinstances = array_filter($instances, static function (stdClass $instance) use ($method) {
return (strcmp($instance->enrol, $method) == 0);
});

if (!empty($options['delete'])) {
// Ensure user is able to delete the instances.
foreach ($methodinstances as $methodinstance) {
if (!$plugin->can_delete_instance($methodinstance)) {
$errors['errorcannotdeleteenrolment'] = new lang_string('errorcannotdeleteenrolment',
'tool_uploadcourse', $plugin->get_instance_name($methodinstance));
break;
}
}
} else if (!empty($options['disable'])) {
// Ensure user is able to toggle instance statuses.
foreach ($methodinstances as $methodinstance) {
if (!$plugin->can_hide_show_instance($methodinstance)) {
$errors['errorcannotdisableenrolment'] =
new lang_string('errorcannotdisableenrolment', 'tool_uploadcourse',
$plugin->get_instance_name($methodinstance));

break;
}
}
} else {
// Ensure user is able to create/update instance.
$methodinstance = empty($methodinstances) ? null : reset($methodinstances);
if ((empty($methodinstance) && !$plugin->can_add_instance($courseid)) ||
(!empty($methodinstance) && !$plugin->can_edit_instance($methodinstance))) {

$errors['errorcannotcreateorupdateenrolment'] =
new lang_string('errorcannotcreateorupdateenrolment', 'tool_uploadcourse',
$plugin->get_instance_name($methodinstance));
$errors['errorcannotcreateorupdateenrolment'] =
new lang_string('errorcannotcreateorupdateenrolment', 'tool_uploadcourse',
$plugin->get_instance_name($methodinstance));

break;
break;
}
}
}
}
Expand Down Expand Up @@ -1079,8 +1107,15 @@ protected function process_enrolment_data($course) {
$instance->enrolenddate = $instance->enrolstartdate;
}

// Sort out the given role. This does not filter the roles allowed in the course.
// Sort out the given role.
if (isset($method['role'])) {
$role = $method['role'];
if (!$this->validate_role_context($course->id, $role)) {
$this->error('contextrolenotallowed',
new lang_string('contextrolenotallowed', 'core_role', $role));
break;
}

$roleids = tool_uploadcourse_helper::get_role_ids();
if (isset($roleids[$method['role']])) {
$instance->roleid = $roleids[$method['role']];
Expand All @@ -1093,6 +1128,41 @@ protected function process_enrolment_data($course) {
}
}

/**
* Check if role is allowed in course context
*
* @param int $courseid course context.
* @param string $role Role.
* @return bool
*/
protected function validate_role_context(int $courseid, string $role) : bool {
if (empty($this->assignableroles[$courseid])) {
$coursecontext = \context_course::instance($courseid);
$this->assignableroles[$courseid] = get_assignable_roles($coursecontext, ROLENAME_SHORT);
}
if (!in_array($role, $this->assignableroles[$courseid])) {
return false;
}
return true;
}

/**
* Check if role is allowed at this context level.
*
* @param int $roleid Role ID.
* @return bool
*/
protected function validate_role_context_level(int $roleid) : bool {
if (empty($this->contextlevels[$roleid])) {
$this->contextlevels[$roleid] = get_role_contextlevels($roleid);
}

if (!in_array(CONTEXT_COURSE, $this->contextlevels[$roleid])) {
return false;
}
return true;
}

/**
* Reset the current course.
*
Expand Down
5 changes: 5 additions & 0 deletions admin/tool/uploadcourse/classes/helper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,11 @@ public static function get_role_names($data, &$errors = array()) {
continue;
}
$rolenames['role_' . $rolesids[$matches[1]]] = $value;
} else if (preg_match('/^(.+)?_role$/', $field, $matches)) {
if (!isset($rolesids[$value])) {
$invalidroles[] = $value;
break;
}
}

}
Expand Down
4 changes: 4 additions & 0 deletions admin/tool/uploadcourse/classes/processor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@ public function execute($tracker = null) {

$data = array_merge($data, $course->get_data(), array('id' => $course->get_id()));
$tracker->output($this->linenb, true, $status, $data);
if ($course->has_errors()) {
$errors++;
$tracker->output($this->linenb, false, $course->get_errors(), $data);
}
} else {
$errors++;
$tracker->output($this->linenb, false, $course->get_errors(), $data);
Expand Down
25 changes: 25 additions & 0 deletions admin/tool/uploadcourse/tests/behat/create.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,28 @@ Feature: An admin can create courses using a CSV file
And I should see "Field 3: b"
And I should see "Field 4: Hello"
And I should see "Field 5: Some text"

@javascript
Scenario: Validation of role for uploaded courses
Given I navigate to "Users > Permissions > Define roles" in site administration
And I click on "Add a new role" "button"
And I click on "Continue" "button"
And I set the following fields to these values:
| Short name | notallowed |
| Custom full name | notallowed |
| contextlevel80 | 1 |
And I click on "Create this role" "button"
And I navigate to "Courses > Upload courses" in site administration
And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_role.csv" file to "File" filemanager
And I click on "Preview" "button"
And I should see "Invalid role names: notexist"
And I should see "Role notallowed not allowed in this context."
When I click on "Upload courses" "button"
And I should see "Course created"
And I should see "Courses total: 3"
And I should see "Courses created: 1"
And I should see "Courses errors: 2"
And I should see "Invalid role names: notexist"
And I should see "Role notallowed not allowed in this context."
And I am on site homepage
And I should see "coursez"
88 changes: 88 additions & 0 deletions admin/tool/uploadcourse/tests/course_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1561,6 +1561,94 @@ public function test_mess_with_frontpage() {
$this->assertArrayHasKey('cannotrenameshortnamealreadyinuse', $co->get_errors());
}

/**
* Test when role doesn't exist.
*
* @covers \tool_uploadcourse_course::prepare
*/
public function test_role_not_exist() {
$this->resetAfterTest();
$this->setAdminUser();

$mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
$updatemode = tool_uploadcourse_processor::UPDATE_NOTHING;

$upload = new tool_uploadcourse_course($mode, $updatemode, [
'category' => 1,
'fullname' => 'Testing',
'shortname' => 'T101',
'enrolment_1' => 'manual',
'enrolment_1_role' => 'notexist'
]);

$this->assertFalse($upload->prepare());
$this->assertArrayHasKey('invalidroles', $upload->get_errors());
}

/**
* Test when role not allowed in course context.
*
* @covers \tool_uploadcourse_course::proceed
*/
public function test_role_not_allowed() {
$this->resetAfterTest();
$this->setAdminUser();

$roleid = create_role('New student role', 'student2', 'New student description', 'student');
set_role_contextlevels($roleid, [CONTEXT_BLOCK]);

$mode = tool_uploadcourse_processor::MODE_CREATE_NEW;
$updatemode = tool_uploadcourse_processor::UPDATE_NOTHING;

$upload = new tool_uploadcourse_course($mode, $updatemode, [
'category' => 1,
'fullname' => 'Testing',
'shortname' => 'T101',
'enrolment_1' => 'manual',
'enrolment_1_role' => 'student2'
]);

$this->assertFalse($upload->prepare());
$this->assertArrayHasKey('contextrolenotallowed', $upload->get_errors());
}

/**
* Test when role is allowed.
*
* @covers \tool_uploadcourse_course::proceed
*/
public function test_role_allowed() {
global $DB;

$this->resetAfterTest();
$this->setAdminUser();

$mode = tool_uploadcourse_processor::MODE_UPDATE_ONLY;
$updatemode = tool_uploadcourse_processor::UPDATE_MISSING_WITH_DATA_OR_DEFAUTLS;

$course = $this->getDataGenerator()->create_course([
'shortname' => 'c1',
]);

$instances = enrol_get_instances($course->id, true);
$studentrole = $DB->get_record('role', ['shortname' => 'student']);
$teacherrole = $DB->get_record('role', ['shortname' => 'teacher']);
$instance = reset($instances);
$this->assertEquals($studentrole->id, $instance->roleid);

$upload = new tool_uploadcourse_course($mode, $updatemode, [
'shortname' => 'c1',
'enrolment_1' => 'manual',
'enrolment_1_role' => 'teacher'
]);

$this->assertTrue($upload->prepare());
$upload->proceed();
$instances = enrol_get_instances($course->id, true);
$instance = reset($instances);
$this->assertEquals($teacherrole->id, $instance->roleid);
}

/**
* Get custom field plugin generator
*
Expand Down
4 changes: 4 additions & 0 deletions admin/tool/uploadcourse/tests/fixtures/enrolment_role.csv
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
shortname,fullname,category,enrolment_1,enrolment_1_role
CX,coursex,1,manual,notexist
CY,coursey,1,manual,notallowed
CZ,coursez,1,manual,student
1 change: 1 addition & 0 deletions lang/en/role.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@
$string['contentbank:upload'] = 'Upload new content to the content bank';
$string['contentbank:useeditor'] = 'Create or edit content using a content type editor';
$string['context'] = 'Context';
$string['contextrolenotallowed'] = 'Role <b>{$a}</b> not allowed in this context.';
$string['course:activityvisibility'] = 'Hide/show activities';
$string['course:bulkmessaging'] = 'Send a message to many people';
$string['course:create'] = 'Create courses';
Expand Down

0 comments on commit 82e3a9c

Please sign in to comment.