Determine when a proctored test version is complete instead of changing its type.

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(@_);

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);
 	}

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;

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 = '';

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<test_version_is_complete($c, $setVersion, $problem)>
+
+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

templates/ContentGenerator/GatewayQuiz.html.ep

@@ -411,9 +411,7 @@
 		</div>
 	% }
 	%
-	% # 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,