PushoverAgent: HTML message support #2264
Conversation
Pushover has supported displaying the `message` as HTML in its apps for about three years. https://updates.pushover.net/post/117525190347/html-message-support
app/models/agents/pushover_agent.rb
Outdated
| @@ -30,6 +30,7 @@ class PushoverAgent < Agent | |||
| * `sound` - the name of one of the sounds supported by device clients to override the user's default sound choice. [See PushOver docs for sound options.](https://pushover.net/api#sounds) | |||
| * `retry` - Required for emergency priority - Specifies how often (in seconds) the Pushover servers will send the same notification to the user. Minimum value: `30` | |||
| * `expire` - Required for emergency priority - Specifies how many seconds your notification will continue to be retried for (every retry seconds). Maximum value: `86400` | |||
| * `html` - set to `1` to have Pushover's apps display the `message` content as HTML | |||
There was a problem hiding this comment.
Maybe this should be a true/false field, following our conventions.
app/models/agents/pushover_agent.rb
Outdated
| @@ -84,6 +86,7 @@ def receive(incoming_events) | |||
| sound | |||
| retry | |||
| expire | |||
| html | |||
| ].each do |key| | |||
| if value = String.try_convert(interpolated[key].presence) | |||
| case key | |||
There was a problem hiding this comment.
Add the following lines here to accept true as a value for html:
when 'html'
case value
when 'true', '1'
value = '1'
else
next
endThere was a problem hiding this comment.
I did mostly this, but explicitly set value = '0' in the else instead of just going to the next key.
Per feedback on huginn#2264
|
@knu Addressed your comments, let me know if you want the two commits squashed into one. :) |
|
@dgw Great! And yes, please squash them. |
|
Oh wait, if if value = interpolated['html'].presence
post_params['html'] =
case value.to_s
when 'true', '1'
'1'
else
'0'
end
end |
|
(That is because |
For reasons that are not clear, String.try_convert(true) yields nil, not a string. Go figure.
|
*angry nerd noises* Now I understand why Gary Bernhardt dropped that line in his "Wat" talk about "languages that suck". 🤣 |
|
There’s nothing wrong with try_convert, it is meant to work that way. Anyway, thanks for your contribution, I’ll try out HTML messages! |
|
Ha, I was going to squash at least the first two commits on approval. Oh well. I don't understand why |
| if value = interpolated['html'].presence | ||
| post_params['html'] = | ||
| case value.to_s | ||
| when 'true', '1' |
There was a problem hiding this comment.
Is there a reason why we want to support 1 as true here? We normally rely on the boolify that only accepts true and "true".
There was a problem hiding this comment.
Pushover's API documentation gives 1 as the value to pass, so it made sense to support what their docs said to do.
Pushover has supported displaying the
messageas HTML in its apps for about three years. https://updates.pushover.net/post/117525190347/html-message-support