mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-12 20:12:15 +02:00
Code Issues Projects Releases Wiki Activity

MDL-65381 grade_report: fix empty separate groups own grades access

Browse Source 
On separate groups' courses, users without groups could not access
their own grade items.
This commit is contained in:
Didier 'OdyX' Raboud 2019-04-18 15:33:16 +02:00 committed by Juan Leyva
parent 26649f5750
commit 74a582c502
2 changed files with 64 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
grade/report/user/classes/external/user.php vendored
View File

@ -88,7 +88,8 @@ class user extends external_api {
core_user::require_active_user($user);
// Check if we can view the user group (if any).
// When userid == 0, we are retrieving all the users, we'll check then if a groupid is required.
if (!groups_user_groups_visible($course, $user->id)) {
// User are always in their own group, also when they don't have groups.
if ($userid != $USER->id && !groups_user_groups_visible($course, $user->id)) {
throw new moodle_exception('notingroup');
}
}
@ -107,21 +108,24 @@ class user extends external_api {
throw new moodle_exception('nopermissiontoviewgrades', 'error');
}
if (!empty($groupid)) {
// Determine is the group is visible to user.
if (!groups_group_visible($groupid, $course)) {
throw new moodle_exception('notingroup');
}
} else {
// Check to see if groups are being used here.
if ($groupmode = groups_get_course_groupmode($course)) {
$groupid = groups_get_course_group($course);
// Determine is the group is visible to user (this is particullary for the group 0).
// User are always in their own group, also when they don't have groups.
if ($userid != $USER->id) {
if (!empty($groupid)) {
// Determine if the group is visible to user.
if (!groups_group_visible($groupid, $course)) {
throw new moodle_exception('notingroup');
}
} else {
$groupid = 0;
// Check to see if groups are being used here.
if ($groupmode = groups_get_course_groupmode($course)) {
$groupid = groups_get_course_group($course);
// Determine if the group is visible to the user (this is particularly for group 0).
if (!groups_group_visible($groupid, $course)) {
throw new moodle_exception('notingroup');
}
} else {
$groupid = 0;
}
}
}

62
grade/report/user/tests/externallib_test.php
View File

@ -42,7 +42,7 @@ class externallib_test extends externallib_advanced_testcase {
* @param int $s2grade Student 2 grade
* @return array Course and users instances
*/
private function load_data(int $s1grade, int $s2grade): array {
private function load_data(int $s1grade, int $s2grade, int $s3grade): array {
global $DB;
$course = $this->getDataGenerator()->create_course(['groupmode' => SEPARATEGROUPS, 'groupmodeforce' => 1]);
@ -54,6 +54,10 @@ class externallib_test extends externallib_advanced_testcase {
$student2 = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($student2->id, $course->id, $studentrole->id);
// Student 3 is in no groups.
$student3 = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($student3->id, $course->id, $studentrole->id);
$teacherrole = $DB->get_record('role', ['shortname' => 'teacher']);
$teacher = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id);
@ -74,7 +78,8 @@ class externallib_test extends externallib_advanced_testcase {
$student1grade = ['userid' => $student1->id, 'rawgrade' => $s1grade, 'idnumber' => 'testidnumber1'];
$student2grade = ['userid' => $student2->id, 'rawgrade' => $s2grade, 'idnumber' => 'testidnumber2'];
$studentgrades = [$student1->id => $student1grade, $student2->id => $student2grade];
$student3grade = ['userid' => $student3->id, 'rawgrade' => $s3grade, 'idnumber' => 'testidnumber3'];
$studentgrades = [$student1->id => $student1grade, $student2->id => $student2grade, $student3->id => $student3grade];
assign_grade_item_update($assignment, $studentgrades);
return [
@ -82,6 +87,7 @@ class externallib_test extends externallib_advanced_testcase {
$teacher,
$student1,
$student2,
$student3,
$assignment
];
}
@ -95,8 +101,9 @@ class externallib_test extends externallib_advanced_testcase {
$s1grade = 80;
$s2grade = 60;
$s3grade = 50;
list($course, $teacher, $student1, $student2, $assignment) = $this->load_data($s1grade, $s2grade);
list($course, $teacher, $student1, $student2, $student3, $assignment) = $this->load_data($s1grade, $s2grade, $s3grade);
// A teacher must see all student grades (in their group only).
$this->setUser($teacher);
@ -129,8 +136,9 @@ class externallib_test extends externallib_advanced_testcase {
$s1grade = 80;
$s2grade = 60;
$s3grade = 50;
list($course, $teacher, $student1, $student2, $assignment) = $this->load_data($s1grade, $s2grade);
list($course, $teacher, $student1, $student2, $student3, $assignment) = $this->load_data($s1grade, $s2grade, $s3grade);
// A user can see his own grades.
$this->setUser($student1);
@ -144,6 +152,22 @@ class externallib_test extends externallib_advanced_testcase {
$student1returnedgrade = (int) $studentgrade['tables'][0]['tabledata'][2]['grade']['content'];
$this->assertEquals($s1grade, $student1returnedgrade);
// A user can see his own even when in no groups.
$this->setUser($student3);
$studentgrade = user_external::get_grades_table($course->id, $student3->id);
$studentgrade = external_api::clean_returnvalue(user_external::get_grades_table_returns(), $studentgrade);
// No warnings returned.
$this->assertTrue(count($studentgrade['warnings']) == 0);
$this->assertTrue(count($studentgrade['tables']) == 1);
$student3returnedgrade = (int) $studentgrade['tables'][0]['tabledata'][2]['grade']['content'];
$this->assertEquals($s3grade, $student3returnedgrade);
// Expect exception when user is not indicated.
$this->setUser($student3);
$this->expectException(\required_capability_exception::class);
user_external::get_grades_table($course->id);
}
/**
@ -156,8 +180,9 @@ class externallib_test extends externallib_advanced_testcase {
$s1grade = 80;
$s2grade = 60;
$s3grade = 50;
list($course, $teacher, $student1, $student2, $assignment) = $this->load_data($s1grade, $s2grade);
list($course, $teacher, $student1, $student2, $student3, $assignment) = $this->load_data($s1grade, $s2grade, $s3grade);
$this->setUser($student2);
@ -179,7 +204,8 @@ class externallib_test extends externallib_advanced_testcase {
$s1grade = 80;
$s2grade = 60;
list($course, $teacher, $student1, $student2, $assignment) = $this->load_data($s1grade, $s2grade);
$s3grade = 50;
list($course, $teacher, $student1, $student2, $student3, $assignment) = $this->load_data($s1grade, $s2grade, $s3grade);
// Redirect events to the sink, so we can recover them later.
$sink = $this->redirectEvents();
@ -226,8 +252,9 @@ class externallib_test extends externallib_advanced_testcase {
$s1grade = 80;
$s2grade = 60;
$s3grade = 50;
list($course, $teacher, $student1, $student2, $assignment) = $this->load_data($s1grade, $s2grade);
list($course, $teacher, $student1, $student2, $student3, $assignment) = $this->load_data($s1grade, $s2grade, $s3grade);
// A teacher must see all student grades (in their group only).
$this->setUser($teacher);
@ -277,8 +304,10 @@ class externallib_test extends externallib_advanced_testcase {
$this->assertFalse($studentgrades['usergrades'][0]['gradeitems'][0]['gradeisoverridden']);
$this->assertEquals('B-', $studentgrades['usergrades'][0]['gradeitems'][0]['lettergradeformatted']);
$this->assertEquals(1, $studentgrades['usergrades'][0]['gradeitems'][0]['rank']);
$this->assertEquals(2, $studentgrades['usergrades'][0]['gradeitems'][0]['numusers']);
$this->assertEquals(70, $studentgrades['usergrades'][0]['gradeitems'][0]['averageformatted']);
$this->assertEquals(3, $studentgrades['usergrades'][0]['gradeitems'][0]['numusers']);
$this->assertEquals(
round(array_sum([$s1grade, $s2grade, $s3grade]) / 3, 2),
$studentgrades['usergrades'][0]['gradeitems'][0]['averageformatted']);
// Course grades.
$this->assertEquals('course', $studentgrades['usergrades'][0]['gradeitems'][1]['itemtype']);
@ -297,8 +326,10 @@ class externallib_test extends externallib_advanced_testcase {
$this->assertFalse($studentgrades['usergrades'][0]['gradeitems'][1]['gradeisoverridden']);
$this->assertEquals('B-', $studentgrades['usergrades'][0]['gradeitems'][1]['lettergradeformatted']);
$this->assertEquals(1, $studentgrades['usergrades'][0]['gradeitems'][1]['rank']);
$this->assertEquals(2, $studentgrades['usergrades'][0]['gradeitems'][1]['numusers']);
$this->assertEquals(70, $studentgrades['usergrades'][0]['gradeitems'][1]['averageformatted']);
$this->assertEquals(3, $studentgrades['usergrades'][0]['gradeitems'][1]['numusers']);
$this->assertEquals(
round(array_sum([$s1grade, $s2grade, $s3grade]) / 3, 2),
$studentgrades['usergrades'][0]['gradeitems'][1]['averageformatted']);
// Now, override and lock a grade.
$gradegrade = \grade_grade::fetch(['itemid' => $studentgrades['usergrades'][0]['gradeitems'][0]['id'],
@ -325,8 +356,9 @@ class externallib_test extends externallib_advanced_testcase {
$s1grade = 80;
$s2grade = 60;
$s3grade = 50;
list($course, $teacher, $student1, $student2, $assignment) = $this->load_data($s1grade, $s2grade);
list($course, $teacher, $student1, $student2, $student3, $assignment) = $this->load_data($s1grade, $s2grade, $s3grade);
grade_set_setting($course->id, 'report_user_showrank', 1);
grade_set_setting($course->id, 'report_user_showpercentage', 1);
@ -374,8 +406,10 @@ class externallib_test extends externallib_advanced_testcase {
$this->assertNull($studentgrades['usergrades'][0]['gradeitems'][0]['gradeisoverridden']);
$this->assertEquals('B-', $studentgrades['usergrades'][0]['gradeitems'][0]['lettergradeformatted']);
$this->assertEquals(1, $studentgrades['usergrades'][0]['gradeitems'][0]['rank']);
$this->assertEquals(2, $studentgrades['usergrades'][0]['gradeitems'][0]['numusers']);
$this->assertEquals(70, $studentgrades['usergrades'][0]['gradeitems'][0]['averageformatted']);
$this->assertEquals(3, $studentgrades['usergrades'][0]['gradeitems'][0]['numusers']);
$this->assertEquals(
round(array_sum([$s1grade, $s2grade, $s3grade]) / 3, 2),
$studentgrades['usergrades'][0]['gradeitems'][0]['averageformatted']);
// Check that the idnumber for assignment grades is equal to the cmid.
$this->assertEquals((string) $studentgrades['usergrades'][0]['gradeitems'][0]['cmid'],