From bba75cb18606b76624e4fb72c6d388ef48d371c1 Mon Sep 17 00:00:00 2001 From: Ankit Agarwal Date: Mon, 2 Jan 2017 14:26:35 +0530 Subject: [PATCH 1/2] MDL-57296 gradebook: Fix issue with gradebook while collapsing grades --- grade/report/grader/lib.php | 43 ++++++++++++++++--- .../grader/tests/behat/switch_views.feature | 4 ++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php index d154df1320c..8e4560f280a 100644 --- a/grade/report/grader/lib.php +++ b/grade/report/grader/lib.php @@ -45,6 +45,13 @@ class grade_report_grader extends grade_report { */ private $allgrades; + /** + * Contains all grade items expect GRADE_TYPE_NONE. + * + * @var array $allgradeitems + */ + private $allgradeitems; + /** * Array of errors for bulk grades updating. * @var array $gradeserror @@ -519,6 +526,22 @@ class grade_report_grader extends grade_report { return $this->users; } + /** + * Load all grade items. + */ + protected function get_allgradeitems() { + if (!empty($this->allgradeitems)) { + return $this->allgradeitems; + } + $allgradeitems = grade_item::fetch_all(array('courseid' => $this->courseid)); + // But hang on - don't include ones which are set to not show the grade at all. + $this->allgradeitems = array_filter($allgradeitems, function($item) { + return $item->gradetype != GRADE_TYPE_NONE; + }); + + return $this->allgradeitems; + } + /** * we supply the userids in this query, and get all the grades * pulls out all the grades, this does not need to worry about paging @@ -567,6 +590,19 @@ class grade_report_grader extends grade_report { } } } + + // Pre fill grades for any remaining items which might be collapsed. + $allgradeitems = $this->get_allgradeitems(); + foreach ($userids as $userid) { + foreach ($allgradeitems as $itemid => $gradeitem) { + if (!isset($this->allgrades[$userid][$itemid])) { + $this->allgrades[$userid][$itemid] = new grade_grade(); + $this->allgrades[$userid][$itemid]->itemid = $itemid; + $this->allgrades[$userid][$itemid]->userid = $userid; + $this->allgrades[$userid][$itemid]->grade_item = $gradeitem; + } + } + } } /** @@ -918,12 +954,7 @@ class grade_report_grader extends grade_report { // grade items (in case one has been hidden) as the course total shown needs to be adjusted for this particular // user. if (!$this->canviewhidden) { - $allgradeitems = grade_item::fetch_all(array('courseid' => $this->courseid)); - - // But hang on - don't include ones which are set to not show the grade at all. - $allgradeitems = array_filter($allgradeitems, function($item) { - return $item->gradetype != GRADE_TYPE_NONE; - }); + $allgradeitems = $this->get_allgradeitems(); } foreach ($this->users as $userid => $user) { diff --git a/grade/report/grader/tests/behat/switch_views.feature b/grade/report/grader/tests/behat/switch_views.feature index 775f455c05b..54bed2bbf64 100644 --- a/grade/report/grader/tests/behat/switch_views.feature +++ b/grade/report/grader/tests/behat/switch_views.feature @@ -12,6 +12,7 @@ Feature: We can change what we are viewing on the grader report | username | firstname | lastname | email | | teacher1 | Teacher | 1 | teacher1@example.com | | student1 | Student | 1 | student1@example.com | + | student2 | Student | 1 | student2@example.com | And the following "course enrolments" exist: | user | course | role | | teacher1 | C1 | editingteacher | @@ -94,6 +95,9 @@ Feature: We can change what we are viewing on the grader report | capability | permission | | moodle/grade:viewhidden | Prevent | And I log out + And the following "course enrolments" exist: + | user | course | role | + | student2 | C1 | student | And I log in as "teacher1" And I follow "Course 1" And I navigate to "View > Grader report" in the course gradebook From 6c6bd6e1ad9f66c9034aeef19306ca7d18e4f4b7 Mon Sep 17 00:00:00 2001 From: Ankit Agarwal Date: Tue, 24 Jan 2017 15:48:25 +0530 Subject: [PATCH 2/2] MDL-57296 gradelib: Fix get_hidding_effect for grade_type_none items --- grade/report/grader/lib.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php index 8e4560f280a..90cbd539497 100644 --- a/grade/report/grader/lib.php +++ b/grade/report/grader/lib.php @@ -565,11 +565,16 @@ class grade_report_grader extends grade_report { WHERE g.itemid = gi.id AND gi.courseid = :courseid {$this->userselect}"; $userids = array_keys($this->users); + $allgradeitems = $this->get_allgradeitems(); if ($grades = $DB->get_records_sql($sql, $params)) { foreach ($grades as $graderec) { $grade = new grade_grade($graderec, false); - $this->allgrades[$graderec->userid][$graderec->itemid] = $grade; + if (!empty($allgradeitems[$graderec->itemid])) { + // Note: Filter out grades which have a grade type of GRADE_TYPE_NONE. + // Only grades without this type are present in $allgradeitems. + $this->allgrades[$graderec->userid][$graderec->itemid] = $grade; + } if (in_array($graderec->userid, $userids) and array_key_exists($graderec->itemid, $this->gtree->get_items())) { // some items may not be present!! $this->grades[$graderec->userid][$graderec->itemid] = $grade; $this->grades[$graderec->userid][$graderec->itemid]->grade_item = $this->gtree->get_item($graderec->itemid); // db caching @@ -592,7 +597,6 @@ class grade_report_grader extends grade_report { } // Pre fill grades for any remaining items which might be collapsed. - $allgradeitems = $this->get_allgradeitems(); foreach ($userids as $userid) { foreach ($allgradeitems as $itemid => $gradeitem) { if (!isset($this->allgrades[$userid][$itemid])) {