diff --git a/admin/tool/uploadcourse/classes/course.php b/admin/tool/uploadcourse/classes/course.php index 4a785114232..655d75b87e1 100644 --- a/admin/tool/uploadcourse/classes/course.php +++ b/admin/tool/uploadcourse/classes/course.php @@ -1022,16 +1022,10 @@ class tool_uploadcourse_course { } $enrolmentplugins = tool_uploadcourse_helper::get_enrolment_plugins(); - $instances = enrol_get_instances($course->id, false); foreach ($enrolmentdata as $enrolmethod => $method) { - $instance = null; - foreach ($instances as $i) { - if ($i->enrol == $enrolmethod) { - $instance = $i; - break; - } - } + $plugin = $enrolmentplugins[$enrolmethod]; + $instance = $plugin->find_instance($method, $course->id); $todelete = isset($method['delete']) && $method['delete']; $todisable = isset($method['disable']) && $method['disable']; diff --git a/admin/tool/uploadcourse/tests/behat/cohorts.feature b/admin/tool/uploadcourse/tests/behat/cohorts.feature index 4edf6c8a9c0..4441ed60995 100644 --- a/admin/tool/uploadcourse/tests/behat/cohorts.feature +++ b/admin/tool/uploadcourse/tests/behat/cohorts.feature @@ -38,14 +38,15 @@ Feature: An admin can create courses with cohort enrolments using a CSV file Given I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_cohort.csv" file to "File" filemanager And I click on "Preview" "button" And I should see "Unknown cohort (Not exist)!" - And I should see "Cohort Cohort 3 not allowed in this context." + And I should see "Cohort CV3 not allowed in this context." When I click on "Upload courses" "button" And I should see "Unknown cohort (Not exist)!" - And I should see "Cohort Cohort 3 not allowed in this context." - And I should see "Cohort Cohort 4 not allowed in this context." + And I should see "Cohort CV3 not allowed in this context." + And I should see "Cohort CV4 not allowed in this context." + And I should see "Invalid role names: student1" And I should see "Courses created: 2" And I should see "Courses updated: 0" - And I should see "Courses errors: 3" + And I should see "Courses errors: 4" And I am on the "Course 1" "enrolment methods" page Then I should not see "Cohort sync (Cohort 3 - Student)" And I am on the "Course 2" "enrolment methods" page @@ -54,6 +55,13 @@ Feature: An admin can create courses with cohort enrolments using a CSV file And I should see "Cohort sync (Cohort 5 - Student)" And I click on "Edit" "link" in the "Cohort 5" "table_row" And the field "Add to group" matches value "None" + And I navigate to "Courses > Upload courses" in site administration + And I set the field "Upload mode" to "Create new courses, or update existing ones" + And I set the field "Update mode" to "Update with CSV data only" + And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_missing_fields.csv" file to "File" filemanager + And I click on "Preview" "button" + And I should see "Missing value for mandatory fields: cohortidnumber, role" + And "Upload courses" "button" should exist @javascript Scenario: Validation of groups for uploaded courses with cohort enrolments @@ -103,3 +111,18 @@ Feature: An admin can create courses with cohort enrolments using a CSV file And the field "Add to group" matches value "group1" And I am on the "Course 1" "groups" page And I should see "group1" + + @javascript + Scenario: Upload multiple enrolment methods of same type to the same course + Given I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_multiple.csv" file to "File" filemanager + And I click on "Preview" "button" + When I click on "Upload courses" "button" + And I should see "Courses updated: 2" + And I am on the "Course 1" "enrolment methods" page + Then I should see "Cohort sync (Cohort 1 - Student)" + And I should see "Cohort sync (Cohort 4 - Non-editing teacher)" + And I click on "Edit" "link" in the "Cohort 1" "table_row" + And the field "Assign role" matches value "Student" + And I press "Cancel" + And I click on "Edit" "link" in the "Cohort 4" "table_row" + And the field "Assign role" matches value "Non-editing teacher" diff --git a/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort.csv b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort.csv index 929130d6d35..351de24872d 100644 --- a/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort.csv +++ b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort.csv @@ -1,5 +1,6 @@ -shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_cohortname +shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_cohortidnumber C1,Course 1,CAT1,cohort,student,Not exist -C1,Course 1,CAT1,cohort,student,Cohort 3 -C2,Course 2,CAT2,cohort,student,Cohort 4 -C3,Course 3,CAT1,cohort,student,Cohort 5 +C1,Course 1,CAT1,cohort,student,CV3 +C2,Course 2,CAT2,cohort,student,CV4 +C3,Course 3,CAT1,cohort,student,CV5 +C4,Course 3,CAT1,cohort,student1,CV6 diff --git a/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_addtogroup.csv b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_addtogroup.csv index d5a60f95f49..ea7dbaf55de 100644 --- a/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_addtogroup.csv +++ b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_addtogroup.csv @@ -1,3 +1,3 @@ -shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_cohortname,enrolment_1_addtogroup -C2,Course 2,CAT2,cohort,student,Cohort 2,1 -C3,Course 3,CAT1,cohort,student,Cohort 1,0 +shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_cohortidnumber,enrolment_1_addtogroup +C2,Course 2,CAT2,cohort,student,CV2,1 +C3,Course 3,CAT1,cohort,student,CV1,0 diff --git a/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_addtogroup_groupname.csv b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_addtogroup_groupname.csv index e9b289fc63e..13663c3faae 100644 --- a/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_addtogroup_groupname.csv +++ b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_addtogroup_groupname.csv @@ -1,2 +1,2 @@ -shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_cohortname,enrolment_1_addtogroup,enrolment_1_groupname -C1,Course 1,CAT1,cohort,student,Cohort 1,0,group1 +shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_cohortidnumber,enrolment_1_addtogroup,enrolment_1_groupname +C1,Course 1,CAT1,cohort,student,CV1,0,group1 diff --git a/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_groups.csv b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_groups.csv index 615ab7690f0..49308f9d096 100644 --- a/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_groups.csv +++ b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_groups.csv @@ -1,3 +1,3 @@ -shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_cohortname,enrolment_1_groupname -C1,Course 1,CAT1,cohort,student,Cohort 1,notexist -C1,Course 1,CAT1,cohort,student,Cohort 1,group1 +shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_cohortidnumber,enrolment_1_groupname +C1,Course 1,CAT1,cohort,student,CV1,notexist +C1,Course 1,CAT1,cohort,student,CV1,group1 diff --git a/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_missing_fields.csv b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_missing_fields.csv new file mode 100644 index 00000000000..2a8fce6dae2 --- /dev/null +++ b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_missing_fields.csv @@ -0,0 +1,2 @@ +shortname,fullname,category_idnumber,enrolment_1 +C1,Course 1,CAT1,cohort diff --git a/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_multiple.csv b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_multiple.csv new file mode 100644 index 00000000000..63dae29775e --- /dev/null +++ b/admin/tool/uploadcourse/tests/fixtures/enrolment_cohort_multiple.csv @@ -0,0 +1,3 @@ +shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_1_cohortidnumber +C1,Course 1,CAT1,cohort,student,CV1 +C1,Course 1,CAT1,cohort,teacher,CV4 diff --git a/enrol/cohort/lib.php b/enrol/cohort/lib.php index a24701a96d8..6c2859bb074 100644 --- a/enrol/cohort/lib.php +++ b/enrol/cohort/lib.php @@ -550,20 +550,37 @@ class enrol_cohort_plugin extends enrol_plugin { } } - if (!isset($enrolmentdata['cohortname'])) { - $errors['missingmandatoryfields'] = - new lang_string('missingmandatoryfields', 'tool_uploadcourse', - 'cohortname'); + if (!isset($enrolmentdata['cohortidnumber'])) { + $missingmandatoryfields = 'cohortidnumber'; } else { - $cohortname = $enrolmentdata['cohortname']; - // Cohort name is not unique. - $cohortid = $DB->get_field('cohort', 'MIN(id)', ['name' => $cohortname]); + $cohortidnumber = $enrolmentdata['cohortidnumber']; + // Cohort idnumber is unique. + $cohortid = $DB->get_field('cohort', 'id', ['idnumber' => $cohortidnumber]); if (!$cohortid) { $errors['unknowncohort'] = - new lang_string('unknowncohort', 'cohort', $cohortname); + new lang_string('unknowncohort', 'cohort', $cohortidnumber); } } + + if (!isset($enrolmentdata['role'])) { + // We require role since we need it to identify enrol instance. + if (isset($missingmandatoryfields)) { + $missingmandatoryfields .= ', role'; + } else { + $missingmandatoryfields = 'role'; + } + $errors['missingmandatoryfields'] = + new lang_string('missingmandatoryfields', 'tool_uploadcourse', + $missingmandatoryfields); + } else { + $roleid = $DB->get_field('role', 'id', ['shortname' => $enrolmentdata['role']]); + if (!$roleid) { + $errors['unknownrole'] = + new lang_string('unknownrole', 'error', s($enrolmentdata['role'])); + } + } + return $errors; } @@ -577,9 +594,11 @@ class enrol_cohort_plugin extends enrol_plugin { public function fill_enrol_custom_fields(array $enrolmentdata, int $courseid) : array { global $DB; - // Cohort name is not unique. - $enrolmentdata['customint1'] = - $DB->get_field('cohort', 'MIN(id)', ['name' => $enrolmentdata['cohortname']]); + if (isset($enrolmentdata['cohortidnumber'])) { + // Cohort idnumber is unique. + $enrolmentdata['customint1'] = + $DB->get_field('cohort', 'id', ['idnumber' => $enrolmentdata['cohortidnumber']]); + } if (isset($enrolmentdata['addtogroup'])) { if ($enrolmentdata['addtogroup'] == COHORT_NOGROUP) { @@ -602,10 +621,12 @@ class enrol_cohort_plugin extends enrol_plugin { */ public function validate_plugin_data_context(array $enrolmentdata, ?int $courseid = null) : ?lang_string { $error = null; - $cohortid = $enrolmentdata['customint1']; - $coursecontext = \context_course::instance($courseid); - if (!cohort_get_cohort($cohortid, $coursecontext)) { - $error = new lang_string('contextcohortnotallowed', 'cohort', $enrolmentdata['cohortname']); + if (isset($enrolmentdata['customint1'])) { + $cohortid = $enrolmentdata['customint1']; + $coursecontext = \context_course::instance($courseid); + if (!cohort_get_cohort($cohortid, $coursecontext)) { + $error = new lang_string('contextcohortnotallowed', 'cohort', $enrolmentdata['cohortidnumber']); + } } return $error; } @@ -634,6 +655,33 @@ class enrol_cohort_plugin extends enrol_plugin { public function is_csv_upload_supported(): bool { return true; } + + /** + * Finds matching instances for a given course. + * + * @param array $enrolmentdata enrolment data. + * @param int $courseid Course ID. + * @return stdClass|null Matching instance + */ + public function find_instance(array $enrolmentdata, int $courseid) : ?stdClass { + global $DB; + $instances = enrol_get_instances($courseid, false); + + $instance = null; + if (isset($enrolmentdata['cohortidnumber']) && isset($enrolmentdata['role'])) { + $cohortid = $DB->get_field('cohort', 'id', ['idnumber' => $enrolmentdata['cohortidnumber']]); + $roleid = $DB->get_field('role', 'id', ['shortname' => $enrolmentdata['role']]); + if ($cohortid && $roleid) { + foreach ($instances as $i) { + if ($i->enrol == 'cohort' && $i->customint1 == $cohortid && $i->roleid == $roleid) { + $instance = $i; + break; + } + } + } + } + return $instance; + } } /** diff --git a/enrol/cohort/tests/lib_test.php b/enrol/cohort/tests/lib_test.php index 6968f5107e2..238f4ef81bf 100644 --- a/enrol/cohort/tests/lib_test.php +++ b/enrol/cohort/tests/lib_test.php @@ -226,12 +226,18 @@ class lib_test extends \advanced_testcase { $course = $this->getDataGenerator()->create_course(['category' => $cat1->id, 'shortname' => 'ANON']); - $cohort1 = $this->getDataGenerator()->create_cohort(['contextid' => \context_coursecat::instance($cat1->id)->id]); - $cohort2 = $this->getDataGenerator()->create_cohort(['contextid' => \context_coursecat::instance($cat2->id)->id]); + $cohort1 = $this->getDataGenerator()->create_cohort([ + 'contextid' => \context_coursecat::instance($cat1->id)->id, + 'idnumber' => 'one', + ]); + $cohort2 = $this->getDataGenerator()->create_cohort([ + 'contextid' => \context_coursecat::instance($cat2->id)->id, + 'idnumber' => 'two', + ]); $enrolmentdata = [ 'customint1' => $cohort2->id, - 'cohortname' => $cohort2->name, + 'cohortidnumber' => $cohort2->idnumber, ]; $error = $cohortplugin->validate_plugin_data_context($enrolmentdata, $course->id); $this->assertInstanceOf('lang_string', $error); @@ -239,7 +245,7 @@ class lib_test extends \advanced_testcase { $enrolmentdata = [ 'customint1' => $cohort1->id, - 'cohortname' => $cohort1->name, + 'cohortidnumber' => $cohort1->idnumber, ]; $error = $cohortplugin->validate_plugin_data_context($enrolmentdata, $course->id); $this->assertNull($error); @@ -257,22 +263,25 @@ class lib_test extends \advanced_testcase { $cat = $this->getDataGenerator()->create_category(); $course = $this->getDataGenerator()->create_course(['category' => $cat->id, 'shortname' => 'ANON']); - $cohort = $this->getDataGenerator()->create_cohort(['contextid' => \context_coursecat::instance($cat->id)->id]); + $cohort = $this->getDataGenerator()->create_cohort([ + 'contextid' => \context_coursecat::instance($cat->id)->id, + 'idnumber' => 'one', + ]); $group = $this->getDataGenerator()->create_group(['courseid' => $course->id]); - $enrolmentdata['cohortname'] = $cohort->name; + $enrolmentdata['cohortidnumber'] = $cohort->idnumber; $enrolmentdata = $cohortplugin->fill_enrol_custom_fields($enrolmentdata, $course->id); $this->assertArrayHasKey('customint1', $enrolmentdata); $this->assertEquals($cohort->id, $enrolmentdata['customint1']); $this->assertArrayNotHasKey('customint2', $enrolmentdata); - $enrolmentdata['cohortname'] = 'notexist'; + $enrolmentdata['cohortidnumber'] = 'notexist'; $enrolmentdata = $cohortplugin->fill_enrol_custom_fields($enrolmentdata, $course->id); $this->assertArrayHasKey('customint1', $enrolmentdata); - $this->assertNull($enrolmentdata['customint1']); + $this->assertFalse($enrolmentdata['customint1']); $this->assertArrayNotHasKey('customint2', $enrolmentdata); - $enrolmentdata['cohortname'] = $cohort->name; + $enrolmentdata['cohortidnumber'] = $cohort->idnumber; $enrolmentdata['addtogroup'] = COHORT_NOGROUP; $enrolmentdata = $cohortplugin->fill_enrol_custom_fields($enrolmentdata, $course->id); @@ -313,8 +322,14 @@ class lib_test extends \advanced_testcase { $group1 = $this->getDataGenerator()->create_group(['courseid' => $course->id, 'name' => 'Group 1']); - $cohort1 = $this->getDataGenerator()->create_cohort(['contextid' => \context_coursecat::instance($cat1->id)->id]); - $cohort2 = $this->getDataGenerator()->create_cohort(['contextid' => \context_coursecat::instance($cat2->id)->id]); + $cohort1 = $this->getDataGenerator()->create_cohort([ + 'contextid' => \context_coursecat::instance($cat1->id)->id, + 'idnumber' => 'one', + ]); + $cohort2 = $this->getDataGenerator()->create_cohort([ + 'contextid' => \context_coursecat::instance($cat2->id)->id, + 'idnumber' => 'two', + ]); enrol::enable_plugin('cohort', false); @@ -328,13 +343,14 @@ class lib_test extends \advanced_testcase { enrol::enable_plugin('cohort', true); - // Unknown cohort name. - $enrolmentdata['cohortname'] = 'test'; + // Unknown cohort idnumber and missing role. + $enrolmentdata['cohortidnumber'] = 'test'; $errors = $cohortplugin->validate_enrol_plugin_data($enrolmentdata); + $this->assertArrayHasKey('missingmandatoryfields', $errors); $this->assertArrayHasKey('unknowncohort', $errors); // Non-valid 'addtogroup' option. - $enrolmentdata['cohortname'] = $cohort1->name; + $enrolmentdata['cohortidnumber'] = $cohort1->idnumber; $enrolmentdata['addtogroup'] = 2; $errors = $cohortplugin->validate_enrol_plugin_data($enrolmentdata, $course->id); $this->assertArrayHasKey('erroraddtogroup', $errors); @@ -346,7 +362,7 @@ class lib_test extends \advanced_testcase { $this->assertArrayHasKey('erroraddtogroupgroupname', $errors); // Cohort is not allowed on a given category context. - $enrolmentdata['cohortname'] = $cohort2->name; + $enrolmentdata['cohortidnumber'] = $cohort2->idnumber; $errors = $cohortplugin->validate_enrol_plugin_data($enrolmentdata, $course->id); $this->assertArrayHasKey('contextnotallowed', $errors); @@ -355,8 +371,14 @@ class lib_test extends \advanced_testcase { $errors = $cohortplugin->validate_enrol_plugin_data($enrolmentdata, $course->id); $this->assertArrayHasKey('errorinvalidgroup', $errors); + // Unknown role. + $enrolmentdata['role'] = 'test'; + $errors = $cohortplugin->validate_enrol_plugin_data($enrolmentdata, $course->id); + $this->assertArrayHasKey('unknownrole', $errors); + // Valid data when trying to create a group. - $enrolmentdata['cohortname'] = $cohort1->name; + $enrolmentdata['cohortidnumber'] = $cohort1->idnumber; + $enrolmentdata['role'] = 'student'; $enrolmentdata['addtogroup'] = 1; unset($enrolmentdata['groupname']); $errors = $cohortplugin->validate_enrol_plugin_data($enrolmentdata, $course->id); @@ -375,4 +397,80 @@ class lib_test extends \advanced_testcase { $this->assertEmpty($errors); } + /** + * Test the behaviour of find_instance(). + * + * @covers ::find_instance + */ + public function test_find_instance() { + global $DB; + $this->resetAfterTest(); + + $cat = $this->getDataGenerator()->create_category(); + $course = $this->getDataGenerator()->create_course(['category' => $cat->id, 'shortname' => 'ANON']); + + $cohort1 = $this->getDataGenerator()->create_cohort([ + 'contextid' => \context_coursecat::instance($cat->id)->id, + 'idnumber' => 'one', + ]); + $cohort2 = $this->getDataGenerator()->create_cohort([ + 'contextid' => \context_coursecat::instance($cat->id)->id, + 'idnumber' => 'two', + ]); + + $cohort3 = $this->getDataGenerator()->create_cohort([ + 'contextid' => \context_coursecat::instance($cat->id)->id, + 'idnumber' => 'three', + ]); + + $studentrole = $DB->get_record('role', ['shortname' => 'student']); + $teacherrole = $DB->get_record('role', ['shortname' => 'teacher']); + $managerrole = $DB->get_record('role', ['shortname' => 'manager']); + $cohortplugin = enrol_get_plugin('cohort'); + + // Add three cohort enrol instances. + $instanceid1 = $cohortplugin->add_instance($course, ['customint1' => $cohort1->id, 'roleid' => $teacherrole->id]); + $instanceid2 = $cohortplugin->add_instance($course, ['customint1' => $cohort2->id, 'roleid' => $managerrole->id]); + $instanceid3 = $cohortplugin->add_instance($course, ['customint1' => $cohort2->id, 'roleid' => $studentrole->id]); + + $instance1 = $DB->get_record('enrol', ['id' => $instanceid1]); + $instance2 = $DB->get_record('enrol', ['id' => $instanceid2]); + + $enrolmentdata = []; + $instance = $cohortplugin->find_instance($enrolmentdata, $course->id); + $this->assertNull($instance); + + // Unknown idnumber. + $enrolmentdata['cohortidnumber'] = 'test'; + $instance = $cohortplugin->find_instance($enrolmentdata, $course->id); + $this->assertNull($instance); + + // Unknown role. + $enrolmentdata['role'] = 'test'; + $enrolmentdata['cohortidnumber'] = $cohort1->idnumber; + $instance = $cohortplugin->find_instance($enrolmentdata, $course->id); + $this->assertNull($instance); + + // Cohort3 instance has not matching role and cohort. + $enrolmentdata['role'] = $teacherrole->shortname; + $enrolmentdata['cohortidnumber'] = $cohort3->idnumber; + $instance = $cohortplugin->find_instance($enrolmentdata, $course->id); + $this->assertNull($instance); + + // Cohort2 instance has matching cohort, but not matching role. + $enrolmentdata['role'] = $teacherrole->shortname; + $enrolmentdata['cohortidnumber'] = $cohort2->idnumber; + $instance = $cohortplugin->find_instance($enrolmentdata, $course->id); + $this->assertNull($instance); + + $enrolmentdata['role'] = $teacherrole->shortname; + $enrolmentdata['cohortidnumber'] = $cohort1->idnumber; + $instance = $cohortplugin->find_instance($enrolmentdata, $course->id); + $this->assertEquals($instance1->id, $instance->id); + + $enrolmentdata['role'] = $managerrole->shortname; + $enrolmentdata['cohortidnumber'] = $cohort2->idnumber; + $instance = $cohortplugin->find_instance($enrolmentdata, $course->id); + $this->assertEquals($instance2->id, $instance->id); + } } diff --git a/enrol/guest/lib.php b/enrol/guest/lib.php index 2e6ddf8f2b8..a9b7d9b6eb8 100644 --- a/enrol/guest/lib.php +++ b/enrol/guest/lib.php @@ -497,6 +497,28 @@ class enrol_guest_plugin extends enrol_plugin { public function is_csv_upload_supported(): bool { return true; } + + /** + * Finds matching instances for a given course. + * + * @param array $enrolmentdata enrolment data. + * @param int $courseid Course ID. + * @return stdClass|null Matching instance + */ + public function find_instance(array $enrolmentdata, int $courseid) : ?stdClass { + + $instances = enrol_get_instances($courseid, false); + $instance = null; + foreach ($instances as $i) { + if ($i->enrol == 'guest') { + // There can be only one guest enrol instance so find first available. + $instance = $i; + break; + } + } + return $instance; + } + } /** diff --git a/enrol/guest/tests/lib_test.php b/enrol/guest/tests/lib_test.php new file mode 100644 index 00000000000..1a35b5801b7 --- /dev/null +++ b/enrol/guest/tests/lib_test.php @@ -0,0 +1,59 @@ +. + +/** + * Guest enrolment tests. + * + * @package enrol_guest + * @category phpunit + * @copyright 2023 Ilya Tregubov + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace enrol_guest; + +class lib_test extends \advanced_testcase { + + /** + * Test the behaviour of find_instance(). + * + * @covers ::find_instance + */ + public function test_find_instance() { + global $DB; + $this->resetAfterTest(); + + $cat = $this->getDataGenerator()->create_category(); + // When we create a course, a guest enrolment instance is also created. + $course = $this->getDataGenerator()->create_course(['category' => $cat->id, 'shortname' => 'ANON']); + + $guestplugin = enrol_get_plugin('guest'); + + $expected = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => 'guest']); + + // Let's try to add second instance - only 1 guest instance is possible. + $instanceid2 = null; + // Have to do this check since add_instance doesn't block adding second instance for guest plugin. + if ($guestplugin->can_add_instance($course->id)) { + $instanceid2 = $guestplugin->add_instance($course, []); + } + $this->assertNull($instanceid2); + + $enrolmentdata = []; + $actual = $guestplugin->find_instance($enrolmentdata, $course->id); + $this->assertEquals($expected->id, $actual->id); + } + +} diff --git a/enrol/manual/lib.php b/enrol/manual/lib.php index 88be7ee0666..9299f4b86b8 100644 --- a/enrol/manual/lib.php +++ b/enrol/manual/lib.php @@ -655,6 +655,26 @@ class enrol_manual_plugin extends enrol_plugin { return true; } + /** + * Finds matching instances for a given course. + * + * @param array $enrolmentdata enrolment data. + * @param int $courseid Course ID. + * @return stdClass|null Matching instance + */ + public function find_instance(array $enrolmentdata, int $courseid) : ?stdClass { + + $instances = enrol_get_instances($courseid, false); + $instance = null; + foreach ($instances as $i) { + if ($i->enrol == 'manual') { + // There can be only one manual enrol instance so find first available. + $instance = $i; + break; + } + } + return $instance; + } } /** diff --git a/enrol/manual/tests/lib_test.php b/enrol/manual/tests/lib_test.php index 0f4b09bdc10..ce51f186a7d 100644 --- a/enrol/manual/tests/lib_test.php +++ b/enrol/manual/tests/lib_test.php @@ -761,4 +761,32 @@ class lib_test extends \advanced_testcase { ], ]; } + + /** + * Test the behaviour of find_instance(). + * + * @covers ::find_instance + */ + public function test_find_instance() { + global $DB; + $this->resetAfterTest(); + + $cat = $this->getDataGenerator()->create_category(); + // When we create a course, a manual enrolment instance is also created. + $course = $this->getDataGenerator()->create_course(['category' => $cat->id, 'shortname' => 'ANON']); + + $teacherrole = $DB->get_record('role', ['shortname' => 'teacher']); + $manualplugin = enrol_get_plugin('manual'); + + $expected = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => 'manual']); + + // Let's try to add second instance - only 1 manual instance is possible. + $instanceid2 = $manualplugin->add_instance($course, ['roleid' => $teacherrole->id]); + $this->assertNull($instanceid2); + + $enrolmentdata = []; + $actual = $manualplugin->find_instance($enrolmentdata, $course->id); + $this->assertEquals($expected->id, $actual->id); + } + } diff --git a/enrol/self/lib.php b/enrol/self/lib.php index 7793ccb9f1a..ab838958b3c 100644 --- a/enrol/self/lib.php +++ b/enrol/self/lib.php @@ -1088,6 +1088,28 @@ class enrol_self_plugin extends enrol_plugin { public function is_csv_upload_supported(): bool { return true; } + + /** + * Finds matching instances for a given course. + * + * @param array $enrolmentdata enrolment data. + * @param int $courseid Course ID. + * @return stdClass|null Matching instance + */ + public function find_instance(array $enrolmentdata, int $courseid) : ?stdClass { + + $instances = enrol_get_instances($courseid, false); + $instance = null; + foreach ($instances as $i) { + if ($i->enrol == 'self') { + // This is bad - we can not really distinguish between self instances. So grab first available. + $instance = $i; + break; + } + } + return $instance; + } + } /** diff --git a/enrol/self/tests/self_test.php b/enrol/self/tests/self_test.php index 351fdfeec34..53d2cc82d86 100644 --- a/enrol/self/tests/self_test.php +++ b/enrol/self/tests/self_test.php @@ -961,4 +961,32 @@ class self_test extends \advanced_testcase { // Self enrol has 2 enrol actions -- edit and unenrol. $this->assertCount(2, $actions); } + + /** + * Test the behaviour of find_instance(). + * + * @covers ::find_instance + */ + public function test_find_instance() { + global $DB; + $this->resetAfterTest(); + + $cat = $this->getDataGenerator()->create_category(); + // When we create a course, a self enrolment instance is also created. + $course = $this->getDataGenerator()->create_course(['category' => $cat->id, 'shortname' => 'ANON']); + + $teacherrole = $DB->get_record('role', ['shortname' => 'teacher']); + $selfplugin = enrol_get_plugin('self'); + + $instanceid1 = $DB->get_record('enrol', ['courseid' => $course->id, 'enrol' => 'self']); + + // Let's add a second instance. + $instanceid2 = $selfplugin->add_instance($course, ['roleid' => $teacherrole->id]); + + $enrolmentdata = []; + // The first instance should be returned - due to sorting in enrol_get_instances(). + $actual = $selfplugin->find_instance($enrolmentdata, $course->id); + $this->assertEquals($instanceid1->id, $actual->id); + } + } diff --git a/enrol/upgrade.txt b/enrol/upgrade.txt index 8d193cc614e..801e224bbd2 100644 --- a/enrol/upgrade.txt +++ b/enrol/upgrade.txt @@ -1,6 +1,16 @@ This files describes API changes in /enrol/* - plugins, information provided here is intended especially for developers. +=== 4.4 === + +* New find_instance() function has been created. It finds a matching enrolment instance in a given course using provided + enrolment data (for example cohort idnumber for cohort enrolment). Defaults to null. Override this function in your + enrolment plugin if you want it to be supported in CSV course upload. Please be aware that sometimes it is not possible + to uniquely find an instance based on given data. For example lti entolment records do not have any unique data except + id in mod_lti_enrol table. Such plugins should not be supported in CSV course upload. The exception is self enrolment + plugin since it is already supported in CSV course upload. For self enrolment plugin, find_instance() returns first + available instance in the course. + === 4.3 === * New is_csv_upload_supported() function has been created. It checks whether enrolment plugin is supported diff --git a/lib/enrollib.php b/lib/enrollib.php index 3452d1ca50a..457357a0fff 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -3510,4 +3510,17 @@ abstract class enrol_plugin { return null; } + /** + * Finds matching instances for a given course. + * + * @param array $enrolmentdata enrolment data. + * @param int $courseid Course ID. + * @return stdClass|null Matching instance + */ + public function find_instance(array $enrolmentdata, int $courseid) : ?stdClass { + + // By default, we assume we can't uniquely identify an instance so better not update any. + // Plugins can override this if they can uniquely identify an instance. + return null; + } }