-
Notifications
You must be signed in to change notification settings - Fork 136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Firebase for Push notifications #685
base: master
Are you sure you want to change the base?
Conversation
daltonfury42
commented
Oct 26, 2021
•
edited
Loading
edited
- Move sentry/alerting to it's own file.
- Register to backend for notifications for token.
Preview URL for this branch is https://firebase.dpmhbfuiok7f8.amplifyapp.com. Please wait for sometime for the build to finish. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is still a work in progress item, but we could merge it if it doesn't break anything.
import { useLinkDevice } from 'store/asyncActions'; | ||
|
||
// Public key generated from firebase console | ||
const vapidKey = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, it's okay to expose this key, right?
@@ -11,7 +11,9 @@ const appSlice = createSlice({ | |||
initialState: { | |||
errorText: '', | |||
infoText: '', | |||
notificationPermission: null, // This state value is initialised by the notification service. | |||
// This value is initilised at start by services/notification/system.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: typo for initialised
*/ | ||
export const linkDevice = (deviceId) => ({ | ||
method: 'put', | ||
url: `/owner/link?deviceId=${deviceId}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking this resource should be named something else. I feel that owner doesn't fit its purpose. Do you think /notifications/subscribe
would fit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another approach I can think of is simply a /notifications
endpoint with body
{
subscribe: <BOOL>,
deviceId: <ID>
}
for PUT and PATCH.
|
||
const registerForNotifications = () => { | ||
getToken(messaging, { vapidKey }) | ||
.then((deviceId) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, is deviceId generated internally by firebase? Is it unique for every device?