Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to grading tests for other users. #2641

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions htdocs/js/GatewayQuiz/gateway.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
1 change: 1 addition & 0 deletions lib/WeBWorK.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 10 additions & 3 deletions lib/WeBWorK/Authen/Proctor.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 8 additions & 1 deletion lib/WeBWorK/ConfigValues.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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./
),
drgrice1 marked this conversation as resolved.
Show resolved Hide resolved
type => 'permission'
},
{
Expand Down
75 changes: 57 additions & 18 deletions lib/WeBWorK/ContentGenerator/GatewayQuiz.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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')
)
)
)
Expand Down Expand Up @@ -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})
Expand All @@ -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');
}
}
}
}
Expand All @@ -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')
Expand All @@ -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};
}
Expand Down Expand Up @@ -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),
Expand All @@ -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;
Expand Down Expand Up @@ -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),
Expand All @@ -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) {
Expand Down
84 changes: 49 additions & 35 deletions templates/ContentGenerator/GatewayQuiz.html.ep
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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!')
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
? (
Expand Down Expand Up @@ -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>
Expand Down