diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php index 3cef0a93363..1caf36f4fa8 100644 --- a/grade/report/grader/lib.php +++ b/grade/report/grader/lib.php @@ -1782,9 +1782,10 @@ class grade_report_grader extends grade_report { */ protected static function filter_collapsed_categories($courseid, $collapsed) { global $DB; - if (empty($collapsed)) { - $collapsed = array('aggregatesonly' => array(), 'gradesonly' => array()); - } + // Ensure we always have an element for aggregatesonly and another for gradesonly, no matter it's empty. + $collapsed['aggregatesonly'] = $collapsed['aggregatesonly'] ?? []; + $collapsed['gradesonly'] = $collapsed['gradesonly'] ?? []; + if (empty($collapsed['aggregatesonly']) && empty($collapsed['gradesonly'])) { return $collapsed; } @@ -1805,12 +1806,23 @@ class grade_report_grader extends grade_report { */ protected static function get_collapsed_preferences($courseid) { if ($collapsed = get_user_preferences('grade_report_grader_collapsed_categories'.$courseid)) { - return json_decode($collapsed, true); + $collapsed = json_decode($collapsed, true); + // Ensure we always have an element for aggregatesonly and another for gradesonly, no matter it's empty. + $collapsed['aggregatesonly'] = $collapsed['aggregatesonly'] ?? []; + $collapsed['gradesonly'] = $collapsed['gradesonly'] ?? []; + return $collapsed; } // Try looking for old location of user setting that used to store all courses in one serialized user preference. + $collapsed = ['aggregatesonly' => [], 'gradesonly' => []]; // Use this if old settings are not found. + $collapsedall = []; + $oldprefexists = false; if (($oldcollapsedpref = get_user_preferences('grade_report_grader_collapsed_categories')) !== null) { + $oldprefexists = true; if ($collapsedall = unserialize_array($oldcollapsedpref)) { + // Ensure we always have an element for aggregatesonly and another for gradesonly, no matter it's empty. + $collapsedall['aggregatesonly'] = $collapsedall['aggregatesonly'] ?? []; + $collapsedall['gradesonly'] = $collapsedall['gradesonly'] ?? []; // We found the old-style preference, filter out only categories that belong to this course and update the prefs. $collapsed = static::filter_collapsed_categories($courseid, $collapsedall); if (!empty($collapsed['aggregatesonly']) || !empty($collapsed['gradesonly'])) { @@ -1819,17 +1831,21 @@ class grade_report_grader extends grade_report { $collapsedall['gradesonly'] = array_diff($collapsedall['gradesonly'], $collapsed['gradesonly']); if (!empty($collapsedall['aggregatesonly']) || !empty($collapsedall['gradesonly'])) { set_user_preference('grade_report_grader_collapsed_categories', serialize($collapsedall)); - } else { - unset_user_preference('grade_report_grader_collapsed_categories'); } } - } else { - // We found the old-style preference, but it is unreadable, discard it. - unset_user_preference('grade_report_grader_collapsed_categories'); } - } else { - $collapsed = array('aggregatesonly' => array(), 'gradesonly' => array()); } + + // Arrived here, if the old pref exists and it doesn't contain + // more information, it means that the migration of all the + // data to new, by course, preferences is completed, so + // the old one can be safely deleted. + if ($oldprefexists && + empty($collapsedall['aggregatesonly']) && + empty($collapsedall['gradesonly'])) { + unset_user_preference('grade_report_grader_collapsed_categories'); + } + return $collapsed; } diff --git a/grade/tests/report_graderlib_test.php b/grade/tests/report_graderlib_test.php index 431d129a0ac..84a5ab64f9d 100644 --- a/grade/tests/report_graderlib_test.php +++ b/grade/tests/report_graderlib_test.php @@ -230,6 +230,226 @@ class core_grade_report_graderlib_testcase extends advanced_testcase { $this->assertEquals(count($toobigvalue['gradesonly']) - 1, count($report1->collapsed['gradesonly'])); } + /** + * Test some special cases of the conversion from old preferences to new ones + * + * @covers \grade_report_grader::get_collapsed_preferences + * @covers \grade_report_grader::filter_collapsed_categories + */ + public function test_old_collapsed_preferences() { + $this->resetAfterTest(true); + + $user1 = $this->getDataGenerator()->create_user(); + $course1 = $this->getDataGenerator()->create_course(); + $course2 = $this->getDataGenerator()->create_course(); + $course3 = $this->getDataGenerator()->create_course(); + + $course1cats = $course2cats = $course3cats = []; + for ($i = 0; $i < 10; $i++) { + $course1cats[] = $this->create_grade_category($course1)->id; + $course2cats[] = $this->create_grade_category($course2)->id; + $course3cats[] = $this->create_grade_category($course3)->id; + } + + $report1 = $this->create_report($course1); + // Collapse all the cats in course1. + foreach ($course1cats as $catid) { + $report1->process_action('cg'. $catid, 'switch_minus'); + } + + // Expand all the cats in course2. + $report2 = $this->create_report($course2); + foreach ($course2cats as $catid) { + $report2->process_action('cg'.$catid, 'switch_minus'); + $report2->process_action('cg'.$catid, 'switch_plus'); + } + + // Collapse odd cats and expand even cats in course3. + $report3 = $this->create_report($course3); + foreach ($course3cats as $catid) { + $report3->process_action('cg'.$catid, 'switch_minus'); + if (($i++) % 2) { + $report3->process_action('cg'.$catid, 'switch_plus'); + } + } + + $report1 = $this->create_report($course1); + $this->assertEquals(10, count($report1->collapsed['aggregatesonly'])); + $this->assertEquals(0, count($report1->collapsed['gradesonly'])); + $report2 = $this->create_report($course2); + $this->assertEquals(0, count($report2->collapsed['aggregatesonly'])); + $this->assertEquals(10, count($report2->collapsed['gradesonly'])); + $report3 = $this->create_report($course3); + $this->assertEquals(5, count($report3->collapsed['aggregatesonly'])); + $this->assertEquals(5, count($report3->collapsed['gradesonly'])); + + // Use the preferences generated for user1 and set it in the old format for other users. + + // User2: both gradesonly and aggregatesonly. + $user2 = $this->getDataGenerator()->create_user(); + $alldata = [ + 'gradesonly' => array_merge( + $report1->collapsed['gradesonly'], + $report2->collapsed['gradesonly'], + $report3->collapsed['gradesonly']), + 'aggregatesonly' => array_merge( + $report1->collapsed['aggregatesonly'], + $report2->collapsed['aggregatesonly'], + $report3->collapsed['aggregatesonly']), + ]; + set_user_preference('grade_report_grader_collapsed_categories', serialize($alldata), $user2); + + $this->setUser($user2); + $convertedreport1 = $this->create_report($course1); + $this->assertEquals($report1->collapsed['gradesonly'], $convertedreport1->collapsed['gradesonly']); + $this->assertEquals($report1->collapsed['aggregatesonly'], $convertedreport1->collapsed['aggregatesonly']); + $newprefs1 = get_user_preferences('grade_report_grader_collapsed_categories' . $course1->id); // Also verify new prefs. + $this->assertEquals($report1->collapsed['gradesonly'], json_decode($newprefs1, true)['gradesonly']); + $this->assertEquals($report1->collapsed['aggregatesonly'], json_decode($newprefs1, true)['aggregatesonly']); + + $convertedreport2 = $this->create_report($course2); + $this->assertEquals($report2->collapsed['gradesonly'], $convertedreport2->collapsed['gradesonly']); + $this->assertEquals($report2->collapsed['aggregatesonly'], $convertedreport2->collapsed['aggregatesonly']); + $newprefs2 = get_user_preferences('grade_report_grader_collapsed_categories' . $course2->id); // Also verify new prefs. + $this->assertEquals($report2->collapsed['gradesonly'], json_decode($newprefs2, true)['gradesonly']); + $this->assertEquals($report2->collapsed['aggregatesonly'], json_decode($newprefs2, true)['aggregatesonly']); + + $convertedreport3 = $this->create_report($course3); + $this->assertEquals($report3->collapsed['gradesonly'], $convertedreport3->collapsed['gradesonly']); + $this->assertEquals($report3->collapsed['aggregatesonly'], $convertedreport3->collapsed['aggregatesonly']); + $newprefs3 = get_user_preferences('grade_report_grader_collapsed_categories' . $course3->id); // Also verify new prefs. + $this->assertEquals($report3->collapsed['gradesonly'], json_decode($newprefs3, true)['gradesonly']); + $this->assertEquals($report3->collapsed['aggregatesonly'], json_decode($newprefs3, true)['aggregatesonly']); + + // Make sure the old style user preference is removed now. + $this->assertEmpty(get_user_preferences('grade_report_grader_collapsed_categories')); + + // User3: only gradesonly (missing aggregatesonly). + $user3 = $this->getDataGenerator()->create_user(); + $alldata = [ + 'gradesonly' => array_merge( + $report1->collapsed['gradesonly'], + $report2->collapsed['gradesonly'], + $report3->collapsed['gradesonly']), + ]; + set_user_preference('grade_report_grader_collapsed_categories', serialize($alldata), $user3); + + $this->setUser($user3); + $convertedreport1 = $this->create_report($course1); + $this->assertEquals($report1->collapsed['gradesonly'], $convertedreport1->collapsed['gradesonly']); + $this->assertEquals([], $convertedreport1->collapsed['aggregatesonly']); + $newprefs1 = get_user_preferences('grade_report_grader_collapsed_categories' . $course1->id); // Also verify new prefs. + $this->assertNull($newprefs1); + + $convertedreport2 = $this->create_report($course2); + $this->assertEquals($report2->collapsed['gradesonly'], $convertedreport2->collapsed['gradesonly']); + $this->assertEquals([], $convertedreport2->collapsed['aggregatesonly']); + $newprefs2 = get_user_preferences('grade_report_grader_collapsed_categories' . $course2->id); // Also verify new prefs. + $this->assertEquals($report2->collapsed['gradesonly'], json_decode($newprefs2, true)['gradesonly']); + $this->assertEquals([], json_decode($newprefs2, true)['aggregatesonly']); + + $convertedreport3 = $this->create_report($course3); + $this->assertEquals($report3->collapsed['gradesonly'], $convertedreport3->collapsed['gradesonly']); + $this->assertEquals([], $convertedreport3->collapsed['aggregatesonly']); + $newprefs3 = get_user_preferences('grade_report_grader_collapsed_categories' . $course3->id); // Also verify new prefs. + $this->assertEquals($report3->collapsed['gradesonly'], json_decode($newprefs3, true)['gradesonly']); + $this->assertEquals([], json_decode($newprefs3, true)['aggregatesonly']); + + // Make sure the old style user preference is removed now. + $this->assertEmpty(get_user_preferences('grade_report_grader_collapsed_categories')); + + // User4: only aggregatesonly (missing gradesonly). + $user4 = $this->getDataGenerator()->create_user(); + $alldata = [ + 'aggregatesonly' => array_merge( + $report1->collapsed['aggregatesonly'], + $report2->collapsed['aggregatesonly'], + $report3->collapsed['aggregatesonly']), + ]; + set_user_preference('grade_report_grader_collapsed_categories', serialize($alldata), $user4); + + $this->setUser($user4); + $convertedreport1 = $this->create_report($course1); + $this->assertEquals([], $convertedreport1->collapsed['gradesonly']); + $this->assertEquals($report1->collapsed['aggregatesonly'], $convertedreport1->collapsed['aggregatesonly']); + $newprefs1 = get_user_preferences('grade_report_grader_collapsed_categories' . $course1->id); // Also verify new prefs. + $this->assertEquals([], json_decode($newprefs1, true)['gradesonly']); + $this->assertEquals($report1->collapsed['aggregatesonly'], json_decode($newprefs1, true)['aggregatesonly']); + + $convertedreport2 = $this->create_report($course2); + $this->assertEquals([], $convertedreport2->collapsed['gradesonly']); + $this->assertEquals($report2->collapsed['aggregatesonly'], $convertedreport2->collapsed['aggregatesonly']); + $newprefs2 = get_user_preferences('grade_report_grader_collapsed_categories' . $course2->id); // Also verify new prefs. + $this->assertNull($newprefs2); + + $convertedreport3 = $this->create_report($course3); + $this->assertEquals([], $convertedreport3->collapsed['gradesonly']); + $this->assertEquals($report3->collapsed['aggregatesonly'], $convertedreport3->collapsed['aggregatesonly']); + $newprefs3 = get_user_preferences('grade_report_grader_collapsed_categories' . $course3->id); // Also verify new prefs. + $this->assertEquals([], json_decode($newprefs3, true)['gradesonly']); + $this->assertEquals($report3->collapsed['aggregatesonly'], json_decode($newprefs3, true)['aggregatesonly']); + + // Make sure the old style user preference is removed now. + $this->assertEmpty(get_user_preferences('grade_report_grader_collapsed_categories')); + + // User5: both missing gradesonly and aggregatesonly. + $user5 = $this->getDataGenerator()->create_user(); + $alldata = []; + set_user_preference('grade_report_grader_collapsed_categories', serialize($alldata), $user5); + + $this->setUser($user5); + $convertedreport1 = $this->create_report($course1); + $this->assertEquals([], $convertedreport1->collapsed['gradesonly']); + $this->assertEquals([], $convertedreport1->collapsed['aggregatesonly']); + $newprefs1 = get_user_preferences('grade_report_grader_collapsed_categories' . $course1->id); // Also verify new prefs. + $this->assertNull($newprefs1); + + $convertedreport2 = $this->create_report($course2); + $this->assertEquals([], $convertedreport2->collapsed['gradesonly']); + $this->assertEquals([], $convertedreport2->collapsed['aggregatesonly']); + $newprefs2 = get_user_preferences('grade_report_grader_collapsed_categories' . $course2->id); // Also verify new prefs. + $this->assertNull($newprefs2); + + $convertedreport3 = $this->create_report($course3); + $this->assertEquals([], $convertedreport3->collapsed['gradesonly']); + $this->assertEquals([], $convertedreport3->collapsed['aggregatesonly']); + $newprefs3 = get_user_preferences('grade_report_grader_collapsed_categories' . $course3->id); // Also verify new prefs. + $this->assertNull($newprefs3); + + // Make sure the old style user preference is removed now. + $this->assertEmpty(get_user_preferences('grade_report_grader_collapsed_categories')); + + // User6: both empty gradesonly and aggregatesonly. + $user6 = $this->getDataGenerator()->create_user(); + $alldata = [ + 'gradesonly' => [], + 'aggregatesonly' => [] + ]; + set_user_preference('grade_report_grader_collapsed_categories', serialize($alldata), $user6); + + $this->setUser($user6); + $convertedreport1 = $this->create_report($course1); + $this->assertEquals([], $convertedreport1->collapsed['gradesonly']); + $this->assertEquals([], $convertedreport1->collapsed['aggregatesonly']); + $newprefs1 = get_user_preferences('grade_report_grader_collapsed_categories' . $course1->id); // Also verify new prefs. + $this->assertNull($newprefs1); + + $convertedreport2 = $this->create_report($course2); + $this->assertEquals([], $convertedreport2->collapsed['gradesonly']); + $this->assertEquals([], $convertedreport2->collapsed['aggregatesonly']); + $newprefs2 = get_user_preferences('grade_report_grader_collapsed_categories' . $course2->id); // Also verify new prefs. + $this->assertNull($newprefs2); + + $convertedreport3 = $this->create_report($course3); + $this->assertEquals([], $convertedreport3->collapsed['gradesonly']); + $this->assertEquals([], $convertedreport3->collapsed['aggregatesonly']); + $newprefs3 = get_user_preferences('grade_report_grader_collapsed_categories' . $course3->id); // Also verify new prefs. + $this->assertNull($newprefs3); + + // Make sure the old style user preference is removed now. + $this->assertEmpty(get_user_preferences('grade_report_grader_collapsed_categories')); + } + /** * Tests the get_right_rows function with one 'normal' and one 'ungraded' quiz. *