Messaging push notification improvements #2966
Conversation
…rity, separate notification channel
OrangeAndGreen
requested review from
Jignesh-dimagi,
pm-dimagi and
shubham1g5
📝 WalkthroughWalkthroughThis change adds two new string resources for a messaging notification channel across multiple language resource files (default, Portuguese, and Tigrinya). In addition to defining the title and description for the messaging channel, the default strings file also updates existing messaging notification titles by adding bell emojis. The Java classes are updated accordingly: in the notification manager class, a new constant for the messaging channel is defined and used to create the channel with maximum importance, while the Firebase messaging service is updated to dynamically select the appropriate notification channel, adjust notification priority, and include a large icon when handling messaging-related notifications. Sequence Diagram(s)sequenceDiagram
participant NM as CommCareNoficationManager
participant NMgr as NotificationManager
NM->>NMgr: createNotificationChannel(NOTIFICATION_CHANNEL_MESSAGING_ID, title, description, IMPORTANCE_MAX)
sequenceDiagram
participant FMS as CommCareFirebaseMessagingService
participant Builder as NotificationCompat.Builder
participant NMgr as NotificationManager
FMS->>FMS: Receive push notification
FMS->>FMS: Check if action equals "CCC_MESSAGE"
FMS->>Builder: Set channel = messaging channel\nSet priority = IMPORTANCE_MAX\nDecode large icon
Builder->>NMgr: Build and dispatch notification
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🔇 Additional comments (11)
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
Can we add a screenshot to show the notification after these changes in PR.
app/src/org/commcare/services/CommCareFirebaseMessagingService.java
Outdated
Show resolved
Hide resolved
|
@damagatchi retest this please |
Drawing a red circle on the icon when there are unread messages.
app/src/org/commcare/services/CommCareFirebaseMessagingService.java
Outdated
Show resolved
Hide resolved
| Intent intent = null; | ||
| String action = payloadData.get("action"); | ||
| String notificationChannel = CommCareNoficationManager.NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID; | ||
| int priority = NotificationCompat.PRIORITY_HIGH; |
There was a problem hiding this comment.
Priority should be default - PRIORITY_DEFAULT
There was a problem hiding this comment.
I went with PRIORITY_HIGH to keep the existing behavior for default notifications (i.e. any not related to messaging). That's the value currently set in the master code
app/src/org/commcare/services/CommCareFirebaseMessagingService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
As a general guideling icons either need to be classified as per device densitites (mdpi/hdpi/xhdpi etc) or needs to be a vector drawable.
There was a problem hiding this comment.
I replaced the graphic with a vector drawable. Actually make that two vector drawables, so we can better support dark mode in push notifications.
Messaging icon improvements
https://dimagi.atlassian.net/browse/CCCT-710
cross-request: dimagi/commcare-core#1455
Product Description
Improvements to push notifications related to messaging.
-Large icon shown on right side
-Bell emojis surrounding notification title
-New notification channel for messaging
-Default max priority (to show over current user activity)
Views of the collapsed and expanded notifications:
Notification alongside the usually-present active session notification:
Technical Summary
Added code to create the new channel in addition to other existing channels.
Added code for choosing the channel, priority, and large icon when creating a messaging notification.
Feature Flag
ConnectID Messaging
Safety Assurance
Safety story
Dev testing of the updated notifications.
Automated test coverage
No automated tests for ConnectID yet.
QA Plan
Send a message from HQ to the ConnectID user while the user is not on a messaging page.
Verify the described changes to the notification are shown.
Labels and Review