metalmatze / alertmanager-bot

[deprecated] Bot for Prometheus' Alertmanager
MIT License
661 stars 148 forks source link

Fixing empty subscribed user list '@' #199

Closed nielsole closed 2 years ago

nielsole commented 2 years ago

chat.Username is not always set. "len(string) > 0" evaluates to false for both the empty string and nil.

Closes https://github.com/metalmatze/alertmanager-bot/issues/196

this was not tested

metalmatze commented 2 years ago

This should be testable with a unit test really good. I'll give you some time for that, otherwise taking a look myself potentially. Let me know if anything is unclear.

nielsole commented 2 years ago

Added a unit test and replaced another occurrence where Firstname could be nil. I never saw that occurrence be nil, but I guess graceful handling of empty fields is always a good idea.

nielsole commented 2 years ago

Merry Christmas btw :)

metalmatze commented 2 years ago

Thank you so much for this! Hope you had a great time off as well!