Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

MDL-29572 Grade Import: Avoid creating invalid temporary rows; Provid…

…e feedback on error
  • Loading branch information...
commit 643e341cc0777ce5296e51950c6bfb231b7fe4cd 1 parent 9b8555f
@jrchamp jrchamp authored
View
65 grade/import/csv/index.php
@@ -187,6 +187,25 @@
core_php_time_limit::raise();
raise_memory_limit(MEMORY_EXTRA);
+ $userfields = array(
+ 'userid' => array(
+ 'field' => 'id',
+ 'label' => 'id',
+ ),
+ 'useridnumber' => array(
+ 'field' => 'idnumber',
+ 'label' => 'idnumber',
+ ),
+ 'useremail' => array(
+ 'field' => 'email',
+ 'label' => 'email address',
+ ),
+ 'username' => array(
+ 'field' => 'username',
+ 'label' => 'username',
+ ),
+ );
+
$csvimport->init();
$newgradeitems = array(); // temporary array to keep track of what new headers are processed
@@ -225,39 +244,23 @@
}
switch ($t0) {
- case 'userid': //
- if (!$user = $DB->get_record('user', array('id' => $value))) {
- // user not found, abort whole import
- import_cleanup($importcode);
- echo $OUTPUT->notification("user mapping error, could not find user with id \"$value\"");
- $status = false;
- break 3;
- }
- $studentid = $value;
- break;
+ case 'userid':
case 'useridnumber':
- if (empty($value) || !$user = $DB->get_record('user', array('idnumber' => $value))) {
- // user not found, abort whole import
- import_cleanup($importcode);
- echo $OUTPUT->notification("user mapping error, could not find user with idnumber \"$value\"");
- $status = false;
- break 3;
- }
- $studentid = $user->id;
- break;
case 'useremail':
- if (!$user = $DB->get_record('user', array('email' => $value))) {
- import_cleanup($importcode);
- echo $OUTPUT->notification("user mapping error, could not find user with email address \"$value\"");
- $status = false;
- break 3;
- }
- $studentid = $user->id;
- break;
case 'username':
- if (!$user = $DB->get_record('user', array('username' => $value))) {
+ // Skip invalid row with blank user field.
+ if (empty($value)) {
+ continue 3;
+ }
+
+ if (!$user = $DB->get_record('user', array($userfields[$t0]['field'] => $value))) {
+ // User not found, abort whole import.
import_cleanup($importcode);
- echo $OUTPUT->notification("user mapping error, could not find user with username \"$value\"");
+ $usermappingerrorobj = new stdClass();
+ $usermappingerrorobj->field = $userfields[$t0]['label'];
+ $usermappingerrorobj->value = $value;
+ echo $OUTPUT->notification(get_string('usermappingerror', 'grades', $usermappingerrorobj));
+ unset($usermappingerrorobj);
$status = false;
break 3;
}
@@ -383,7 +386,7 @@
// user not found, abort whole import
$status = false;
import_cleanup($importcode);
- echo $OUTPUT->notification('user mapping error, could not find user!');
+ echo $OUTPUT->notification(get_string('usermappingerrorusernotfound', 'grades'));
break;
}
@@ -391,7 +394,7 @@
// not allowed to import into this group, abort
$status = false;
import_cleanup($importcode);
- echo $OUTPUT->notification('user not member of current group, can not update!');
+ echo $OUTPUT->notification(get_string('usermappingerrorcurrentgroup', 'grades'));
break;
}
View
27 grade/import/lib.php
@@ -45,6 +45,8 @@ function get_new_importcode() {
function grade_import_commit($courseid, $importcode, $importfeedback=true, $verbose=true) {
global $CFG, $USER, $DB, $OUTPUT;
+ $failed = false;
+ $executionerrors = false;
$commitstart = time(); // start time in case we need to roll back
$newitemids = array(); // array to hold new grade_item ids from grade_import_newitem table, mapping array
@@ -57,11 +59,11 @@ function grade_import_commit($courseid, $importcode, $importfeedback=true, $verb
// instances of the new grade_items created, cached
// in case grade_update fails, so that we can remove them
$instances = array();
- $failed = false;
foreach ($newitems as $newitem) {
// get all grades with this item
- if ($grades = $DB->get_records('grade_import_values', array('newgradeitem' => $newitem->id))) {
+ $gradeimportparams = array('newgradeitem' => $newitem->id, 'importcode' => $importcode, 'importer' => $USER->id);
+ if ($grades = $DB->get_records('grade_import_values', $gradeimportparams)) {
/// create a new grade item for this - must use false as second param!
/// TODO: we need some bounds here too
$gradeitem = new grade_item(array('courseid'=>$courseid, 'itemtype'=>'manual', 'itemname'=>$newitem->itemname), false);
@@ -104,7 +106,8 @@ function grade_import_commit($courseid, $importcode, $importfeedback=true, $verb
return false;
}
// get all grades with this item
- if ($grades = $DB->get_records('grade_import_values', array('itemid' => $itemid))) {
+ $gradeimportparams = array('itemid' => $itemid, 'importcode' => $importcode, 'importer' => $USER->id);
+ if ($grades = $DB->get_records('grade_import_values', $gradeimportparams)) {
// make the grades array for update_grade
foreach ($grades as $grade) {
@@ -112,7 +115,11 @@ function grade_import_commit($courseid, $importcode, $importfeedback=true, $verb
$grade->feedback = false; // ignore it
}
if (!$gradeitem->update_final_grade($grade->userid, $grade->finalgrade, 'import', $grade->feedback)) {
- $failed = 1;
+ $errordata = new stdClass();
+ $errordata->itemname = $gradeitem->itemname;
+ $errordata->userid = $grade->userid;
+ $executionerrors[] = get_string('errorsettinggrade', 'grades', $errordata);
+ $failed = true;
break 2;
}
}
@@ -120,11 +127,17 @@ function grade_import_commit($courseid, $importcode, $importfeedback=true, $verb
$modifieditems[] = $itemid;
}
+ }
- if (!empty($failed)) {
- import_cleanup($importcode);
- return false;
+ if ($failed) {
+ if ($executionerrors && $verbose) {
+ echo $OUTPUT->notification(get_string('gradeimportfailed', 'grades'));
+ foreach ($executionerrors as $errorstr) {
+ echo $OUTPUT->notification($errorstr);
+ }
}
+ import_cleanup($importcode);
+ return false;
}
}
View
5 lang/en/grades.php
@@ -180,6 +180,7 @@
$string['errornocourse'] = 'Could not get course information';
$string['errorreprintheadersnonnumeric'] = 'Received non-numeric value for reprint-headers';
$string['errorsavegrade'] = 'Could not save grade, sorry.';
+$string['errorsettinggrade'] = 'Error saving "{$a->itemname}" grade for userid {$a->userid}';
$string['errorupdatinggradecategoryaggregateonlygraded'] = 'Error updating the "Aggregate only non-empty grades" setting of grade category ID {$a->id}';
$string['errorupdatinggradecategoryaggregateoutcomes'] = 'Error updating the "Include outcomes in aggregation" setting of grade category ID {$a->id}';
$string['errorupdatinggradecategoryaggregatesubcats'] = 'Error updating the "Aggregate including subcategories" setting of grade category ID {$a->id}';
@@ -253,6 +254,7 @@
$string['gradehistorylifetime'] = 'Grade history lifetime';
$string['gradehistorylifetime_help'] = 'This specifies the length of time you want to keep history of changes in grade related tables. It is recommended to keep it as long as possible. If you experience performance problems or have limited database space, try to set lower value.';
$string['gradeimport'] = 'Grade import';
+$string['gradeimportfailed'] = 'Grade Import failed during commit. Details:';
$string['gradeitem'] = 'Grade item';
$string['gradeitemaddusers'] = 'Exclude from grading';
$string['gradeitemadvanced'] = 'Advanced grade item options';
@@ -661,6 +663,9 @@
$string['user'] = 'User';
$string['usergrade'] = 'User {$a->fullname} ({$a->useridnumber}) on item {$a->gradeidnumber}';
$string['userid'] = 'User ID';
+$string['usermappingerror'] = 'User mapping error: Could not find user with {$a->field} of "{$a->value}".';
+$string['usermappingerrorusernotfound'] = 'User mapping error. Could not find user.';
+$string['usermappingerrorcurrentgroup'] = 'User is not a member of current group.';
$string['userpreferences'] = 'User preferences';
$string['userenrolmentsuspended'] = 'User enrolment suspended';
$string['useweighted'] = 'Use weighted';
Please sign in to comment.
Something went wrong with that request. Please try again.