TwinePlatform / twine-monolith

⬛️ Monorepo for the Twine platform
https://twine-together.com
GNU Affero General Public License v3.0
5 stars 3 forks source link

☁️ api triggered in app notifications #368

Open astroash opened 4 years ago

astroash commented 4 years ago

A place to discuss implementation for in app api triggered notifications. Requirements:

Suggestion:

eliasmalik commented 4 years ago

End point: GET /notifications/volunteers & /notifications/admins

Just GET /notifications would be sufficient, the role can be inferred from authentication.

user: enum (volunteer | admin)

This should be a foreign key into access_role.

validFrom: Date
validTo: Date
active: boolean

What does active tell us/mean that can't be inferred from valid_from/valid_to?

only 1 message at a time

This can be removed by returning an array of messages. However, if we do that we need to think about how multiple messages are going to be displayed.

If we really wanted to we could categorise the messages to show different icons

Yeah, we can probably put this stuff in meta.

Other thoughts:

  1. Do we want to track who has seen which notifications?
  2. IMO these notifications are supposed to be ephemeral. See them once, confirm/close, then they're gone forever. You don't keep seeing them every time you open the app. All the apps I've used do it this way.
astroash commented 4 years ago

Just GET /notifications would be sufficient, the role can be inferred from authentication

Makes sense. We should probably name this route /notifications/volunteer-app for future proofing

This should be a foreign key into access_role.

👍🏽

What does active tell us/mean that can't be inferred from valid_from/valid_to?

I was thinking active could be set for early removal, but I guess we can just change validUntil

This can be removed by returning an array of messages. However, if we do that we need to think about how multiple messages are going to be displayed.

I’ll bare this in mind whilst styling the modals. This may be an unnecessary feature for its anticipated use case. Will add if it’s a simple addition.

Do we want to track who has seen which notifications?

This can be retroactively worked out from valid dates and sessions logs

IMO these notifications are supposed to be ephemeral. See them once, confirm/close, then they're gone forever. You don't keep seeing them every time you open the app. All the apps I've used do it this way.

What examples are you thinking off? I was basing my thinking about banking apps maintenance alerts which are displayed on every load (in line on Monzo & new modal on NatWest)

eliasmalik commented 4 years ago

What examples are you thinking off? I was basing my thinking about banking apps maintenance alerts which are displayed on every load (in line on Monzo & new modal on NatWest)

The Monzo notifications that appear each time usually appear inline at the top of the transaction feed, no?

The ones i remember are tide, citymapper, Metrobank, maybe LinkedIn not sure.

I'm not gonna die on this hill tbh, just an IMO.

astroash commented 4 years ago

The Monzo notifications that appear each time usually appear inline at the top of the transaction feed, no?

Ya, I mentioned that.

I guess it will greatly depend on the types of messages. How would you imagine implementing a one time only display message?

eliasmalik commented 4 years ago

How would you imagine implementing a one time only display message?

I thought we were talking about fullscreen(ish) modals, but maybe we're talking about different things.

Ya, I mentioned that.

If we're talking about non-fullscreen notifications, then I've no objection to them appearing each time.

astroash commented 4 years ago

We were talking about the same thing, I think you just missed the brackets in my response.

I was asking how you imagined implementing a full screen modal that only appears once. We’re you imagining keeping a record of who had viewed it so that the server can check if it’s valid for that user?

eliasmalik commented 4 years ago

We’re you imagining keeping a record of who had viewed it so that the server can check if it’s valid for that user?

nah i would have just done it client side with whatever the equivalent of localStorage is

astroash commented 4 years ago

Something like an array of shown message ids, that the client checks the api response id against? That sounds like a lot less overhead than what I was imagining.

I guess it depends on what the type of messages Edward imagines showing with this feature.

Another complication is that both CB_ADMIN & VOLUNTEER_ADMIN can access the admin section of the native app so we cannot have a foreign key for messages. Possible solutions:

Let me know if you have a preference.

Updated proposal:

eliasmalik commented 4 years ago

Something like an array of shown message ids, that the client checks the api response id against?

Basically yeah

I guess it depends on what the type of messages Edward imagines showing with this feature.

Yeah sure. I think a lot of the infrastructure for this can be built without needing to answer this question right now, but it would be good to kick it back up to the others so we know what we're doing eventually.

Another complication is that both CB_ADMIN & VOLUNTEER_ADMIN can access the admin section of the native app so we cannot have a foreign key for messages.

I would consider this a benefit rather than a complication (which means my preference is to duplicate messages in the DB if we want to show them to both types of admin)