Introducing Notifications #637

baronhsieh2005 · 2024-11-18T19:03:59Z

Our app can receive notifications now!

Changes

Using FCM, our app could receive notifications by sending their FCM Registration token, a device-based token that can be used by admin to send notifications by FCM service, to the backend.
For every new token generated, we would store/update it in SharedPreferences. Therefore, to fetch notification tokens, we can easily do so reading SharedPreferences.
Currently, we implemented network requests by posting the registration token to the backend so they can send notifications as admin and deleting the registration token from the backend so users won't receive notifications when they are logged out.
There are more to come. For now, this implementation would be sufficient to send pca notifications by the add/drop period. We would add new notification preferences (so users can customize what notifications they want to receive) as new notifications are being introduced. @Divak2004 is working on the settings UI.

Documentation and Testing:

Firebase Notification Testing Console: https://console.firebase.google.com/project/penn-mobile-android/notification/compose
About FCM Messages: https://firebase.google.com/docs/cloud-messaging/concept-options
Receive Messages in an Android App: https://firebase.google.com/docs/cloud-messaging/android/receive

Divak2004 · 2024-11-18T21:18:54Z

PennMobile/src/main/java/com/pennapps/labs/pennmobile/MainActivity.kt

+            ActivityResultContracts.RequestPermission(),
+        ) { isGranted: Boolean ->
+            if (isGranted) {
+                // FCM SDK (and your app) can post notifications.


If isGranted is true, does that just mean you can send notifications even though nothing is written here.

doubling this question

Is this something where we can choose to do something (ex: sending a "success!" message to the user), but it's also okay not to do anything (since notifications will be shown as enabled in the background?)

I don't think we have to do anything, but having TODOs in main is generally considered bad practice. What behavior should we expect if notifications aren't enabled? Is this information generally available or should it be stored somewhere? (oops, answered by scrolling down LOL)

I don't think we have to do anything then. How do other ppl generally use this function?

Divak2004 · 2024-11-18T21:20:07Z

PennMobile/src/main/java/com/pennapps/labs/pennmobile/MainActivity.kt

+            ) {
+                // FCM SDK (and your app) can post notifications.
+            } else if (shouldShowRequestPermissionRationale(Manifest.permission.POST_NOTIFICATIONS)) {
+                // TODO: display an educational UI explaining to the user the features that will be enabled


Is there some method to keep track of the user already rejecting notifications?

It seems the "shouldShowRequestPermissionRationale" function actually checks the Manifest.permission.POST_NOTIFICATIONS to see whether these permissions was just rejected (meaning ask again on-create), or if it was rejected + "don't show again" box was checked (meaning don't ask again on-create)

When a user rejects, the specific type of rejection is stored within Manifest.permission.POST_NOTIFICATIONS. Pretty Cool

again, try to clean up TODOs. Personally, I don't believe we really need an explanation for this and am comfortable with doing nothing.

Divak2004

Left some questions above

meiron03

See comments. Also, figure out how u want to update token in backend when onNewToken() is called.

meiron03 · 2024-11-22T18:10:36Z

PennMobile/build.gradle

@@ -99,6 +99,7 @@ dependencies {
    testImplementation platform(libs.androidx.compose.bom)
    androidTestImplementation platform(libs.androidx.compose.bom)
    implementation platform(libs.firebase.bom)
+    implementation(libs.firebase.messaging)


why does this have parens but the other imports don't? Can we standardize formatting?

meiron03 · 2024-11-22T18:15:42Z

PennMobile/src/main/java/com/pennapps/labs/pennmobile/MainActivity.kt

@@ -91,6 +107,7 @@ class MainActivity : AppCompatActivity() {
            setTheme(R.style.DarkBackground)
        }
        Utils.getCurrentSystemTime()
+        askNotificationPermission()


why are we asking for notification permission every time the activity starts? It is probably annoying if they don't want notifications and the app keeps asking... I feel like the app should just ask once.

udp: I didn't realize that android had a "never ask again" feature. Someone correct me if I'm wrong, but this case should be handled right.

meiron03 · 2024-11-22T18:20:28Z

PennMobile/src/main/java/com/pennapps/labs/pennmobile/MainActivity.kt

+            ) {
+                // FCM SDK (and your app) can post notifications.
+            } else if (shouldShowRequestPermissionRationale(Manifest.permission.POST_NOTIFICATIONS)) {
+                // TODO: display an educational UI explaining to the user the features that will be enabled


again, try to clean up TODOs. Personally, I don't believe we really need an explanation for this and am comfortable with doing nothing.

meiron03 · 2024-11-22T18:32:35Z

PennMobile/src/main/res/values/colors.xml

@@ -72,5 +72,6 @@
    <color name="light_blue_200">#FF81D4FA</color>
    <color name="light_blue_600">#FF039BE5</color>
    <color name="light_blue_900">#FF01579B</color>
+    <color name="penn_red">#990000</color>


we already have PennRed... Please remove one of them.

meiron03 · 2024-11-22T18:41:56Z

PennMobile/src/main/java/com/pennapps/labs/pennmobile/api/viewmodels/LoginWebviewViewmodel.kt

+class LoginWebviewViewmodel : ViewModel() {
+    suspend fun sendToken(
+        mNotificationAPI: NotificationAPI,
+        notGuest: Boolean,


can we negate this here. (I know our guest mode stuff is kinda fucked up but notGuest is just an extremely cursed variable name)

baronhsieh2005 added 6 commits November 11, 2024 20:37

Draft for notifs

8281125

Notif Networking

5791015

DeleteToken Fix

7e05cf8

Minor fixes

6ea307f

Fix MoreFragment initial bug

2f35e44

Rename and Remove delete Authorization

37ad439

baronhsieh2005 requested review from vavali08, meiron03, Akula112233, trinif, Divak2004 and joemacd November 18, 2024 19:04

Divak2004 reviewed Nov 18, 2024

View reviewed changes

meiron03 requested changes Nov 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introducing Notifications #637

Introducing Notifications #637

baronhsieh2005 commented Nov 18, 2024

Divak2004 Nov 18, 2024

Akula112233 Nov 19, 2024

meiron03 Nov 22, 2024 •

edited

Loading

Divak2004 Nov 18, 2024

Akula112233 Nov 19, 2024 •

edited

Loading

meiron03 Nov 22, 2024

Divak2004 left a comment •

edited

Loading

meiron03 left a comment

meiron03 Nov 22, 2024

meiron03 Nov 22, 2024

meiron03 Nov 22, 2024

meiron03 Nov 22, 2024

meiron03 Nov 22, 2024

meiron03 Nov 22, 2024

Introducing Notifications #637

Are you sure you want to change the base?

Introducing Notifications #637

Conversation

baronhsieh2005 commented Nov 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meiron03 Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Akula112233 Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Divak2004 left a comment • edited Loading

Choose a reason for hiding this comment

meiron03 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meiron03 Nov 22, 2024 •

edited

Loading

Akula112233 Nov 19, 2024 •

edited

Loading

Divak2004 left a comment •

edited

Loading