franck-boullier commented 6 years ago

The problem:

As of today for the notification type 'case_updated' we are triggerring an event each time any field in a case is updated.
There are some fields in Unee-T that are only visible in the dashboard (BZFE) interface and not in the MEFE (this is by design)

Consequence: MEFE only user receive notifications about changes they can't see in the MEFE.

The possible solutions:

Short term fix:

Default disable the notification type 'case_updated' for the MEFE users

Possible long term fix:

Add an information in the payload to differenciate notifications based on the updated fields. If the field is not available in the MEFE, then we indicate that this notification is not intended for MEFE users but only for BZFE users.

@kaihendry do you see any issue with that approach?

franck-boullier commented 6 years ago

In the DEV and PROD environment, all the existing users have had their preference for notification type caseUpdaed changed to 'false' in the Mongo database.

@kaihendry what do we need to do so that all the users that we will invite to a case have their notification preference for caseUpdated changed to 'false' by default?

kaihendry commented 6 years ago

The default is controlled by the migrations iiuc https://github.com/unee-t/frontend/blob/77172b72096e4e5f920ddb0199f19cd42d949fc2/imports/migrations/6_add_more_notifcations.js

@nbiton do we create a new migration with the default changed?

nbiton commented 6 years ago

Since @franck-boullier already changed the value to 'false' manually for all existing users (and I think that's fine, as it hasn't introduced a new field to the schema), we just need to change the following code line to make sure new/new+invited users have it set to 'false' when they're created: https://github.com/unee-t/frontend/blob/master/imports/api/custom-users.js#L27

kaihendry commented 6 years ago

Going to assume this bug is fixed by making the default to opt in for _caseupdated.

franck-boullier commented 6 years ago

We only implemented a short term fix. We need to work on the long term solution as described earlier

Possible long term fix:

Add an information in the payload to differentiate notifications based on the updated fields. If the field is not available in the MEFE, then we indicate that this notification is not intended for MEFE users but only for BZFE users.

kaihendry commented 6 years ago

Silly idea is give to users a preview or history of the sort of notifications that come down each channel on /notification-settings

https://github.com/unee-t/frontend/blob/master/imports/ui/notification-settings/notification-settings.jsx

franck-boullier commented 6 years ago

I like that idea!

kaihendry commented 6 years ago

After discussing with @nbiton it seems a lot of work for little gain. We should be focusing on making existing notifications more useful / more signal.

Not exactly sure on the UI of how to show these email renders either.

franck-boullier commented 6 years ago

After discussing with @nbiton it seems a lot of work for little gain

This seems to contradict the parallel conversation that we are having in Slack on this right now... I am very confused, maybe because I don't understand what is the work that @kaihendry is referring to :thinking:

Maybe best to have a single place for the same conversation (i.e this issue in GH) so we better understand what we want to do/where we want to go

kaihendry commented 6 years ago

This bug is/was about showing / previewing what they about to receive.

The discussion on #general is about making notifications a digest. And filter out what users don't want. Separate issues in my mind.

franck-boullier commented 5 years ago

Why is this critical:

This is one of the main feedback from "power users" today: notifications are way to noisy up to the point that they become useless.

We need a way for power user to fine tune notifications.

Ideas and suggestion:

Only notify me if the case is created by a user is role X
Only notify me if the message is created by a user is role X
Only notify me if the message is for a case with a severity A
Only notify me if the case created is a severity A
etc...

Next Steps:

@kaihendry is looking at this and uses GH notifications as one source of inspiration...

franck-boullier commented 5 years ago

More thoughts on notifications:

Key principles:

How I am notified:

This should be a Global parameters for Notifications mechanism for the user. Option could be:

email,
In App,
WhatsApp,
WeChat,
Other

Granularity:

Global level parameters for notifications (All Units and All Cases)
Unit level parameters for notifications (for a specific unit, all cases in that unit)
Case level parameters for notifications (for a specific case).Options for the user (notify me if...)

Notification options

Notification about Units

I am invited to role a Unit

Global, Unit but NOT Case:

A New case is created (only available to users who have access to the dashboard/paying plans)

