From 1d11f17e8fe213d5b2e14eb9232a66a8524358c2 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 14 Oct 2026 21:17:34 -0500 Subject: [PATCH] Determine when a proctored test version is complete instead of changing its type. Previously a proctored test version was converted to an unproctored test when the last allowed submission was made so that it could be reviewed without proctor authorization. If time expired before all submissions were used, the version remained proctored and could not be reviewed without proctor authorization (issue #3239). Furthermore, once converted, a version whose close date was extended or that was given more attempts could be submitted without proctor authorization. Now the assignment type of a proctored test version is never changed. Instead, a version is considered complete if at least one submission has been recorded and the student can no longer record answers for the version because all attempts have been used or time has expired. Proctor authorization is not required to view a complete version. Since this is determined from the current data for the version, a complete version that is extended or given more attempts requires proctor authorization again. The version_last_attempt_time is now only used in place of the current time to determine if answers can be recorded or checked for a proctored test when answers are being submitted, so that a complete version that is viewed is not treated as if it can still be submitted. Determining if a version is complete is done so as to add as little database work as possible to requests. The number of recorded attempts is only obtained if the result depends on it, and pages that already have a problem for the version (the set version list and the grades page) pass it in. The proctor authentication result is saved for the request, since it is checked both during authentication and by Authz::checkSet, and the set version obtained is saved in the Authz merged set cache so that it is not obtained again. The set version list links complete proctored versions to the unproctored route so that the proctor authentication module is not called during authentication for those. Fixes #3239. Co-Authored-By: Claude Opus 5.5 --- lib/WeBWorK/Authen/Proctor.pm | 26 ++++++- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 73 ++++--------------- lib/WeBWorK/ContentGenerator/Grades.pm | 32 +++++--- lib/WeBWorK/ContentGenerator/ProblemSet.pm | 6 +- lib/WeBWorK/Utils/Sets.pm | 62 ++++++++++++++++ .../ContentGenerator/GatewayQuiz.html.ep | 4 +- 6 files changed, 128 insertions(+), 75 deletions(-) diff --git a/lib/WeBWorK/Authen/Proctor.pm b/lib/WeBWorK/Authen/Proctor.pm index 46d655104a..c72ecede03 100644 --- a/lib/WeBWorK/Authen/Proctor.pm +++ b/lib/WeBWorK/Authen/Proctor.pm @@ -10,7 +10,8 @@ WeBWorK::Authen::Proctor - Authenticate gateway test proctors. use strict; use warnings; -use WeBWorK::Utils qw(x); +use WeBWorK::Utils qw(x); +use WeBWorK::Utils::Sets qw(test_version_is_complete); use constant GENERIC_ERROR_MESSAGE => x('Invalid user ID or password.'); @@ -39,6 +40,29 @@ sub verify { # A user with the proctor_quiz_login permission does not need separate proctor authorization to enter # (or view) a proctored test. They can simply use their own login. return 1 if $c->authz->hasPermissions($c->authen->{user_id}, 'proctor_quiz_login'); + + # Proctor authorization is not needed to view a version that is not a proctored test or that is complete. For + # the proctored routes this method is called twice in a request, once during authentication and again by + # Authz::checkSet, so the result is saved for the second call. The set version is also saved in the Authz + # merged set cache so that Authz::checkSet does not need to obtain it again from the database. + if (defined $c->stash('versionID')) { + $c->{proctorNotRequired} //= do { + my $setVersion = $c->authz->{merged_set}; + unless ($setVersion + && $setVersion->set_id eq $c->stash('setID') + && $setVersion->user_id eq $c->param('effectiveUser') + && ($setVersion->version_id // 0) == $c->stash('versionID')) + { + $setVersion = $c->db->getMergedSetVersion($c->param('effectiveUser'), + $c->stash('setID'), $c->stash('versionID')); + $c->authz->{merged_set} = $setVersion if $setVersion; + } + $setVersion + && ($setVersion->assignment_type ne 'proctored_gateway' + || test_version_is_complete($c, $setVersion)) ? 1 : 0; + }; + return 1 if $c->{proctorNotRequired}; + } } return $self->SUPER::verify(@_); diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index c4091d5b24..637f73819b 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -113,9 +113,10 @@ sub can_showSolutions ($c, $user, $permissionLevel, $effectiveUser, $set, $probl return $c->can_showCorrectAnswers($user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet); } -# Allow for a version_last_attempt_time which is the time the set was submitted. If that is present we use that instead -# of the current time to decide if answers can be recorded. This deals with the time between the submission time and -# the proctor authorization. +# When answers are being submitted for a proctored test, allow for a version_last_attempt_time which is the time the set +# was submitted. If that is present we use that instead of the current time to decide if answers can be recorded. This +# deals with the time between the submission time and the proctor authorization. Otherwise the current time must be +# used, since a proctored test version is never converted to an unproctored test. # IMPORTANT: This method is called directly by the LoginProctor.pm package without a proper GatewayQuiz object, and so # it can not call any of the object methods in the GatewayQuiz package or use anything that is not set for a generic # ContentGenerator object. Also make sure that the conditions for recording an unsubmitted test are kept in sync with @@ -142,7 +143,7 @@ sub can_recordAnswers ($c, $user, $permissionLevel, $effectiveUser, $set, $probl } my $submitTime = - ($set->assignment_type eq 'proctored_gateway' && $set->version_last_attempt_time) + ($c->param('submitAnswers') && $set->assignment_type eq 'proctored_gateway' && $set->version_last_attempt_time) ? $set->version_last_attempt_time : $c->submitTime; @@ -170,9 +171,9 @@ sub can_recordAnswers ($c, $user, $permissionLevel, $effectiveUser, $set, $probl return 0; } -# Allow for a version_last_attempt_time which is the time the set was submitted. If that is present, then use that -# instead of the current time to decide if answers can be checked. This deals with the time between the submission time -# and the proctor authorization. +# When answers are being submitted for a proctored test, allow for a version_last_attempt_time which is the time the set +# was submitted. If that is present, then use that instead of the current time to decide if answers can be checked. +# This deals with the time between the submission time and the proctor authorization. sub can_checkAnswers ($c, $user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet, $submitAnswers = 0) { my $authz = $c->authz; @@ -181,7 +182,7 @@ sub can_checkAnswers ($c, $user, $permissionLevel, $effectiveUser, $set, $proble && !$authz->hasPermissions($user->user_id, 'can_check_and_submit_answers'); my $submitTime = - ($set->assignment_type eq 'proctored_gateway' && $set->version_last_attempt_time) + ($c->param('submitAnswers') && $set->assignment_type eq 'proctored_gateway' && $set->version_last_attempt_time) ? $set->version_last_attempt_time : $c->submitTime; @@ -1141,62 +1142,18 @@ async sub initialize ($c) { $c->{scoreRecordedMessage} = \@scoreRecordedMessage; $c->{ltiGradePassbackMessage} = $ltiGradePassbackMessage; - # Additional set-level database manipulation: We want to save the time that a set was submitted, and for proctored - # tests we want to reset the assignment type after a set is submitted for the last time so that it's possible to - # look at it later without getting proctor authorization. + # Additional set-level database manipulation: Save the time that a set was submitted if the answers are being + # recorded, or if the first submission occurs after the due date. if ( - ( - $c->{submitAnswers} - && ( - $will{recordAnswers} - || (!$set->version_last_attempt_time - && $c->submitTime > $set->due_date + $ce->{gatewayGracePeriod}) - ) - ) - || ( - $set->assignment_type eq 'proctored_gateway' - && ( - ($userID eq $effectiveUserID && !$can{recordAnswersNextTime}) - || ( - $userID ne $effectiveUserID - && $authz->hasPermissions($userID, 'record_answers_when_acting_as_student') - && $set->attempts_per_version > 0 - && ($problem->num_correct + $problem->num_incorrect + ($c->{submitAnswers} ? 1 : 0) >= - $set->attempts_per_version) - ) - ) - ) + $c->{submitAnswers} + && ($will{recordAnswers} + || (!$set->version_last_attempt_time && $c->submitTime > $set->due_date + $ce->{gatewayGracePeriod})) ) { - # Save the submission time if we're recording the answer, or if the first submission occurs after the due_date. - $set->version_last_attempt_time($timeNowInt) - if ( - $c->{submitAnswers} - && ( - $will{recordAnswers} - || (!$set->version_last_attempt_time - && $c->submitTime > $set->due_date + $ce->{gatewayGracePeriod}) - ) - ); - - $set->assignment_type('gateway') - if ( - $set->assignment_type eq 'proctored_gateway' - && ( - ($userID eq $effectiveUserID && !$can{recordAnswersNextTime}) - || ( - $userID ne $effectiveUserID - && $can{recordAnswers} - && $set->attempts_per_version > 0 - && ($problem->num_correct + $problem->num_incorrect + ($c->{submitAnswers} ? 1 : 0) >= - $set->attempts_per_version) - ) - ) - ); + $set->version_last_attempt_time($timeNowInt); # Save only parameters that determine access to the set version. my $cleanSet = $db->getSetVersion($effectiveUserID, $set->set_id, $versionID); - $cleanSet->assignment_type($set->assignment_type); $cleanSet->version_last_attempt_time($set->version_last_attempt_time); $db->putSetVersion($cleanSet); } diff --git a/lib/WeBWorK/ContentGenerator/Grades.pm b/lib/WeBWorK/ContentGenerator/Grades.pm index 827c53f4b6..501ef2dfde 100644 --- a/lib/WeBWorK/ContentGenerator/Grades.pm +++ b/lib/WeBWorK/ContentGenerator/Grades.pm @@ -12,9 +12,14 @@ use Mojo::Util qw(xml_escape); use WeBWorK::Utils qw(wwRound); use WeBWorK::Utils::DateTime qw(before); use WeBWorK::Utils::JITAR qw(jitar_id_to_seq); -use WeBWorK::Utils::Sets qw(grade_set format_set_name_display restricted_set_message); use WeBWorK::Utils::ProblemProcessing qw(compute_unreduced_score); use WeBWorK::HTML::StudentNav qw(studentNav); +use WeBWorK::Utils::Sets qw( + grade_set + format_set_name_display + restricted_set_message + test_version_is_complete +); use WeBWorK::Localize; sub initialize ($c) { @@ -242,22 +247,22 @@ sub displayStudentGrades ($c) { next; } - # Only show link if the problems can be seen, and it is not a proctored test - # since students cannot directly access them until after it is graded, at - # which time it reverts to a normal test. + my $hideScore = defined $set->hide_score + && ($set->hide_score eq 'Y' || $set->hide_score eq 'BeforeAnswerDate' && time < $set->answer_date); + + # Only show link if the problems can be seen, and it is not an incomplete proctored test version, since + # students cannot access those without proctor authorization. If the score is shown, then the problems + # for the version are obtained below and are used to determine if the version is complete. $item->{link} = '' - if $set->assignment_type eq 'proctored_gateway' - || ( + if ( defined $set->hide_work && ($set->hide_work eq 'Y' || $set->hide_work eq 'BeforeAnswerDate' && time < $set->answer_date) - ); + ) + || ($hideScore && $set->assignment_type eq 'proctored_gateway' && !test_version_is_complete($c, $set)); # If the set has hide_score set, then nothing left to do. - next - if defined $set->hide_score - && ($set->hide_score eq 'Y' - || $set->hide_score eq 'BeforeAnswerDate' && time < $set->answer_date); + next if $hideScore; # This is a test version, and the scores can be shown, so also show score of template set. $allItems{$setTemplateID}{message} = '' if $allItems{$setTemplateID}{hide_score}; @@ -268,6 +273,11 @@ sub displayStudentGrades ($c) { grade_set($db, $set, $studentID, $setIsVersioned, 1); $total_right = wwRound(2, $total_right); + $item->{link} = '' + if $setIsVersioned + && $set->assignment_type eq 'proctored_gateway' + && !test_version_is_complete($c, $set, $problem_records->[0]); + # Save set grades. $item->{grade_total} = $total; $item->{grade_total_right} = $total_right; diff --git a/lib/WeBWorK/ContentGenerator/ProblemSet.pm b/lib/WeBWorK/ContentGenerator/ProblemSet.pm index 1c720a9f71..af01ba4251 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemSet.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemSet.pm @@ -13,7 +13,7 @@ use WeBWorK::Utils qw(wwRound); use WeBWorK::Utils::DateTime qw(after); use WeBWorK::Utils::Files qw(path_is_subdir); use WeBWorK::Utils::Rendering qw(renderPG); -use WeBWorK::Utils::Sets qw(is_restricted grade_set format_set_name_display); +use WeBWorK::Utils::Sets qw(is_restricted grade_set format_set_name_display test_version_is_complete); use WeBWorK::Localize; use WeBWorK::AchievementItems; use WeBWorK::HTML::StudentNav qw(studentNav); @@ -254,12 +254,14 @@ sub gateway_body ($c) { my $verSubmits = defined $Problem ? $Problem->num_correct + $Problem->num_incorrect : 0; my $maxSubmits = $verSet->attempts_per_version || 0; + my $proctored = $verSet->assignment_type =~ /proctored/ && !test_version_is_complete($c, $verSet, $Problem); + # Build data hash for this version. my $data = {}; $data->{id} = $set->set_id . ',v' . $verSet->version_id; $data->{version} = $verSet->version_id; $data->{start} = $c->formatDateTime($verSet->version_creation_time, $ce->{studentDateDisplayFormat}); - $data->{proctored} = $verSet->assignment_type =~ /proctored/; + $data->{proctored} = $proctored; # Display close date if this is not a timed test. my $closeText = ''; diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index a9e9f17522..228350fd43 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -17,6 +17,7 @@ our @EXPORT_OK = qw( is_restricted get_test_problem_position restricted_set_message + test_version_is_complete ); sub format_set_name_internal ($set_name) { @@ -290,6 +291,45 @@ sub restricted_set_message($c, $set, $status) { } } +sub test_version_is_complete ($c, $setVersion, $problem = undef) { + return 0 unless $setVersion->version_last_attempt_time; + + my $authz = $c->authz; + my $userID = $setVersion->user_id; + + # All problems in a test version have the same number of attempts, so the attempts for the first problem are used. + # This is determined in a closure so that the database queries are only made if this information is needed below. + my $attemptsUsed = sub { + unless ($problem) { + my ($problemID) = $c->db->listProblemVersions($userID, $setVersion->set_id, $setVersion->version_id); + return 0 unless defined $problemID; + $problem = $c->db->getProblemVersion($userID, $setVersion->set_id, $setVersion->version_id, $problemID); + return 0 unless $problem; + } + return ($problem->num_correct // 0) + ($problem->num_incorrect // 0); + }; + + if (after($setVersion->due_date + $c->ce->{gatewayGracePeriod})) { + return 0 + if $authz->hasPermissions($userID, + after($setVersion->answer_date) ? 'record_answers_after_answer_date' : 'record_answers_after_due_date'); + return $attemptsUsed->() > 0; + } + + my $attemptsPerVersion = $setVersion->attempts_per_version || 0; + my $canRecordWithAttempts = $authz->hasPermissions($userID, 'record_answers_after_open_date_with_attempts'); + my $canRecordWithoutAttempts = + $attemptsPerVersion + ? $authz->hasPermissions($userID, 'record_answers_after_open_date_without_attempts') + : $canRecordWithAttempts; + return 0 if $canRecordWithAttempts && $canRecordWithoutAttempts; + + my $attempts = $attemptsUsed->(); + return 0 unless $attempts; + my $hasAttemptsLeft = !$attemptsPerVersion || $attempts < $attemptsPerVersion; + return !($hasAttemptsLeft ? $canRecordWithAttempts : $canRecordWithoutAttempts); +} + 1; =head1 NAME @@ -389,4 +429,26 @@ Checks for and returns any restricted messages for the given set and status. C<$status> can be either 'conditional' for conditional release, or 'lti' to check for lis_source_did when using homework grade passback. +=head2 test_version_is_complete + +Usage: C + +Returns true if at least one submission has been recorded for the test version +C<$setVersion> (a merged set version), and the user the version belongs to can +no longer record answers for it, either because all attempts for the version +have been used or because time has expired. This is determined from the +permissions of that user in the same way that it is determined when the test is +submitted. + +The optional C<$problem> should be a problem version from C<$setVersion>. If it +is given, then the number of recorded attempts is taken from it. Otherwise the +first problem of the version is obtained from the database to find the number of +recorded attempts. + +This is used to determine if a proctored test version can be viewed without +proctor authorization. Since this is determined from the current data for the +version, a version that is complete is no longer complete if more attempts are +allowed or its close date is extended, and then proctor authorization is +required again. A version with no recorded submissions is never complete. + =cut diff --git a/templates/ContentGenerator/GatewayQuiz.html.ep b/templates/ContentGenerator/GatewayQuiz.html.ep index fd25dda3dd..c744c04ada 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -411,9 +411,7 @@ % } % - % # Make sure that the 'action' in the form points to the same set version. Using the assignment type (rather than - % # the current route) also gets a URL that will not require a proctor login if a proctored test has been submitted - % # for the last time, since the assignment type has already been reset to 'gateway' in that case. + % # Make sure that the 'action' in the form points to the same set version. % my $action = url_for( % $c->{set}->assignment_type =~ /proctored/ ? 'proctored_gateway_quiz_version' : 'gateway_quiz_version', % setID => $setID,