From a92d87f8fa9315ea2512b2ef9df82a506361114b Mon Sep 17 00:00:00 2001 From: Daniel Wang Date: Mon, 18 Jun 2018 17:41:23 -0700 Subject: [PATCH] Fix round robin populate bug --- src/oncall/scheduler/default.py | 6 +++--- src/oncall/scheduler/round-robin.py | 21 ++++++++++++++------- test/test_scheduler.py | 6 +++--- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/oncall/scheduler/default.py b/src/oncall/scheduler/default.py index d7a0d554..45c71f94 100644 --- a/src/oncall/scheduler/default.py +++ b/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) diff --git a/src/oncall/scheduler/round-robin.py b/src/oncall/scheduler/round-robin.py index 74a72eff..ac4a6735 100644 --- a/src/oncall/scheduler/round-robin.py +++ b/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) \ No newline at end of file diff --git a/test/test_scheduler.py b/test/test_scheduler.py index 95afc65d..06e5786d 100644 --- a/test/test_scheduler.py +++ b/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()