Merge pull request #2641 from somiaj/grade-test-for-others

Updates to grading tests for other users.

Author: Alex-Jordan

htdocs/js/GatewayQuiz/gateway.js

@@ -158,22 +158,20 @@
 
 		const remainingTime = serverDueTime - browserTime + timeDelta;
 
-		if (!timerDiv.dataset.acting) {
-			if (remainingTime <= 10 - gracePeriod) {
-				if (sessionStorage.getItem('gatewayAlertStatus')) {
-					sessionStorage.removeItem('gatewayAlertStatus');
-
-					// Submit the test if time is expired and near the end of grace period.
-					actuallySubmit = true;
-					submitAnswers.click();
-				}
-			} else {
-				// Set the timer text and check alerts at page load.
-				updateTimer();
+		if (!timerDiv.dataset.acting && remainingTime <= 10 - gracePeriod) {
+			if (sessionStorage.getItem('gatewayAlertStatus')) {
+				sessionStorage.removeItem('gatewayAlertStatus');
 
-				// Start the timer.
-				setInterval(updateTimer, 1000);
+				// Submit the test if time is expired and near the end of grace period.
+				actuallySubmit = true;
+				submitAnswers.click();
 			}
+		} else {
+			// Set the timer text and check alerts at page load.
+			updateTimer();
+
+			// Start the timer.
+			setInterval(updateTimer, 1000);
 		}
 	}
 

lib/WeBWorK.pm

@@ -217,6 +217,7 @@ async sub dispatch ($c) {
 				# current server time during a gateway quiz, and that definitely should not revoke proctor
 				# authorization.
 				delete $c->authen->session->{proctor_authorization_granted};
+				delete $c->authen->session->{acting_proctor};
 			}
 			return 1;
 		} else {

lib/WeBWorK/Authen/Proctor.pm

@@ -96,10 +96,17 @@ sub verify_normal_user {
 	# is 'No', then the verify method will have returned 1, and this never happens.  For an ongoing login session, only
 	# a key with versioned set information is accepted, and that version must match the requested set version.  The set
 	# id will not have a version when opening a new version. For that new proctor credentials are required.
-	if ($self->{login_type} eq 'proctor_login'
-		&& $c->stash('setID') =~ /,v\d+$/
+	if (
+		$self->{login_type} eq 'proctor_login'
 		&& $c->authen->session('proctor_authorization_granted')
-		&& $c->authen->session('proctor_authorization_granted') eq $c->stash('setID'))
+		&& (
+			(
+				$c->stash('setID') =~ /,v\d+$/
+				&& $c->authen->session('proctor_authorization_granted') eq $c->stash('setID')
+			)
+			|| $c->authen->session('acting_proctor')
+		)
+		)
 	{
 		return 1;
 	} else {

lib/WeBWorK/ConfigValues.pm

@@ -529,7 +529,14 @@ sub getConfigValues ($ce) {
 				var  => 'permissionLevels{record_answers_when_acting_as_student}',
 				doc  => x('Can submit answers for a student'),
 				doc2 => x(
-					'When acting as a student, this permission level and higher can submit answers for that student.'),
+					'When acting as a student, this permission level and higher can submit answers for that student, '
+						. 'which includes starting and grading test versions.  This permission should only be turned '
+						. 'on temporarily and set back to "nobody" after you are done submitting answers for a '
+						. 'student.  Leaving this permission on is dangerous, as you could unintentionally submit '
+						. 'answers for a student, which can use up their total number of attempts.  Further, if you '
+						. 'are viewing an open test version, your answers on each page will be saved when you move '
+						. q/between pages, which will overwrite the student's saved answers./
+				),
 				type => 'permission'
 			},
 			{

lib/WeBWorK/ContentGenerator/GatewayQuiz.pm

@@ -141,9 +141,12 @@ sub can_recordAnswers ($c, $user, $permissionLevel, $effectiveUser, $set, $probl
 
 	if ($user->user_id ne $effectiveUser->user_id) {
 		# If the user is not allowed to record answers as another user, return that permission.  If the user is allowed
-		# to record only set version answers, then allow that between the open and close dates, and so drop out of this
-		# conditional to the usual one.
-		return 1 if $authz->hasPermissions($user->user_id,  'record_answers_when_acting_as_student');
+		# to record an unsubmitted test, allow that.  If the user is allowed to record only set version answers, then
+		# allow that between the open and close dates, and so drop out of this conditional to the usual one.
+		return 1
+			if $authz->hasPermissions($user->user_id, 'record_answers_when_acting_as_student')
+			|| $c->can_gradeUnsubmittedTest($user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet,
+				$submitAnswers);
 		return 0 if !$authz->hasPermissions($user->user_id, 'record_set_version_answers_when_acting_as_student');
 	}
 
@@ -224,6 +227,15 @@ sub can_checkAnswers ($c, $user, $permissionLevel, $effectiveUser, $set, $proble
 	return 0;
 }
 
+# If user can use the problem grader, and the test is past due and has not been submitted, allow them to submit.
+sub can_gradeUnsubmittedTest ($c, $user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet, $submitAnswers = 0)
+{
+	return
+		!$submitAnswers
+		&& $c->can_showProblemGrader($user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet)
+		&& (after($set->due_date + $c->ce->{gatewayGracePeriod}) && !$set->version_last_attempt_time);
+}
+
 sub can_showScore ($c, $user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet) {
 	return
 		$c->authz->hasPermissions($user->user_id, 'view_hidden_work')
@@ -533,7 +545,7 @@ async sub pre_header_initialize ($c) {
 							$authz->hasPermissions($userID, 'record_answers_when_acting_as_student')
 							|| $authz->hasPermissions($userID, 'create_new_set_version_when_acting_as_student')
 						)
-						&& $c->param('createnew_ok')
+						&& $c->param('submit_for_student_ok')
 					)
 				)
 				)
@@ -597,24 +609,18 @@ async sub pre_header_initialize ($c) {
 					|| $authz->hasPermissions($userID, 'create_new_set_version_when_acting_as_student'))
 				)
 			{
-
-				$c->{invalidSet} = $c->maketext(
+				$c->stash->{actingConfirmation} = $c->maketext(
 					'You are acting as user [_1].  If you continue, you will create a new version of '
 						. 'this test for that user, which will count against their allowed maximum '
 						. 'number of versions for the current time interval.  In general, this is not '
 						. 'what you want to do.  Please be sure that you want to do this before clicking '
 						. 'the "Create New Test Version" button below.  Alternatively, click "Cancel".',
 					$effectiveUserID
 				);
-				$c->{invalidVersionCreation} = 1;
+				$c->stash->{actingConfirmationButton} = $c->maketext('Create New Test Version');
 
 			} elsif ($effectiveUserID ne $userID) {
-				$c->{invalidSet} = $c->maketext(
-					'You are acting as user [_1], and do not have the permission to create a new test version '
-						. 'when acting as another user.',
-					$effectiveUserID
-				);
-				$c->{invalidVersionCreation} = 2;
+				$c->{actingCreationError} = 1;
 
 			} elsif (($maxAttemptsPerVersion == 0 || $currentNumAttempts < $maxAttemptsPerVersion)
 				&& $c->submitTime < $set->due_date() + $ce->{gatewayGracePeriod})
@@ -641,11 +647,29 @@ async sub pre_header_initialize ($c) {
 			if (
 				($currentNumAttempts < $maxAttemptsPerVersion)
 				&& ($effectiveUserID eq $userID
-					|| $authz->hasPermissions($userID, 'record_set_version_answers_when_acting_as_student'))
+					|| $authz->hasPermissions($userID, 'record_set_version_answers_when_acting_as_student')
+					|| $authz->hasPermissions($userID, 'record_answers_when_acting_as_student'))
 				)
 			{
 				if (between($set->open_date(), $set->due_date() + $ce->{gatewayGracePeriod}, $c->submitTime)) {
 					$versionIsOpen = 1;
+
+					# If acting as another user, then the user has permissions to record answers for the
+					# student which is dangerous for open test versions. Give a warning unless the user
+					# has already confirmed they understand the risk.
+					if ($effectiveUserID ne $userID && !$c->param('submit_for_student_ok')) {
+						$c->stash->{actingConfirmation} = $c->maketext(
+							'You are trying to view an open test version for [_1] and have the permission to submit '
+								. 'answers for that user.  This is dangerous, as your answers can overwrite the '
+								. q/student's answers as you move between test pages, preview, or check answers.  /
+								. 'If you are planing to submit answers for this student, click "View Test Version" '
+								. 'below to continue.  If you only want to view the test version, click "Cancel" '
+								. 'below, then disable the permission to record answers when acting as a student '
+								. 'before viewing open test versions.',
+							$effectiveUserID
+						);
+						$c->stash->{actingConfirmationButton} = $c->maketext('View Test Version');
+					}
 				}
 			}
 		}
@@ -654,6 +678,13 @@ async sub pre_header_initialize ($c) {
 		$c->{invalidSet} = $c->maketext('This test is closed.  No new test versions may be taken.');
 	}
 
+	if ($c->stash->{actingConfirmation}) {
+		# Store session while waiting for confirmation for proctored tests.
+		$c->authen->session(acting_proctor => 1) if $c->{assignment_type} eq 'proctored_gateway';
+		return;
+	}
+	delete $c->authen->session->{acting_proctor};
+
 	# If the proctor session key does not have a set version id, then add it.  This occurs when a student
 	# initially enters a proctored test, since the version id is not determined until just above.
 	if ($c->authen->session('proctor_authorization_granted')
@@ -663,8 +694,8 @@ async sub pre_header_initialize ($c) {
 		else                   { delete $c->authen->session->{proctor_authorization_granted}; }
 	}
 
-	# If the set or problem is invalid, then delete any proctor session keys and return.
-	if ($c->{invalidSet}) {
+	# If the set is invalid, then delete any proctor session keys and return.
+	if ($c->{invalidSet} || $c->{actingCreationError}) {
 		if (defined $c->{assignment_type} && $c->{assignment_type} eq 'proctored_gateway') {
 			delete $c->authen->session->{proctor_authorization_granted};
 		}
@@ -740,6 +771,7 @@ async sub pre_header_initialize ($c) {
 		checkAnswers          => $c->can_checkAnswers(@args),
 		recordAnswersNextTime => $c->can_recordAnswers(@args, $c->{submitAnswers}),
 		checkAnswersNextTime  => $c->can_checkAnswers(@args, $c->{submitAnswers}),
+		gradeUnsubmittedTest  => $c->can_gradeUnsubmittedTest(@args, $c->{submitAnswers}),
 		showScore             => $c->can_showScore(@args),
 		showProblemScores     => $c->can_showProblemScores(@args),
 		showWork              => $c->can_showWork(@args),
@@ -754,6 +786,12 @@ async sub pre_header_initialize ($c) {
 	$c->{can}  = \%can;
 	$c->{will} = \%will;
 
+	# Issue a warning if a test has not been submitted, but can still be graded by the instructor.
+	$c->addbadmessage(
+		$c->maketext(
+			'This test version is past due, but has not been graded. You can still grade the test for this user.')
+	) if $can{gradeUnsubmittedTest} && $userID ne $effectiveUserID;
+
 	# Set up problem numbering and multipage variables.
 
 	my @problemNumbers;
@@ -1330,7 +1368,8 @@ sub path ($c, $args) {
 		$args,
 		'WeBWorK'   => $navigation_allowed ? $c->url_for('root')     : '',
 		$courseName => $navigation_allowed ? $c->url_for('set_list') : '',
-		$setID eq 'Undefined_Set' || $c->{invalidSet}
+		$setID eq 'Undefined_Set'
+			|| $c->{invalidSet} || $c->{actingCreationError} || $c->stash->{actingConfirmation}
 		? ($setID => '')
 		: (
 			$c->{set}->set_id           => $c->url_for('problem_list', setID => $c->{set}->set_id),
@@ -1344,7 +1383,7 @@ sub nav ($c, $args) {
 	my $userID          = $c->param('user');
 	my $effectiveUserID = $c->param('effectiveUser');
 
-	return '' if $c->{invalidSet};
+	return '' if $c->{invalidSet} || $c->{actingCreationError} || $c->stash->{actingConfirmation};
 
 	# Set up and display a student navigation for those that have permission to act as a student.
 	if ($c->authz->hasPermissions($userID, 'become_student') && $effectiveUserID ne $userID) {

templates/ContentGenerator/GatewayQuiz.html.ep

@@ -65,39 +65,45 @@
 % my $userID          = param('user');
 % my $effectiveUserID = param('effectiveUser');
 %
-% # If the set or problem is invalid, then show that information and exit.
+% # If the set is invalid, then show that information and exit.
 % if ($c->{invalidSet}) {
 	<div class="alert alert-danger mb-2">
 		<div class="mb-2">
-			% if ($c->{invalidVersionCreation}) {
-				<%= maketext(
-					'The selected test ([_1]) is not a valid test for [_2] (acted as by [_3]).',
-					$setID, $effectiveUserID, $userID
-				) =%>
-			% } else {
-				<%= maketext(
-					'The selected test ([_1]) is not a valid test for [_2].',
-					$setID, $effectiveUserID
-				) =%>
-			% }
+			<%= maketext('The selected test ([_1]) is not a valid test for [_2].', $setID, $effectiveUserID) =%>
 		</div>
 		<div><%= $c->{invalidSet} %></div>
-		% if ($c->{invalidVersionCreation} && $c->{invalidVersionCreation} == 1) {
-			<div class="mt-3">
-				<%= link_to maketext('Create New Test Version') => $c->systemLink(
-						url_for,
-						params => { effectiveUser => $effectiveUserID, user => $userID, createnew_ok => 1 }
-					),
-					class => 'btn btn-primary'
-				=%>
-				<%= link_to maketext('Cancel') => $c->systemLink(
-						url_for('problem_list', setID => $setID),
-						params => { effectiveUser => $effectiveUserID, user => $userID }
-					),
-					class => 'btn btn-primary'
-				=%>
-			</div>
-		% }
+	</div>
+	% last;
+% }
+% # If user tried to create a test version for another user without correct permissions, show message and exit.
+% if ($c->{actingCreationError}) {
+	<div class="alert alert-danger mb-2">
+		<%= maketext(
+			'You are acting as user [_1] and do not have the permission to create a new test version when acting '
+				. 'as another user.',
+			$effectiveUserID
+		) =%>
+	</div>
+	% last;
+% }
+% # Get confirmation before creating new test version or working on an open test for another user.
+% if (stash->{actingConfirmation}) {
+	<div class="alert alert-danger mb-2">
+		<div class="mb-2"><%= stash->{actingConfirmation} =%></div>
+		<div>
+			<%= link_to stash->{actingConfirmationButton} => $c->systemLink(
+					url_for,
+					params => { effectiveUser => $effectiveUserID, user => $userID, submit_for_student_ok => 1 }
+				),
+				class => 'btn btn-primary'
+			=%>
+			<%= link_to maketext('Cancel') => $c->systemLink(
+					url_for('problem_list', setID => $setID =~ s/,v\d+$//r),
+					params => { effectiveUser => $effectiveUserID, user => $userID }
+				),
+				class => 'btn btn-primary'
+			=%>
+		</div>
 	</div>
 	%
 	% last;
@@ -233,7 +239,7 @@
 % # Remaining output of test headers.
 % # Display timer or information about elapsed time, output link, and information about any recorded score if not
 % # submitAnswers or checkAnswers.
-% if ($c->{can}{recordAnswersNextTime}) {
+% if ($c->{can}{recordAnswersNextTime} && before($c->{set}->due_date + $ce->{gatewayGracePeriod}, $submitTime)) {
 	% my $timeLeft = $c->{set}->due_date - int($submitTime);    # This is in seconds
 	%
 	% # Print the timer if there is less than 24 hours left.
@@ -267,11 +273,13 @@
 		) =%>
 	% }
 	%
-	% if ($timeLeft < 60 && $timeLeft > 0 && !$authz->hasPermissions($userID, 'record_answers_when_acting_as_student')) {
+	% if ($timeLeft < 60 && $timeLeft > 0 && !$c->{can}{gradeUnsubmittedTest}
+			% && !$authz->hasPermissions($userID, 'record_answers_when_acting_as_student')) {
 		<div class="alert alert-danger d-inline-block mb-2 p-1">
 			<strong><%= maketext('You have less than 1 minute to complete this test.') %></strong>
 		</div>
-	% } elsif ($timeLeft <= 0 && !$authz->hasPermissions($userID, 'record_answers_when_acting_as_student')) {
+	% } elsif ($timeLeft <= 0 && !$c->{can}{gradeUnsubmittedTest} &&
+			% !$authz->hasPermissions($userID, 'record_answers_when_acting_as_student')) {
 		<div class="alert alert-danger d-inline-block mb-2 p-1">
 			<strong>
 				<%= maketext('You are out of time!')
@@ -408,6 +416,10 @@
 			<%= hidden_field newPage => '' =%>
 			<%= hidden_field currentPage => $pageNumber =%>
 		% }
+		% # Keep track that a user has confirmed it is okay to submit for a student.
+		% if (param('submit_for_student_ok')) {
+			<%= hidden_field submit_for_student_ok => 1 =%>
+		% }
 		%
 		% # Set up links between problems and, for multi-page tests, pages.
 		% for my $i (0 .. $#$pg_results) {
@@ -651,11 +663,16 @@
 		%
 		<div class="submit-buttons-container col-12 mb-2">
 			<%= submit_button maketext('Preview Test'), name => 'previewAnswers', class => 'btn btn-primary mb-1' =%>
+			% if ($c->{can}{checkAnswersNextTime}
+					% && (!$c->{can}{recordAnswersNextTime} || $c->{can}{showProblemGrader})) {
+				<%= submit_button maketext('Check Test'), name => 'checkAnswers', class => 'btn btn-primary mb-1' =%>
+			% }
 			% if ($c->{can}{recordAnswersNextTime}) {
 				<%= tag('input',
 					type  => 'submit',
 					name  => 'submitAnswers',
-					value => maketext('Grade Test'),
+					value => $effectiveUserID ne $userID
+						? maketext('Grade Test for [_1]', $effectiveUserID) : maketext('Grade Test'),
 					class => 'btn btn-primary mb-1',
 					$c->{set}->attempts_per_version
 					? (
@@ -685,9 +702,6 @@
 					: ()
 				) =%>
 			% }
-			% if ($c->{can}{checkAnswersNextTime} && !$c->{can}{recordAnswersNextTime}) {
-				<%= submit_button maketext('Check Test'), name => 'checkAnswers', class => 'btn btn-primary mb-1' =%>
-			% }
 		</div>
 		% if ($numProbPerPage && $numPages > 1 && $c->{can}{recordAnswersNextTime}) {
 			<p><em><%= maketext('Note: grading the test grades all problems, not just those on this page.') %></em></p>