From 736a89c4275e2339825f6aa5b691952909a0b50b Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Fri, 3 Feb 2017 15:50:42 +0200 Subject: [PATCH] Fix bug causing `when am I on call` to error out. This was happening if the person is not on any schedule. Fixes #51 --- lib/pagerbot/action_manager.rb | 9 +++++++-- lib/pagerbot/utilities.rb | 1 + templates/lookup_person_irc.erb | 6 +++++- templates/lookup_person_slack.erb | 6 +++++- test/unit/pagerbot/action_manager.rb | 13 +++++++++++++ 5 files changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/pagerbot/action_manager.rb b/lib/pagerbot/action_manager.rb index 7541aad..b7a5341 100644 --- a/lib/pagerbot/action_manager.rb +++ b/lib/pagerbot/action_manager.rb @@ -115,7 +115,12 @@ def lookup_person(query, event_data) # person goes on any on call. if query[:schedule] == 'call' next_oncall = @pagerduty.next_oncall(person.id, nil) - schedule = @pagerduty.find_schedule(next_oncall[:schedule][:name]) + if next_oncall[:schedule].nil? + schedule = nil + next_oncall = nil + else + schedule = @pagerduty.find_schedule(next_oncall[:schedule][:name]) + end else schedule = @pagerduty.find_schedule(query[:schedule]) next_oncall = @pagerduty.next_oncall(person.id, schedule.id) @@ -126,7 +131,7 @@ def lookup_person(query, event_data) person: person, schedule: schedule } - if next_oncall + unless next_oncall.nil? output_data[:time] = person.parse_time(next_oncall[:start]) end diff --git a/lib/pagerbot/utilities.rb b/lib/pagerbot/utilities.rb index d0cb508..e1f683c 100644 --- a/lib/pagerbot/utilities.rb +++ b/lib/pagerbot/utilities.rb @@ -107,6 +107,7 @@ def self.display_time(time) end def self.display_schedule(schedule, text=nil) + return 'any schedule' if schedule.nil? url = PagerBot.pagerduty.uri_base+"/schedules##{schedule.id}" link_to(url, text || schedule.name) end diff --git a/templates/lookup_person_irc.erb b/templates/lookup_person_irc.erb index e48f70f..759189e 100644 --- a/templates/lookup_person_irc.erb +++ b/templates/lookup_person_irc.erb @@ -1,5 +1,9 @@ <% if time.nil? %> -<%= person.name %> is not scheduled to go on <%= schedule.name %> +<%= person.name %> is not scheduled to go on <% if schedule.nil? %> +any schedule +<% else %> +<%= schedule.name %> +<% end %> <% elsif time < Time.now %> <%= person.name %> is on <%= schedule.name %> now <% else %> diff --git a/templates/lookup_person_slack.erb b/templates/lookup_person_slack.erb index 8baf3b9..2e1b7bd 100644 --- a/templates/lookup_person_slack.erb +++ b/templates/lookup_person_slack.erb @@ -1,5 +1,9 @@ <% if time.nil? %> -<%= person.name %> is not scheduled to go on <%= utilities.display_schedule(schedule) %>. +<%= person.name %> is not scheduled to go on <% if schedule.nil? %> +any schedule +<% else %> +<%= utilities.display_schedule(schedule) %> +<% end %> <% elsif time < Time.now %> <%= person.name %> is on <%= utilities.display_schedule(schedule) %> now. <% else %> diff --git a/test/unit/pagerbot/action_manager.rb b/test/unit/pagerbot/action_manager.rb index fc86552..ac7e1a1 100644 --- a/test/unit/pagerbot/action_manager.rb +++ b/test/unit/pagerbot/action_manager.rb @@ -122,6 +122,19 @@ def event_data(opts={nick: "karl"}) assert_equal("Karl-Aksel Puulmann is on Primary breakage now", response.fetch(:message)) end + + it 'special case: on schedule `call` should succeed if person is not to go on any schedule' do + @pagerduty.expects(:next_oncall) + .with('P123456', nil) + .returns schedule: nil + + response = @manager.lookup_person({ + :person => "karl", + :schedule => "call" + }, event_data) + + assert_equal("Karl-Aksel Puulmann is not scheduled to go on any schedule", response.fetch(:message)) + end end describe 'who is on schedule X' do