Fightpandemics: Notification Settings [Desktop][Mobile] - for users to turn Notifications on or off
I want to be able to turn Notifications on or off, so I can receive Notifications when I want them (when I have an active post) and turn them off when I don't need them.
Expected results: A Notification Settings page where users can toggle Notifications to be either on or off.
Actual results: Does not exist
To start (version 1), we can just have booleans on the user - on or off for all emailNotifications. Later we can add webNotifications and controls by type.
This can be broken up into 3 parts.
a. B/E update mongoose model to include booleans.
b. F/E update form - to include Booleans.
Initial design: https://fightpandemics.slack.com/archives/C018ZNAD62Y/p1598344009136700
c. We also need a special backend route for unsubscribing from email notifications via email - we'd need to authenticate with some other kind of token then what we use for auth
nfranklin9
All 14 comments
Future work: settings for Web App Frequency: Daily, Weekly, Monthly
For initial launch, we're just doing on/off
nfranklin9
on 3 Sep 2020
- The User model modification is proposed as below, I'm going to move on with this change if we all agree:
const individualUserSchema = new Schema(
{
authId: {
...
notificationMask: {
default: 0,
type: Number,
},
...
}
- Questions: above change will be applied to "IndividualUser", how about "OrganizationUser"?
ddveloper
on 13 Oct 2020
I am targeting a bitMask format (but I don't know if there is another good type than Number to represent it).
it will be easy to scale up to support other combinations, the tradeoff is not so readable, a lookup table is required for decoding it
default 0 will be for notification disabled
ddveloper
on 13 Oct 2020
I think a bit mask will be hard to work with. We could do something like this instead?
{
...
notificationPreferences: {
likes: {
instant: Boolean,
daily: Boolean,
weekly: Boolean,
biweekly: Boolean,
},
comments: {
instant: Boolean,
daily: Boolean,
weekly: Boolean,
biweekly: Boolean,
},
posts: {
instant: Boolean,
daily: Boolean,
weekly: Boolean,
biweekly: Boolean,
},
},
...
}
More verbose but easier to read and work with.
mannykary
on 13 Oct 2020
In the user model we can add
notificationPreferences: {
message: {
type: String,
enum: ["instant", "daily", "weekly", "biweekly"],
required: true,
default: "instant",
},
likes: {
type: String,
enum: ["instant", "daily", "weekly", "biweekly"],
required: true,
default: "instant",
},
comments: {
type: String,
enum: ["instant", "daily", "weekly", "biweekly"],
required: true,
default: "instant",
},
posts: {
type: String,
enum: ["instant", "daily", "weekly", "biweekly"],
required: true,
default: "instant",
},
},
Zaydme
on 13 Oct 2020
@Zaydme your model assumes that the options are mutually exclusive but the user needs to be able select multiple options (i.e. for likes, they might want both instant and daily)
mannykary
on 13 Oct 2020
@Zaydme your model assumes that the options are mutually exclusive but the user needs to be able select multiple options (i.e. for likes, they might want both instant and daily)
@mannykary
maybe we can have an array of selected frequencies, that would be
notificationPreferences: {
message: [{
type: String,
enum: ["instant", "daily", "weekly", "biweekly"],
required: true,
default: ["instant"],
}],
likes: [{
type: String,
enum: ["instant", "daily", "weekly", "biweekly"],
required: true,
default: ["instant"],
}],
comments: [{
type: String,
enum: ["instant", "daily", "weekly", "biweekly"],
required: true,
default: ["instant"],
}],
posts: [{
type: String,
enum: ["instant", "daily", "weekly", "biweekly"],
required: true,
default: ["instant"],
}],
},
I like Manny's version : 0 what do you think @joshmorel
ddveloper
on 13 Oct 2020
@Zaydme your model assumes that the options are mutually exclusive but the user needs to be able select multiple options (i.e. for likes, they might want both instant and daily)
from UX perspective, I think giving single choices probably will be good enough, while having multiple choices at the backend doesn't hurt
ddveloper
on 13 Oct 2020
maybe we can have an array of selected frequencies, that would be
That's a little better, but thinking about how the Lambda and the notification preferences API will query for and filter out recipients that are not subscribed, it is a bit harder to work with.
Consider the Notification Preferences frontend and its corresponding API. The frontend would show checkboxes for each option. The checkboxes need to map to the appropriate setting in the model, and it's harder to do that mapping if the preferences for each notification type is an array.
For example, let's say all the options are checked for "likes", and the user unchecks "biweekly"... the request to the backend (truncated for brevity) would be:
PATCH /api/users/current
{
notificationPreferences: {
...
likes: ["instant", "daily", "weekly"]
...
}
}
vs using 1 to 1 booleans for each distinct setting:
PATCH /api/users/current
{
notificationPreferences: {
...
likes: {
instant: true,
daily: true,
weekly: true,
biweekly: false,
}
...
}
}
The first method we'd need to construct the array in the frontend to build the request when the form is submitted, based on the selected options, whereas for the second method all we'd need to do is build the object of booleans which is easy since it maps 1 to 1 with the checkboxes.
Now consider the Notification Lambda. It needs to query for the user's notification preferences, so if the preferences are represented like so (truncated for brevity)
notificationPreferences = {
...
likes: ["instant", "daily", "weekly"]
..
}
If the Notification Lambda is querying for all recipients that want to receive a "biweekly" email, it would have to do an O(n) lookup on the array:
if (notificationPreferences.likes.includes("biweekly")
(not that bad for a single user, but if there are a lot of users, it's O(n) for each user, so it becomes O(n^2). I suppose this could be improved if the array was converted to a set for O(1) lookup)
If it's an object of booleans:
notificationPreferences = {
...
likes: {
instant: true,
daily: true,
weekly: true,
biweekly: true
}
..
}
Then it's O(1) lookup for each user, and O(n) overall:
if (notificationPreferences.likes.biweekly)
Considering that the Lambda has limited memory and also needs to run quickly (max run time of a Lambda is 15 minutes), I think every bit of time/space saved matters.
mannykary
on 13 Oct 2020
Yeah that makes sense
Zaydme
on 13 Oct 2020
update: add new fields in User Schema, tested OK with fake F/E (able to get and set)
const userSchema = new Schema(
{
about: { maxLength: 100, trim: true, type: String },
email: {
required: true,
type: String,
unique: true,
uniqueCaseInsensitive: true,
validator: isValidEmail,
},
notifyPrefs: {
message: {
instant: { default: true, type: Boolean },
daily: { default: false, type: Boolean },
weekly: { default: false, type: Boolean },
biweekly: { default: false, type: Boolean },
},
likes: {
instant: { default: false, type: Boolean },
daily: { default: false, type: Boolean },
weekly: { default: true, type: Boolean },
biweekly: { default: false, type: Boolean },
},
comments: {
instant: { default: false, type: Boolean },
daily: { default: true, type: Boolean },
weekly: { default: false, type: Boolean },
biweekly: { default: false, type: Boolean },
},
posts: {
instant: { default: false, type: Boolean },
daily: { default: true, type: Boolean },
weekly: { default: false, type: Boolean },
biweekly: { default: false, type: Boolean },
}
},
location: Object,
photo: String,
},
{ collection: "users", timestamps: true },
);
backend /unsubscribe GET and PATCH endpoint is added in #1677 , tested with Postman (set token in req.headers)
note: the 2 fields are encoded in the token { userId, expireDate }
ddveloper
on 21 Oct 2020
an update: working on frontend now in #1677 to complete the loop, estimated finish time, these 2 days
ddveloper
on 22 Oct 2020
Related issues
rabbi08
路
10Comments
doheekim93
路
8Comments
ppagarawal
路
4Comments
joshmorel
路
7Comments
galeza
路
10Comments
Most helpful comment
That's a little better, but thinking about how the Lambda and the notification preferences API will query for and filter out recipients that are not subscribed, it is a bit harder to work with.
Consider the Notification Preferences frontend and its corresponding API. The frontend would show checkboxes for each option. The checkboxes need to map to the appropriate setting in the model, and it's harder to do that mapping if the preferences for each notification type is an array.
For example, let's say all the options are checked for "likes", and the user unchecks "biweekly"... the request to the backend (truncated for brevity) would be:
PATCH /api/users/current
vs using 1 to 1 booleans for each distinct setting:
PATCH /api/users/current
The first method we'd need to construct the array in the frontend to build the request when the form is submitted, based on the selected options, whereas for the second method all we'd need to do is build the object of booleans which is easy since it maps 1 to 1 with the checkboxes.
Now consider the Notification Lambda. It needs to query for the user's notification preferences, so if the preferences are represented like so (truncated for brevity)
If the Notification Lambda is querying for all recipients that want to receive a "biweekly" email, it would have to do an O(n) lookup on the array:
(not that bad for a single user, but if there are a lot of users, it's O(n) for each user, so it becomes O(n^2). I suppose this could be improved if the array was converted to a set for O(1) lookup)
If it's an object of booleans:
Then it's O(1) lookup for each user, and O(n) overall:
Considering that the Lambda has limited memory and also needs to run quickly (max run time of a Lambda is 15 minutes), I think every bit of time/space saved matters.