Feature/harsh jenny/push notifications #171
Conversation
…database have fcm tokens, and all sessions have flags for notifSent
ALUM/ALUM/ALUMApp.swift
Outdated
| return true | ||
| } | ||
| } | ||
| // class AppDelegate: NSObject, UIApplicationDelegate { |
There was a problem hiding this comment.
If this isn't being used then we can just delete the comments
ALUM/ALUM/ALUMApp.swift
Outdated
| print("did receive FCM Token") | ||
| // let deviceToken:[String: String] = ["token": fcmToken ?? ""] | ||
| currentUser.fcmToken = fcmToken | ||
| // currentUser.fcmToken = Messaging.messaging().fcmToken |
ALUM/ALUM/ALUMApp.swift
Outdated
| Messaging.messaging().apnsToken = deviceToken | ||
| } | ||
|
|
||
| func application(_ application: UIApplication, didFailToRegisterForRemoteNotificationsWithError error: Error) { |
There was a problem hiding this comment.
Let's also remove any functions that aren't being used for anything.
backend/src/errors/service.ts
Outdated
| @@ -47,7 +48,7 @@ export class ServiceError extends CustomError { | |||
|
|
|||
| static MENTOR_WAS_NOT_SAVED = new ServiceError(11, 404, MENTOR_WAS_NOT_SAVED); | |||
|
|
|||
| static MENTEE_WAS_NOT_SAVED = new ServiceError(11, 404, MENTEE_WAS_NOT_SAVED); | |||
| static MENTEE_WAS_NOT_SAVED = new ServiceError(12, 404, MENTEE_WAS_NOT_SAVED); | |||
|
|
|||
| static INVALID_ROLE_WAS_FOUND = new ServiceError(11, 404, INVALID_ROLE_WAS_FOUND); | |||
There was a problem hiding this comment.
Small fix but shouldn't have Error Codes be the same so ig make this 13
backend/src/routes/sessions.ts
Outdated
| @@ -51,19 +53,24 @@ router.post( | |||
| } | |||
| const accessToken = mentor.personalAccessToken; | |||
| const data = await getCalendlyEventDate(req.body.calendlyURI, accessToken); | |||
| const startDate = new Date(2023, 0o5, 0o5, 17, 0, 0, 0); | |||
| const endDate = new Date(2023, 0o5, 0o5, 18, 0, 0, 0); | |||
There was a problem hiding this comment.
What's the purpose of the startDate and endDate fields? Is it to ensure that startTime and endTime exists? Because I was thinking of calendly doesn't return the start and end time of the event it makes sense to just return an error rather than filling in some random event time.
| console.log("Notification sent successfully:", response); | ||
| }) | ||
| .catch((error) => { | ||
| console.error("Error sending notification:", error); |
There was a problem hiding this comment.
This should throw an error rather than logging a message. When calling the function in the route it should catch the error and then print the error statement there.
| const dateNow = new Date(); | ||
| const mentee = await Mentee.findById(session.menteeId); | ||
| if (!mentee) { | ||
| throw InternalError.ERROR_GETTING_MENTEE; |
There was a problem hiding this comment.
Not sure if throwing an error would be the best way to handle errors in this scenario. Suppose that we have 10 sessions and only the 5th one is corrupted. From my understanding the function will get to the 5th session but throw an error. However, it won't continue to the next session and send the proper notification for that session. I think its best for the function to just continue to the next session rather than ending the loop altogether.
| if (session.preSessionCompleted) { | ||
| const menteeNotif = await sendNotification( | ||
| "You have an upcoming session.", | ||
| `Ready for your session with ${mentor.name} in 24 hours? \u{1F440}`, |
There was a problem hiding this comment.
Since the only difference between the if statement is the mentee and mentor message, the if-else statement should modify a variable for the message to be sent based on the pre-session notes. Then after the if-else statement send the notification. This will help clean up the code and reduce repitition.
| const dateNow = new Date(); | ||
| const mentee = await Mentee.findById(session.menteeId); | ||
| if (!mentee) { | ||
| throw InternalError.ERROR_GETTING_MENTEE; |
There was a problem hiding this comment.
Same idea about the errors in the Upcoming session function.
| if (!mentor) { | ||
| throw InternalError.ERROR_GETTING_MENTOR; | ||
| } | ||
| if (dateNow.getTime() - session.endTime.getTime() >= 0) { |
There was a problem hiding this comment.
Lets not send it immediately after the session ends. Maybe give some buffer time like 5-10 mins.
|
Looks good to me. |
Tracking Info
Resolves #126
Make sure your branch name conforms to:
<feature|staging|bugfix|...>/<username>/<3-4 word description separated by dashes>. Otherwise, please rename your branch and create a new PR.Changes
We added the notification system of the ALUM app. Here are some key points:
• Each device/user gets a unique FCM token from Firebase. We wrote the routes to obtain the token and send it to our backend. Notifications are sent to devices using the token.
• In
notifications.ts, the basic outline for notification sending is outlined in the function sendNotification. The sendNotification function is used across files whenever a notification needs to be sent.• We set up cron jobs to automatically send notifications to users in specific repeated cases outlined in the Figma (i.e. 24 hours before a session, right after a session). The cron job is used for the instances where the notification is time based. Currently, we have it set to run every 1 minute - this can be updated.
Testing
How did you confirm your changes worked?
I built the app on my phone and was able to receive notifications, and used console outputs to see that the fcm token was sent correctly.
Confirmation of Change
TODO...