Skip to content

Commit 5d7b7fd

Browse files
committed
Fix issue #2272.
Make sure that the `$userRecord` that is passed to the `fieldHTML` method in `ProblemSetDetail.pm` exists before trying to access it. Note the actual error occurs for the `counts_parent_grade` column of the problem record which is the only problem record field that is of the "choose" type (which is why this does not occur for non JITAR sets). The earlier checks that the $userRecord exists are needed as the hacked method of accessing the data directly via the hash key rather than using the accessor turns the undefined value into a hash reference if that is done. Then the check if the $userRecord exists in the "choose" case later will fail because it will exist. I don't know how long this bug has existed, but I know it goes back to at least WeBWorK 2.17 (I suspect it goes back considerably further though). The fieldHTML method in general needs a rewrite. There is to much hackery in use there. The comment on lines 855-857 is about part of what is causing this problem. That comment dates back to 2007, and the problem it refers to goes back further.
1 parent d8a32be commit 5d7b7fd

File tree

1 file changed

+6
-4
lines changed

1 file changed

+6
-4
lines changed

lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,7 @@ sub fieldHTML ($c, $userID, $setID, $problemID, $globalRecord, $userRecord, $fie
856856
# avoiding errors if the access method is undefined. That seems a bit suspect, but it's used below so we'll
857857
# leave it here.
858858
push(@gVals, $globalRecord->{$f});
859-
push(@uVals, $userRecord->{$f});
859+
push(@uVals, $userRecord ? $userRecord->{$f} : '');
860860
push(@bVals, '');
861861
}
862862
# I don't like this, but combining multiple values is a bit messy
@@ -865,7 +865,7 @@ sub fieldHTML ($c, $userID, $setID, $problemID, $globalRecord, $userRecord, $fie
865865
$blankfield = join(':', @bVals);
866866
} else {
867867
$globalValue = $globalRecord->{$field};
868-
$userValue = $userRecord->{$field};
868+
$userValue = $userRecord ? $userRecord->{$field} : '';
869869
}
870870

871871
# Use defined instead of value in order to allow 0 to printed, e.g. for the 'value' field.
@@ -956,11 +956,13 @@ sub fieldHTML ($c, $userID, $setID, $problemID, $globalRecord, $userRecord, $fie
956956
my @fields = split(/:/, $field);
957957
my @part_values;
958958
for (@fields) {
959-
push(@part_values, $forUsers && $userRecord->$_ ne '' ? $userRecord->$_ : $globalRecord->$_);
959+
push(@part_values,
960+
$forUsers && $userRecord && $userRecord->$_ ne '' ? $userRecord->$_ : $globalRecord->$_);
960961
}
961962
$value = join(':', @part_values);
962963
} elsif (!$value) {
963-
$value = ($forUsers && $userRecord->$field ne '' ? $userRecord->$field : $globalRecord->$field);
964+
$value =
965+
$forUsers && $userRecord && $userRecord->$field ne '' ? $userRecord->$field : $globalRecord->$field;
964966
}
965967

966968
$inputType = $c->select_field(

0 commit comments

Comments
 (0)