diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 27c4ae87327bb..f32def9a70c42 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -885,74 +885,47 @@ public function has_overrides() { * Returns user override * * Algorithm: For each assign setting, if there is a matching user-specific override, - * then use that otherwise, if there are group-specific overrides, return the most - * lenient combination of them. If neither applies, leave the assign setting unchanged. + * then use that otherwise, if there are group-specific overrides, use the one with the + * lowest sort order. If neither applies, leave the assign setting unchanged. * * @param int $userid The userid. - * @return override if exist + * @return stdClass The override */ public function override_exists($userid) { global $DB; - // Check for user override. - $override = $DB->get_record('assign_overrides', array('assignid' => $this->get_instance()->id, 'userid' => $userid)); + // Gets an assoc array containing the keys for defined user overrides only. + $getuseroverride = function($userid) use ($DB) { + $useroverride = $DB->get_record('assign_overrides', ['assignid' => $this->get_instance()->id, 'userid' => $userid]); + return $useroverride ? get_object_vars($useroverride) : []; + }; - if (!$override) { - $override = new stdClass(); - $override->duedate = null; - $override->cutoffdate = null; - $override->allowsubmissionsfromdate = null; - } + // Gets an assoc array containing the keys for defined group overrides only. + $getgroupoverride = function($userid) use ($DB) { + $groupings = groups_get_user_groups($this->get_instance()->course, $userid); - // Check for group overrides. - $groupings = groups_get_user_groups($this->get_instance()->course, $userid); + if (empty($groupings[0])) { + return []; + } - if (!empty($groupings[0])) { // Select all overrides that apply to the User's groups. list($extra, $params) = $DB->get_in_or_equal(array_values($groupings[0])); $sql = "SELECT * FROM {assign_overrides} - WHERE groupid $extra AND assignid = ?"; + WHERE groupid $extra AND assignid = ? ORDER BY sortorder ASC"; $params[] = $this->get_instance()->id; - $records = $DB->get_records_sql($sql, $params); - - // Combine the overrides. - $duedates = array(); - $cutoffdates = array(); - $allowsubmissionsfromdates = array(); - - foreach ($records as $gpoverride) { - if (isset($gpoverride->duedate)) { - $duedates[] = $gpoverride->duedate; - } - if (isset($gpoverride->cutoffdate)) { - $cutoffdates[] = $gpoverride->cutoffdate; - } - if (isset($gpoverride->allowsubmissionsfromdate)) { - $allowsubmissionsfromdates[] = $gpoverride->allowsubmissionsfromdate; - } - } - // If there is a user override for a setting, ignore the group override. - if (is_null($override->allowsubmissionsfromdate) && count($allowsubmissionsfromdates)) { - $override->allowsubmissionsfromdate = min($allowsubmissionsfromdates); - } - if (is_null($override->cutoffdate) && count($cutoffdates)) { - if (in_array(0, $cutoffdates)) { - $override->cutoffdate = 0; - } else { - $override->cutoffdate = max($cutoffdates); - } - } - if (is_null($override->duedate) && count($duedates)) { - if (in_array(0, $duedates)) { - $override->duedate = 0; - } else { - $override->duedate = max($duedates); - } - } - - } - - return $override; + $groupoverride = $DB->get_record_sql($sql, $params, IGNORE_MULTIPLE); + + return $groupoverride ? get_object_vars($groupoverride) : []; + }; + + // Later arguments clobber earlier ones with array_merge. The two helper functions + // return arrays containing keys for only the defined overrides. So we get the + // desired behaviour as per the algorithm. + return (object)array_merge( + ['duedate' => null, 'cutoffdate' => null, 'allowsubmissionsfromdate' => null], + $getgroupoverride($userid), + $getuseroverride($userid) + ); } /** diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php index fac227384ffb4..193f7fa0b656b 100644 --- a/mod/assign/tests/locallib_test.php +++ b/mod/assign/tests/locallib_test.php @@ -2680,6 +2680,119 @@ public function test_get_plugins_file_areas() { $this->assertEquals(2, $usingfilearea); } + /** + * Test override exists + * + * This function needs to obey the group override logic as per the assign grading table and + * the overview block. + */ + public function test_override_exists() { + global $DB; + + $this->setAdminUser(); + + $course = $this->getDataGenerator()->create_course(); + + // Create an assign instance. + $assign = $this->create_instance(['course' => $course]); + $assigninstance = $assign->get_instance(); + + // Create users. + $users = [ + 'Only in group A' => $this->getDataGenerator()->create_user(), + 'Only in group B' => $this->getDataGenerator()->create_user(), + 'In group A and B (no user override)' => $this->getDataGenerator()->create_user(), + 'In group A and B (user override)' => $this->getDataGenerator()->create_user(), + 'In no groups' => $this->getDataGenerator()->create_user() + ]; + + // Enrol users. + foreach ($users as $user) { + $this->getDataGenerator()->enrol_user($user->id, $course->id); + } + + // Create groups. + $groupa = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $groupb = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + + // Add members to groups. + // Group A. + $this->getDataGenerator()->create_group_member( + ['groupid' => $groupa->id, 'userid' => $users['Only in group A']->id]); + $this->getDataGenerator()->create_group_member( + ['groupid' => $groupa->id, 'userid' => $users['In group A and B (no user override)']->id]); + $this->getDataGenerator()->create_group_member( + ['groupid' => $groupa->id, 'userid' => $users['In group A and B (user override)']->id]); + + // Group B. + $this->getDataGenerator()->create_group_member( + ['groupid' => $groupb->id, 'userid' => $users['Only in group B']->id]); + $this->getDataGenerator()->create_group_member( + ['groupid' => $groupb->id, 'userid' => $users['In group A and B (no user override)']->id]); + $this->getDataGenerator()->create_group_member( + ['groupid' => $groupb->id, 'userid' => $users['In group A and B (user override)']->id]); + + // Overrides for each of the groups, and a user override. + $overrides = [ + // Override for group A, highest priority (numerically lowest sortorder). + [ + 'assignid' => $assigninstance->id, + 'groupid' => $groupa->id, + 'userid' => null, + 'sortorder' => 1, + 'allowsubmissionsfromdate' => 1, + 'duedate' => 2, + 'cutoffdate' => 3 + ], + // Override for group B, lower priority (numerically higher sortorder). + [ + 'assignid' => $assigninstance->id, + 'groupid' => $groupb->id, + 'userid' => null, + 'sortorder' => 2, + 'allowsubmissionsfromdate' => 5, + 'duedate' => 6, + 'cutoffdate' => 6 + ], + // User override. + [ + 'assignid' => $assigninstance->id, + 'groupid' => null, + 'userid' => $users['In group A and B (user override)']->id, + 'sortorder' => null, + 'allowsubmissionsfromdate' => 7, + 'duedate' => 8, + 'cutoffdate' => 9 + ], + ]; + + // Kinda hacky, need to add the ID to the overrides in the above array + // for later. + foreach ($overrides as &$override) { + $override['id'] = $DB->insert_record('assign_overrides', $override); + } + + $returnedoverrides = array_reduce(array_keys($users), function($carry, $description) use ($users, $assign) { + return $carry + ['For user ' . lcfirst($description) => $assign->override_exists($users[$description]->id)]; + }, []); + + // Test we get back the correct override from override_exists (== checks all object members match). + // User only in group A should see the group A override. + $this->assertTrue($returnedoverrides['For user only in group A'] == (object)$overrides[0]); + // User only in group B should see the group B override. + $this->assertTrue($returnedoverrides['For user only in group B'] == (object)$overrides[1]); + // User in group A and B, with no user override should see the group A override + // as it has higher priority (numerically lower sortorder). + $this->assertTrue($returnedoverrides['For user in group A and B (no user override)'] == (object)$overrides[0]); + // User in group A and B, with a user override should see the user override + // as it has higher priority (numerically lower sortorder). + $this->assertTrue($returnedoverrides['For user in group A and B (user override)'] == (object)$overrides[2]); + // User with no overrides should get nothing. + $this->assertNull($returnedoverrides['For user in no groups']->duedate); + $this->assertNull($returnedoverrides['For user in no groups']->cutoffdate); + $this->assertNull($returnedoverrides['For user in no groups']->allowsubmissionsfromdate); + } + /** * Test the quicksave grades processor */