Merge pull request #2457 from Alex-Jordan/course-config-grader

add problem_grader permission
openwebwork · Aug 6, 2024 · b6322b5 · b6322b5
2 parents c9a78b7 + ed0ca36
commit b6322b5
Show file tree

Hide file tree

Showing 12 changed files with 64 additions and 36 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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/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
@@ -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 $@" }; }
 

diff --git 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')) {
 	<div class="alert alert-danger p-1 mb-0"><%= maketext('You are not authorized to grade assignments.') %></div>
 	% last;
 % }

diff --git 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
@@ -549,7 +549,7 @@
 										<i class="icon far fa-eye" data-alt="<%= maketext('View') %>"></i>
 									<% 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
@@ -186,7 +186,7 @@
 			<th><%= maketext('# of Active Students') %></th>
 			<% for (@$numActive) { %><td class="text-center"><%= $_ %></td><% } =%>
 		</tr>
-		% if ($authz->hasPermissions(param('user'), 'score_sets')) {
+		% if ($authz->hasPermissions(param('user'), 'problem_grader')) {
 			<tr>
 				<th><%= maketext('Manual Grader') %></th>
 				% for (@$problems) {

diff --git 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')) {
 			<td>
 				<%= link_to(maketext('Grade Problem') =>
 					$c->systemLink(url_for('instructor_problem_grader', problemID => $problemID))) =%>