MDL-80599 core: Refactor enrol methods

Move update instance logic to its own method and cover with tests.
Also update enrol plugins to work woth new methods.
This commit is contained in:
Ilya Tregubov 2024-02-05 15:08:46 +08:00
parent 78229db6a1
commit 56fb55683a
10 changed files with 173 additions and 54 deletions

View File

@ -1129,34 +1129,7 @@ class tool_uploadcourse_course {
break;
}
// Now update values.
$modifiedinstance = $instance;
// Sort out the start, end and date.
$modifiedinstance->enrolstartdate = (isset($method['startdate']) ? strtotime($method['startdate']) : 0);
$modifiedinstance->enrolenddate = (isset($method['enddate']) ? strtotime($method['enddate']) : 0);
// Is the enrolment period set?
if (isset($method['enrolperiod']) && !empty($method['enrolperiod'])) {
if (preg_match('/^\d+$/', $method['enrolperiod'])) {
$method['enrolperiod'] = (int)$method['enrolperiod'];
} else {
// Try and convert period to seconds.
$method['enrolperiod'] = strtotime('1970-01-01 GMT + ' . $method['enrolperiod']);
}
$modifiedinstance->enrolperiod = $method['enrolperiod'];
}
if ($instance->enrolstartdate > 0 && isset($method['enrolperiod'])) {
$modifiedinstance->enrolenddate = $instance->enrolstartdate + $method['enrolperiod'];
}
if ($instance->enrolenddate > 0) {
$modifiedinstance->enrolperiod = $instance->enrolenddate - $instance->enrolstartdate;
}
if ($instance->enrolenddate < $instance->enrolstartdate) {
$modifiedinstance->enrolenddate = $instance->enrolstartdate;
}
// Sort out the given role.
// Validate role context again since course is created.
if (isset($method['role']) || isset($method['roleid'])) {
if (isset($method['role'])) {
$role = $method['role'];
@ -1166,22 +1139,14 @@ class tool_uploadcourse_course {
$role = $DB->get_field('role', 'shortname', ['id' => $roleid], MUST_EXIST);
}
if (!$this->validate_role_context($course->id, $roleid)) {
$this->error('contextrolenotallowed',
new lang_string('contextrolenotallowed', 'core_role', $role));
$this->error('contextrolenotallowed', new lang_string('contextrolenotallowed', 'core_role', $role));
break;
}
$roleids = tool_uploadcourse_helper::get_role_ids();
if (in_array($roleid, $roleids)) {
$modifiedinstance->roleid = $roleid;
}
}
// Sort out custom instance name.
if (isset($method['name'])) {
$modifiedinstance->name = $method['name'];
}
// Now update values.
// Sort out plugin specific fields.
$modifiedinstance = $plugin->update_enrol_plugin_data($course->id, $method, $instance);
$plugin->update_instance($instance, $modifiedinstance);
} else {
foreach ($errors as $key => $message) {

View File

@ -31,7 +31,7 @@ Feature: An admin can create courses with cohort enrolments using a CSV file
| enrol_plugins_enabled | manual,guest,self |
And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_cohort.csv" file to "File" filemanager
When I click on "Preview" "button"
Then I should see "Cohort sync plugin is disabled"
Then I should see "Cohort sync enrol plugin is disabled"
@javascript
Scenario: Validation of cohorts for uploaded courses

View File

@ -0,0 +1 @@
plugindisabled,enrol_cohort

View File

@ -29,9 +29,11 @@ $string['cohort:unenrol'] = 'Unenrol suspended users';
$string['defaultgroupnametext'] = '{$a->name} cohort {$a->increment}';
$string['enrolcohortsynctask'] = 'Cohort enrolment sync task';
$string['instanceexists'] = 'Cohort is already synchronised with selected role';
$string['plugindisabled'] = 'Cohort sync plugin is disabled';
$string['pluginname'] = 'Cohort sync';
$string['pluginname_desc'] = 'Cohort enrolment plugin synchronises cohort members with course participants.';
$string['status'] = 'Active';
$string['creategroup'] = 'Create new group';
$string['privacy:metadata:core_group'] = 'Enrol cohort plugin can create a new group or use an existing group to add all the members of the cohort.';
// Deprecated since Moodle 4.5.
$string['plugindisabled'] = 'Cohort sync plugin is disabled';

View File

@ -515,11 +515,7 @@ class enrol_cohort_plugin extends enrol_plugin {
public function validate_enrol_plugin_data(array $enrolmentdata, ?int $courseid = null): array {
global $DB;
$errors = [];
if (!enrol_is_enabled('cohort')) {
$errors['plugindisabled'] =
new lang_string('plugindisabled', 'enrol_cohort');
}
$errors = parent::validate_enrol_plugin_data($enrolmentdata, $courseid);
if (isset($enrolmentdata['addtogroup'])) {
$addtogroup = $enrolmentdata['addtogroup'];

View File

@ -393,11 +393,7 @@ class enrol_meta_plugin extends enrol_plugin {
public function validate_enrol_plugin_data(array $enrolmentdata, ?int $courseid = null): array {
global $DB;
$errors = [];
if (!enrol_is_enabled('meta')) {
$errors['plugindisabled'] =
new lang_string('plugindisabled', 'plugin');
}
$errors = parent::validate_enrol_plugin_data($enrolmentdata, $courseid);
if (isset($enrolmentdata['addtogroup'])) {
$addtogroup = $enrolmentdata['addtogroup'];

View File

@ -23,6 +23,8 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
use core\plugininfo\enrol;
defined('MOODLE_INTERNAL') || die();
@ -1733,4 +1735,96 @@ class enrollib_test extends advanced_testcase {
$this->setUser($user2);
$this->assertFalse(enrol_selfenrol_available($course->id));
}
/**
* Test the behaviour of validate_enrol_plugin_data().
*
* @covers ::validate_enrol_plugin_data
*/
public function test_validate_enrol_plugin_data(): void {
$this->resetAfterTest();
// Plugin is disabled in system.
enrol::enable_plugin('manual', false);
$manualplugin = enrol_get_plugin('manual');
$enrolmentdata = [];
$errors = $manualplugin->validate_enrol_plugin_data($enrolmentdata);
$this->assertArrayHasKey('plugindisabled', $errors);
$this->assertArrayNotHasKey('errorunsupportedmethod', $errors);
$categoryplugin = enrol_get_plugin('category');
$errors = $categoryplugin->validate_enrol_plugin_data($enrolmentdata);
$this->assertArrayHasKey('errorunsupportedmethod', $errors);
}
/**
* Test the behaviour of update_enrol_plugin_data().
*
* @covers ::update_enrol_plugin_data
*/
public function test_update_enrol_plugin_data(): void {
global $DB;
$this->resetAfterTest();
$manualplugin = enrol_get_plugin('manual');
$admin = get_admin();
$this->setUser($admin);
$enrolmentdata = [];
$cat = $this->getDataGenerator()->create_category();
$course = $this->getDataGenerator()->create_course(['category' => $cat->id, 'shortname' => 'ANON']);
$instance = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => 'manual'], '*', MUST_EXIST);
$teacherroleid = $DB->get_field('role', 'id', ['shortname' => 'teacher']);
$editingteacherroleid = $DB->get_field('role', 'id', ['shortname' => 'editingteacher']);
$enrolmentdata['startdate'] = '3 Feb 2024';
$enrolmentdata['enddate'] = '4 Feb 2024';
$enrolmentdata['role'] = 'teacher';
$enrolmentdata['name'] = 'testinstance';
$expectedinstance = $instance;
$expectedinstance->enrolstartdate = strtotime($enrolmentdata['startdate']);
$expectedinstance->enrolenddate = strtotime($enrolmentdata['enddate']);
$expectedinstance->role = $teacherroleid;
$expectedinstance->name = $enrolmentdata['name'];
$expectedinstance->enrolperiod = $expectedinstance->enrolenddate - $expectedinstance->enrolstartdate;
$modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance);
$this->assertEquals($expectedinstance, $modifiedinstance);
$enrolmentdata['roleid'] = $editingteacherroleid;
unset($enrolmentdata['startdate']);
unset($enrolmentdata['enddate']);
unset($enrolmentdata['role']);
$enrolmentdata['enrolperiod'] = $modifiedinstance->enrolperiod++;
$expectedinstance->roleid = $editingteacherroleid;
$expectedinstance->enrolstartdate = 0;
$expectedinstance->enrolenddate = 0;
$expectedinstance->enrolperiod++;
$modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance);
$this->assertEquals($expectedinstance, $modifiedinstance);
$enrolmentdata['startdate'] = '3 Feb 2024';
$enrolmentdata['enrolperiod'] = 3600;
$expectedinstance->enrolstartdate = strtotime($enrolmentdata['startdate']);
$expectedinstance->enrolperiod = $enrolmentdata['enrolperiod'];
$expectedinstance->enrolenddate = $expectedinstance->enrolstartdate + $enrolmentdata['enrolperiod'];
$modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance);
$this->assertEquals($expectedinstance, $modifiedinstance);
$enrolmentdata['enddate'] = '5 Feb 2024';
unset($enrolmentdata['enrolperiod']);
$expectedinstance->enrolenddate = strtotime($enrolmentdata['startdate']);
$expectedinstance->enrolperiod = $expectedinstance->enrolenddate - $expectedinstance->enrolstartdate;
$modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance);
$this->assertEquals($expectedinstance, $modifiedinstance);
$enrolmentdata['enrolperiod'] = '2hours';
$expectedinstance->enrolperiod = 7200;
$expectedinstance->enrolenddate = $expectedinstance->enrolstartdate + $expectedinstance->enrolperiod;
$modifiedinstance = $manualplugin->update_enrol_plugin_data($course->id, $enrolmentdata, $instance);
$this->assertEquals($expectedinstance, $modifiedinstance);
}
}

View File

@ -132,6 +132,7 @@ $string['periodend'] = 'until {$a}';
$string['periodnone'] = 'enrolled {$a}';
$string['periodstart'] = 'from {$a}';
$string['periodstartend'] = 'from {$a->start} until {$a->end}';
$string['plugindisabled'] = '{$a} enrol plugin is disabled';
$string['recovergrades'] = 'Recover user\'s old grades if possible';
$string['rolefromthiscourse'] = '{$a->role} (Assigned in this course)';
$string['rolefrommetacourse'] = '{$a->role} (Inherited from parent course)';

View File

@ -3536,7 +3536,12 @@ abstract class enrol_plugin {
$errors['errorunsupportedmethod'] =
new lang_string('errorunsupportedmethod', 'tool_uploadcourse',
get_class($this));
} else {
$plugin = $this->get_name();
if (!enrol_is_enabled($plugin)) {
$pluginname = get_string('pluginname', 'enrol_' . $plugin);
$errors['plugindisabled'] = new lang_string('plugindisabled', 'enrol', $pluginname);
}
}
return $errors;
}
@ -3728,4 +3733,56 @@ abstract class enrol_plugin {
message_send($message);
}
/**
* Updates enrol plugin instance with provided data.
* @param int $courseid Course ID.
* @param array $enrolmentdata enrolment data.
* @param stdClass $instance Instance to update.
*
* @return stdClass updated instance
*/
public function update_enrol_plugin_data(int $courseid, array $enrolmentdata, stdClass $instance): stdClass {
global $DB;
// Sort out the start, end and date.
$instance->enrolstartdate = (isset($enrolmentdata['startdate']) ? strtotime($enrolmentdata['startdate']) : 0);
$instance->enrolenddate = (isset($enrolmentdata['enddate']) ? strtotime($enrolmentdata['enddate']) : 0);
// Is the enrolment period set?
if (!empty($enrolmentdata['enrolperiod'])) {
if (preg_match('/^\d+$/', $enrolmentdata['enrolperiod'])) {
$enrolmentdata['enrolperiod'] = (int)$enrolmentdata['enrolperiod'];
} else {
// Try and convert period to seconds.
$enrolmentdata['enrolperiod'] = strtotime('1970-01-01 GMT + ' . $enrolmentdata['enrolperiod']);
}
$instance->enrolperiod = $enrolmentdata['enrolperiod'];
}
if ($instance->enrolstartdate > 0 && isset($enrolmentdata['enrolperiod'])) {
$instance->enrolenddate = $instance->enrolstartdate + $enrolmentdata['enrolperiod'];
}
if ($instance->enrolenddate > 0) {
$instance->enrolperiod = $instance->enrolenddate - $instance->enrolstartdate;
}
if ($instance->enrolenddate < $instance->enrolstartdate) {
$instance->enrolenddate = $instance->enrolstartdate;
}
// Sort out the given role.
if (isset($enrolmentdata['role']) || isset($enrolmentdata['roleid'])) {
if (isset($enrolmentdata['role'])) {
$roleid = $DB->get_field('role', 'id', ['shortname' => $enrolmentdata['role']], MUST_EXIST);
} else {
$roleid = $enrolmentdata['roleid'];
}
$instance->roleid = $roleid;
}
// Sort out custom instance name.
if (isset($enrolmentdata['name'])) {
$instance->name = $enrolmentdata['name'];
}
return $instance;
}
}

