From f78c9b37c5a92e100eecdd953e0bc58346e69ffa Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Tue, 9 Jul 2024 14:58:22 -0700 Subject: [PATCH 1/2] add problem_grader permission --- conf/defaults.config | 1 + lib/WeBWorK/ConfigValues.pm | 9 +++++++++ lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 2 +- lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm | 2 +- lib/WeBWorK/ContentGenerator/Problem.pm | 2 +- .../ContentGenerator/Instructor/ProblemGrader.html.ep | 2 +- .../Instructor/ProblemGrader/siblings.html.ep | 2 +- .../ContentGenerator/Instructor/ProblemSetDetail.html.ep | 2 +- .../ContentGenerator/Instructor/Stats/set_stats.html.ep | 2 +- .../ContentGenerator/ProblemSet/problem_list_row.html.ep | 2 +- 10 files changed, 18 insertions(+), 8 deletions(-) diff --git a/conf/defaults.config b/conf/defaults.config index 6a3521b8ef..dbebc7c6b4 100644 --- a/conf/defaults.config +++ b/conf/defaults.config @@ -793,6 +793,7 @@ $authen{admin_module} = ['WeBWorK::Authen::Basic_TheLastOption']; become_student => "professor", access_instructor_tools => "ta", score_sets => "professor", + problem_grader => "professor", send_mail => "professor", receive_feedback => ['ta', 'professor', 'admin'], diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index b7d837005c..dc0ae1ce29 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -516,6 +516,15 @@ sub getConfigValues ($ce) { doc2 => x('Only this permission level and higher get buttons for sending email to the instructor.'), type => 'permission' }, + { + var => 'permissionLevels{problem_grader}', + doc => x('Can use problem grader'), + doc2 => x( + 'This permission level and higher can use the problem grader (both the grader that is available ' + . 'on a problem page and the set-wide probelem grader).' + ), + type => 'permission' + }, { var => 'permissionLevels{record_answers_when_acting_as_student}', doc => x('Can submit answers for a student'), diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index a9b67d7813..06e2ec4ea8 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -118,7 +118,7 @@ sub can_showProblemGrader ($c, $user, $permissionLevel, $effectiveUser, $set, $p my $authz = $c->authz; return ($authz->hasPermissions($user->user_id, 'access_instructor_tools') - && $authz->hasPermissions($user->user_id, 'score_sets') + && $authz->hasPermissions($user->user_id, 'problem_grader') && $set->set_id ne 'Undefined_Set' && !$c->{invalidSet}); } diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm b/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm index 1a2d0db2aa..c23ad33e43 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm @@ -48,7 +48,7 @@ async sub initialize ($c) { unless $c->stash->{set} && $c->stash->{problem} && $authz->hasPermissions($userID, 'access_instructor_tools') - && $authz->hasPermissions($userID, 'score_sets'); + && $authz->hasPermissions($userID, 'problem_grader'); # Get all users of the set, and restrict to the sections or recitations that are allowed for the user if such # restrictions are defined. For gateway sets only get users for which versions exist. The users are sorted by diff --git a/lib/WeBWorK/ContentGenerator/Problem.pm b/lib/WeBWorK/ContentGenerator/Problem.pm index 59c65455f0..067848c758 100644 --- a/lib/WeBWorK/ContentGenerator/Problem.pm +++ b/lib/WeBWorK/ContentGenerator/Problem.pm @@ -96,7 +96,7 @@ sub can_showProblemGrader ($c, $user, $effectiveUser, $set, $problem) { my $authz = $c->authz; return ($authz->hasPermissions($user->user_id, 'access_instructor_tools') - && $authz->hasPermissions($user->user_id, 'score_sets') + && $authz->hasPermissions($user->user_id, 'problem_grader') && $set->set_id ne 'Undefined_Set' && !$c->{invalidSet}); } diff --git a/templates/ContentGenerator/Instructor/ProblemGrader.html.ep b/templates/ContentGenerator/Instructor/ProblemGrader.html.ep index f4826e550b..60a6d260bf 100644 --- a/templates/ContentGenerator/Instructor/ProblemGrader.html.ep +++ b/templates/ContentGenerator/Instructor/ProblemGrader.html.ep @@ -33,7 +33,7 @@ % last; % } % -% unless ($authz->hasPermissions(param('user'), 'score_sets')) { +% unless ($authz->hasPermissions(param('user'), 'problem_grader')) {
<%= maketext('You are not authorized to grade assignments.') %>
% last; % } diff --git a/templates/ContentGenerator/Instructor/ProblemGrader/siblings.html.ep b/templates/ContentGenerator/Instructor/ProblemGrader/siblings.html.ep index da167a24f1..c06270d982 100644 --- a/templates/ContentGenerator/Instructor/ProblemGrader/siblings.html.ep +++ b/templates/ContentGenerator/Instructor/ProblemGrader/siblings.html.ep @@ -2,7 +2,7 @@ % % unless ($set % && $authz->hasPermissions(param('user'), 'access_instructor_tools') - % && $authz->hasPermissions(param('user'), 'score_sets')) + % && $authz->hasPermissions(param('user'), 'problem_grader')) % { % last; % } diff --git a/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep b/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep index 87b6241b54..d13d74d409 100644 --- a/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep +++ b/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep @@ -549,7 +549,7 @@ <% end =%> % } - % if ($authz->hasPermissions(param('user'), 'score_sets')) { + % if ($authz->hasPermissions(param('user'), 'problem_grader')) { <%= link_to $c->systemLink(url_for( 'instructor_problem_grader', setID => $setID, diff --git a/templates/ContentGenerator/Instructor/Stats/set_stats.html.ep b/templates/ContentGenerator/Instructor/Stats/set_stats.html.ep index ebdb7e3818..547be0b391 100644 --- a/templates/ContentGenerator/Instructor/Stats/set_stats.html.ep +++ b/templates/ContentGenerator/Instructor/Stats/set_stats.html.ep @@ -186,7 +186,7 @@ <%= maketext('# of Active Students') %> <% for (@$numActive) { %><%= $_ %><% } =%> - % if ($authz->hasPermissions(param('user'), 'score_sets')) { + % if ($authz->hasPermissions(param('user'), 'problem_grader')) { <%= maketext('Manual Grader') %> % for (@$problems) { diff --git a/templates/ContentGenerator/ProblemSet/problem_list_row.html.ep b/templates/ContentGenerator/ProblemSet/problem_list_row.html.ep index 6ce9b303ae..879a244027 100644 --- a/templates/ContentGenerator/ProblemSet/problem_list_row.html.ep +++ b/templates/ContentGenerator/ProblemSet/problem_list_row.html.ep @@ -75,7 +75,7 @@ % } % # Grader % if ($authz->hasPermissions(param('user'), 'access_instructor_tools') - % && $authz->hasPermissions(param('user'), 'score_sets')) { + % && $authz->hasPermissions(param('user'), 'problem_grader')) { <%= link_to(maketext('Grade Problem') => $c->systemLink(url_for('instructor_problem_grader', problemID => $problemID))) =%> From 3f47412dbf1611a0d7c50611acd62315ef0feed5 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 10 Jul 2024 20:23:24 -0500 Subject: [PATCH 2/2] Make the problem_grader permission all that is needed to use the single problem grader. For this the `putUserProblem`, `putProblemVersion`, and `putPastAnswer` WebworkWebservice commands now have the more lax `problem_grader` permission. However, the methods themselves theck the `modify_student_data` permssion, and only allow the "status" and "comment_string" to be changed by users that do not have the `modify_student_data` permission. --- lib/WebworkWebservice.pm | 10 ++-- lib/WebworkWebservice/ProblemActions.pm | 64 +++++++++++++++---------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/lib/WebworkWebservice.pm b/lib/WebworkWebservice.pm index 862058bf06..1bd113b619 100644 --- a/lib/WebworkWebservice.pm +++ b/lib/WebworkWebservice.pm @@ -262,10 +262,12 @@ sub command_permission { setProblemTags => 'modify_tags', # WebworkWebservice::ProblemActions - getUserProblem => 'access_instructor_tools', - putUserProblem => 'modify_student_data', - putProblemVersion => 'modify_student_data', - putPastAnswer => 'modify_student_data', + getUserProblem => 'access_instructor_tools', + # Note: The modify_student_data permission is checked in the following three methods and only the status and + # comment_string can actually be modified by users with the problem_grader permission only. + putUserProblem => 'problem_grader', + putProblemVersion => 'problem_grader', + putPastAnswer => 'problem_grader', tidyPGCode => 'access_instructor_tools', convertCodeToPGML => 'access_instructor_tools', diff --git a/lib/WebworkWebservice/ProblemActions.pm b/lib/WebworkWebservice/ProblemActions.pm index 76df6216fd..d615cb2ba7 100644 --- a/lib/WebworkWebservice/ProblemActions.pm +++ b/lib/WebworkWebservice/ProblemActions.pm @@ -47,17 +47,22 @@ sub putUserProblem { my $userProblem = $db->getUserProblem($params->{user_id}, $params->{set_id}, $params->{problem_id}); if (!$userProblem) { return { text => 'User problem not found.' }; } - for ( - 'source_file', 'value', 'max_attempts', 'showMeAnother', - 'showMeAnotherCount', 'prPeriod', 'prCount', 'problem_seed', - 'status', 'attempted', 'last_answer', 'num_correct', - 'num_incorrect', 'att_to_open_children', 'counts_parent_grade', 'sub_status', - 'flags' - ) - { - $userProblem->{$_} = $params->{$_} if defined($params->{$_}); + if ($self->c->authz->hasPermissions($self->authen->{user_id}, 'modify_student_data')) { + for ( + 'source_file', 'value', 'max_attempts', 'showMeAnother', + 'showMeAnotherCount', 'prPeriod', 'prCount', 'problem_seed', + 'attempted', 'last_answer', 'num_correct', 'num_incorrect', + 'att_to_open_children', 'counts_parent_grade', 'sub_status', 'flags' + ) + { + $userProblem->{$_} = $params->{$_} if defined $params->{$_}; + } } + # The status is the only thing that users with the problem_grader permission can change. + # This method can not be called without the problem_grader permission. + $userProblem->{status} = $params->{status} if defined $params->{status}; + # Remove the needs_grading flag if the mark_graded parameter is set. $userProblem->{flags} =~ s/:needs_grading$// if $params->{mark_graded}; @@ -81,17 +86,22 @@ sub putProblemVersion { $db->getProblemVersion($params->{user_id}, $params->{set_id}, $params->{version_id}, $params->{problem_id}); if (!$problemVersion) { return { text => 'Problem version not found.' }; } - for ( - 'source_file', 'value', 'max_attempts', 'showMeAnother', - 'showMeAnotherCount', 'prPeriod', 'prCount', 'problem_seed', - 'status', 'attempted', 'last_answer', 'num_correct', - 'num_incorrect', 'att_to_open_children', 'counts_parent_grade', 'sub_status', - 'flags' - ) - { - $problemVersion->{$_} = $params->{$_} if defined($params->{$_}); + if ($self->c->authz->hasPermissions($self->authen->{user_id}, 'modify_student_data')) { + for ( + 'source_file', 'value', 'max_attempts', 'showMeAnother', + 'showMeAnotherCount', 'prPeriod', 'prCount', 'problem_seed', + 'attempted', 'last_answer', 'num_correct', 'num_incorrect', + 'att_to_open_children', 'counts_parent_grade', 'sub_status', 'flags' + ) + { + $problemVersion->{$_} = $params->{$_} if defined($params->{$_}); + } } + # The status is the only thing that users with the problem_grader permission can change. + # This method can not be called without the problem_grader permission. + $problemVersion->{status} = $params->{status} if defined $params->{status}; + # Remove the needs_grading flag if the mark_graded parameter is set. $problemVersion->{flags} =~ s/:needs_grading$// if $params->{mark_graded}; @@ -116,14 +126,20 @@ sub putPastAnswer { $pastAnswer->{user_id} = $params->{user_id} if $params->{user_id}; - for ( - 'set_id', 'problem_id', 'source_file', 'timestamp', - 'scores', 'answer_string', 'comment_string', 'problem_seed' - ) - { - $pastAnswer->{$_} = $params->{$_} if defined($params->{$_}); + if ($self->c->authz->hasPermissions($self->authen->{user_id}, 'modify_student_data')) { + for ( + 'set_id', 'problem_id', 'source_file', 'timestamp', + 'scores', 'answer_string', 'comment_string', 'problem_seed' + ) + { + $pastAnswer->{$_} = $params->{$_} if defined($params->{$_}); + } } + # The comment_string is the only thing that users with the problem_grader permission can change. + # This method can not be called without the problem_grader permission. + $pastAnswer->{comment_string} = $params->{comment_string} if defined $params->{comment_string}; + eval { $db->putPastAnswer($pastAnswer) }; if ($@) { return { text => "putPastAnswer $@" }; }