Skip to content

Commit

Permalink
MDL-58578 core_calendar: Properly respect priorities on overview
Browse files Browse the repository at this point in the history 
Prior to this patch, if a user was in two groups, and an override
existed for both groups in an assignment the override
visually lower on the override list would be displayed on the
overview, whereas the one visually higher would be displayed
in the assignment grading table.
  • Loading branch information
@cameorn1730
cameorn1730 committed Apr 27, 2017
1 parent 5ccddd2 commit a776415
Show file tree
Hide file tree
Showing 7 changed files with 213 additions and 7 deletions.
2 changes: 1 addition & 1 deletion calendar/classes/local/event/strategies/raw_event_retrieval_strategy.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ protected function get_raw_events_legacy_implementation(
$subquery = "SELECT ev.modulename,
ev.instance,
ev.eventtype,
MAX(ev.priority) as priority
MIN(ev.priority) as priority
FROM {event} ev
$subquerywhere
GROUP BY ev.modulename, ev.instance, ev.eventtype";
Expand Down
4 changes: 2 additions & 2 deletions calendar/lib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
/**
* CALENDAR_EVENT_USER_OVERRIDE_PRIORITY - Constant for the user override priority.
*/
define('CALENDAR_EVENT_USER_OVERRIDE_PRIORITY', 9999999);
define('CALENDAR_EVENT_USER_OVERRIDE_PRIORITY', 0);

/**
* CALENDAR_EVENT_TYPE_STANDARD - Standard events.
Expand Down Expand Up @@ -1233,7 +1233,7 @@ function calendar_get_events($tstart, $tend, $users, $groups, $courses, $withdur
$subquery = "SELECT ev.modulename,
ev.instance,
ev.eventtype,
MAX(ev.priority) as priority
MIN(ev.priority) as priority
FROM {event} ev
$subquerywhere
GROUP BY ev.modulename, ev.instance, ev.eventtype";
Expand Down
4 changes: 2 additions & 2 deletions calendar/tests/lib_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public function test_calendar_get_events_with_overrides() {
$events = calendar_get_events($timestart, $timeend, $group12student->id, $groups, $course->id);
$this->assertCount(1, $events);
$event = reset($events);
$this->assertEquals('Assignment 1 due date - Group B override', $event->name);
$this->assertEquals('Assignment 1 due date - Group A override', $event->name);
// Get events for user that belongs to group A and has no user override events.
$this->setUser($group1student);
$events = calendar_get_events($timestart, $timeend, $group1student->id, $groups, $course->id);
Expand Down Expand Up @@ -524,7 +524,7 @@ public function test_get_legacy_events_with_overrides() {
$events = calendar_get_legacy_events($timestart, $timeend, $group12student->id, $groups, $course->id);
$this->assertCount(1, $events);
$event = reset($events);
$this->assertEquals('Assignment 1 due date - Group B override', $event->name);
$this->assertEquals('Assignment 1 due date - Group A override', $event->name);

// Get events for user that belongs to group A and has no user override events.
$this->setUser($group1student);
Expand Down
183 changes: 183 additions & 0 deletions calendar/tests/local_api_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -675,4 +675,187 @@ public function test_get_legacy_events_with_disabled_module() {
$event = reset($events);
$this->assertEquals('assign', $event->modulename);
}

/**
* Test for \core_calendar\local\api::get_legacy_events() when there are user and group overrides.
*/
public function test_get_legacy_events_with_overrides() {
$generator = $this->getDataGenerator();

$course = $generator->create_course();

$plugingenerator = $this->getDataGenerator()->get_plugin_generator('mod_assign');
if (!isset($params['course'])) {
$params['course'] = $course->id;
}

$instance = $plugingenerator->create_instance($params);

// Create users.
$useroverridestudent = $generator->create_user();
$group1student = $generator->create_user();
$group2student = $generator->create_user();
$group12student = $generator->create_user();
$nogroupstudent = $generator->create_user();

// Enrol users.
$generator->enrol_user($useroverridestudent->id, $course->id, 'student');
$generator->enrol_user($group1student->id, $course->id, 'student');
$generator->enrol_user($group2student->id, $course->id, 'student');
$generator->enrol_user($group12student->id, $course->id, 'student');
$generator->enrol_user($nogroupstudent->id, $course->id, 'student');

// Create groups.
$group1 = $generator->create_group(['courseid' => $course->id]);
$group2 = $generator->create_group(['courseid' => $course->id]);

// Add members to groups.
$generator->create_group_member(['groupid' => $group1->id, 'userid' => $group1student->id]);
$generator->create_group_member(['groupid' => $group2->id, 'userid' => $group2student->id]);
$generator->create_group_member(['groupid' => $group1->id, 'userid' => $group12student->id]);
$generator->create_group_member(['groupid' => $group2->id, 'userid' => $group12student->id]);
$now = time();

// Events with the same module name, instance and event type.
$events = [
[
'name' => 'Assignment 1 due date',
'description' => '',
'format' => 0,
'courseid' => $course->id,
'groupid' => 0,
'userid' => 2,
'modulename' => 'assign',
'instance' => $instance->id,
'eventtype' => 'due',
'timestart' => $now,
'timeduration' => 0,
'visible' => 1
], [
'name' => 'Assignment 1 due date - User override',
'description' => '',
'format' => 1,
'courseid' => 0,
'groupid' => 0,
'userid' => $useroverridestudent->id,
'modulename' => 'assign',
'instance' => $instance->id,
'eventtype' => 'due',
'timestart' => $now + 86400,
'timeduration' => 0,
'visible' => 1,
'priority' => CALENDAR_EVENT_USER_OVERRIDE_PRIORITY
], [
'name' => 'Assignment 1 due date - Group A override',
'description' => '',
'format' => 1,
'courseid' => $course->id,
'groupid' => $group1->id,
'userid' => 2,
'modulename' => 'assign',
'instance' => $instance->id,
'eventtype' => 'due',
'timestart' => $now + (2 * 86400),
'timeduration' => 0,
'visible' => 1,
'priority' => 1,
], [
'name' => 'Assignment 1 due date - Group B override',
'description' => '',
'format' => 1,
'courseid' => $course->id,
'groupid' => $group2->id,
'userid' => 2,
'modulename' => 'assign',
'instance' => $instance->id,
'eventtype' => 'due',
'timestart' => $now + (3 * 86400),
'timeduration' => 0,
'visible' => 1,
'priority' => 2,
],
];

foreach ($events as $event) {
calendar_event::create($event, false);
}

$timestart = $now - 100;
$timeend = $now + (3 * 86400);
$groups = [$group1->id, $group2->id];

// Get user override events.
$this->setUser($useroverridestudent);
$events = calendar_get_legacy_events($timestart, $timeend, $useroverridestudent->id, $groups, $course->id);
$this->assertCount(1, $events);
$event = reset($events);
$this->assertEquals('Assignment 1 due date - User override', $event->name);

// Get event for user with override but with the timestart and timeend parameters only covering the original event.
$events = calendar_get_legacy_events($timestart, $now, $useroverridestudent->id, $groups, $course->id);
$this->assertCount(0, $events);

// Get events for user that does not belong to any group and has no user override events.
$this->setUser($nogroupstudent);
$events = calendar_get_legacy_events($timestart, $timeend, $nogroupstudent->id, $groups, $course->id);
$this->assertCount(1, $events);
$event = reset($events);
$this->assertEquals('Assignment 1 due date', $event->name);

// Get events for user that belongs to groups A and B and has no user override events.
$this->setUser($group12student);
$events = calendar_get_legacy_events($timestart, $timeend, $group12student->id, $groups, $course->id);
$this->assertCount(1, $events);
$event = reset($events);
$this->assertEquals('Assignment 1 due date - Group A override', $event->name);

// Get events for user that belongs to group A and has no user override events.
$this->setUser($group1student);
$events = calendar_get_legacy_events($timestart, $timeend, $group1student->id, $groups, $course->id);
$this->assertCount(1, $events);
$event = reset($events);
$this->assertEquals('Assignment 1 due date - Group A override', $event->name);

// Add repeating events.
$repeatingevents = [
[
'name' => 'Repeating site event',
'description' => '',
'format' => 1,
'courseid' => SITEID,
'groupid' => 0,
'userid' => 2,
'repeatid' => $event->id,
'modulename' => '0',
'instance' => 0,
'eventtype' => 'site',
'timestart' => $now + 86400,
'timeduration' => 0,
'visible' => 1,
],
[
'name' => 'Repeating site event',
'description' => '',
'format' => 1,
'courseid' => SITEID,
'groupid' => 0,
'userid' => 2,
'repeatid' => $event->id,
'modulename' => '0',
'instance' => 0,
'eventtype' => 'site',
'timestart' => $now + (2 * 86400),
'timeduration' => 0,
'visible' => 1,
],
];

foreach ($repeatingevents as $event) {
calendar_event::create($event, false);
}

// Make sure repeating events are not filtered out.
$events = calendar_get_legacy_events($timestart, $timeend, true, true, true);
$this->assertCount(3, $events);
}
}
2 changes: 1 addition & 1 deletion calendar/tests/raw_event_retrieval_strategy_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public function test_get_raw_event_strategy_with_overrides() {
$events = $retrievalstrategy->get_raw_events([$group12student->id], $groups, [$course->id]);
$this->assertCount(1, $events);
$event = reset($events);
$this->assertEquals('Assignment 1 due date - Group B override', $event->name);
$this->assertEquals('Assignment 1 due date - Group A override', $event->name);

// Get events for user that belongs to group A and has no user override events.
$this->setUser($group1student);
Expand Down
23 changes: 23 additions & 0 deletions lib/db/upgrade.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2575,6 +2575,10 @@ function xmldb_main_upgrade($oldversion) {
$record->nextruntime = $nextruntime;
$DB->insert_record('task_adhoc', $record);

// This same task is queued again in a later step, but if we already queue it here
// then there is no need to queue it again. We use this flag in the second step.
$refresheventsadhocadded = true;

// Main savepoint reached.
upgrade_main_savepoint(true, 2017030700.00);
}
Expand Down Expand Up @@ -2834,5 +2838,24 @@ function xmldb_main_upgrade($oldversion) {
upgrade_main_savepoint(true, 2017041801.00);
}

if ($oldversion < 2017042600.01) {
// If the previous step didn't execute and queue the task.
if (!isset($refresheventsadhocadded)) {
// Create adhoc task for upgrading of existing calendar events.
$record = new \stdClass();
$record->classname = "\\core\\task\\refresh_mod_calendar_events_task";
$record->component = 'core';

// Next run time based from nextruntime computation in \core\task\manager::queue_adhoc_task().
$nextruntime = time() - 1;
$record->nextruntime = $nextruntime;
$DB->insert_record('task_adhoc', $record);

}

// Main savepoint reached.
upgrade_main_savepoint(true, 2017042600.01);
}

return true;
}
2 changes: 1 addition & 1 deletion version.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

defined('MOODLE_INTERNAL') || die();

$version = 2017042600.00; // YYYYMMDD = weekly release date of this DEV branch.
$version = 2017042600.01; // YYYYMMDD = weekly release date of this DEV branch.
// RR = release increments - 00 in DEV branches.
// .XX = incremental changes.

Expand Down

0 comments on commit a776415

Please sign in to comment.