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,