Any level for a case (Global, Unit, Case)

I am assigned to a case (only available to users who have access to the dashboard/paying plans).
I am invited to a case (only available to users who have access to the dashboard/paying plans).
Next Step is updated (this includes update to the Next Step Date and/or the Next Step field)
Solution is updated
Deadline is updated
Case is Resolved
Any New Message
New Message from Role X
New Message from Role Y
New Message from Role Z
If severity of the case is 'X' (ex: Blocker or Critical) then Notify me of all changes.Other Notifications:

Notification for Inspection Reports:

A New Inspection Report is created

Notifications for Inventory Items:

A new Inventory Item is created
An Inventory Item is removed (hidden/marked as obsolete)
An Inventory Item is moved to a different unit.

kaihendry commented 5 years ago

Message system

First step to tackle the problem is to list all the notification types (which you have done to have extent), like some big dumb message system. In Github parlance this (I believe is) called an "App integration" in which you register Web hooks for every notification type. The Web delivery hook callback is where the logic is processed to deliver that notification.

The message system basically decides whether the notification goes out based on whether a user is logged in or not. Therefore this initial intake is best handled by the frontend, since only the frontend knows if the user is logged in and present.

But there is a lot more to it.

For example do we want a system so that we track the state of whether the message is read or not? Github does this. But it makes things a LOT more complicated.

Do we have a system where users can @mention each other?

security We must ensure our fine grained permissions are in affect. I.e. how do we prove that a user is entitled to receive a message? Every "App integration" will probably have to be on a user level to ensure a notification doesn't get incorrectly sent. Testing this will be challenging alone. We need a function or service that can tell us easily if user A can actually read message XYZ. Update: Bugzilla backend can tell us this already, and the message payloads currently actually tell us who is able to see the payload.

can we design something simpler or use an existing medium A message system alone is quite an engineering challenge. This probably shouldn't hold any state of the user preferences. It must have a strong test coverage to prove that the events come through. Alternatively if we had a system to generate secret URLs and allow users to share those secret URLs and share them over their chosen medium, e.g. WhatsApp or email, then this notification system requirement becomes unnecessary.

Frontend

The user frontend to this effectively has the logic to register the Web hooks in the "big dumb message system". The "message system" must be proven to work first. That it can correctly route messages a user is allowed to see to the user (security). Github's interface looks like https://github.com/settings/notifications

Like Github, we should be massively simplifying this for the user.

Delivery hooks

I think the delivery hooks themselves can be a standalone service.

The hooks that actually deliver the message, have to be smart. Smart enough to:

act on preferences know the user preferences whether they actually want to receive the message (their preferences, probably stored on Mongo)
delivery ideally able to tell if they read it or acted on it or if it was delivered at all (sms receipts)
unsubscribe or mute allow the user to interact with the frontend or perhaps directly with the delivery hooks to make a note of their preferences
interaction ensure in some instances that they can reply and then message gets put back in the case

This service probably should be designed with its own API frontend and backend to send the notification over:

sms
whatsapp
email
anything

Implementing each of these backends will be a challenge requiring it own sets of testing etc etc

franck-boullier commented 5 years ago

Thanks for the detailed contribution @kaihendry. My initial reactions/comments/answers to some of your questions:

For example do we want a system so that we track the state of whether the message is read or not?

Yes definitely

Github does this. But it makes things a LOT more complicated.

Not really: and idea that was already discussed a few time was for us to use the already existing BZ tag mechanism to add a tag read by xxx to the relevant message(s) each time a given user has seen a given message

Do we have a system where users can @mention each other?

Not part of the scope for now (and in the forseeable future)

how do we prove that a user is entitled to receive a message?

If a user is party to a case (creator, assignee, invited) then he/she is entitled to receive the message relative to this case. And as @kaihendry already mentioned the info 'who is allowed to see the message' is already included in our notification payload.

a system to generate secret URLs and allow users to share those secret URLs and share them over their chosen medium, e.g. WhatsApp or email, then this notification system requirement becomes unnecessary.

Interesting idea but not sure how this would work in the real world (what would be a typical user story to illustrate that?)