[ntfy] priority level is not clearly communicated to the user

tdavis6 commented 9 months ago

⚠️ Please verify that this bug has NOT been raised before.

[X] I checked and didn't find similar issue

🛡️ Security Policy

[X] I agree to have read this project Security Policy

Description

When a notification is triggered, it sends to ntfy with a priority of 5. This is the incorrect behavior because it is set to send with a priority of 4. The double chevron indicates a priority of 4, while the triple chevron indicates a priority of 5.

Thank you so much for such an awesome app!

👟 Reproduction steps

This is triggered by setting a notification to use ntfy, with a priority of 4. Then, if a monitor goes down, the notification is sent with a priority of 5.

👀 Expected behavior

It would be expected that the notification is sent with a priority of 4.

😓 Actual Behavior

The notification was sent with a priority of 5.

🐻 Uptime-Kuma Version

1.23.8

💻 Operating System and Arch

Synology DSM 7.2.1-69057 Update 3

🌐 Browser

Firefox 120.0.1

🐋 Docker Version

20.10.23

🟩 NodeJS Version

No response

📝 Relevant log output

No response

tdavis6 commented 9 months ago

So sorry, just read the ntfy.js file and this appears to be the intended behavior. This is based on lines 44-45.

CommanderStorm commented 9 months ago

The behaviour is this way since https://github.com/louislam/uptime-kuma/pull/2863

https://github.com/louislam/uptime-kuma/blob/ad4629cb038c5b47fb04053f48c6fa905644e386/server/notification-providers/ntfy.js#L44-L45

=> down monitors have n+1 priority

tdavis6 commented 9 months ago

Thank you so much for the quick reply!

CommanderStorm commented 9 months ago

appears to be the intended behavior

Missing documentation / confusing UX is a bug too (!). I think this should have a help text in the frontend

CommanderStorm commented 9 months ago

@tdavis6 What do you think about such a helptext ? I am struggling to find a formulation which does not include max(5, priority + 1). Do you have any good ideas?

CommanderStorm commented 9 months ago

Okay. I think I found a way which does not include a formula. Oppinions?

tdavis6 commented 9 months ago

I think that looks good! However, if the set priority is 3 and it bumps it to 4 that might not be clear. I'm trying to think of something that would make that behavior clear, without being too long to put there.

tdavis6 commented 9 months ago

Perhaps "All events are sent with this priority, except DOWN-events, which have a priority one above this, with a maximum of 5". Although this might be too long.

tdavis6 commented 9 months ago

Thank you so much for helping me!

sevmonster commented 9 months ago

I wasn't aware of this functionality. Would it be too much to have a separate priority for down alerts?

CommanderStorm commented 9 months ago

Resolved by https://github.com/louislam/uptime-kuma/pull/4175

louislam / uptime-kuma