Skip to content

Commit

Permalink
MDL-29572 Grade Import: Avoid creating invalid temporary rows; Provid…
Browse files Browse the repository at this point in the history 
…e feedback on error
  • Loading branch information
@jrchamp
jrchamp committed Mar 23, 2014
1 parent 9b8555f commit 643e341
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 38 deletions.
65 changes: 34 additions & 31 deletions grade/import/csv/index.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -383,15 +386,15 @@
// 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;
}

if ($separatemode and !groups_is_member($currentgroup, $studentid)) {
// 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;
}

Expand Down
27 changes: 20 additions & 7 deletions grade/import/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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);
Expand Down Expand Up @@ -104,27 +106,38 @@ 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) {
if (!$importfeedback) {
$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;
}
}
//$itemdetails -> idnumber = $gradeitem->idnumber;
$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;
}
}

Expand Down
5 changes: 5 additions & 0 deletions lang/en/grades.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}';
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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';
Expand Down

0 comments on commit 643e341

Please sign in to comment.