HTBox / allReady

This repo contains the code for allReady, an open-source solution focused on increasing awareness, efficiency and impact of preparedness campaigns as they are delivered by humanitarian and disaster response organizations in local communities.
http://www.htbox.org/projects/allready
MIT License
891 stars 627 forks source link

Proposal: Notification Framework #389

Open MisterJames opened 8 years ago

MisterJames commented 8 years ago

We have an existing notification mechanism that has served the immediate needs of the MVP. I would like to propose that we augment that and build out a framework for rich notifications that improves the experience for the end user, supports self-directed levels of notifications from the user, visible notifications for logged in users as well as opt-out lists.

Important I think this issue should only be used to help guide direction and establish overall guidance...I don't intend this to be a spec. This is just a brain-storming exercise that may or may not reveal elements that are desirable for the application and HTBox in general.

New User Experience

A user would see the following behaviors from the application:

  1. The ability to set a preferred contact method (email, SMS, both, none)
  2. The ability to set the types of notifications they wish to receive
  3. The ability to manage notifications (mark all as read, archive)
  4. A listing of notifications tied to their profile when logged in to the web site
  5. A notification icon in the navigation bar indicating outstanding notifications in the site
  6. An automatic "digest" of notifications - not receiving more than 1 communication over a period of time (similar to what Slack does)
  7. Rich, template-based email

    Site Changes

    • A notification icon will appear in various colors/states to indicate the presence of notifications
    • A notifications page will be added that shows all non-archived notifications, sorted reverse chronologically

      Model Changes

    • A new flag at the user profile level would be a hard/fast "kill switch" for notifications. This would be a straight toggle that would prevent any notifications from going to the user.
    • We would need to introduce a new entity type, likely outside of Azure SQL, that would contain the list of notifications keyed per user (I'm thinking something like table storage). There would be a tracking flag or record for when each notification has been seen.
    • User preferences for type of notifications to receive and the preferred method of receipt will be added

      System Changes

    • We need to take inventory of all notification opportunities (per #186) and create appropriate templates for email and SMS
    • When a user "quits" or closes their profile, the user profile "kill switch" would be thrown to prevent future notifications and all digest notifications would be suppressed
    • There will be no more direct calls to the notification components; controllers will invoke commands via the bus when data changes are made, and appropriate events will be raised back on the bus. Only the event subscribers will

These changes could be implemented over a series of related, iterative pull requests. Seeking some feedback here thinking @BillWagner @tonysurma @Harmonym @stimms @dpaquette as a start, but anyone welcome!

bcbeatty commented 8 years ago

Are you thinking the UI could work similar to GitHub's notifications?

joelhulen commented 8 years ago

Along the lines of notifications, we may want to think about implementing a scheduled task (via WebJob or similar) that sends out aggregated notification emails (as you mentioned) as well as reminder-type notifications. For instance, if someone has volunteered for an activity that occurs on Saturday, you may send them a reminder on Wednesday about that activity.

For displaying notifications, I think what would be nice instead of having a separate notification page, is to have a slide-out pane that displays the notifications wherever you are without having to leave your current page. Disqus does this very well.

bcbeatty commented 8 years ago

This also could tie into the Cordova app to receive notifications, Finally a use case for the app vs mobile web

mheggeseth commented 8 years ago

Why wouldn't we want to store notification instances and their statuses in SQL? Would handling them via some medium other than the SQL DB present any dev/test challenges?

stimms commented 8 years ago

A scheduled task is certainly an approach for sending out messages but I would posit that it isn't how most of these large messaging/notification type applications work. I don't get notified at midnight or some arbitrary time that there are notifications for me instead I get them after some period of time has passed since the first notification. This is largely better because then we aren't seeing spikes in load at any particular time and people aren't optimally missing messages (message received at 12:01 and notifications ran one minute prior).

This approach could likely be implemented with a timeout pattern which we could implemented with ease on top of service bus using delayed delivery.

As for persistence I like table storage for this. SQL storage is a scarce resource and for cloud scenarios should be a last stop or used when developers aren't hyper-awesome like we all are. How is that for a controversial opinion?

UI-wise I really like

No new notifications image

New notifications image

Notifications open image

I'd have clear all on there as well as some way to clear individual ones. Also some way to collapse this thing.

stimms commented 8 years ago

On a related note did you know there is no stop sign in font awesome?

joelhulen commented 8 years ago

I agree about not sending notifications at an arbitrary time. The scheduled job would run at regular intervals and check against some data store for messages that need to be added to the queue. The messages would be either aggregated ones, or reminders of some sort. This means that we would need some logic to set the send time, whether it's x minutes since the first message in an aggregate, as you mentioned, or a reminder. For near-instant messages, they'd just be added to the Azure Queue right away. Of course for reminders and other messages that need to be sent 24 hours prior to the event, or some other relative time, we need to take into account the time zone of the event time and/or the user.

That said, I think it would be a really good idea to have the users set their time zone when they register for an account.

As far as what you mocked up, @stimms, that's exactly what I was thinking when I suggested having a slide-out side menu containing the messages. I also like the message badge that is contextual to the state of the messages.

MisterJames commented 8 years ago

@stimms and @joelhulen I like the timeout pattern and I think that we would also need something of the "high priority" variety of message as well. I spoke with @tonysurma at the code-a-thon about this...for example, if the timeout hadn't yet passed, but an SMS needed to go out to say "this has been cancelled due to radioactivity, don't go under any circumstance" it would be bad if it sat in a holding pattern for 12 hours. Otherwise, I like a lot of what is here.

So there would be three kinds of message:

Any reason for any more? If an "immediate" priority message enters the queue, should it also pick up the others? (You also have 18 other unread messages in three campagins...).

stimms commented 8 years ago

I would hesitate to pollute your high priority items with trivia from the general message stream.

There is a huge radiation leak, you must flee for your life. Also: Daisy had puppies!

There is no reason that there couldn't be a high and low priority message stream. In my mind the timeout simply handles triggering the sending of a summary rather than actually holding the messages. I would put messages in table storage with

joelhulen commented 8 years ago

I think "immediate" was always on the table. I imagine that would just go into a specialized Azure queue that fires on receipt to send the notification.

One thing I was thinking that would be slick is if we made use of SignalR to have notices appear on the site in real time without the user needing to refresh. I know there are ways to do this without SignalR, but it does have a nice standard way of pushing messages from the server via websockets, that falls back automatically to long polling for those poor souls still using old browsers.

stevejgordon commented 8 years ago

I was just looking for something to pick up and wondered if there were plans to look at this feature and perhaps break it down into chunks?

I like the idea of something similar to GItHub notifications as a way to view things that have occurred since you last view the list or marked as read. Coupled with emails as necessary for subscribed items. I like the idea of realtime updates as well.

BillWagner commented 8 years ago

@stevejgordon I like the idea of breaking this into smaller tasks. If you'd like to create new issues for those tasks, and reference this one, that would be great.

That said, note that this isn't scheduled for either of our upcoming milestones. We are in the process of planning a pilot deployment for a campaign, and it will help that succeed if we focus on the issues that are scheduled for the next milestones when we are developing code.

stevejgordon commented 8 years ago

@BillWagner Thanks Bill. I'll have a think on it and suggest a breakdown. Would a task list here or separate issues be best practice? Also since this isn't top priority I'll see what I can offer to other milestone issues first.

BillWagner commented 8 years ago

@stevejgordon Three questions:

  1. You've become more involved in the project. Thank you. I'd like to add you to the list of team members (same privileges), and then we can formally assign issues to you as you work on them.
  2. We're still learning, but I think the right plan is to create a task list on this issue. After we are satisfied that it's the correct breakdown, we'll make issues from each task to associate a task with a PR.
  3. Yes, it would be awesome to have focus on the current milestone. Those are the tasks we need for the next pilot.

Again, thanks for your participation and help!

stevejgordon commented 8 years ago

@BillWagner You're welcome. It's a great cause and fun to learn and work with ASP.NET 5 as well. I'm happy to be added to the list of team members, thanks.

mgmccarthy commented 8 years ago

@BillWagner @MisterJames I wouldn't mind picking up apiece of this Issue (if it's still relevant). The piece I would like to work on would be allowing the user to set their communication preferences (email/sms or both).

To put in my two cents on the rest of the items discussed here.

As far as retaining notifications after they've been sent. I'm really not a big fan of that. Email and Sms already have the ability to store older messages by default. So, we get that for free instead of having to write code for it.

Also, as far as "grouping" the notifications, I'm not too sure that's needed. Under what type of scenario would someone be inundated with messages (maybe an org admin???)... I can't see regular volunteers benefiting from that type of functionality.

BillWagner commented 8 years ago

Bump to @tonysurma for triage between now and the 1.0 release.

tonysurma commented 8 years ago

@mgmccarthy I would talk with @MisterJames and have the two of you figure out if this is still key technically for how we are doing this now that we have a bunch of other parts in

mgmccarthy commented 8 years ago

@MisterJames, this seems like a "nice to have", and not a requirement for the initial release of the system. If you agree, we could keep this backlogged until we pick it up.

If we DO need to track what notifications have been sent, for the existing sms confirmation messages going out to requestors for the incoming getasmokealaram integration, I can add tracking to these messages in a new db table somewhere to "prep" for this issue.

Obviously, this framework is bigger than just that single integration, so when we pick it up, it would be good to break into smaller chunks

stevejgordon commented 7 years ago

@MisterJames @mgmccarthy It might be time to look at picking this up again. Once of the scenarios we have is for admins of various levels to be be able to message out to different groups of users. See #1786

The more I think on notifications I'm of the opinion it might be nice to have somewhere on the site where a user can see anything they've been sent, rather than relying on them seeing emails in each case. Like the notifications here on GitHub.

Thoughts?

mgmccarthy commented 7 years ago

@stevejgordon I agree with everything you said. If the need to see everything the user has ever sent is for historical purposes ("did I sent that notification out, let me check" or "what exactly was in that notification I sent?"), then this should be fairly straightforward.

Right now, we're utilizing queue's for notifications, so we already have a centralized location that handles the queued messages and then based on the type of notification, sends the notification using the appropriate medium. This functionality lives in web jobs.

Since the current functionality lives in webjobs, and since we definitely want to take an async/eventually consistent approach to writing all sent notifications to some type of persistence, I'm assuming we can use queue's again on the web job end.

The only challenge (and this challenge keeps coming up over and over again, Issues #1639 and #1626) is we need access to a db context in order to write the send notifications to. Write now, our DbContext is attached directly to our web project, so we don't have access to it in the project that holds the web jobs.

I think @dpaquette was going to look into getting the DbContext/data broken out into it's own project so we can utilize it in webjobs? He mentioned it here a couple months ago

I'd also like to add that it would be nice to get an Issue on the board (if one does not already exists), where registered users can set their notification preferences (email/sms/voice/carrier pigeon). This way, when the system dispatches a "notification", the message we use will be medium agnostic, and the centralized location that is responsible for actually picking these notification off the queue can also look up communication preference for each user and dispatch the notification via the appropriate medium(s).

stevejgordon commented 7 years ago

@mgmccarthy I believe it is possible to put context and model into a library project with EF Core. I've done it with EF in the past but there is a bit more to it with Core from the small bits I've seen. I think the challenge is around the migrations command needing to be run from the directory of the web app as dotnet.exe need to be able to load the app. But I think you can pass in a path to the actual context library. We'd need to check it out more though and consider any deployment issues it might raise.

An alternative if we just want to talk to a specific notification history table might be to use new context within the web job library as really it only needs to know about a couple of the tables. That might be the better approach. No need to share a complex context unless the web job needs the extra tables. There is the issue of keeping them in sync though. Obviously we don't want both issuing migrations and if a model changes, it needs to be reflected in both.

I'd also like to add that it would be nice to get an Issue on the board (if one does not already exists), where registered users can set their notification preferences (email/sms/voice/carrier pigeon).

I'm thinking the same. I think preference of comms is important (although there may be cases were we override if the message demands). We might also want to include a "quiet hours" type feature where a user can block notifications and they are queued to send once the user is back online. I can imagine volunteers in different time zones being a bit annoying if they get SMS messages in the middle of the night for example!

I think that's what this issue was aiming to address originally. It's quite a complex piece but maybe we can agree the priorities and start fleshing out some smaller issues to address each part.

mgmccarthy commented 7 years ago

:+1: for this:

An alternative if we just want to talk to a specific notification history table might be to use new context within the web job library as really it only needs to know about a couple of the tables

if we can simplify and start this way, I think it's a good approach. Since we really don't know how or why the historical data is going to be used, we can at least capture it in a separate db/context.

and a :+1: for this as well:

We might also want to include a "quiet hours" type feature where a user can block notifications and they are queued to send once the user is back online. I can imagine volunteers in different time zones being a bit annoying if they get SMS messages in the middle of the night for example!

@dpaquette and I had to work this way when building out the scheduled Hangfire jobs for the sms notifications going out... we had to convert to the user's time zone before scheduling the next job so they didn't end up getting sms notifications at 2am.

stevejgordon commented 7 years ago

In terms of quiet time it would also be when an org admin does a send to all type message for volunteers in say a specific itinerary