Update notification screen whenever new notifications are received

[Jignesh-dimagi]

Technical Summary

https://dimagi.atlassian.net/browse/QA-8229

Labels and Review

• Do we need to enhance the manual QA test coverage ? If yes, the "QA Note" label is set correctly
• Does the PR introduce any major changes worth communicating ? If yes, the "Release Note" label is set and a "Release Note" is specified in PR description.
• Risk label is set correctly
• The set of people pinged as reviewers is appropriate for the level of risk of the change

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces a new registerForNewNotification() method in PushNotificationActivity that registers the activity to receive notification broadcasts via NotificationBroadcastHelper. When a broadcast is received, the method triggers pushNotificationViewModel.loadNotifications(false) to refresh the notification list. The method is called during onCreate() to ensure the activity subscribes to new notification broadcasts upon initialization.

Sequence Diagram

sequenceDiagram
    participant Activity as PushNotificationActivity
    participant Helper as NotificationBroadcastHelper
    participant ViewModel as pushNotificationViewModel
    
    Activity->>Activity: onCreate()
    Activity->>Activity: registerForNewNotification()
    Activity->>Helper: Register broadcast receiver
    Helper-->>Activity: Receiver registered
    
    Note over Helper: New notification arrives
    Helper->>Activity: Broadcast notification event
    Activity->>ViewModel: loadNotifications(false)
    ViewModel-->>Activity: Refresh notification list

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• The addition is straightforward: one new public method with broadcast registration and one initialization call
• Logic density is low—method wraps existing helper and ViewModel functionality
• Primary focus should be verifying correct broadcast receiver lifecycle management and ensuring loadNotifications(false) is the appropriate refresh trigger

Possibly related PRs

• Managed read/unread pn status and its icon #3383: Introduces ACTION_NEW_NOTIFICATIONS, broadcasting logic, and receiver handling that this PR builds upon by subscribing PushNotificationActivity to these broadcasts.

Suggested labels

skip-integration-tests

Suggested reviewers

• OrangeAndGreen
• shubham1g5

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is incomplete. While it includes a technical summary link and a checklist template, it lacks critical sections such as Product Description, rationale, design decisions, feature flags, safety story, automated test coverage details, and QA plan.	Complete the PR description by filling out all required sections: explain the user-facing effects, provide design rationale, describe safety testing and confidence measures, document test coverage, detail the QA plan, and verify all labels are set appropriately.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically describes the main change: adding a mechanism to update the notification screen whenever new notifications are received.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jignesh/fix/qa-8229

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8694a0 and 6d3c2eb.

📒 Files selected for processing (1)

• app/src/org/commcare/activities/PushNotificationActivity.kt (3 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: shubham1g5
Repo: dimagi/commcare-android PR: 0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Lint Code Base

🔇 Additional comments (3)

app/src/org/commcare/activities/PushNotificationActivity.kt (3)

18-18: LGTM!

The import is necessary for the new broadcast registration functionality.

---

34-34: ****

The lifecycle management is properly handled. NotificationBroadcastHelper.registerForNotifications() receives the Activity as a LifecycleOwner and uses DefaultLifecycleObserver to automatically register the broadcast receiver in onResume() and unregister it in onPause(). No manual cleanup in the activity is needed—the helper manages the receiver's lifecycle automatically, preventing memory leaks.

Likely an incorrect or invalid review comment.

---

119-126: No lifecycle management issues—the broadcast registration is already properly handled.

The original review concerns about memory leaks and missing unregistration logic are unfounded. NotificationBroadcastHelper uses Android's DefaultLifecycleObserver pattern to automatically register the receiver in onResume() and unregister in onPause(), ensuring proper cleanup without manual intervention. No onDestroy() override is needed.

The only valid suggestion is to make registerForNewNotification() private since it's only called internally from onCreate():

-    fun registerForNewNotification() {
+    private fun registerForNewNotification() {
         NotificationBroadcastHelper.registerForNotifications(this, this) {
             // Whenever new notification is received, signalling is calling retrieve_notifications api
             // so whenever this broadcast is received, new notification is already stored in local DB
             // that's the reason that isRefreshed = false is required
             pushNotificationViewModel.loadNotifications(false)
         }
     }

Likely an incorrect or invalid review comment.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Jignesh-dimagi]

@damagatchi retest this please