Commit 7c5263b5 authored by Ghada El-Zoghbi's avatar Ghada El-Zoghbi
Browse files

Bug 1600116: Validate group shortname across all institutions.

Validate that the group shortname is unique across all institutions
instead of just the institution that is being processed.

Make sure the mandatory fields (shortname, displayname, roles)
in the upload csv are not empty as the group shortname validation
relies on the display name.

Also, improve the error message thrown during the submit when
the groupname is not unique.

behatnotneeded

Change-Id: I23278de9bb52f1289535924f1accf3fcb9d559ce
parent aec9fbd0
......@@ -145,18 +145,31 @@ function uploadcsv_validate(Pieform $form, $values) {
$editroles = $line[$formatkeylookup['editroles']];
}
// Make sure these three mandatory fields are populated.
if (empty($shortname)) {
$csverrors->add($i, get_string('uploadcsverrormandatoryfieldnotspecified', 'admin', $i, 'shortname'));
}
if (empty($displayname)) {
$csverrors->add($i, get_string('uploadcsverrormandatoryfieldnotspecified', 'admin', $i, 'displayname'));
}
if (empty($grouptype)) {
$csverrors->add($i, get_string('uploadcsverrormandatoryfieldnotspecified', 'admin', $i, 'roles'));
}
if (!preg_match('/^[a-zA-Z0-9_.-]{2,255}$/', $shortname)) {
$csverrors->add($i, get_string('uploadgroupcsverrorinvalidshortname', 'admin', $i, $shortname));
}
if (isset($shortnames[$shortname])) {
// Duplicate shortname within this file.
$csverrors->add($i, get_string('uploadgroupcsverrorshortnamealreadytaken', 'admin', $i, $shortname));
$validshortname = group_generate_shortname($displayname);
$csverrors->add($i, get_string('uploadgroupcsverrorshortnamealreadytaken1', 'admin', $i, $shortname, $validshortname));
}
else if (!$values['updategroups']) {
// The groupname must be new
if (record_exists('group', 'shortname', $shortname, 'institution', $institution)) {
$csverrors->add($i, get_string('uploadgroupcsverrorshortnamealreadytaken', 'admin', $i, $shortname));
if (record_exists('group', 'shortname', $shortname)) {
$validshortname = group_generate_shortname($displayname);
$csverrors->add($i, get_string('uploadgroupcsverrorshortnamealreadytaken1', 'admin', $i, $shortname, $validshortname));
}
}
else if ($values['updategroups']) {
......@@ -175,12 +188,12 @@ function uploadcsv_validate(Pieform $form, $values) {
if (isset($displaynames[strtolower($displayname)])) {
// Duplicate displayname within this file
$csverrors->add($i, get_string('uploadgroupcsverrorsgroupnamealreadyexists', 'admin', $i, $displayname));
$csverrors->add($i, get_string('uploadgroupcsverrordisplaynamealreadyexists', 'admin', $i, $displayname));
}
else if (!$values['updategroups']) {
// The displayname must be new
if (get_records_sql_array('SELECT id FROM {group} WHERE LOWER(TRIM(name)) = ?', array(strtolower(trim($displayname))))) {
$csverrors->add($i, get_string('uploadgroupcsverrorgroupnamealreadyexists', 'admin', $i, $displayname));
$csverrors->add($i, get_string('uploadgroupcsverrordisplaynamealreadyexists', 'admin', $i, $displayname));
}
}
else {
......@@ -194,7 +207,7 @@ function uploadcsv_validate(Pieform $form, $values) {
$shortname,
$institution
))) {
$csverrors->add($i, get_string('uploadgroupcsverrorgroupnamealreadyexists', 'admin', $i, $displayname));
$csverrors->add($i, get_string('uploadgroupcsverrordisplaynamealreadyexists', 'admin', $i, $displayname));
}
}
$displaynames[strtolower($displayname)] = 1;
......
......@@ -647,12 +647,12 @@ $string['uploadcsvpagedescription6'] = '<p>Here you can upload new users via a <
%s';
$string['uploadcsverrortoomanyusers'] = 'You have too many lines in your CSV file. Your file should not contain more than %s.';
$string['uploadgroupcsverrorgroupnamealreadyexists'] = 'Error on line %s of your file: The groupname "%s" already exists.';
$string['uploadgroupcsverrordisplaynamealreadyexists'] = 'Error on line %s of your file: The displayname "%s" already exists.';
$string['uploadgroupcsverrorinvalidshortname'] = 'Error on line %s of your file: The shortname "%s" is invalid.';
$string['uploadgroupcsverrorshortnamemissing'] = 'Error on line %s of your file: The group with the shortname "%s" does not exist.';
$string['uploadgroupcsverrorinvalidgrouptype'] = 'Error on line %s of your file: The grouptype "%s" is invalid.';
$string['uploadgroupcsverrorinvalideditroles'] = 'Error on line %s of your file: The value for editroles "%s" is invalid.';
$string['uploadgroupcsverrorshortnamealreadytaken'] = 'Error on line %s of your file: The shortname "%s" is already taken.';
$string['uploadgroupcsverrorshortnamealreadytaken1'] = 'Error on line %s of your file: The shortname "%s" is already taken. A valid alternative is "%s"';
$string['uploadgroupcsverrorusernamesnotlastfield'] = 'The "usernames" field must be the last field in the header.';
$string['uploadgroupcsverroropencontrolled'] = 'Line %s: Groups cannot have both open and controlled membership.';
$string['uploadgroupcsverroropenrequest'] = 'Line %s: Groups with open membership cannot allow membership requests.';
......
......@@ -356,11 +356,12 @@ function group_create($data) {
if (!empty($data['shortname'])) {
// make sure it is unique and is correct length
$shortname = group_generate_shortname($data['shortname']);
$shortname = group_generate_shortname($data['name']);
// If we want to retain the supplied shortname we need to make sure it can be done
if (!empty($data['retainshortname'])) {
if ($shortname != $data['shortname']) {
throw new UserException('group_create: problem with supplied shortname ' . $data['shortname'] . ' not matching allowed shortname ' . $shortname);
throw new UserException('group_create: The supplied shortname \'' . $data['shortname'] .
'\' is already taken. This shortname \'' . $shortname . '\' is available.');
}
}
$data['shortname'] = $shortname;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment