Skip to content

Commit

Permalink
MDL-68676 mod_assign: check accessallgroups in can_edit_submission()
Browse files Browse the repository at this point in the history 
In can_edit_submission() when the assign group mode is SEPARATEGROUPS
check if the user has the moodle/site:accessallgroups capability before
checking if they are a shared groupmember. Due to the teamsubmission
setting students might not be in a group but can still submit, which
means they are not returned in get_shared_group_members().
  • Loading branch information
@rhell4
rhell4 committed Jun 22, 2024
1 parent 2bf886f commit 1b5cc6b
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 9 deletions.
3 changes: 2 additions & 1 deletion mod/assign/locallib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6327,7 +6327,8 @@ public function can_edit_submission($userid, $graderid = 0) {
}

$cm = $this->get_course_module();
if (groups_get_activity_groupmode($cm) == SEPARATEGROUPS) {
if (groups_get_activity_groupmode($cm) == SEPARATEGROUPS &&
!has_capability('moodle/site:accessallgroups', $this->context, $graderid)) {
$sharedgroupmembers = $this->get_shared_group_members($cm, $graderid);
return in_array($userid, $sharedgroupmembers);
}
Expand Down
28 changes: 20 additions & 8 deletions mod/assign/tests/behat/bulk_remove_submissions.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ Feature: Bulk remove submissions
| teacher1 | Teacher | 1 | teacher1@example.com |
| student1 | Student | 1 | student1@example.com |
| student2 | Student | 2 | student2@example.com |
| student3 | Student | 3 | student3@example.com |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
| student1 | C1 | student |
| student2 | C1 | student |
| student3 | C1 | student |
And the following "groups" exist:
| name | course | idnumber |
| Group 1 | C1 | G1 |
Expand Down Expand Up @@ -79,8 +81,13 @@ Feature: Bulk remove submissions
Then I should not see "Remove submission" in the "Choose operation" "select"

@javascript @skip_chrome_zerosize
Scenario: Notification should be displayed when non-group users are selected for submission bulk removal
in separate group mode
Scenario: Bulk remove submission when shared group users are added to the bulk
removing submissions process in separate group mode without access all groups capability
Given the following "group members" exist:
| user | group |
| teacher1 | G1 |
| student1 | G1 |
| student2 | G1 |
Given the following "activity" exists:
| activity | assign |
| course | C1 |
Expand All @@ -93,25 +100,26 @@ Feature: Bulk remove submissions
| assign | user | onlinetext |
| Test assignment name | student1 | I'm the student1 submission |
| Test assignment name | student2 | I'm the student2 submission |
| Test assignment name | student3 | I'm the student3 submission |
And the following "role capability" exists:
| role | editingteacher |
| mod/assign:editothersubmission | allow |
| moodle/site:accessallgroups | prevent |
And I am on the "Test assignment name" Activity page logged in as teacher1
And I follow "View all submissions"
And I should see "I'm the student1 submission"
And I should see "I'm the student2 submission"
And I should not see "I'm the student3 submission"
And I set the field "selectall" to "1"
When I set the field "operation" to "Remove submission"
And I click on "Go" "button" confirming the dialogue

Then I should see "I'm the student1 submission"
And I should see "I'm the student2 submission"
And I should see "The submission of Student 1 cannot be removed"
And I should see "The submission of Student 2 cannot be removed"
Then I should not see "I'm the student1 submission"
Then I should not see "I'm the student2 submission"

@javascript @skip_chrome_zerosize
Scenario: Bulk remove submission when group users are added to the bulk
removing submissions process in separate group mode
Scenario: Bulk remove submission when group users and non-group users are added to the bulk
removing submissions process in separate group mode with access all groups capability
Given the following "group members" exist:
| user | group |
| student1 | G1 |
Expand All @@ -128,15 +136,19 @@ Feature: Bulk remove submissions
| assign | user | onlinetext |
| Test assignment name | student1 | I'm the student1 submission |
| Test assignment name | student2 | I'm the student2 submission |
| Test assignment name | student3 | I'm the student3 submission |
And the following "role capability" exists:
| role | editingteacher |
| mod/assign:editothersubmission | allow |
| moodle/site:accessallgroups | allow |
And I am on the "Test assignment name" Activity page logged in as teacher1
And I follow "View all submissions"
And I should see "I'm the student1 submission"
And I should see "I'm the student2 submission"
And I should see "I'm the student3 submission"
And I set the field "selectall" to "1"
When I set the field "operation" to "Remove submission"
And I click on "Go" "button" confirming the dialogue
Then I should not see "I'm the student1 submission"
And I should not see "I'm the student2 submission"
And I should not see "I'm the student3 submission"
17 changes: 17 additions & 0 deletions mod/assign/tests/locallib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3437,6 +3437,7 @@ public function test_can_edit_submission_separategroups_with_editothersubmission
$student2 = $this->getDataGenerator()->create_and_enrol($course, 'student');
$student3 = $this->getDataGenerator()->create_and_enrol($course, 'student');
$student4 = $this->getDataGenerator()->create_and_enrol($course, 'student');
$student5 = $this->getDataGenerator()->create_and_enrol($course, 'student');

$grouping = $this->getDataGenerator()->create_grouping(array('courseid' => $course->id));
$group1 = $this->getDataGenerator()->create_group(['courseid' => $course->id]);
Expand All @@ -3454,6 +3455,7 @@ public function test_can_edit_submission_separategroups_with_editothersubmission
'submissiondrafts' => 1,
'groupingid' => $grouping->id,
'groupmode' => SEPARATEGROUPS,
'preventsubmissionnotingroup' => 0,
]);

// Add the capability to the new \assignment for student 1.
Expand All @@ -3467,12 +3469,27 @@ public function test_can_edit_submission_separategroups_with_editothersubmission
$this->assertTrue($assign->can_edit_submission($student2->id, $student1->id));
$this->assertFalse($assign->can_edit_submission($student3->id, $student1->id));
$this->assertFalse($assign->can_edit_submission($student4->id, $student1->id));
$this->assertFalse($assign->can_edit_submission($student5->id, $student1->id));

// Verify other students do not have the ability to edit submissions for other users.
$this->assertTrue($assign->can_edit_submission($student2->id, $student2->id));
$this->assertFalse($assign->can_edit_submission($student1->id, $student2->id));
$this->assertFalse($assign->can_edit_submission($student3->id, $student2->id));
$this->assertFalse($assign->can_edit_submission($student4->id, $student2->id));
$this->assertFalse($assign->can_edit_submission($student5->id, $student2->id));

// Add the required capability to edit other submissions and to view all groups to the teacher.
$roleid = create_role('Dummy role 2', 'dummyrole2', 'dummy role description');
assign_capability('mod/assign:editothersubmission', CAP_ALLOW, $roleid, $assign->get_context()->id);
assign_capability('moodle/site:accessallgroups', CAP_ALLOW, $roleid, $assign->get_context()->id);
role_assign($roleid, $teacher->id, $assign->get_context()->id);

// Verify the teacher has the ability to edit submissions for other users including users not in a group.
$this->assertTrue($assign->can_edit_submission($student1->id, $teacher->id));
$this->assertTrue($assign->can_edit_submission($student2->id, $teacher->id));
$this->assertTrue($assign->can_edit_submission($student3->id, $teacher->id));
$this->assertTrue($assign->can_edit_submission($student4->id, $teacher->id));
$this->assertTrue($assign->can_edit_submission($student5->id, $teacher->id));
}

/**
Expand Down

0 comments on commit 1b5cc6b

Please sign in to comment.