Skip to content

Commit

Permalink
MDL-67636 gradebook: Prevent exposing hidden grade in locked category
Browse files Browse the repository at this point in the history
  • Loading branch information
John Yao committed Nov 19, 2020
1 parent 6ef4e66 commit 11886d3
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 4 deletions.
92 changes: 92 additions & 0 deletions grade/tests/behat/grade_hidden_items_locked_category.feature
View file
@@ -0,0 +1,92 @@
@core @core_grades
Feature: Hidden grade items should be hidden when grade category is locked, but should be visible in overridden category
In order to verify existing grades items display as expected
As an teacher
I need to modify grade items and grade categories
I need to ensure existing grades display in an expected manner

Background:
Given the following "courses" exist:
| fullname | shortname | category | groupmode |
| Course 1 | C1 | 0 | 1 |
And the following "users" exist:
| username | firstname | lastname | email | idnumber |
| teacher1 | Teacher | 1 | teacher1@example.com | t1 |
| student1 | Student | 1 | student1@example.com | s1 |
And the following "course enrolments" exist:
| user | course | role |
| teacher1 | C1 | editingteacher |
| student1 | C1 | student |
And I log in as "admin"
And I am on "Course 1" course homepage
And I navigate to "Setup > Gradebook setup" in the course gradebook
And I press "Add category"
And I set the following fields to these values:
| Category name | Test locked category |
And I press "Save changes"
And I press "Add grade item"
And I set the following fields to these values:
| Item name | Hidden item |
| Hidden | 1 |
| Grade category | Test locked category |
And I press "Save changes"
And I log out
And I log in as "teacher1"
And I am on "Course 1" course homepage
And I navigate to "View > Grader report" in the course gradebook
And I turn editing mode on
And I give the grade "50.00" to the user "Student 1" for the grade item "Hidden item"
And I press "Save changes"
And I navigate to "Setup > Gradebook setup" in the course gradebook
And I set the following settings for grade item "Test locked category":
| Locked | 1 |
And I press "Save changes"
And I log out

Scenario: Hidden grade items in locked category is hidden for teacher
Given I log in as "teacher1"
And I am on "Course 1" course homepage
And I navigate to "View > User report" in the course gradebook
And I select "Myself" from the "View report as" singleselect
When I select "Student 1" from the "Select all or one user" singleselect
Then the following should exist in the "user-grade" table:
| Grade item | Calculated weight | Grade | Range | Percentage | Contribution to course total |
| Test locked category total | 100.00 % | 50.00 | 0–100 | 50.00 % | - |
| Course total | - | 50.00 | 0–100 | 50.00 % | - |

Scenario: Hidden grade items in locked category is hidden for student
Given I log in as "student1"
And I am on "Course 1" course homepage
When I navigate to "User report" in the course gradebook
Then the following should exist in the "user-grade" table:
| Grade item | Calculated weight | Grade | Range | Percentage | Contribution to course total |
| Test locked category total | 100.00 % | - | 0–100 | - | - |
| Course total | - | - | 0–100 | - | - |
And I should not see "Hidden item"

