search

hubot-archive / hubot-pager-me

PagerDuty integration for Hubot
https://www.npmjs.com/package/hubot-pager-me
MIT License
73 stars 93 forks source link

No oncalls returns if `HUBOT_PAGERDUTY_SCHEDULES` is unset #118

Closed RussellRollins closed 5 years ago

RussellRollins commented 5 years ago

I believe this issue was introduced in: https://github.com/hubot-scripts/hubot-pager-me/pull/117

If the HUBOT_PAGERDUTY_SCHEDULES environment variable is unset, allowed_schedules is an empty array.

I think the intention of the line:

if !allowed_schedules or schedule.id in allowed_schedules

Is to allow all results if the environment variable is unset, however, the !allowed_schedules check is not truthy for empty arrays:

https://repl.it/repls/LoudLimpOolanguage

So instead, it allows no results.

I suspect a better check would be

if !allowed_schedules.length or schedule.id in allowed_schedules
stephenyeargin commented 5 years ago

My fault in the code review. 🐼 We were looking towards the completion of the new filters, but I forgot to guard backwards compatibility.

stephenyeargin commented 5 years ago

There's also another issue -- if a schedule doesn't have a person assigned to it, it will come through the list regardless.

RussellRollins commented 5 years ago

@stephenyeargin no worries, caught the issue today when a hubot got automatically upgraded. I'm happy to open a PR to fix the first issue, but I'm not sure I fully understand the second.

stephenyeargin commented 5 years ago

Second issue isn't as critical since it's only showing up in the logs. My PR cleans up the logic a bit to avoid getting to that block.

stephenyeargin commented 5 years ago

Released in v3.1.1