From ec43371a3ff958ef313a02134b51b0eab7c9b8bd Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Tue, 24 Oct 2017 11:14:21 +0800 Subject: [PATCH] MDL-45068 groups: fixes to group import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - If 'coursename' is specified in the CSV it should match the course short name - thanks Yusuf Yılmaz for the patch - If 'idnumber' is specified but 'groupidnumber' is not, idnumber should be used for matching the course idnumber only - If 'groupingname' is not specified, there should be no notices (regression from MDL-42514) - If 'coursename' or 'idnumber' column is present, it can contain empty values in some/all lines --- group/import.php | 18 ++++++----- group/tests/behat/groups_import.feature | 31 +++++++++++++++++++ .../fixtures/groups_import_multicourse.csv | 6 ++++ 3 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 group/tests/fixtures/groups_import_multicourse.csv diff --git a/group/import.php b/group/import.php index 2c09e6df76b..ae0923bcdb5 100644 --- a/group/import.php +++ b/group/import.php @@ -118,7 +118,7 @@ if ($mform_post->is_cancelled()) { //decode encoded commas $record[$header[$key]] = preg_replace($csv_encode, $csv_delimiter, trim($value)); } - if ($record[$header[0]]) { + if (trim($rawline) !== '') { // add a new group to the database // add fields to object $user @@ -134,28 +134,32 @@ if ($mform_post->is_cancelled()) { } } - if (isset($newgroup->idnumber)){ + if (!empty($newgroup->idnumber)) { //if idnumber is set, we use that. //unset invalid courseid if (!$mycourse = $DB->get_record('course', array('idnumber'=>$newgroup->idnumber))) { echo $OUTPUT->notification(get_string('unknowncourseidnumber', 'error', $newgroup->idnumber)); unset($newgroup->courseid);//unset so 0 doesn't get written to database + } else { + $newgroup->courseid = $mycourse->id; } - $newgroup->courseid = $mycourse->id; - } else if (isset($newgroup->coursename)){ + } else if (!empty($newgroup->coursename)) { //else use course short name to look up //unset invalid coursename (if no id) - if (!$mycourse = $DB->get_record('course', array('shortname', $newgroup->coursename))) { + if (!$mycourse = $DB->get_record('course', array('shortname' => $newgroup->coursename))) { echo $OUTPUT->notification(get_string('unknowncourse', 'error', $newgroup->coursename)); unset($newgroup->courseid);//unset so 0 doesn't get written to database + } else { + $newgroup->courseid = $mycourse->id; } - $newgroup->courseid = $mycourse->id; } else { //else use use current id $newgroup->courseid = $id; } + unset($newgroup->idnumber); + unset($newgroup->coursename); //if courseid is set if (isset($newgroup->courseid)) { @@ -196,7 +200,7 @@ if ($mform_post->is_cancelled()) { } // Add group to grouping - if (!empty($newgroup->groupingname) || is_numeric($newgroup->groupingname)) { + if (isset($newgroup->groupingname) && strlen($newgroup->groupingname)) { $groupingname = $newgroup->groupingname; if (! $groupingid = groups_get_grouping_by_name($newgroup->courseid, $groupingname)) { $data = new stdClass(); diff --git a/group/tests/behat/groups_import.feature b/group/tests/behat/groups_import.feature index 0bec1e371be..95019f10907 100644 --- a/group/tests/behat/groups_import.feature +++ b/group/tests/behat/groups_import.feature @@ -8,12 +8,14 @@ Feature: Importing of groups and groupings Given the following "courses" exist: | fullname | shortname | category | | Course 1 | C1 | 0 | + | Course 2 | C2 | 0 | And the following "users" exist: | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@example.com | And the following "course enrolments" exist: | user | course | role | | teacher1 | C1 | editingteacher | + | teacher1 | C2 | editingteacher | @javascript Scenario: Import groups and groupings as teacher @@ -110,3 +112,32 @@ Feature: Importing of groups and groupings And I press "Edit group settings" And the field "id_idnumber" matches value "" And I press "Cancel" + + @javascript + Scenario: Import groups into multiple courses as a teacher + Given I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "Users > Groups" in current page administration + And I press "Import groups" + When I upload "group/tests/fixtures/groups_import_multicourse.csv" file to "Import" filemanager + And I press "Import groups" + Then I should see "Group group7 added successfully" + And I should see "Unknown course named \"C-non-existing\"" + And I should see "Group group8 added successfully" + And I should not see "group-will-not-be-created" + And I should see "Group group9 added successfully" + And I should see "Group group10 added successfully" + And I press "Continue" + And I should see "group10" + And I should see "group7" + And I should see "group8" + And I should not see "group9" + And I should not see "group-will-not-be-created" + And I am on "Course 2" course homepage + And I navigate to "Users > Groups" in current page administration + And I should see "group9" + And I should not see "group-will-not-be-created" + And I should not see "group7" + And I should not see "group8" + And I should not see "group10" + And I log out diff --git a/group/tests/fixtures/groups_import_multicourse.csv b/group/tests/fixtures/groups_import_multicourse.csv new file mode 100644 index 00000000000..cbedd6006c0 --- /dev/null +++ b/group/tests/fixtures/groups_import_multicourse.csv @@ -0,0 +1,6 @@ +coursename,groupname +C1,group7 +C-non-existing,group-will-not-be-created +C1,group8 +C2,group9 +,group10