From 2fecfa597b84fdce6a6fc1d6e6f36704901d35e2 Mon Sep 17 00:00:00 2001 From: Sara Arjona Date: Fri, 22 Mar 2024 13:45:10 +0100 Subject: [PATCH] MDL-80875 uploadcourse: Fix params validation --- .../uploadcourse/tests/behat/enrolments.feature | 52 ++++++++++++++++++++++ .../tests/fixtures/enrolment_multiple.csv | 2 + .../tests/fixtures/enrolment_multiple_delete.csv | 2 + enrol/self/lib.php | 8 ++-- lib/enrollib.php | 8 ++++ 5 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 admin/tool/uploadcourse/tests/fixtures/enrolment_multiple.csv create mode 100644 admin/tool/uploadcourse/tests/fixtures/enrolment_multiple_delete.csv diff --git a/admin/tool/uploadcourse/tests/behat/enrolments.feature b/admin/tool/uploadcourse/tests/behat/enrolments.feature index 90f3060bfc5..03dc9931832 100644 --- a/admin/tool/uploadcourse/tests/behat/enrolments.feature +++ b/admin/tool/uploadcourse/tests/behat/enrolments.feature @@ -102,3 +102,55 @@ Feature: An admin can update courses enrolments using a CSV file Then I should see "Course updated" And I am on the "Course 1" "enrolment methods" page And I should not see "Guest access" in the "generaltable" "table" + + @javascript + Scenario: Re-upload a file using CSV data only after deleting the enrolments method + Given I navigate to "Plugins > Enrolments > Manage enrol plugins" in site administration + And I click on "Enable" "link" in the "Course meta link" "table_row" + And the following "cohort" exists: + | name | Cohort1 | + | idnumber | Cohort1 | + And the following "category" exists: + | name | Cat 1 | + | category | 0 | + | idnumber | CAT1 | + And I navigate to "Courses > Upload courses" in site administration + And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_multiple.csv" file to "File" filemanager + And I click on "Preview" "button" + And I click on "Upload courses" "button" + And I am on the "Course 2" "enrolment methods" page + And I should see "Self enrolment (Student)" in the "generaltable" "table" + And I should see "Cohort sync (Cohort1 - Student)" in the "generaltable" "table" + And I should see "Guest access" in the "generaltable" "table" + And I should see "Manual enrolments" in the "generaltable" "table" + And I should see "Course meta link (Course 1)" in the "generaltable" "table" + And I navigate to "Courses > Upload courses" in site administration + # Delete all enrolment methods. + And I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_multiple_delete.csv" file to "File" filemanager + And I set the field "Upload mode" to "Only update existing courses" + And I set the field "Update mode" to "Update with CSV data only" + And I set the field "Allow deletes" to "Yes" + And I click on "Preview" "button" + And I click on "Upload courses" "button" + And I should see "Course updated" + And I am on the "Course 2" "enrolment methods" page + And I should not see "Self enrolment (Student)" in the "generaltable" "table" + And I should not see "Cohort sync (Cohort1 - Student)" in the "generaltable" "table" + And I should not see "Guest access" in the "generaltable" "table" + And I should not see "Manual enrolments" in the "generaltable" "table" + And I should not see "Course meta link (Course 1)" in the "generaltable" "table" + # Re-upload again the CSV file, to add again the enrolment methods. + And I navigate to "Courses > Upload courses" in site administration + When I upload "admin/tool/uploadcourse/tests/fixtures/enrolment_multiple.csv" file to "File" filemanager + And I set the field "Upload mode" to "Only update existing courses" + And I set the field "Update mode" to "Update with CSV data only" + And I set the field "Allow deletes" to "Yes" + And I click on "Preview" "button" + And I click on "Upload courses" "button" + And I am on the "Course 2" "enrolment methods" page + Then I should see "Self enrolment (Student)" in the "generaltable" "table" + And I should see "Cohort sync (Cohort1 - Student)" in the "generaltable" "table" + And I should see "Guest access" in the "generaltable" "table" + And I should see "Manual enrolments" in the "generaltable" "table" + And I should see "Course meta link (Course 1)" in the "generaltable" "table" + And I navigate to "Courses > Upload courses" in site administration diff --git a/admin/tool/uploadcourse/tests/fixtures/enrolment_multiple.csv b/admin/tool/uploadcourse/tests/fixtures/enrolment_multiple.csv new file mode 100644 index 00000000000..60f0768a2bf --- /dev/null +++ b/admin/tool/uploadcourse/tests/fixtures/enrolment_multiple.csv @@ -0,0 +1,2 @@ +shortname,fullname,category_idnumber,enrolment_1,enrolment_1_role,enrolment_2,enrolment_2_role,enrolment_2_cohortidnumber,enrolment_3,enrolment_4,enrolment_4_role,enrolment_5,enrolment_5_metacoursename +C2,Course 2,CAT1,self,student,cohort,student,Cohort1,guest,manual,student,meta,C1 diff --git a/admin/tool/uploadcourse/tests/fixtures/enrolment_multiple_delete.csv b/admin/tool/uploadcourse/tests/fixtures/enrolment_multiple_delete.csv new file mode 100644 index 00000000000..8ec824556d9 --- /dev/null +++ b/admin/tool/uploadcourse/tests/fixtures/enrolment_multiple_delete.csv @@ -0,0 +1,2 @@ +shortname,fullname,category_idnumber,enrolment_1,enrolment_1_delete,enrolment_2,enrolment_2_role,enrolment_2_cohortidnumber,enrolment_2_delete,enrolment_3,enrolment_3_delete,enrolment_4,enrolment_4_delete,enrolment_5,enrolment_5_metacoursename,enrolment_5_delete +C2,Course 2,CAT1,self,1,cohort,student,Cohort1,1,guest,1,manual,1,meta,C1,1 diff --git a/enrol/self/lib.php b/enrol/self/lib.php index 3c14687b18f..5ecfb19aee1 100644 --- a/enrol/self/lib.php +++ b/enrol/self/lib.php @@ -1039,12 +1039,14 @@ class enrol_self_plugin extends enrol_plugin { } } - if (($data['expirynotify'] > 0 || $data['customint2']) && $data['expirythreshold'] < 86400) { + if (array_key_exists('expirynotify', $data) + && ($data['expirynotify'] > 0 || $data['customint2']) + && $data['expirythreshold'] < 86400) { $errors['expirythreshold'] = get_string('errorthresholdlow', 'core_enrol'); } // Now these ones are checked by quickforms, but we may be called by the upload enrolments tool, or a webservive. - if (core_text::strlen($data['name']) > 255) { + if (array_key_exists('name', $data) && core_text::strlen($data['name']) > 255) { $errors['name'] = get_string('err_maxlength', 'form', 255); } $validstatus = array_keys($this->get_status_options()); @@ -1072,7 +1074,7 @@ class enrol_self_plugin extends enrol_plugin { 'expirynotify' => $validexpirynotify, 'roleid' => $validroles ); - if ($data['expirynotify'] != 0) { + if (array_key_exists('expirynotify', $data) && $data['expirynotify'] != 0) { $tovalidate['expirythreshold'] = PARAM_INT; } $typeerrors = $this->validate_param_types($data, $tovalidate); diff --git a/lib/enrollib.php b/lib/enrollib.php index a940bb9c9db..2b6f81807ba 100644 --- a/lib/enrollib.php +++ b/lib/enrollib.php @@ -3517,6 +3517,9 @@ abstract class enrol_plugin { $errors = array(); $invalidstr = get_string('invaliddata', 'error'); foreach ($rules as $fieldname => $rule) { + if (!array_key_exists($fieldname, $data)) { + continue; + } if (is_array($rule)) { if (!in_array($data[$fieldname], $rule)) { $errors[$fieldname] = $invalidstr; @@ -3576,6 +3579,8 @@ abstract class enrol_plugin { * @return lang_string|null Error */ public function validate_plugin_data_context(array $enrolmentdata, ?int $courseid = null): ?lang_string { + global $DB; + if ($courseid) { $enrolmentdata += ['courseid' => $courseid, 'id' => 0, 'status' => ENROL_INSTANCE_ENABLED]; $instance = (object)[ @@ -3584,6 +3589,9 @@ abstract class enrol_plugin { 'status' => $enrolmentdata['status'], 'type' => $this->get_name(), ]; + if (array_key_exists('role', $enrolmentdata)) { + $instance->roleid = $DB->get_field('role', 'id', ['shortname' => $enrolmentdata['role']]); + } $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)); -- 2.11.4.GIT