feat: add push notification filtering settings #3207
Conversation
UncleSamtoshi
commented
Sep 13, 2023
•
UncleSamtoshi
commented
- Adds the ability to set and retrieve push notification filters
- Push Notifications will be on by default and before they are sent we will check to make sure notifications are enabled and that the individual notification type and subtype is not disabled
- NOTE: This is implemented using a string for NotificationType instead of an enum. This allows for the notification service to not need any context about the notifications that are passing through it.
UncleSamtoshi
force-pushed
the
add-notification-filtering
branch
3 times, most recently
from
b409b74 to
2c45ce1
Compare
UncleSamtoshi
requested review from
bodymindarts,
dolcalmi and
nicolasburtey
UncleSamtoshi
changed the title
src/graphql/public/root/mutation/account-update-push-notification-settings.ts
Outdated
Show resolved
Hide resolved
UncleSamtoshi
force-pushed
the
add-notification-filtering
branch
from
aadb6c4 to
3a0d2d3
Compare
UncleSamtoshi
force-pushed
the
add-notification-filtering
branch
from
bba12c7 to
54771b2
Compare
UncleSamtoshi
force-pushed
the
add-notification-filtering
branch
from
a1ae361 to
337cffe
Compare
| ...sendNotificationArgs, | ||
| data: { | ||
| ...data, | ||
| NotificationCategory: notificationCategory, |
There was a problem hiding this comment.
not sure what is supposed to be inside { data } versus at the root level (ie: { ...sendNotification, ... }) but reading here would make me think NotificationCategory: notificationCategory, is more a metadata thing and should be one level up in the root
There was a problem hiding this comment.
That makes sense, I will move it up a level.
There was a problem hiding this comment.
Actually I think it may make sense to keep it where it is at. It is preferable that all notifications that have a notification category receive it as a part of their metadata. Since this method already has logic relating to handling notification categories I think it is alright for it to augment the metadata.
| @@ -0,0 +1,28 @@ | |||
| import { | |||
There was a problem hiding this comment.
I'm wondering how much we should add to the legacy-integration. We're aiming at trimming this folder down. @vindard would have more context
|
great PR. looks good overall, some nits to look at. |
