diff --git a/htdocs/js/GatewayQuiz/gateway.js b/htdocs/js/GatewayQuiz/gateway.js index 4aecb3f37e..365389560c 100644 --- a/htdocs/js/GatewayQuiz/gateway.js +++ b/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); } } diff --git a/lib/WeBWorK.pm b/lib/WeBWorK.pm index 319ea19c82..95499668ca 100644 --- a/lib/WeBWorK.pm +++ b/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 { diff --git a/lib/WeBWorK/Authen/Proctor.pm b/lib/WeBWorK/Authen/Proctor.pm index 2cd91db890..272098df2b 100644 --- a/lib/WeBWorK/Authen/Proctor.pm +++ b/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 { diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 58955fc34e..b1427ea81e 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/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' }, { diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 36ae2cb148..9dfa6f5c65 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/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,8 +609,7 @@ 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 ' @@ -606,15 +617,10 @@ async sub pre_header_initialize ($c) { . '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) { diff --git a/templates/ContentGenerator/GatewayQuiz.html.ep b/templates/ContentGenerator/GatewayQuiz.html.ep index e6d77dcd54..b575931f0b 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/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}) {
- % 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) =%>
<%= $c->{invalidSet} %>
- % if ($c->{invalidVersionCreation} && $c->{invalidVersionCreation} == 1) { -
- <%= 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' - =%> -
- % } +
+ % last; +% } +% # If user tried to create a test version for another user without correct permissions, show message and exit. +% if ($c->{actingCreationError}) { +
+ <%= 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 + ) =%> +
+ % last; +% } +% # Get confirmation before creating new test version or working on an open test for another user. +% if (stash->{actingConfirmation}) { +
+
<%= stash->{actingConfirmation} =%>
+
+ <%= 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' + =%> +
% % 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')) {
<%= maketext('You have less than 1 minute to complete this test.') %>
- % } 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')) {
<%= 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 @@ %
<%= 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' =%> - % }
% if ($numProbPerPage && $numPages > 1 && $c->{can}{recordAnswersNextTime}) {

<%= maketext('Note: grading the test grades all problems, not just those on this page.') %>