Skip to content

Conversation

AndrewCheung360
Copy link
Member

Overview

Implement Basic FCM Service Setup

Changes Made

  • Implemented basic Firebase Messaging Service setup
  • Implemented FCM token repository for getting and updating fcm token
  • Added firebase messaging library to dependencies
  • Added POST notification permissions to manifest

Test Coverage

  • See recording below

Next Steps

  • Set up notification permission request (next PR)
  • Handle chat notifications
  • Add send fcm token to backend functionality

Screenshots

Screen.Recording.2025-10-08.at.6.10.49.PM.mov

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Implements basic Firebase Cloud Messaging (FCM) service setup to enable push notifications in the Hustle app. This is the foundation for notification functionality.

  • Added Firebase Messaging dependency and service registration
  • Created FCM token repository for managing notification tokens
  • Implemented Firebase Messaging Service with notification handling

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
gradle/libs.versions.toml Added firebase-messaging dependency declaration
app/build.gradle.kts Included firebase-messaging library in app dependencies
app/src/main/java/com/cornellappdev/hustle/di/AppModule.kt Added DI bindings for FCM token repository and Firebase Messaging
app/src/main/java/com/cornellappdev/hustle/data/repository/FcmTokenRepository.kt Created repository interface and implementation for FCM token management
app/src/main/java/com/cornellappdev/hustle/data/remote/HustleFirebaseMessagingService.kt Implemented Firebase Messaging Service for handling push notifications
app/src/main/AndroidManifest.xml Registered Firebase Messaging Service in app manifest

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

@zachseidner1 zachseidner1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks great :)
just a couple questions on how you determined what flags to use


private fun showNotification(title: String, body: String, data: Map<String, String>) {
val intent = Intent(this, MainActivity::class.java).apply {
flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you decide on these flags, perhaps leave a comment about this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Admittedly, I didn't think too much about these flags and their functionality and mainly followed example code, but I believe the new task is to prevent crashes if the app is launched from a service in the background, and the clear top flag is to prevent duplicate instances of activities if say we minimized the app and then clicked on the notification to open it. I'll probably add a comment before merging

this,
data.hashCode(),
intent,
PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same with these flags?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar situation as above. Update current flag is to make sure the data associated with the notification is the correct and current one (eg. potential hash collision with pending intent request codes for 2 separate messages) and immutable flag is required by Android for security reasons I believe to make sure malicious apps can't hijack your notifications or something like that. Will also add comment before merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants