Skip to content

Commit

Permalink
Fix issue openwebwork#2272.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
drgrice1 committed Dec 6, 2023
1 parent 7d0f568 commit 5287df3
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,7 @@ sub fieldHTML ($c, $userID, $setID, $problemID, $globalRecord, $userRecord, $fie
# avoiding errors if the access method is undefined. That seems a bit suspect, but it's used below so we'll
# leave it here.
push(@gVals, $globalRecord->{$f});
push(@uVals, $userRecord->{$f});
push(@uVals, $userRecord ? $userRecord->{$f} : '');
push(@bVals, '');
}
# I don't like this, but combining multiple values is a bit messy
Expand All @@ -865,7 +865,7 @@ sub fieldHTML ($c, $userID, $setID, $problemID, $globalRecord, $userRecord, $fie
$blankfield = join(':', @bVals);
} else {
$globalValue = $globalRecord->{$field};
$userValue = $userRecord->{$field};
$userValue = $userRecord ? $userRecord->{$field} : '';
}

# Use defined instead of value in order to allow 0 to printed, e.g. for the 'value' field.
Expand Down Expand Up @@ -956,11 +956,13 @@ sub fieldHTML ($c, $userID, $setID, $problemID, $globalRecord, $userRecord, $fie
my @fields = split(/:/, $field);
my @part_values;
for (@fields) {
push(@part_values, $forUsers && $userRecord->$_ ne '' ? $userRecord->$_ : $globalRecord->$_);
push(@part_values,
$forUsers && $userRecord && $userRecord->$_ ne '' ? $userRecord->$_ : $globalRecord->$_);
}
$value = join(':', @part_values);
} elsif (!$value) {
$value = ($forUsers && $userRecord->$field ne '' ? $userRecord->$field : $globalRecord->$field);
$value =
$forUsers && $userRecord && $userRecord->$field ne '' ? $userRecord->$field : $globalRecord->$field;
}

$inputType = $c->select_field(
Expand Down

0 comments on commit 5287df3

Please sign in to comment.