-
Notifications
You must be signed in to change notification settings - Fork 115
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
Push Notifications Package #2081
Conversation
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
e0b43d5
to
9e659fd
Compare
8196063
to
a11b478
Compare
a11b478
to
ccb1f77
Compare
This reverts commit b753be5.
FIREBASE_PRIVATE_KEY_BASE64: parseString({ default: 'LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tClBMQUNFSE9MREVSX0tFWV9GT1JfREVWRUxPUE1FTlQKLS0tLS1FTkQgUFJJVkFURSBLRVktLS0tLQo=' }), | ||
|
||
// APP ID for the Google Firebase Messaging project | ||
FIREBASE_PROJECT_ID: parseString({ default: 'dydx-v4' }), |
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.
does eng need access to this (eventually)?
const initializeFirebaseApp = () => { | ||
const defaultGoogleApplicationCredentials: { [key: string]: string } = { | ||
project_id: config.FIREBASE_PROJECT_ID, | ||
private_key: Buffer.from(config.FIREBASE_PRIVATE_KEY_BASE64, 'base64').toString('ascii').replace(/\\n/g, '\n'), |
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.
comment for what this is doing/why needed
message: 'Initializing Firebase App', | ||
}); | ||
|
||
const serviceAccount: ServiceAccount = defaultGoogleApplicationCredentials; |
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.
why do we need this line? combine with l13?
|
||
import config from '../config'; | ||
|
||
const initializeFirebaseApp = () => { |
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.
existing syntax for functions is function initializeFirebaseApp(): void {
/* eslint-disable no-template-curly-in-string */ | ||
import { LocalizationKey, LanguageCode } from './types'; | ||
|
||
export const LOCALIZED_MESSAGES: Record<LanguageCode, Record<LocalizationKey, string>> = { |
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.
existing convention is separate with dashes, e.g. localized-messages.ts
@@ -0,0 +1,152 @@ | |||
export enum NotificationType { |
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.
include enum description to be consistent
|
||
export function createNotification<T extends NotificationType>( | ||
type: T, | ||
dynamicValues: T extends NotificationType.DEPOSIT_SUCCESS |
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.
this typing seems really complicated, anyway to simplify?
@@ -0,0 +1,132 @@ | |||
import { DateTime } from 'luxon'; |
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.
token-table
is a little vague. Can we be a bit more specific? perhaps firebase-notification-token-table
?
import { IsoString } from '../types'; | ||
import WalletModel from './wallet-model'; | ||
|
||
class TokenModel extends Model { |
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.
comment for what this is used for
|
||
type IsoString = string; | ||
|
||
export interface TokenCreateObject { |
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.
Token -> FirebaseNotificationToken ?
Changelist
Add a new package to enable sending push notifications via Firebase
Test Plan
[Describe how this PR was tested (if applicable)]