MDL-43820 enrol_cohort: Override find_instance

For cohort enrol method we can have multiple instances. We can match
those by cohort idnumber and role provided.

Also adding some extra validation and update tests.

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"

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

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

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

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

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

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

enrol/cohort/lib.php

@@ -550,20 +550,37 @@ public function validate_enrol_plugin_data(array $enrolmentdata, ?int $courseid
             }
         }
 
-        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 @@ public function validate_enrol_plugin_data(array $enrolmentdata, ?int $courseid
     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 @@ public function fill_enrol_custom_fields(array $enrolmentdata, int $courseid) :
      */
     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 @@ public function add_custom_instance(stdClass $course, ?array $fields = null): ?i
     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;
+    }
 }
 
 /**