Scenario: Hidden grade items in overridden category should show
Given I log in as "teacher1"
And I am on "Course 1" course homepage
And I navigate to "Setup > Gradebook setup" in the course gradebook
And I press "Add category"
And I set the following fields to these values:
| Category name | Test overridden category B|
And I press "Save changes"
And I press "Add grade item"
And I set the following fields to these values:
| Item name | Cat b item |
| Grade category | Test overridden category B |
And I press "Save changes"
When I navigate to "View > Grader report" in the course gradebook
And I turn editing mode on
And I give the grade "50.00" to the user "Student 1" for the grade item "Test overridden category B total"
And I press "Save changes"
And I log out
And I log in as "student1"
And I am on "Course 1" course homepage
And I navigate to "User report" in the course gradebook
Then the following should exist in the "user-grade" table:
| Grade item | Calculated weight | Grade | Range | Percentage | Contribution to course total |
| Test locked category total | 50.00 % | - | 0–100 | - | - |
| Test overridden category B total | 50.00 % | 50.00 | 0–100 | 50.00 % | - |
| Course total | - | - | 0–200 | - | - |
10 changes: 7 additions & 3 deletions lib/grade/grade_grade.php
View file
Expand Up @@ -780,13 +780,15 @@ public static function get_hiding_affected(&$grade_grades, &$grade_items) {
$dependson[$grade_grade->itemid] = $grade_items[$grade_grade->itemid]->depends_on();
if ($grade_grade->is_excluded()) {
//nothing to do, aggregation is ok
continue;
} else if ($grade_grade->is_hidden()) {
$hiddenfound = true;
$altered[$grade_grade->itemid] = null;
$alteredaggregationstatus[$grade_grade->itemid] = 'dropped';
$alteredaggregationweight[$grade_grade->itemid] = 0;
} else if ($grade_grade->is_locked() or $grade_grade->is_overridden()) {
// no need to recalculate locked or overridden grades
} else if ($grade_grade->is_overridden()) {
// No need to recalculate overridden grades.
continue;
} else {
if (!empty($dependson[$grade_grade->itemid])) {
$dependencydepth[$grade_grade->itemid] = 1;
Expand Down Expand Up @@ -846,9 +848,11 @@ public static function get_hiding_affected(&$grade_grades, &$grade_items) {
} else {
// depends on altered grades - we should try to recalculate if possible
if ($grade_items[$do]->is_calculated() or
(!$grade_items[$do]->is_category_item() and !$grade_items[$do]->is_course_item())
(!$grade_items[$do]->is_category_item() and !$grade_items[$do]->is_course_item()) or
($grade_items[$do]->is_category_item() and $grade_items[$do]->is_locked())
) {
// This is a grade item that is not a category or course and has been affected by grade hiding.
// Or a grade item that is a category and it is locked.
// I guess this means it is a calculation that needs to be recalculated.
$unknown[$do] = $grade_grades[$do]->finalgrade;
unset($todo[$key]);
Expand Down
2 changes: 1 addition & 1 deletion lib/grade/grade_item.php
View file
Expand Up @@ -1658,7 +1658,7 @@ public function depends_on($reset_cache=false) {
return $this->dependson_cache;
}

if ($this->is_locked()) {
if ($this->is_locked() && !$this->is_category_item()) {
// locked items do not need to be regraded
$this->dependson_cache = array();
return $this->dependson_cache;
Expand Down
99 changes: 99 additions & 0 deletions lib/grade/tests/grade_grade_test.php
View file
Expand Up @@ -570,4 +570,103 @@ public function sub_test_grade_grade_deleted_event() {
$gg = grade_grade::fetch(array('userid' => $u2->id, 'itemid' => $gi->id));
$this->assertEmpty($gg);
}

/**
* Tests get_hiding_affected by locked category and overridden grades.
*/
public function test_category_get_hiding_affected() {
$generator = $this->getDataGenerator();

// Create the data we need for the tests.
$course1 = $generator->create_course();
$user1 = $generator->create_and_enrol($course1, 'student');
$assignment2 = $generator->create_module('assign', ['course' => $course1->id]);

// Create a category item.
$gradecategory = new grade_category(array('courseid' => $course1->id, 'fullname' => 'test'), false);
$gradecategoryid = $gradecategory->insert();

// Create one hidden grade item.
$gradeitem1a = new grade_item($generator->create_grade_item(
[
'courseid' => $course1->id,
'itemtype' => 'mod',
'itemmodule' => 'assign',
'iteminstance' => $assignment2->id,
'categoryid' => $gradecategoryid,
'hidden' => 1,
]
), false);
grade_update('mod/assign', $gradeitem1a->courseid, $gradeitem1a->itemtype, $gradeitem1a->itemmodule, $gradeitem1a->iteminstance,
$gradeitem1a->itemnumber, ['userid' => $user1->id]);

// Get category grade item.
$gradeitem = $gradecategory->get_grade_item();
// Reset needsupdate to allow set_locked.
$gradeitem->needsupdate = 0;
$gradeitem->update();
// Lock category grade item.
$gradeitem->set_locked(1);

$hidingaffectedlocked = $this->call_get_hiding_affected($course1, $user1);
// Since locked category now should be recalculated.
// The number of unknown items is 2, this includes category item and course item.
$this->assertEquals(2, count($hidingaffectedlocked['unknown']));

// Unlock category.
$gradeitem->set_locked(0);
$hidingaffectedunlocked = $this->call_get_hiding_affected($course1, $user1);
// When category unlocked, hidden item should exist in altered items.
$this->assertTrue(in_array($gradeitem1a->id, array_keys($hidingaffectedunlocked['altered'])));

// This creates all the grade_grades we need.
grade_regrade_final_grades($course1->id);

// Set grade override.
$gradegrade = grade_grade::fetch([
'userid' => $user1->id,
'itemid' => $gradeitem->id,
]);
// Set override grade grade, and check that grade submission has been overridden.
$gradegrade->set_overridden(true);
$this->assertEquals(true, $gradegrade->is_overridden());
$hidingaffectedoverridden = $this->call_get_hiding_affected($course1, $user1);
// No need to recalculate overridden grades.
$this->assertTrue(in_array($gradegrade->itemid, array_keys($hidingaffectedoverridden['alteredaggregationstatus'])));
$this->assertEquals('used', $hidingaffectedoverridden['alteredaggregationstatus'][$gradegrade->itemid]);
}

/**
* Call get_hiding_affected().
* @param stdClass $course The course object
* @param stdClass $user The student object
* @return array
*/
private function call_get_hiding_affected($course, $user) {
global $DB;

$items = grade_item::fetch_all(array('courseid' => $course->id));
$grades = array();
$sql = "SELECT g.*
FROM {grade_grades} g
JOIN {grade_items} gi ON gi.id = g.itemid
WHERE g.userid = :userid AND gi.courseid = :courseid";
if ($gradesrecords = $DB->get_records_sql($sql, ['userid' => $user->id, 'courseid' => $course->id])) {
foreach ($gradesrecords as $grade) {
$grades[$grade->itemid] = new grade_grade($grade, false);
}
unset($gradesrecords);
}
foreach ($items as $itemid => $gradeitem) {
if (!isset($grades[$itemid])) {
$gradegrade = new grade_grade();
$gradegrade->userid = $user->id;
$gradegrade->itemid = $gradeitem->id;
$grades[$itemid] = $gradegrade;
}
$gradeitem->grade_item = $gradeitem;
}

return grade_grade::get_hiding_affected($grades, $items);
}
}

0 comments on commit 11886d3

Please sign in to comment.