Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notifications: Provide notifications for Device/Instance crashes #4295

Closed
6 tasks done
Tracked by #277
joepavitt opened this issue Jul 31, 2024 · 17 comments
Closed
6 tasks done
Tracked by #277

Notifications: Provide notifications for Device/Instance crashes #4295

joepavitt opened this issue Jul 31, 2024 · 17 comments
Assignees
Labels
area:api Work on the platform API headline Something to highlight in the release size:M - 3 Sizing estimation point task A piece of work that isn't necessarily tied to a specific Epic or Story.
Milestone

Comments

@joepavitt
Copy link
Contributor

joepavitt commented Jul 31, 2024

Using the new Notifications API we should alert the user via the notifications system in FlowFuse that instances have crashed or restarted unexpectedly.

Tasks

Preview Give feedback

Tracking Issues

Preview Give feedback
  1. Steve-Mcl
  2. Steve-Mcl
  3. Steve-Mcl
@joepavitt joepavitt added this to the 2.8 milestone Jul 31, 2024
@joepavitt joepavitt moved this to Todo in 🛠 Development Jul 31, 2024
@joepavitt joepavitt added task A piece of work that isn't necessarily tied to a specific Epic or Story. size:M - 3 Sizing estimation point area:api Work on the platform API labels Jul 31, 2024
@joepavitt joepavitt moved this from Todo to Up Next in 🛠 Development Aug 14, 2024
@Steve-Mcl
Copy link
Contributor

Joe, since the notification API seems to only support a single recipient who should receive said notifications?

@joepavitt
Copy link
Contributor Author

Send multiple notifications? Find members of team, loop over, send notification to each?

@Steve-Mcl
Copy link
Contributor

@joepavitt

Send multiple notifications? Find members of team, loop over, send notification to each?

members? or just owners?

would that include admins of the platform too?

What if a team has 100+ members?

Also, not certain if/how/when we would clear this (if at all)?

@joepavitt
Copy link
Contributor Author

@hardillb who receives an email when instances go down, members + owner, or just owners?

No need for platform admins to be made aware

@joepavitt
Copy link
Contributor Author

Also, not certain if/how/when we would clear this (if at all)?

If the user clicks the notification, they'll be navigated to the instance overview, at which point, notification is marked as read

@hardillb
Copy link
Contributor

It's configurable

image

@Steve-Mcl
Copy link
Contributor

Also, not certain if/how/when we would clear this (if at all)?

If the user clicks the notification, they'll be navigated to the instance overview, at which point, notification is marked as read

Yeah, i know, but what if the application is restarted successfully (problem is resolved) - do we leave the notification in the other 99 users inbox?

@Steve-Mcl
Copy link
Contributor

It's configurable

image

That is specific to emails not notifications @hardillb (not is scope as far as I can see)

@hardillb
Copy link
Contributor

@Steve-Mcl it's the answer to Joe's question

@joepavitt
Copy link
Contributor Author

Can we read from that same setting to drive the in-app notification too?

@Steve-Mcl
Copy link
Contributor

Can we read from that same setting to drive the in-app notification too?

Yes, we can.

@Steve-Mcl
Copy link
Contributor

Also, not certain if/how/when we would clear this (if at all)?

If the user clicks the notification, they'll be navigated to the instance overview, at which point, notification is marked as read

Yeah, i know, but what if the application is restarted successfully (problem is resolved) - do we leave the notification in the other 99 users inbox?

@joepavitt ^

@joepavitt
Copy link
Contributor Author

yes - we do

@Steve-Mcl
Copy link
Contributor

Steve-Mcl commented Aug 23, 2024

Can we read from that same setting to drive the in-app notification too?

Yes, we can.

Bah - the email options are EE only.

Possible directions that spring to mind are (listed most difficult to easiest)

  1. We modify the email preferences so that it is dual purpose (emails and notification) (hide email section for CE)
    • Will require the settings data to be restructured due to current format
    /* current settings object ... */
    { 
        "emailAlerts": { "crash": true,  "safe": true, "recipients": "both"  } 
    }
    /* would need to be transformed for all instances to ... */
    { 
        "alerts": {
            "email": { "crash": true, "safe": true },
            "notification": { "crash": true, "safe": true },
            "recipients": "both"
        }
    }
  2. We duplicate the email preferences as "notifications"
    • This is cleaner in implementation due to separation of concerns
    • Settings can be stored without affecting existing structure
    /* existing settings object - unchanged */
    {
        "emailAlerts": { "crash": true,  "safe": true, "recipients": "both" }
    }
    /* add new setting notificationAlerts */
    {
        "notificationAlerts": { "crash": true,  "safe": true, "recipients": "both" } 
    }
  3. We just send the crashed notification to all members + owners (no options)
  4. We just send the crashed notification to owners (no options)
  5. We dont send crash notification on CE platforms (i.e. becomes an EE feature)

Mockups

image

image


@joepavitt I couldnt find in the owner/parent issues if notifications are EE only but since they are NOT implemented under the EE file structure I have assumed this feature is for all?

@Steve-Mcl Steve-Mcl self-assigned this Aug 23, 2024
@Steve-Mcl Steve-Mcl moved this from Up Next to In Design in 🛠 Development Aug 23, 2024
@joepavitt
Copy link
Contributor Author

Correct this is not an EE-Only feature - first iteration, send to all.

We can offer configurations for this at a later date.

@Steve-Mcl
Copy link
Contributor

Steve-Mcl commented Aug 23, 2024

No need for platform admins to be made aware

first iteration, send to all.

I am combining the 2 replies on this matter i.e. will send to Members and Owners

EDIT and excluding viewer/dashboard users

EDIT2:

To clarify - members and owners only (and if an admin happens to be a member or owner, they will get notification too)

@joepavitt joepavitt moved this from In Design to In Progress in 🛠 Development Aug 27, 2024
@Steve-Mcl Steve-Mcl moved this from In Progress to Review in 🛠 Development Aug 27, 2024
@Steve-Mcl Steve-Mcl moved this from Review to In Progress in 🛠 Development Aug 27, 2024
@Steve-Mcl Steve-Mcl moved this from In Progress to Review in 🛠 Development Aug 27, 2024
@joepavitt joepavitt added the headline Something to highlight in the release label Aug 28, 2024
@joepavitt
Copy link
Contributor Author

All tasks completed

@github-project-automation github-project-automation bot moved this from Review to Done in 🛠 Development Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Work on the platform API headline Something to highlight in the release size:M - 3 Sizing estimation point task A piece of work that isn't necessarily tied to a specific Epic or Story.
Projects
Status: Done
Development

No branches or pull requests

3 participants