Pushover Agent #233
Pushover Agent #233
Conversation
| cannot_be_scheduled! | ||
| cannot_create_events! | ||
|
|
||
| @@api_url = 'https://api.pushover.net/1/messages.json' |
There was a problem hiding this comment.
Stylistically, this might be better as a constant. E.g., API_URL.
There was a problem hiding this comment.
And I took care of this as well.
|
This is great work @akilism, thank you! I left a couple of small comments. |
|
❤️ I've been wanting this for a while! Thanks! On Friday, April 18, 2014, Andrew Cantino notifications@github.com wrote:
Dean |
…ylistic changes as well.
…ylistic changes as well.
| end | ||
|
|
||
| def validate_options | ||
| unless options['token'].present? && options['user'].present? && options['expected_receive_period_in_days'].present? |
There was a problem hiding this comment.
You provided default values for the token and the user which means if the user does not empty those fields your validation will always pass, i would change the default options for token and user to an empty string.
Personally i am not a big fan of determinating the state of an agent which is receiving events based on the time frame it received the last event. I would rather base it on the (un)successful delivery of the events it received.
There was a problem hiding this comment.
Yeah I can see that. I'll make this change later today. Really I'd like to take into account the response from pushover as to whether or not the notification was successful, a failure that can be retried or a failure that will never be successful and shouldn't be retried.
|
@cantino any word on this? |
|
Sorry, I missed this. I'll try to look at it tonight-- traveling today. |
| } | ||
|
|
||
| post_params['device'] = event.payload['device'] || options['device'] | ||
| post_params['title'] = event.payload['title'] || event.payload['subject'] || options['title'] |
There was a problem hiding this comment.
The only other change I'd suggest is to use .presence on these, so that empty fields are treated as missing.
post_params['title'] = event.payload['title'].presence || event.payload['subject'].presence || options['title']
and also below, anywhere you think that makes sense. E.g,
url = (event.payload['url'].presence || options['url'] || '').to_s
url_title = (event.payload['url_title'].presence || options['url_title']).to_s
post_params['priority'] = (event.payload['priority'].presence || options['priority']).to_i
etc.
sound, retry, expire, ...
Anything where empty string should be treated as nil.
|
Got it. I'll make this changes tonight! |
|
@cantino Updated. |
|
Thanks @akilism, nice work! |
Pushover Agent
Hi!
I wrote a little Pushover Agent with tests. Supports these optional api parameters with event level override.
devicetitleurlurl_titleprioritytimestampsound