View File

@ -4551,31 +4551,38 @@ function get_complete_user_data($field, $value, $mnethostid = null, $throwexcept
* @param string $password the password to be checked against the password policy
* @param string $errmsg the error message to display when the password doesn't comply with the policy.
* @param stdClass $user the user object to perform password validation against. Defaults to null if not provided.
* @param ?array $errorsarray array of errors with string identifiers.
*
* @return bool true if the password is valid according to the policy. false otherwise.
*/
function check_password_policy($password, &$errmsg, $user = null) {
function check_password_policy($password, &$errmsg, $user = null, ?array &$errorsarray = null) {
global $CFG;
if (!empty($CFG->passwordpolicy) && !isguestuser($user)) {
$errmsg = '';
if (core_text::strlen($password) < $CFG->minpasswordlength) {
$errmsg .= '<div>'. get_string('errorminpasswordlength', 'auth', $CFG->minpasswordlength) .'</div>';
$errorsarray = ($errorsarray !== null) ? [...$errorsarray, 'errorminpasswordlength'] : null;
}
if (preg_match_all('/[[:digit:]]/u', $password, $matches) < $CFG->minpassworddigits) {
$errmsg .= '<div>'. get_string('errorminpassworddigits', 'auth', $CFG->minpassworddigits) .'</div>';
$errorsarray = ($errorsarray !== null) ? [...$errorsarray, 'errorminpassworddigits'] : null;
}
if (preg_match_all('/[[:lower:]]/u', $password, $matches) < $CFG->minpasswordlower) {
$errmsg .= '<div>'. get_string('errorminpasswordlower', 'auth', $CFG->minpasswordlower) .'</div>';
$errorsarray = ($errorsarray !== null) ? [...$errorsarray, 'errorminpasswordlower'] : null;
}
if (preg_match_all('/[[:upper:]]/u', $password, $matches) < $CFG->minpasswordupper) {
$errmsg .= '<div>'. get_string('errorminpasswordupper', 'auth', $CFG->minpasswordupper) .'</div>';
$errorsarray = ($errorsarray !== null) ? [...$errorsarray, 'errorminpasswordupper'] : null;
}
if (preg_match_all('/[^[:upper:][:lower:][:digit:]]/u', $password, $matches) < $CFG->minpasswordnonalphanum) {
$errmsg .= '<div>'. get_string('errorminpasswordnonalphanum', 'auth', $CFG->minpasswordnonalphanum) .'</div>';
$errorsarray = ($errorsarray !== null) ? [...$errorsarray, 'errorminpasswordnonalphanum'] : null;
}
if (!check_consecutive_identical_characters($password, $CFG->maxconsecutiveidentchars)) {
$errmsg .= '<div>'. get_string('errormaxconsecutiveidentchars', 'auth', $CFG->maxconsecutiveidentchars) .'</div>';
$errorsarray = ($errorsarray !== null) ? [...$errorsarray, 'errormaxconsecutiveidentchars'] : null;
}
// Fire any additional password policy functions from plugins.