-
Bug
-
Resolution: Fixed
-
Minor
-
4.4
-
MOODLE_404_STABLE
-
MOODLE_404_STABLE
-
MDL-80875-main -
-
3
-
HQ 2024 Sprint I1.3 Moppies
- Try to upload a course with this csv:
shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role
C1,Course 1,CAT1,self,student
- Go to created course and delete self enrolment method
- Try to upload same csv again (set to update existing course, use csv data only)
- Observe the exception
Undefined array key "expirynotify"
This function looks wrong:
public function validate_plugin_data_context(array $enrolmentdata, ?int $courseid = null) : ?lang_string { |
if ($courseid) { |
$enrolmentdata += ['courseid' => $courseid, 'id' => 0, 'status' => ENROL_INSTANCE_ENABLED]; |
$instance = (object)[
|
'id' => null, |
'courseid' => $courseid, |
'status' => $enrolmentdata['status'], |
'type' => $this->get_name(), |
];
|
$formerrors = $this->edit_instance_validation($enrolmentdata, [], $instance, context_course::instance($courseid)); |
if (!empty($formerrors)) { |
$errors = array_map(fn($key) => "{$key}: {$formerrors[$key]}", array_keys($formerrors)); |
return new lang_string('errorcannotcreateorupdateenrolment', 'tool_uploadcourse', $errors); |
}
|
}
|
return null; |
}
|
It calls edit_instance_validation inside, but those validation will be too aggressive. Its fine to omit some data in csv when uploading. For example for self enrolment you can skip setting expirynotify (some default will be used), but then inside edit_instance_validation it will fail since it will try since expirynotify is not set:
if ($data['expirynotify'] > 0 and $data['expirythreshold'] < 86400) { |
$errors['expirythreshold'] = get_string('errorthresholdlow', 'core_enrol'); |
}
|
|
similarly to other plugins
Also function name suggest that we are checking smth in some context. But default case doesnt' deal with context check at all. So using generic checks here feels out of place