Fix round robin populate bug

src/oncall/scheduler/default.py

@@ -255,7 +255,7 @@ def calculate_future_events(self, schedule, cursor, start_epoch=None):
         # Return future events and the last epoch events were scheduled for.
         return future_events, self.utc_from_naive_date(next_epoch - timedelta(days=7 * period), schedule)
 
-    def find_least_active_available_user_id(self, schedule, future_events, cursor):
+    def find_next_user_id(self, schedule, future_events, cursor):
         team_id = schedule['team_id']
         role_id = schedule['role_id']
         roster_id = schedule['roster_id']
@@ -300,7 +300,7 @@ def schedule(self, team, schedules, dbinfo):
         # Create events in the db, associating a user to them
         # Iterate through events in order of start time to properly assign users
         for schedule, epoch in sorted(events, key=lambda x: min(ev['start'] for ev in x[1])):
-            user_id = self.find_least_active_available_user_id(schedule, epoch, cursor)
+            user_id = self.find_next_user_id(schedule, epoch, cursor)
             if not user_id:
                 logger.info('Failed to find available user')
                 continue
@@ -340,7 +340,7 @@ def populate(self, schedule, start_time, dbinfo):
 
         # Create events in the db, associating a user to them
         for epoch in future_events:
-            user_id = self.find_least_active_available_user_id(schedule, epoch, cursor)
+            user_id = self.find_next_user_id(schedule, epoch, cursor)
             if not user_id:
                 continue
             self.create_events(team_id, schedule['id'], user_id, epoch, role_id, cursor)

src/oncall/scheduler/round-robin.py

@@ -9,19 +9,20 @@ class Scheduler(default.Scheduler):
 
     def guess_last_scheduled_user(self, schedule, start, roster, cursor):
         cursor.execute('''
-                        SELECT MAX(`last_end`), `user_id` FROM
-                        (SELECT `user_id`, MAX(`end`) AS `last_end` FROM `event`
-                         WHERE `team_id` = %s AND `user_id` IN %s AND `end` <= %s
+                        SELECT `last_start`, `user_id` FROM
+                        (SELECT `user_id`, MAX(`start`) AS `last_start` FROM `event`
+                         WHERE `team_id` = %s AND `user_id` IN %s AND `start` <= %s
                          AND `role_id` = %s
-                         GROUP BY `user_id`) t
-                        GROUP BY `user_id`
+                         GROUP BY `user_id` 
+                         ORDER BY `last_start` DESC) t 
+                        LIMIT 1
                         ''', (schedule['team_id'], roster, start, schedule['role_id']))
         if cursor.rowcount != 0:
             return cursor.fetchone()['user_id']
         else:
             return None
 
-    def find_least_active_available_user_id(self, schedule, future_events, cursor):
+    def find_next_user_id(self, schedule, future_events, cursor):
         cursor.execute('''SELECT `user_id` FROM `roster_user`
                            WHERE `roster_id` = %s AND in_rotation = TRUE''',
                        schedule['roster_id'])
@@ -48,7 +49,7 @@ def find_least_active_available_user_id(self, schedule, future_events, cursor):
         last_idx = roster.index(last_user)
         return roster[(last_idx + 1) % len(roster)]
 
-    def create_events(self, team_id, schedule_id, user_id, events, role_id, cursor):
+    def create_events(self, team_id, schedule_id, user_id, events, role_id, cursor, skip_match=True):
         if len(events) == 1:
             [event] = events
             event_args = (team_id, schedule_id, event['start'], event['end'], user_id, role_id)
@@ -73,3 +74,9 @@ def create_events(self, team_id, schedule_id, user_id, events, role_id, cursor):
                     )'''
                 cursor.execute(query, event_args)
         cursor.execute('UPDATE `schedule` SET `last_scheduled_user_id` = %s', user_id)
+
+    def populate(self, schedule, start_time, dbinfo):
+        _, cursor = dbinfo
+        # Null last_scheduled_user to force find_next_user to determine that from the calendar
+        cursor.execute('UPDATE `schedule` SET `last_scheduled_user_id` = NULL')
+        super(Scheduler, self).populate(schedule, start_time, dbinfo)

test/test_scheduler.py

@@ -22,7 +22,7 @@ def test_find_new_user_as_least_active_user(mocker):
     mocker.patch('oncall.scheduler.default.Scheduler.get_busy_user_by_event_range')
     mocker.patch('oncall.scheduler.default.Scheduler.find_least_active_user_id_by_team')
 
-    user_id = scheduler.find_least_active_available_user_id(MOCK_SCHEDULE, [{'start': 0, 'end': 5}], None)
+    user_id = scheduler.find_next_user_id(MOCK_SCHEDULE, [{'start': 0, 'end': 5}], None)
     assert user_id == 123
 
 
@@ -251,7 +251,7 @@ def mock_busy_user_by_range_side_effect(user_ids, team_id, events, cursor):
                      {'start': 570, 'end': 588},
                      {'start': 600, 'end': 700}]
     scheduler = oncall.scheduler.default.Scheduler()
-    scheduler.find_least_active_available_user_id(MOCK_SCHEDULE, future_events, None)
+    scheduler.find_next_user_id(MOCK_SCHEDULE, future_events, None)
 
     mock_active_user_by_team.assert_called_with({456, 789}, 1, 440, 2, None)
 
@@ -270,6 +270,6 @@ def mock_busy_user_by_range_side_effect(user_ids, team_id, events, cursor):
     mock_busy_user_by_range.side_effect = mock_busy_user_by_range_side_effect
     future_events = [{'start': 440, 'end': 570}]
     scheduler = oncall.scheduler.default.Scheduler()
-    assert scheduler.find_least_active_available_user_id(MOCK_SCHEDULE, future_events, None) is None
+    assert scheduler.find_next_user_id(MOCK_SCHEDULE, future_events, None) is None
 
     mock_active_user_by_team.assert_not_called()