Skip to content

Commit

Permalink
MDL-58514 mod_assign: Apply consistent logic for overrides
Browse files Browse the repository at this point in the history 
Previously the assign submission page was using "lenient" logic
for overrides when more than one group override applied to a user
(i.e., use the earliest "open" date and the latest "due" date)
when really it should be using the sortorder as per the assign
grading table.
  • Loading branch information
@cameorn1730
cameorn1730 committed May 3, 2017
1 parent 932bc91 commit 26aedd4
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 55 deletions.
83 changes: 28 additions & 55 deletions mod/assign/locallib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}

/**
Expand Down
113 changes: 113 additions & 0 deletions mod/assign/tests/locallib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down

0 comments on commit 26aedd4

Please sign in to comment.