From 46f78c6388e6f979ce2d849777727b477568a788 Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Tue, 26 Nov 2024 14:26:33 -0700 Subject: [PATCH 1/7] Updates to grading tests for other users. When able to grade a test of another user, change the "Grade Test" button to "Grade Test for UserID" to make it clear the test is being graded for another user. Show the "Check Test" button along side the "Grade Test" button for instructor who can use the problem grader but also have the permission to submit the past due test. Allow instructors who can use the problem grader to grade unsubmitted tests for that user without needing the permission to submit test versions or answers for another user. This way if for some reason a student's test doesn't get submitted correctly (such as a user closing their browser without grading the test or after reaching a grade proctor screen), it can still be graded with their saved answers. Update the timer when acting as another user if it is being shown, just only suppress warnings about running out of time in this case. Update description of record_answers_when_acting_as_student to state that it can also start and grade test versions. Also include a warning about that permission should be disabled (set back to "nobody") after using it as it can interfere with tests, and should probably only be used temporarily. --- htdocs/js/GatewayQuiz/gateway.js | 26 ++++++++--------- lib/WeBWorK/ConfigValues.pm | 9 +++++- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 28 ++++++++++++++++--- .../ContentGenerator/GatewayQuiz.html.ep | 20 +++++++------ 4 files changed, 56 insertions(+), 27 deletions(-) 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/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 58955fc34e..ee5e3daa56 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, as it can interfere with tests. If you have this permission and are viewing a ' + . 'test version for a student that is also working on that version, your answers will be ' + . 'saved for that student when moving between pages, which could reset or change the answers ' + . 'entered in by the student.' + ), type => 'permission' }, { diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 36ae2cb148..1f562e0ade 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') @@ -641,7 +653,8 @@ 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)) { @@ -740,6 +753,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 +768,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; diff --git a/templates/ContentGenerator/GatewayQuiz.html.ep b/templates/ContentGenerator/GatewayQuiz.html.ep index e6d77dcd54..997e818a3e 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -233,7 +233,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} || $c->{can}{gradeUnsubmittedTest}) { % 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 +267,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!') @@ -651,11 +653,16 @@ %
<%= submit_button maketext('Preview Test'), name => 'previewAnswers', class => 'btn btn-primary mb-1' =%> - % if ($c->{can}{recordAnswersNextTime}) { + % 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} || $c->{can}{gradeUnsubmittedTest}) { <%= 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 +692,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.') %>

From 8a501033ac13fc6dc6f19dddb12ab7d093416121 Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Sun, 8 Dec 2024 10:17:54 -0700 Subject: [PATCH 2/7] Updates from review. Don't show the quiz timer for instructors who can submit student answers after the due date. Update the help language to try to better describe how dangerous it is to leave the record_answers_when_acting_as_student permission on. --- lib/WeBWorK/ConfigValues.pm | 8 ++++---- templates/ContentGenerator/GatewayQuiz.html.ep | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index ee5e3daa56..b1427ea81e 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -532,10 +532,10 @@ sub getConfigValues ($ce) { '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, as it can interfere with tests. If you have this permission and are viewing a ' - . 'test version for a student that is also working on that version, your answers will be ' - . 'saved for that student when moving between pages, which could reset or change the answers ' - . 'entered in by the student.' + . '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/templates/ContentGenerator/GatewayQuiz.html.ep b/templates/ContentGenerator/GatewayQuiz.html.ep index 997e818a3e..ed2ed65079 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -233,7 +233,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} || $c->{can}{gradeUnsubmittedTest}) { +% 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. @@ -657,7 +657,7 @@ % && (!$c->{can}{recordAnswersNextTime} || $c->{can}{showProblemGrader})) { <%= submit_button maketext('Check Test'), name => 'checkAnswers', class => 'btn btn-primary mb-1' =%> % } - % if ($c->{can}{recordAnswersNextTime} || $c->{can}{gradeUnsubmittedTest}) { + % if ($c->{can}{recordAnswersNextTime}) { <%= tag('input', type => 'submit', name => 'submitAnswers', From 10abd7908536e6eeb31ac31d4f6df487b66d8cb7 Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Mon, 9 Dec 2024 18:59:28 -0700 Subject: [PATCH 3/7] Add warning when view open test of user and can submit answers. Viewing an open gateway test while acting as a student with the permission to submit answers for that student is dangerous since the user's answers will be saved over the student's answers. In this case, give a warning to the user about the danger and suggest they disable the permission to submit answers as students before viewing the open test version, unless they plan to submit answers for that student. The warning will only appear when they first view the test version unless they back out of the test version loosing the hidden `submit_for_student_ok` parameter. --- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 24 +++++++++-- .../ContentGenerator/GatewayQuiz.html.ep | 41 +++++++++++-------- 2 files changed, 45 insertions(+), 20 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 1f562e0ade..6a007a388b 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -545,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') ) ) ) @@ -618,7 +618,8 @@ async sub pre_header_initialize ($c) { . 'the "Create New Test Version" button below. Alternatively, click "Cancel".', $effectiveUserID ); - $c->{invalidVersionCreation} = 1; + $c->{invalidVersionCreation} = 1; + $c->{confirmSubmitForStudent} = 1; } elsif ($effectiveUserID ne $userID) { $c->{invalidSet} = $c->maketext( @@ -626,7 +627,7 @@ async sub pre_header_initialize ($c) { . 'when acting as another user.', $effectiveUserID ); - $c->{invalidVersionCreation} = 2; + $c->{invalidVersionCreation} = 1; } elsif (($maxAttemptsPerVersion == 0 || $currentNumAttempts < $maxAttemptsPerVersion) && $c->submitTime < $set->due_date() + $ce->{gatewayGracePeriod}) @@ -659,6 +660,23 @@ async sub pre_header_initialize ($c) { { 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->{invalidSet} = $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->{confirmSubmitForStudent} = 1; + } } } } diff --git a/templates/ContentGenerator/GatewayQuiz.html.ep b/templates/ContentGenerator/GatewayQuiz.html.ep index ed2ed65079..fcaaaf38f2 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -68,30 +68,33 @@ % # If the set or problem 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 - ) =%> - % } -
+ % if (!$c->{confirmSubmitForStudent} || $c->{invalidVersionCreation}) { +
+ % 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 + ) =%> + % } +
+ % }
<%= $c->{invalidSet} %>
- % if ($c->{invalidVersionCreation} && $c->{invalidVersionCreation} == 1) { + % if ($c->{confirmSubmitForStudent}) {
- <%= link_to maketext('Create New Test Version') => $c->systemLink( + <%= link_to $c->{invalidVersionCreation} + ? maketext('Create New Test Version') : maketext('View Test Version') => $c->systemLink( url_for, - params => { effectiveUser => $effectiveUserID, user => $userID, createnew_ok => 1 } + 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), + url_for('problem_list', setID => $setID =~ s/,v\d+$//r), params => { effectiveUser => $effectiveUserID, user => $userID } ), class => 'btn btn-primary' @@ -410,6 +413,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) { From 54d6c4aaacbd33f7ff088d30a87253633a68d18f Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Thu, 12 Dec 2024 14:26:12 -0700 Subject: [PATCH 4/7] Rework confirmation to creating/recording answers when acting. When acting as another user with permissions to create or record answers as that user, rework how the confirmation message is stored. Instead of piggy backing off of the `invalidSet` key, use a new `actingConformation` key for this case. This also skips deleting any proctor authorization key while waiting for user confirmation to continue. --- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 26 +++---- .../ContentGenerator/GatewayQuiz.html.ep | 70 ++++++++++--------- 2 files changed, 47 insertions(+), 49 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 6a007a388b..5b156d7b98 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -609,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->{actingConformation} = $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 ' @@ -618,16 +617,11 @@ async sub pre_header_initialize ($c) { . 'the "Create New Test Version" button below. Alternatively, click "Cancel".', $effectiveUserID ); - $c->{invalidVersionCreation} = 1; - $c->{confirmSubmitForStudent} = 1; + $c->{actingConformationCreate} = 1; + return; } 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} = 1; + $c->{actingCreationError} = 1; } elsif (($maxAttemptsPerVersion == 0 || $currentNumAttempts < $maxAttemptsPerVersion) && $c->submitTime < $set->due_date() + $ce->{gatewayGracePeriod}) @@ -665,7 +659,7 @@ async sub pre_header_initialize ($c) { # 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->{invalidSet} = $c->maketext( + $c->{actingConformation} = $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. / @@ -675,7 +669,7 @@ async sub pre_header_initialize ($c) { . 'before viewing open test versions.', $effectiveUserID ); - $c->{confirmSubmitForStudent} = 1; + return; } } } @@ -694,8 +688,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}; } @@ -1368,7 +1362,7 @@ 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->{actingConformation} ? ($setID => '') : ( $c->{set}->set_id => $c->url_for('problem_list', setID => $c->{set}->set_id), @@ -1382,7 +1376,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->{actingConformation}; # 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 fcaaaf38f2..7b251f1f24 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -65,42 +65,46 @@ % 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->{confirmSubmitForStudent} || $c->{invalidVersionCreation}) { -
- % 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->{confirmSubmitForStudent}) { -
- <%= link_to $c->{invalidVersionCreation} - ? maketext('Create New Test Version') : maketext('View Test Version') => $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; +% } +% # 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 ($c->{actingConformation}) { +
+
<%= $c->{actingConformation} =%>
+
+ <%= link_to $c->{actingConformationCreate} + ? maketext('Create New Test Version') : maketext('View Test Version') => $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; From b005d6606c5d454c574c8516eee7ddeb84e9bd0e Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Thu, 12 Dec 2024 16:08:29 -0700 Subject: [PATCH 5/7] Store acting confirmation in session for proctor logins. Store the confirmation state when creating or viewing an open test version for another user with appropriate permissions in the session. This is used to confirm prior proctor authentication and not ask for a second after user confirmation. Credit to drgrice1. --- lib/WeBWorK.pm | 1 + lib/WeBWorK/Authen/Proctor.pm | 13 +++++++++--- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 21 ++++++++++++------- .../ContentGenerator/GatewayQuiz.html.ep | 7 +++---- 4 files changed, 28 insertions(+), 14 deletions(-) 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/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 5b156d7b98..1171fe745d 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -609,7 +609,7 @@ async sub pre_header_initialize ($c) { || $authz->hasPermissions($userID, 'create_new_set_version_when_acting_as_student')) ) { - $c->{actingConformation} = $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 ' @@ -617,8 +617,7 @@ async sub pre_header_initialize ($c) { . 'the "Create New Test Version" button below. Alternatively, click "Cancel".', $effectiveUserID ); - $c->{actingConformationCreate} = 1; - return; + $c->stash->{actingConfirmationButton} = $c->maketext('Create New Test Version'); } elsif ($effectiveUserID ne $userID) { $c->{actingCreationError} = 1; @@ -659,7 +658,7 @@ async sub pre_header_initialize ($c) { # 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->{actingConformation} = $c->maketext( + $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. / @@ -669,7 +668,7 @@ async sub pre_header_initialize ($c) { . 'before viewing open test versions.', $effectiveUserID ); - return; + $c->stash->{actingConfirmationButton} = $c->maketext('View Test Version'); } } } @@ -688,6 +687,13 @@ async sub pre_header_initialize ($c) { else { delete $c->authen->session->{proctor_authorization_granted}; } } + 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 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') { @@ -1362,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} || $c->{actingCreationError} || $c->{actingConformation} + $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), @@ -1376,7 +1383,7 @@ sub nav ($c, $args) { my $userID = $c->param('user'); my $effectiveUserID = $c->param('effectiveUser'); - return '' if $c->{invalidSet} || $c->{actingCreationError} || $c->{actingConformation}; + 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 7b251f1f24..52ab0a94bb 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -87,12 +87,11 @@ % last; % } % # Get confirmation before creating new test version or working on an open test for another user. -% if ($c->{actingConformation}) { +% if ($actingConfirmation) {
-
<%= $c->{actingConformation} =%>
+
<%= $actingConfirmation =%>
- <%= link_to $c->{actingConformationCreate} - ? maketext('Create New Test Version') : maketext('View Test Version') => $c->systemLink( + <%= link_to $actingConfirmationButton => $c->systemLink( url_for, params => { effectiveUser => $effectiveUserID, user => $userID, submit_for_student_ok => 1 } ), From 129631e95c2ca9e4c554dec7feac79985e10ed85 Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Thu, 12 Dec 2024 17:01:55 -0700 Subject: [PATCH 6/7] Use stash to access actingConfirmation variables. --- templates/ContentGenerator/GatewayQuiz.html.ep | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/ContentGenerator/GatewayQuiz.html.ep b/templates/ContentGenerator/GatewayQuiz.html.ep index 52ab0a94bb..b575931f0b 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -87,11 +87,11 @@ % last; % } % # Get confirmation before creating new test version or working on an open test for another user. -% if ($actingConfirmation) { +% if (stash->{actingConfirmation}) {
-
<%= $actingConfirmation =%>
+
<%= stash->{actingConfirmation} =%>
- <%= link_to $actingConfirmationButton => $c->systemLink( + <%= link_to stash->{actingConfirmationButton} => $c->systemLink( url_for, params => { effectiveUser => $effectiveUserID, user => $userID, submit_for_student_ok => 1 } ), From f0aca962a1c34a7efca64af211040ba5f55f170c Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Thu, 12 Dec 2024 17:08:54 -0700 Subject: [PATCH 7/7] Don't delete proctor session when waiting for acting proctor confirmation. --- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 1171fe745d..9dfa6f5c65 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -678,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') @@ -687,13 +694,6 @@ async sub pre_header_initialize ($c) { else { delete $c->authen->session->{proctor_authorization_granted}; } } - 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 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') {