diff --git a/calendar/classes/local/event/data_access/event_vault.php b/calendar/classes/local/event/data_access/event_vault.php
index d2849194caa..68ba6204fd4 100644
--- a/calendar/classes/local/event/data_access/event_vault.php
+++ b/calendar/classes/local/event/data_access/event_vault.php
@@ -45,6 +45,9 @@ use core_calendar\local\interfaces\event_vault_interface;
*/
class event_vault implements event_vault_interface {
+ /**
+ * @var event_factory_interface $factory Factory for creating events.
+ */
private $factory;
/**
@@ -75,6 +78,10 @@ class event_vault implements event_vault_interface {
/**
* Retrieve an array of events for the given user and time constraints.
*
+ * If using this function for pagination then you can provide the last event that you've seen
+ * ($afterevent) and it will be used to appropriately offset the result set so that you don't
+ * receive the same events again.
+ *
* @param \stdClass $user The user for whom the events belong
* @param int|null $timesortfrom Events with timesort from this value (inclusive)
* @param int|null $timesortto Events with timesort until this value (inclusive)
@@ -101,22 +108,42 @@ class event_vault implements event_vault_interface {
throw new limit_invalid_parameter_exception("Limit must be between 1 and 50 (inclusive)");
}
+ $lastseentimesort = null;
$params = ['type' => CALENDAR_EVENT_TYPE_ACTION];
$where = ['type = :type'];
+ if (!is_null($afterevent)) {
+ $lastseentimesort = $afterevent->get_times()->get_sort_time()->getTimestamp();
+ }
+
if ($timesortfrom) {
- $where[] = 'timesort >= :timesortfrom';
- $params['timesortfrom'] = $timesortfrom;
+ if ($lastseentimesort && $lastseentimesort >= $timesortfrom) {
+ $where[] = '((timesort = :timesortfrom1 AND id > :timesortfromid) '.
+ 'OR timesort > :timesortfrom2)';
+ $params['timesortfromid'] = $afterevent->get_id();
+ $params['timesortfrom1'] = $lastseentimesort;
+ $params['timesortfrom2'] = $lastseentimesort;
+ } else {
+ $where[] = 'timesort >= :timesortfrom';
+ $params['timesortfrom'] = $timesortfrom;
+ }
}
if ($timesortto) {
- $where[] = 'timesort <= :timesortto';
- $params['timesortto'] = $timesortto;
- }
-
- if (!is_null($afterevent)) {
- $where[] = 'id > :id';
- $params['id'] = $afterevent->get_id();
+ if ($lastseentimesort && $lastseentimesort > $timesortto) {
+ // The last seen event from this set is after the time sort range which
+ // means all events in this range have been seen, so we can just return
+ // early here.
+ return [];
+ } else if ($lastseentimesort && $lastseentimesort == $timesortto) {
+ $where[] = '((timesort = :timesortto1 AND id > :timesorttoid) OR timesort < :timesortto2)';
+ $params['timesorttoid'] = $afterevent->get_id();
+ $params['timesortto1'] = $timesortto;
+ $params['timesortto2'] = $timesortto;
+ } else {
+ $where[] = 'timesort <= :timesortto';
+ $params['timesortto'] = $timesortto;
+ }
}
$sql = sprintf("SELECT * FROM {event} WHERE %s ORDER BY timesort ASC, id ASC",
@@ -150,8 +177,12 @@ class event_vault implements event_vault_interface {
/**
* Retrieve an array of events for the given user filtered by the course and time constraints.
*
+ * If using this function for pagination then you can provide the last event that you've seen
+ * ($afterevent) and it will be used to appropriately offset the result set so that you don't
+ * receive the same events again.
+ *
* @param \stdClass $user The user for whom the events belong
- * @param array $courses The courses to filter by
+ * @param \stdClass $course The course to filter by
* @param int|null $timesortfrom Events with timesort from this value (inclusive)
* @param int|null $timesortto Events with timesort until this value (inclusive)
* @param event_interface|null $afterevent Only return events after this one
@@ -172,6 +203,7 @@ class event_vault implements event_vault_interface {
throw new limit_invalid_parameter_exception("Limit must be between 1 and 50 (inclusive)");
}
+ $lastseentimesort = null;
$params = [
'type' => CALENDAR_EVENT_TYPE_ACTION,
'courseid' => $course->id,
@@ -181,24 +213,42 @@ class event_vault implements event_vault_interface {
'courseid = :courseid',
];
+ if (!is_null($afterevent)) {
+ $lastseentimesort = $afterevent->get_times()->get_sort_time()->getTimestamp();
+ }
+
if ($timesortfrom) {
- $where[] = 'timesort >= :timesortfrom';
- $params['timesortfrom'] = $timesortfrom;
+ if ($lastseentimesort && $lastseentimesort >= $timesortfrom) {
+ $where[] = '((timesort = :timesortfrom1 AND id > :timesortfromid) '.
+ 'OR timesort > :timesortfrom2)';
+ $params['timesortfromid'] = $afterevent->get_id();
+ $params['timesortfrom1'] = $lastseentimesort;
+ $params['timesortfrom2'] = $lastseentimesort;
+ } else {
+ $where[] = 'timesort >= :timesortfrom';
+ $params['timesortfrom'] = $timesortfrom;
+ }
}
if ($timesortto) {
- $where[] = 'timesort <= :timesortto';
- $params['timesortto'] = $timesortto;
- }
-
- if (!is_null($afterevent)) {
- $where[] = 'id > :id';
- $params['id'] = $afterevent->get_id();
+ if ($lastseentimesort && $lastseentimesort > $timesortto) {
+ // The last seen event from this set is after the time sort range which
+ // means all events in this range have been seen, so we can just return
+ // early here.
+ return [];
+ } else if ($lastseentimesort && $lastseentimesort == $timesortto) {
+ $where[] = '((timesort = :timesortto1 AND id > :timesorttoid) OR timesort < :timesortto2)';
+ $params['timesorttoid'] = $afterevent->get_id();
+ $params['timesortto1'] = $timesortto;
+ $params['timesortto2'] = $timesortto;
+ } else {
+ $where[] = 'timesort <= :timesortto';
+ $params['timesortto'] = $timesortto;
+ }
}
$wheresql = implode(' AND ', $where);
$sql = sprintf("SELECT * FROM {event} WHERE %s ORDER BY timesort ASC, id ASC", $wheresql);
-
$offset = 0;
$events = [];
// We need to continue to pull records from the database until we reach
diff --git a/calendar/classes/local/interfaces/event_vault_interface.php b/calendar/classes/local/interfaces/event_vault_interface.php
index 754a55cf2f0..623af51cdbe 100644
--- a/calendar/classes/local/interfaces/event_vault_interface.php
+++ b/calendar/classes/local/interfaces/event_vault_interface.php
@@ -42,6 +42,9 @@ interface event_vault_interface {
/**
* Retrieve an array of events for the given user and time constraints.
*
+ * If using this function for pagination then you can provide the last event that you've seen
+ * ($afterevent) and it will be used to appropriately offset the result set so that you don't
+ * receive the same events again.
* @param \stdClass $user The user for whom the events belong
* @param int $timesortfrom Events with timesort from this value (inclusive)
* @param int $timesortto Events with timesort until this value (inclusive)
@@ -56,4 +59,28 @@ interface event_vault_interface {
event_interface $afterevent,
$limitnum
);
+
+ /**
+ * Retrieve an array of events for the given user filtered by the course and time constraints.
+ *
+ * If using this function for pagination then you can provide the last event that you've seen
+ * ($afterevent) and it will be used to appropriately offset the result set so that you don't
+ * receive the same events again.
+ *
+ * @param \stdClass $user The user for whom the events belong
+ * @param \stdClass $course The course to filter by
+ * @param int|null $timesortfrom Events with timesort from this value (inclusive)
+ * @param int|null $timesortto Events with timesort until this value (inclusive)
+ * @param event_interface|null $afterevent Only return events after this one
+ * @param int $limitnum Return at most this number of events
+ * @return action_event_interface
+ */
+ public function get_action_events_by_course(
+ \stdClass $user,
+ \stdClass $course,
+ $timesortfrom,
+ $timesortto,
+ event_interface $afterevent,
+ $limitnum
+ );
}
diff --git a/calendar/tests/event_vault_test.php b/calendar/tests/event_vault_test.php
index 823fea6e034..cb1d78ee788 100644
--- a/calendar/tests/event_vault_test.php
+++ b/calendar/tests/event_vault_test.php
@@ -277,6 +277,129 @@ class core_calendar_event_vault_testcase extends advanced_testcase {
$this->assertEquals(sprintf('Event %d', $limit + 5), $events[4]->get_name());
}
+ /**
+ * Test that get_action_events_by_timesort returns events between the
+ * provided timesort values and after the last seen event when one is
+ * provided. This should work even when the event ids aren't ordered the
+ * same as the timesort order.
+ */
+ public function test_get_action_events_by_timesort_non_consecutive_ids() {
+ $this->resetAfterTest(true);
+ $this->setAdminuser();
+
+ $user = $this->getDataGenerator()->create_user();
+ $factory = new action_event_test_factory();
+ $vault = new event_vault($factory);
+
+ /**
+ * The events should be ordered by timesort as follows:
+ *
+ * 1 event 1
+ * 2 event 1
+ * 1 event 2
+ * 2 event 2
+ * 1 event 3
+ * 2 event 3
+ * 1 event 4
+ * 2 event 4
+ * 1 event 5
+ * 2 event 5
+ * 1 event 6
+ * 2 event 6
+ * 1 event 7
+ * 2 event 7
+ * 1 event 8
+ * 2 event 8
+ * 1 event 9
+ * 2 event 9
+ * 1 event 10
+ * 2 event 10
+ */
+ $records = [];
+ for ($i = 1; $i < 11; $i++) {
+ $records[] = create_event([
+ 'name' => sprintf('1 event %d', $i),
+ 'eventtype' => 'user',
+ 'userid' => $user->id,
+ 'timesort' => $i,
+ 'type' => CALENDAR_EVENT_TYPE_ACTION
+ ]);
+ }
+
+ for ($i = 1; $i < 11; $i++) {
+ $records[] = create_event([
+ 'name' => sprintf('2 event %d', $i),
+ 'eventtype' => 'user',
+ 'userid' => $user->id,
+ 'timesort' => $i,
+ 'type' => CALENDAR_EVENT_TYPE_ACTION
+ ]);
+ }
+
+ /**
+ * Expected result set:
+ *
+ * 2 event 4
+ * 1 event 5
+ * 2 event 5
+ * 1 event 6
+ * 2 event 6
+ * 1 event 7
+ * 2 event 7
+ * 1 event 8
+ * 2 event 8
+ */
+ $aftereventid = $records[3]->id;
+ $afterevent = $vault->get_event_by_id($aftereventid);
+ // Offset results by event with name "1 event 4" which has the same timesort
+ // value as the lower boundary of this query (3). Confirm that the given
+ // $afterevent is used to ignore events with the same timesortfrom values.
+ $events = $vault->get_action_events_by_timesort($user, 3, 8, $afterevent);
+
+ $this->assertCount(9, $events);
+ $this->assertEquals('2 event 4', $events[0]->get_name());
+ $this->assertEquals('2 event 8', $events[8]->get_name());
+
+ /**
+ * Expected result set:
+ *
+ * 2 event 4
+ * 1 event 5
+ */
+ $events = $vault->get_action_events_by_timesort($user, 3, 8, $afterevent, 2);
+
+ $this->assertCount(2, $events);
+ $this->assertEquals('2 event 4', $events[0]->get_name());
+ $this->assertEquals('1 event 5', $events[1]->get_name());
+
+ /**
+ * Expected result set:
+ *
+ * 2 event 8
+ */
+ $aftereventid = $records[7]->id;
+ $afterevent = $vault->get_event_by_id($aftereventid);
+ // Offset results by event with name "1 event 8" which has the same timesort
+ // value as the upper boundary of this query (8). Confirm that the given
+ // $afterevent is used to ignore events with the same timesortto values.
+ $events = $vault->get_action_events_by_timesort($user, 3, 8, $afterevent);
+
+ $this->assertCount(1, $events);
+ $this->assertEquals('2 event 8', $events[0]->get_name());
+
+ /**
+ * Expected empty result set.
+ */
+ $aftereventid = $records[18]->id;
+ $afterevent = $vault->get_event_by_id($aftereventid);
+ // Offset results by event with name "2 event 9" which has a timesort
+ // value larger than the upper boundary of this query (9 > 8). Confirm
+ // that the given $afterevent is used for filtering events.
+ $events = $vault->get_action_events_by_timesort($user, 3, 8, $afterevent);
+
+ $this->assertEmpty($events);
+ }
+
/**
* Test that get_action_events_by_course returns events after the
* provided timesort value.
@@ -611,4 +734,147 @@ class core_calendar_event_vault_testcase extends advanced_testcase {
$this->assertEquals(sprintf('Event %d', $limit + 4), $events[3]->get_name());
$this->assertEquals(sprintf('Event %d', $limit + 5), $events[4]->get_name());
}
+
+ /**
+ * Test that get_action_events_by_course returns events between the
+ * provided timesort values and after the last seen event when one is
+ * provided. This should work even when the event ids aren't ordered the
+ * same as the timesort order.
+ */
+ public function test_get_action_events_by_course_non_consecutive_ids() {
+ $this->resetAfterTest(true);
+ $this->setAdminuser();
+
+ $user = $this->getDataGenerator()->create_user();
+ $course1 = $this->getDataGenerator()->create_course();
+ $course2 = $this->getDataGenerator()->create_course();
+ $factory = new action_event_test_factory();
+ $vault = new event_vault($factory);
+
+ $this->setAdminuser();
+ $this->getDataGenerator()->enrol_user($user->id, $course1->id);
+ $this->getDataGenerator()->enrol_user($user->id, $course2->id);
+
+ /**
+ * The events should be ordered by timesort as follows:
+ *
+ * 1 event 1
+ * 2 event 1
+ * 1 event 2
+ * 2 event 2
+ * 1 event 3
+ * 2 event 3
+ * 1 event 4
+ * 2 event 4
+ * 1 event 5
+ * 2 event 5
+ * 1 event 6
+ * 2 event 6
+ * 1 event 7
+ * 2 event 7
+ * 1 event 8
+ * 2 event 8
+ * 1 event 9
+ * 2 event 9
+ * 1 event 10
+ * 2 event 10
+ */
+ $records = [];
+ for ($i = 1; $i < 11; $i++) {
+ $records[] = create_event([
+ 'name' => sprintf('1 event %d', $i),
+ 'eventtype' => 'user',
+ 'userid' => $user->id,
+ 'timesort' => $i,
+ 'type' => CALENDAR_EVENT_TYPE_ACTION,
+ 'courseid' => $course1->id,
+ ]);
+ }
+
+ for ($i = 1; $i < 11; $i++) {
+ $records[] = create_event([
+ 'name' => sprintf('2 event %d', $i),
+ 'eventtype' => 'user',
+ 'userid' => $user->id,
+ 'timesort' => $i,
+ 'type' => CALENDAR_EVENT_TYPE_ACTION,
+ 'courseid' => $course1->id,
+ ]);
+ }
+
+ // Create events for the other course.
+ for ($i = 1; $i < 11; $i++) {
+ $records[] = create_event([
+ 'name' => sprintf('3 event %d', $i),
+ 'eventtype' => 'user',
+ 'userid' => $user->id,
+ 'timesort' => $i,
+ 'type' => CALENDAR_EVENT_TYPE_ACTION,
+ 'courseid' => $course2->id,
+ ]);
+ }
+
+ /**
+ * Expected result set:
+ *
+ * 2 event 4
+ * 1 event 5
+ * 2 event 5
+ * 1 event 6
+ * 2 event 6
+ * 1 event 7
+ * 2 event 7
+ * 1 event 8
+ * 2 event 8
+ */
+ $aftereventid = $records[3]->id;
+ $afterevent = $vault->get_event_by_id($aftereventid);
+ // Offset results by event with name "1 event 4" which has the same timesort
+ // value as the lower boundary of this query (3). Confirm that the given
+ // $afterevent is used to ignore events with the same timesortfrom values.
+ $events = $vault->get_action_events_by_course($user, $course1, 3, 8, $afterevent);
+
+ $this->assertCount(9, $events);
+ $this->assertEquals('2 event 4', $events[0]->get_name());
+ $this->assertEquals('2 event 8', $events[8]->get_name());
+
+ /**
+ * Expected result set:
+ *
+ * 2 event 4
+ * 1 event 5
+ */
+ $events = $vault->get_action_events_by_course($user, $course1, 3, 8, $afterevent, 2);
+
+ $this->assertCount(2, $events);
+ $this->assertEquals('2 event 4', $events[0]->get_name());
+ $this->assertEquals('1 event 5', $events[1]->get_name());
+
+ /**
+ * Expected result set:
+ *
+ * 2 event 8
+ */
+ $aftereventid = $records[7]->id;
+ $afterevent = $vault->get_event_by_id($aftereventid);
+ // Offset results by event with name "1 event 8" which has the same timesort
+ // value as the upper boundary of this query (8). Confirm that the given
+ // $afterevent is used to ignore events with the same timesortto values.
+ $events = $vault->get_action_events_by_course($user, $course1, 3, 8, $afterevent);
+
+ $this->assertCount(1, $events);
+ $this->assertEquals('2 event 8', $events[0]->get_name());
+
+ /**
+ * Expected empty result set.
+ */
+ $aftereventid = $records[18]->id;
+ $afterevent = $vault->get_event_by_id($aftereventid);
+ // Offset results by event with name "2 event 9" which has a timesort
+ // value larger than the upper boundary of this query (9 > 8). Confirm
+ // that the given $afterevent is used for filtering events.
+ $events = $vault->get_action_events_by_course($user, $course1, 3, 8, $afterevent);
+
+ $this->assertEmpty($events);
+ }
}