Remove toast when failing to send notification clear event

[aaronjwood]

Summary

Removes the toast notification when we fail to send a notification clear event.

Screenshots

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#

Any other notes

I'm sending this out to see if there is interest in doing this. Here is my thinking for removing this notification:

For people that don't expose HA to the internet but still receive notifications there's no reason to continually show a toast every time a notification is swiped away. I personally feel a log is good enough (which the codebase already has) whereas a toast constantly gets in the way. If there was no other means of logging this then I could see the reasoning for having a toast.

My personal situation is nearly identical to the above. I don't expose anything out to the internet, instead I VPN back home when needed. I receive notifications from my cameras and VPN in to view them when needed. If I don't VPN in and swipe notifications away I constantly get the toast that I am removing here.

Would anyone be in favor of taking this patch?

[homeassistant]

Hi @aaronjwood,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[dshokouhi]

We still need a way to notify users that there is a failure. The common user doesn't know to look in the logs when things silently fail which is why we make it a toast.

[aaronjwood]

We still need a way to notify users that there is a failure. The common user doesn't know to look in the logs when things silently fail which is why we make it a toast.

Okay, how about making this a setting somewhere then? I'm not familiar enough with this codebase yet to see how to stitch all of that together, but I could dig into it if there's interest.

[dshokouhi]

Okay, how about making this a setting somewhere then? I'm not familiar enough with this codebase yet to see how to stitch all of that together, but I could dig into it if there's interest.

A setting would work, you can refer to the SettingsFragment to see how some of the shared preference toggles are used across the application like WebView Debugging.

So what do you do when you want to use an actionable notification in the case you mentioned? Do you just not use that feature or connect to VPN so it can send the event? Wondering if we need to add this there too as we also post a Toast message when the action event fails to send.

[aaronjwood]

Okay, how about making this a setting somewhere then? I'm not familiar enough with this codebase yet to see how to stitch all of that together, but I could dig into it if there's interest.

A setting would work, you can refer to the SettingsFragment to see how some of the shared preference toggles are used across the application like WebView Debugging.

So what do you do when you want to use an actionable notification in the case you mentioned? Do you just not use that feature or connect to VPN so it can send the event? Wondering if we need to add this there too as we also post a Toast message when the action event fails to send.

Thanks. Right now I don't use them unless I am on the VPN. Good idea to wire up a setting for that. Maybe a new section under the notification settings subsection for enabling/disabling toasts? I'm thinking a toggle that says something like "display toast on failure to " where is for each of the areas that toasts are sent from.

[dshokouhi]

I think for now lets stick to the one that users are more likely to encounter, a setting for the event failure when a notification is cleared.

More than likely a user will find the actionable failure more relevant than the clear event (i.e. why didn't the garage door open?). My only real concern with splitting up the setting for each type of toast is that it can become a bit much if we add more features later on. We can always just accept feature requests later on if users want to add more options after they start using this feature.

[JBassett]

Converting to draft until ready.

[dshokouhi]

So in terms of the original intent of this PR as the author does not care for these events at all. Should we just make this a setting to control if the event gets sent at all? If a user never intends to get these events and doesn't care for the toast messages on failure then why send the event at all?

[dshokouhi]

@aaronjwood we had some discussion regarding our usage of Toast messages, I think its ok to just remove the Toast message and direct users to the logs if they don't see the event being sent.

We will still need the ktlint failure to be fixed before being able to proceed here.

[aaronjwood]

Okay great! I'll try to set aside some time to fix the lint error this week so we can get this merged.

[jpelgrom]

Don't forget to also remove the notification_clear_failure string, as it will no longer be used 🙂

[aaronjwood]

Sorry for the delay, should be all set now.