CI-467 - Send message bug solved #3512
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jignesh-dimagi
requested review from
OrangeAndGreen,
conroy-ricketts and
shubham1g5
4 tasks
📝 WalkthroughWalkthroughThis pull request refactors the push notification sync workflow by restructuring the NotificationsSyncWorkerManager API. The changes replace batch-oriented constructors and method invocations with a streamlined interface featuring new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/org/commcare/services/FCMMessageData.java`:
- Line 62: The notification channel for push notifications uses
IMPORTANCE_DEFAULT so NotificationCompat.PRIORITY_MAX is ignored on Android 8+;
in CommCareNotificationManager where the channel for
NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID is created (e.g.,
createNotificationChannels / the channel creation block around line ~197), set
the channel's importance to NotificationManager.IMPORTANCE_HIGH or
IMPORTANCE_MAX instead of IMPORTANCE_DEFAULT, and ensure you recreate or update
the existing channel via NotificationManager.createNotificationChannel (or
delete-and-recreate) so the new importance takes effect on devices that already
have the channel; keep the channel id NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID
and retain NotificationCompat.PRIORITY_MAX on the notification side.
| notificationText = payloadData.get(NOTIFICATION_BODY); | ||
| action = payloadData.get(REDIRECT_ACTION); | ||
| priority = NotificationCompat.PRIORITY_HIGH; | ||
| priority = NotificationCompat.PRIORITY_MAX; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -C3 'NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID|createNotificationChannel|IMPORTANCE_' app/srcRepository: dimagi/commcare-android
Length of output: 6369
Increase channel importance for interruptive push notifications
The push notification channel is created with NotificationManager.IMPORTANCE_DEFAULT (CommCareNoficationManager.java, line 197), which means NotificationCompat.PRIORITY_MAX will be ignored on Android 8+. To achieve interruptive behavior, change the channel importance to IMPORTANCE_HIGH or IMPORTANCE_MAX when creating NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID.
🤖 Prompt for AI Agents
In `@app/src/org/commcare/services/FCMMessageData.java` at line 62, The
notification channel for push notifications uses IMPORTANCE_DEFAULT so
NotificationCompat.PRIORITY_MAX is ignored on Android 8+; in
CommCareNotificationManager where the channel for
NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID is created (e.g.,
createNotificationChannels / the channel creation block around line ~197), set
the channel's importance to NotificationManager.IMPORTANCE_HIGH or
IMPORTANCE_MAX instead of IMPORTANCE_DEFAULT, and ensure you recreate or update
the existing channel via NotificationManager.createNotificationChannel (or
delete-and-recreate) so the new importance takes effect on devices that already
have the channel; keep the channel id NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID
and retain NotificationCompat.PRIORITY_MAX on the notification side.
Contributor
Author
There was a problem hiding this comment.
Please review it again
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine the CommCareNoficationManager file and channel creation code
fd -e java -e kt 'CommCare.*[Nn]otif' --exec cat {} \; | head -300
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for the specific channel creation for NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID
rg -n -A10 -B5 'NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID.*=' app/src/
Length of output: 1982
🏁 Script executed:
#!/bin/bash
# Look for createNotificationChannel calls with importance settings
rg -n -A15 'createNotificationChannel|NotificationChannel\(' app/src/ | grep -A15 -B5 'PUSH_NOTIFICATIONS'
Length of output: 4891
@Jignesh-dimagi, I apologize for my previous incorrect comment. After reviewing the code more carefully, I can confirm that your implementation is correct.
The notification channel NEW_HIGH_PRIORITY_NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID is already created with NotificationManager.IMPORTANCE_HIGH, not IMPORTANCE_DEFAULT as I mistakenly stated earlier.
Current Implementation ✅
Your change to set priority = NotificationCompat.PRIORITY_MAX works correctly because:
- Android 8.0+ (API 26+): The channel importance (
IMPORTANCE_HIGH) determines notification behavior, providing heads-up notifications and sound - Android 7.1 and lower: The notification priority (
PRIORITY_MAX) is used, ensuring interruptive behavior on older devices
Optional Enhancement
If you want even more aggressive notification behavior on Android 8+, you could consider changing the channel importance from IMPORTANCE_HIGH to IMPORTANCE_MAX in the channel creation code. However, IMPORTANCE_HIGH is already quite interruptive and is generally the recommended level for most use cases.
The current implementation properly addresses the bug you described in the PR and should display push notifications interruptively across all Android versions. My apologies for the confusion.
🧬 Code Graph Analysis Results
[app/src/org/commcare/connect/ConnectConstants.java:8-55]
public class ConnectConstants {
public static final int PERSONAL_ID_TASK_ID_OFFSET = 1000;
public final static int CREDENTIAL_PICKER_REQUEST = 2000;
public static final int PERSONAL_ID_SIGN_UP_LAUNCH = 1050;
public static final int CONFIGURE_BIOMETRIC_REQUEST_CODE = 1053;
public static final int NETWORK_ACTIVITY_ID = 7000;
public static final String PERSONALID_MANAGED_LOGIN = "personalid-managed-login";
public static final String CONNECT_MANAGED_LOGIN = "connect-managed-login";
public static final String PIN = "PIN";
public static final String CONNECT_KEY_TOKEN = "access_token";
public static final String CONNECT_KEY_EXPIRES = "expires_in";
public static final String CCC_DEST_OPPORTUNITY_SUMMARY_PAGE = "ccc_opportunity_summary_page";
public static final String CCC_DEST_LEARN_PROGRESS = "ccc_learn_progress";
public static final String CCC_DEST_DELIVERY_PROGRESS = "ccc_delivery_progress";
public static final String CCC_PAYMENT_INFO_CONFIRMATION = "ccc_payment_info_confirmation";
public static final String CCC_PAYMENT_INFO_CONFIRMATION_STATUS = "confirmation_status";
public static final String REDIRECT_ACTION = "action";
public static final String GO_TO_JOB_STATUS = "go_to_job_status";
public static final String SHOW_LAUNCH_BUTTON = "show_launch_button";
public static final String OPPORTUNITY_ID = "opportunity_id";
public static final String PAYMENT_ID = "payment_id";
public static final String PAYMENT_STATUS = "payment_status";
public static final String CONNECT_KEY_DB_KEY = "db_key";
public static final String NEW_APP = "new-app";
public static final String LEARN_APP = "learn-app";
public static final String DELIVERY_APP = "delivery-app";
public final static int PERSONALID_NO_ACTIVITY = ConnectConstants.PERSONAL_ID_TASK_ID_OFFSET;
public final static int PERSONALID_REGISTRATION_SUCCESS = ConnectConstants.PERSONAL_ID_TASK_ID_OFFSET + 1;
public final static int PERSONALID_RECOVERY_SUCCESS = ConnectConstants.PERSONAL_ID_TASK_ID_OFFSET + 2;
public final static int PERSONALID_BIOMETRIC_ENROLL_FAIL = ConnectConstants.PERSONAL_ID_TASK_ID_OFFSET + 4;
public final static int PERSONALID_RECOVERY_WRONG_BACKUPCODE = ConnectConstants.PERSONAL_ID_TASK_ID_OFFSET + 7;
public final static int PERSONALID_DEVICE_CONFIGURATION_FAILED = ConnectConstants.PERSONAL_ID_TASK_ID_OFFSET + 8;
public final static int PERSONALID_RECOVERY_ACCOUNT_ORPHANED = ConnectConstants.PERSONAL_ID_TASK_ID_OFFSET + 9;
public final static int PERSONALID_RECOVERY_ACCOUNT_LOCKED = ConnectConstants.PERSONAL_ID_TASK_ID_OFFSET + 10;
public final static int PERSONALID_LOCATION_PERMISSION_FAILURE = ConnectConstants.PERSONAL_ID_TASK_ID_OFFSET + 11;
public final static int PERSONALID_DEVICE_CONFIGURATION_ISSUE_WARNING = ConnectConstants.PERSONAL_ID_TASK_ID_OFFSET + 12;
public final static String STATUS_APPROVED = "approved";
public final static String NOTIFICATION_TITLE = "title";
public final static String NOTIFICATION_BODY = "body";
public final static String NOTIFICATION_ID = "notification_id";
public final static String NOTIFICATION_TIME_STAMP = "timestamp";
public final static String NOTIFICATION_STATUS = "status";
public final static String NOTIFICATION_MESSAGE_ID = "message_id";
public final static String NOTIFICATION_CHANNEL_ID = "channel";
}
[app/src/org/commcare/utils/FirebaseMessagingUtil.java:72-532]
public class FirebaseMessagingUtil {
public static final String FCM_TOKEN = "fcm_token";
public static final String FCM_TOKEN_TIME = "fcm_token_time";
private final static int FCM_NOTIFICATION = R.string.fcm_notification;
public static final String MESSAGING_UPDATE_BROADCAST = "com.dimagi.messaging.update";
public static String getFCMToken() {
return PreferenceManager
.getDefaultSharedPreferences(CommCareApplication.instance())
.getString(FCM_TOKEN, null);
}
public static long getFCMTokenTime() {
return PreferenceManager
.getDefaultSharedPreferences(CommCareApplication.instance())
.getLong(FCM_TOKEN_TIME, 0);
}
public static void updateFCMToken(String newToken) {
SharedPreferences sharedPreferences = PreferenceManager
.getDefaultSharedPreferences(CommCareApplication.instance());
sharedPreferences.edit().putString(FCM_TOKEN, newToken).apply();
sharedPreferences.edit().putLong(FCM_TOKEN_TIME, System.currentTimeMillis()).apply();
}
public static void verifyToken() {
// TODO: Enable FCM in debug mode
if (!BuildConfig.DEBUG) {
// Retrieve the current Firebase Cloud Messaging (FCM) registration token
FirebaseMessaging.getInstance().getToken().addOnCompleteListener(handleFCMTokenRetrieval());
}
}
private static OnCompleteListener handleFCMTokenRetrieval() {
return (OnCompleteListener<String>)task -> {
if (task.isSuccessful()) {
String token = task.getResult();
if (!StringUtils.isEmpty(token)) {
updateFCMToken(token);
} else {
Logger.exception("Fetching FCM registration token failed",
new Throwable("FCM registration token is empty"));
}
} else {
Throwable throwable = task.getException() != null ? task.getException() : new Throwable(
"Task to fetch FCM registration token failed");
Logger.exception("Fetching FCM registration token failed", throwable);
}
};
}
public static String serializeFCMMessageData(FCMMessageData fcmMessageData) {
byte[] serializedMessageData = SerializationUtil.serialize(fcmMessageData);
String base64EncodedMessageData = Base64.encodeToString(serializedMessageData, Base64.DEFAULT);
return base64EncodedMessageData;
}
public static FCMMessageData deserializeFCMMessageData(String base64EncodedSerializedFCMMessageData) {
if (base64EncodedSerializedFCMMessageData != null) {
byte[] serializedMessageData = Base64.decode(base64EncodedSerializedFCMMessageData, Base64.DEFAULT);
return SerializationUtil.deserialize(serializedMessageData, FCMMessageData.class);
}
return null;
}
/**
* DataSyncer singleton class
*/
private static FirebaseMessagingDataSyncer dataSyncer;
private static FirebaseMessagingDataSyncer getDataSyncer(Context context) {
if (dataSyncer == null) {
synchronized (FirebaseMessagingUtil.class) {
dataSyncer = new FirebaseMessagingDataSyncer(context);
}
}
return dataSyncer;
}
/**
* This is generic function used by both CommCareFirebaseMessagingService and launcher activity.
*
* `@param` context - application context
* `@param` dataPayload - This will be data payload when calling from CommCareFirebaseMessagingService or will
* be intent data payload when calling from launcher activity
* `@param` notificationPayload - This will be notification payload when calling from CommCareFirebaseMessagingService and null when calling from launcher activity
* `@return` Intent - if need for launcher activity to start the activity
*/
public static Intent handleNotification(Context context, Map<String, String> dataPayload, RemoteMessage.Notification notificationPayload, boolean showNotification) {
if (dataPayload == null || dataPayload.isEmpty()) {
if (!showNotificationFromNotificationPayload(context, notificationPayload,showNotification)) {
Logger.exception("Empty push notification", new Throwable(String.format("Empty notification without payload")));
}
return null;
}
FCMMessageData fcmMessageData = new FCMMessageData(dataPayload);
if (hasCccAction(fcmMessageData.getAction())){
return handleCCCActionPushNotification(context, fcmMessageData,showNotification);
} else if (fcmMessageData.getActionType() == FCMMessageData.ActionTypes.SYNC) {
getDataSyncer(context).syncData(fcmMessageData);
return null;
} else {
return handleGeneralApplicationPushNotification(context, fcmMessageData,showNotification);
}
}
/**
* Show notification from notification payload
*
* `@param` context
* `@param` payloadNotification
* `@return`
*/
private static boolean showNotificationFromNotificationPayload(Context context, RemoteMessage.Notification payloadNotification,boolean showNotification) {
if (payloadNotification != null &&
!Strings.isEmptyOrWhitespace(payloadNotification.getTitle()) &&
!Strings.isEmptyOrWhitespace(payloadNotification.getBody()) &&
showNotification) {
Map<String, String> notificationPayload = new HashMap<>();
notificationPayload.put(NOTIFICATION_TITLE, payloadNotification.getTitle());
notificationPayload.put(NOTIFICATION_BODY, payloadNotification.getBody());
handleGeneralApplicationPushNotification(context, new FCMMessageData(notificationPayload),showNotification);
return true;
}
return false;
}
/**
* handle CCC action push notification
*
* `@param` context
* `@param` fcmMessageData
* `@return` Intent
*/
private static Intent handleCCCActionPushNotification(Context context, FCMMessageData fcmMessageData, boolean showNotification) {
if(!cccCheckPassed(context)) { // app doesn't show notification related to CCC if user is not logged in
Logger.log(LogTypes.TYPE_FCM,"CCC push notification sent while user is logout");
return null;
}
ConnectUserDatabaseUtil.turnOnConnectAccess(context);
return switch (fcmMessageData.getAction()) {
case CCC_MESSAGE -> handleCCCMessageChannelPushNotification(context, fcmMessageData,showNotification);
case CCC_DEST_PAYMENTS -> handleCCCPaymentPushNotification(context, fcmMessageData,showNotification);
case CCC_PAYMENT_INFO_CONFIRMATION ->
handleCCCPaymentInfoConfirmationPushNotification(context, fcmMessageData,showNotification);
case CCC_DEST_OPPORTUNITY_SUMMARY_PAGE ->
handleOpportunitySummaryPagePushNotification(context, fcmMessageData,showNotification);
case CCC_DEST_LEARN_PROGRESS ->
handleResumeLearningOrDeliveryJobPushNotification(true, context, fcmMessageData,showNotification);
case CCC_DEST_DELIVERY_PROGRESS ->
handleResumeLearningOrDeliveryJobPushNotification(false, context, fcmMessageData,showNotification);
default -> null;
};
}
/**
* Handle CCC payment push notification
*
* `@param` context
* `@param` fcmMessageData
* `@return` Intent
*/
private static Intent handleCCCPaymentPushNotification(Context context, FCMMessageData fcmMessageData, boolean showNotification) {
Intent intent = getConnectActivityNotification(context, fcmMessageData);
if(showNotification) {
NotificationCompat.Builder fcmNotification = buildNotification(context, intent, fcmMessageData);
int flags = Build.VERSION.SDK_INT >= Build.VERSION_CODES.S
? PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_IMMUTABLE
: PendingIntent.FLAG_UPDATE_CURRENT;
// Yes button intent with payment_id from payload
Intent yesIntent = new Intent(context, PaymentAcknowledgeReceiver.class);
yesIntent.putExtra(OPPORTUNITY_ID, fcmMessageData.getPayloadData().get(OPPORTUNITY_ID));
yesIntent.putExtra(PAYMENT_ID, fcmMessageData.getPayloadData().get(PAYMENT_ID));
yesIntent.putExtra(PAYMENT_STATUS, true);
PendingIntent yesPendingIntent = PendingIntent.getBroadcast(context, 1,
yesIntent, flags);
// No button intent with payment_id from payload
Intent noIntent = new Intent(context, PaymentAcknowledgeReceiver.class);
noIntent.putExtra(OPPORTUNITY_ID, fcmMessageData.getPayloadData().get(OPPORTUNITY_ID));
noIntent.putExtra(PAYMENT_ID, fcmMessageData.getPayloadData().get(PAYMENT_ID));
noIntent.putExtra(PAYMENT_STATUS, false);
PendingIntent noPendingIntent = PendingIntent.getBroadcast(context, 2,
noIntent, flags);
// Add Yes & No action button to the notification
fcmNotification.addAction(0, context.getString(R.string.connect_payment_acknowledge_notification_yes), yesPendingIntent);
fcmNotification.addAction(0, context.getString(R.string.connect_payment_acknowledge_notification_no), noPendingIntent);
showNotification(context, fcmNotification, fcmMessageData);
}
return intent;
}
/**
* Handle CCC resume learning or delivery job push notification
*
* `@param` isLearning
* `@param` context
* `@param` fcmMessageData
* `@return`
*/
private static Intent handleResumeLearningOrDeliveryJobPushNotification(Boolean isLearning, Context context, FCMMessageData fcmMessageData, boolean showNotification) {
if (fcmMessageData.getPayloadData().containsKey(OPPORTUNITY_ID)) {
Intent intent = getConnectActivityNotification(context, fcmMessageData);
intent.putExtra(OPPORTUNITY_ID, fcmMessageData.getPayloadData().get(OPPORTUNITY_ID));
if(showNotification) showNotification(context, buildNotification(context, intent, fcmMessageData),
fcmMessageData);
return intent;
}
String ccc_action = isLearning ? CCC_DEST_LEARN_PROGRESS : CCC_DEST_DELIVERY_PROGRESS;
Logger.exception("Empty push notification for action '" + ccc_action + "'", new Throwable(String.format("Empty notification without 'opportunity_id'")));
return null;
}
/**
* Handle CCC opportunity summary page push notification
*
* `@param` context
* `@param` fcmMessageData
* `@return`
*/
private static Intent handleOpportunitySummaryPagePushNotification(Context context, FCMMessageData fcmMessageData, boolean showNotification) {
if (fcmMessageData.getPayloadData().containsKey(OPPORTUNITY_ID)) {
Intent intent = getConnectActivityNotification(context, fcmMessageData);
intent.putExtra(OPPORTUNITY_ID, fcmMessageData.getPayloadData().get(OPPORTUNITY_ID));
if(showNotification) showNotification(context, buildNotification(context, intent, fcmMessageData),
fcmMessageData);
return intent;
}
Logger.exception("Empty push notification for action 'ccc_opportunity_summary_page'", new Throwable(String.format("Empty notification without 'opportunity_id'")));
return null;
}
/**
* Handle CCC payment info confirmation push notification
*
* `@param` context
* `@param` fcmMessageData
* `@return`
*/
private static Intent handleCCCPaymentInfoConfirmationPushNotification(Context context, FCMMessageData fcmMessageData, boolean showNotification) {
if (fcmMessageData.getPayloadData().containsKey(ConnectConstants.CCC_PAYMENT_INFO_CONFIRMATION_STATUS)) {
Intent intent = getConnectActivityNotification(context, fcmMessageData);
intent.putExtra(ConnectConstants.CCC_PAYMENT_INFO_CONFIRMATION_STATUS, fcmMessageData.getPayloadData().get(ConnectConstants.CCC_PAYMENT_INFO_CONFIRMATION_STATUS));
if(showNotification) showNotification(context, buildNotification(context, intent, fcmMessageData),
fcmMessageData);
return intent;
}
Logger.exception("Empty push notification for action 'ccc_payment_info_confirmation'", new Throwable(String.format("Empty notification without 'confirmation_status'")));
return null;
}
/**
* This will handle general application push notification. No need to return intent as it will be already in launcher activity i.e. DispatchActivity
*
* `@param` context
* `@param` fcmMessageData
* `@return` Intent
*/
private static Intent handleGeneralApplicationPushNotification(Context context, FCMMessageData fcmMessageData, boolean showNotification) {
if(showNotification) {
Intent intent = new Intent(context, DispatchActivity.class);
intent.setAction(Intent.ACTION_MAIN);
intent.addCategory(Intent.CATEGORY_LAUNCHER);
showNotification(context, buildNotification(context, intent, fcmMessageData), fcmMessageData);
}
return null; // This will always null as we are already in DispatchActivity and don't want to start again
}
/**
* Handle CCC messaging/channel push notification
*
* `@param` context
* `@param` fcmMessageData
* `@return` Intent
*/
private static Intent handleCCCMessageChannelPushNotification(Context context, FCMMessageData fcmMessageData, boolean showNotification) {
Intent intent = null;
fcmMessageData.setNotificationChannel(CommCareNoficationManager.NOTIFICATION_CHANNEL_MESSAGING_ID);
fcmMessageData.setLargeIcon(BitmapFactory.decodeResource(context.getResources(), R.drawable.ic_connect_message_large));
boolean isMessage = fcmMessageData.getPayloadData().containsKey(ConnectMessagingMessageRecord.META_MESSAGE_ID);
int notificationTitleId;
String notificationMessage;
String channelId;
if (isMessage) {
notificationTitleId = R.string.connect_messaging_message_notification_title;
notificationMessage = context.getString(R.string.connect_messaging_message_notification_message,
fcmMessageData.getPayloadData().get("channel_name"));
channelId = fcmMessageData.getPayloadData().get("channel");
} else {
//Channel
ConnectMessagingChannelRecord channel = MessageManager.handleReceivedChannel(context,
fcmMessageData.getPayloadData());
notificationTitleId = R.string.connect_messaging_channel_notification_title;
notificationMessage = context.getString(R.string.connect_messaging_channel_notification_message,
channel.getChannelName());
channelId = channel.getChannelId();
}
//Send broadcast so any interested pages can update their UI
Intent broadcastIntent = new Intent(MESSAGING_UPDATE_BROADCAST);
LocalBroadcastManager.getInstance(context).sendBroadcast(broadcastIntent);
if (!ConnectMessageChannelListFragment.isActive &&
!channelId.equals(ConnectMessageFragment.getActiveChannel())) {
//Show push notification
fcmMessageData.setNotificationTitle(context.getString(notificationTitleId));
fcmMessageData.setNotificationText(notificationMessage);
intent = new Intent(context, ConnectMessagingActivity.class);
intent.putExtra(fcmMessageData.getAction(), fcmMessageData.getAction());
intent.putExtra(ConnectMessagingMessageRecord.META_MESSAGE_CHANNEL_ID, channelId);
intent.putExtra(REDIRECT_ACTION, fcmMessageData.getAction());
}
if (intent != null && showNotification) {
showNotification(context, buildNotification(context, intent, fcmMessageData), fcmMessageData);
}
return intent;
}
/**
* Get connect activity notification intent
*
* `@param` context
* `@param` fcmMessageData
* `@return` Intent
*/
private static Intent getConnectActivityNotification(Context context, FCMMessageData fcmMessageData) {
Intent intent = new Intent(context, ConnectActivity.class);
intent.putExtra(REDIRECT_ACTION, fcmMessageData.getAction());
if (fcmMessageData.getPayloadData().containsKey(OPPORTUNITY_ID)) {
intent.putExtra(OPPORTUNITY_ID, fcmMessageData.getPayloadData().get(OPPORTUNITY_ID));
}
return intent;
}
/**
* Build notification
*
* `@param` context
* `@param` intent
* `@param` fcmMessageData
* `@return` NotificationCompat.Builder
*/
private static NotificationCompat.Builder buildNotification(Context context, Intent intent, FCMMessageData fcmMessageData) {
intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP | Intent.FLAG_ACTIVITY_SINGLE_TOP |
Intent.FLAG_ACTIVITY_NEW_TASK);
int flags = Build.VERSION.SDK_INT >= Build.VERSION_CODES.S
? PendingIntent.FLAG_UPDATE_CURRENT | PendingIntent.FLAG_IMMUTABLE
: PendingIntent.FLAG_UPDATE_CURRENT;
Bundle bundleExtras = new Bundle();
intent.putExtra(NOTIFICATION_ID, fcmMessageData.getPayloadData().get(NOTIFICATION_ID));
PendingIntent contentIntent = PendingIntent.getActivity(context, 0, intent, flags);
if (Strings.isEmptyOrWhitespace(fcmMessageData.getNotificationTitle()) && Strings.isEmptyOrWhitespace(fcmMessageData.getNotificationText())) {
Logger.exception("Empty push notification",
new Throwable(String.format("Empty notification for action '%s'", fcmMessageData.getAction())));
}
NotificationCompat.Builder fcmNotification = new NotificationCompat.Builder(context,
fcmMessageData.getNotificationChannel())
.setContentTitle(fcmMessageData.getNotificationTitle())
.setContentText(fcmMessageData.getNotificationText())
.setContentIntent(contentIntent)
.setAutoCancel(true)
.setSmallIcon(R.drawable.commcare_actionbar_logo)
.setPriority(fcmMessageData.getPriority())
.setWhen(System.currentTimeMillis())
.setExtras(bundleExtras);
if (fcmMessageData.getLargeIcon() != null) {
fcmNotification.setLargeIcon(fcmMessageData.getLargeIcon());
}
return fcmNotification;
}
/**
* Show notification
*
* `@param` context
* `@param` notificationBuilder
* `@param` fcmMessageData
*/
private static void showNotification(Context context, NotificationCompat.Builder notificationBuilder,
FCMMessageData fcmMessageData) {
NotificationManager mNM = (NotificationManager)context.getSystemService(NOTIFICATION_SERVICE);
String notificationId = fcmMessageData.getPayloadData().get(NOTIFICATION_ID);
int notifId = !TextUtils.isEmpty(notificationId)
? notificationId.hashCode()
: FCM_NOTIFICATION; // fallback to constant
mNM.notify(notifId, notificationBuilder.build());
FirebaseAnalyticsUtil.reportNotificationEvent(
AnalyticsParamValue.NOTIFICATION_EVENT_TYPE_SHOW,
AnalyticsParamValue.REPORT_NOTIFICATION_METHOD_FIREBASE,
fcmMessageData.getAction(),
notificationId
);
}
/**
* Check if CCC action
*
* `@param` action
* `@return`
*/
private static boolean hasCccAction(String action) {
return action != null && action.contains("ccc_");
}
public static Intent getIntentForPNIfAny(Context context,Intent intent){
if(intent!=null && intent.getExtras()!=null){
Map<String, String> dataPayload = new HashMap<>();
for (String key : intent.getExtras().keySet()) {
String value = intent.getExtras().getString(key);
dataPayload.put(key, value);
}
Intent pnIntent = handleNotification(context,dataPayload,null,false);
if(pnIntent!=null){
intent.replaceExtras(new Bundle()); // clear intents if push notification intents are present else app keeps reloading same PN intents
}
return pnIntent;
}
return null;
}
public static Intent getIntentForPNClick(Context context, PushNotificationRecord pushNotificationRecord){
return cccCheckPassed(context) ? handleNotification(context,PushNotificationApiHelper.INSTANCE.convertPNRecordToPayload(pushNotificationRecord),null ,false): null;
}
public static boolean cccCheckPassed(Context context){
PersonalIdManager personalIdManager = PersonalIdManager.getInstance();
personalIdManager.init(context);
return personalIdManager.isloggedIn();
}
}
🧠 Learnings used
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.
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/connect/MessageManager.java:0-0
Timestamp: 2025-07-29T14:54:47.940Z
Learning: User OrangeAndGreen organizes messaging-related changes into separate PRs rather than mixing them with other Connect work, which aligns with their preference for handling code issues in separate PRs when they are not directly related to the main changes.
Learnt from: Jignesh-dimagi
Repo: dimagi/commcare-android PR: 0
File: :0-0
Timestamp: 2025-05-08T13:40:19.645Z
Learning: PR `#3048` introduces a comprehensive messaging system in the Connect feature, implementing secure encryption using AES-GCM for message content, proper channel management with consent flows, and a well-designed UI separation between sent and received messages with real-time notification integration.
Contributor
Author
There was a problem hiding this comment.
IMPORTANCE_MAX is depreciated and IMPORTANCE_HIGH is only recommended, can you please check.
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Android NotificationManager IMPORTANCE_MAX deprecated
💡 Result:
NotificationManager.IMPORTANCE_MAX isn’t actually deprecated in the Android framework API; it’s documented as “Unused.” That means it has no effect and shouldn’t be relied on. [1]
What to use instead
- For the highest meaningful channel level, use
NotificationManager.IMPORTANCE_HIGH(value 4). [1] - If you’re migrating from the old (pre-channels) notification priorities:
Notification.PRIORITY_MAXis deprecated; the replacement is channel importance, andPRIORITY_MAXmaps toNotificationManager.IMPORTANCE_HIGH. [2]
Note: You might see IMPORTANCE_MAX described in NotificationManagerCompat, but on the platform IMPORTANCE_MAX is still “unused,” so IMPORTANCE_HIGH is the practical top level. [1] [3]
Sources: Android NotificationManager constants [1], Notification priority deprecation/mapping [2], NotificationManagerCompat constants [3].
@Jignesh-dimagi, you are absolutely correct! Thank you for catching that.
According to the Android documentation, NotificationManager.IMPORTANCE_MAX is marked as "Unused" and has no effect. NotificationManager.IMPORTANCE_HIGH is indeed the recommended and highest meaningful importance level for notification channels.
Your current implementation is correct as-is:
✅ Channel: Uses NotificationManager.IMPORTANCE_HIGH (correct)
✅ Notification: Uses NotificationCompat.PRIORITY_MAX (correct for pre-Android 8.0 devices)
This combination provides the most interruptive notification behavior across all Android versions:
- Android 8.0+: Channel importance (
IMPORTANCE_HIGH) controls behavior - Android 7.1 and below: Notification priority (
PRIORITY_MAX) controls behavior
I apologize for the confusion in my previous comment when I mentioned IMPORTANCE_MAX as an option. Your implementation is already following Android best practices! 🎯
🧠 Learnings used
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.
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/adapters/ConnectMessageAdapter.java:0-0
Timestamp: 2025-07-29T14:14:07.954Z
Learning: In the CommCare Android Connect messaging system (ConnectMessageAdapter.java), the team prefers to let getMessage() potentially return null and crash rather than adding defensive null checks. This follows the fail-fast philosophy used throughout the Connect feature to catch programming errors immediately during development.
Learnt from: Jignesh-dimagi
Repo: dimagi/commcare-android PR: 0
File: :0-0
Timestamp: 2025-05-08T13:40:19.645Z
Learning: PR `#3048` introduces a comprehensive messaging system in the Connect feature, implementing secure encryption using AES-GCM for message content, proper channel management with consent flows, and a well-designed UI separation between sent and received messages with real-time notification integration.
Learnt from: Jignesh-dimagi
Repo: dimagi/commcare-android PR: 3093
File: app/res/navigation/nav_graph_connect_messaging.xml:41-45
Timestamp: 2025-05-09T10:57:41.073Z
Learning: In the CommCare Android codebase, the navigation graph for Connect messaging (`nav_graph_connect_messaging.xml`) intentionally uses `channel_id` as the argument name in the connectMessageFragment, despite using `channelId` in other parts of the same navigation graph. This naming difference is by design in the refactored code.
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/connect/MessageManager.java:0-0
Timestamp: 2025-07-29T14:45:13.470Z
Learning: In the CommCare Android Connect messaging system (MessageManager.java), the error handling logic in updateChannelConsent's processFailure callback intentionally attempts to retrieve the channel encryption key even after consent update failure. This behavior was recommended by the server team and the worst case scenario is silently failing to retrieve the encryption key, which is considered acceptable.
Learnt from: pm-dimagi
Repo: dimagi/commcare-android PR: 2956
File: app/src/org/commcare/fragments/connectMessaging/ConnectMessageChannelListFragment.java:58-60
Timestamp: 2025-02-19T15:15:01.935Z
Learning: Error handling for message retrieval in ConnectMessageChannelListFragment's retrieveMessages callback is not required as per user preference.
Learnt from: shubham1g5
Repo: dimagi/commcare-android PR: 3455
File: app/src/org/commcare/connect/network/connectId/parser/RetrieveNotificationsResponseParser.kt:64-93
Timestamp: 2025-12-08T12:01:48.545Z
Learning: In the CommCare Android Connect notification system (RetrieveNotificationsResponseParser.kt), messages are only decrypted using channels that already exist in the database with their encryption keys. When new channels arrive without keys, a separate network flow fetches the channel keys, and then the notification API is re-called to decrypt any messages for those channels. This explains why parseAndSeparateNotifications() correctly uses existingChannels from the database rather than newly parsed channels.
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/connect/MessageManager.java:0-0
Timestamp: 2025-07-29T14:54:47.940Z
Learning: User OrangeAndGreen organizes messaging-related changes into separate PRs rather than mixing them with other Connect work, which aligns with their preference for handling code issues in separate PRs when they are not directly related to the main changes.
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3040
File: app/src/org/commcare/android/database/connect/models/ConnectJobDeliveryFlagRecord.java:39-55
Timestamp: 2025-04-18T20:13:29.655Z
Learning: In the CommCare Android Connect feature, the JSON object passed to `ConnectJobDeliveryFlagRecord.fromJson()` method should never be null based on the implementation design.
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/fragments/connectMessaging/ConnectMessageFragment.java:0-0
Timestamp: 2025-07-29T14:09:49.805Z
Learning: In the CommCare Android Connect messaging system, message retry logic is implemented at the MessageManager level via MessageManager.sendUnsentMessages(). This method is called automatically when the channel list loads (ConnectMessageChannelListFragment), and it attempts to resend any outgoing messages that are not yet confirmed. Individual message fragments do not need to implement their own retry logic.
OrangeAndGreen
approved these changes
Jan 26, 2026
Contributor
OrangeAndGreen
left a comment
OrangeAndGreen
left a comment
There was a problem hiding this comment.
The changes look good to me for fixing the reported issues, and also some nice steps toward making the code easier to understand. It's still rather complex though and I think we would benefit from good documentation on the various ways the notifications are received and acknowledged.
shubham1g5
approved these changes
Jan 26, 2026
Contributor
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
Thanks for the refactor here, think the code structure makes much more sense now.
Comment on lines
+115
to
+116
| showNotification = false, | ||
| syncNotification = true, |
Contributor
There was a problem hiding this comment.
nit: can we add a small 1-line description to these 2 variables defined at class level.
Comment on lines
196
to
199
| parseAndStartNotificationWorker(notificationPayload) // syncNotification is false always as this method will check for sync | ||
| } | ||
| } | ||
| syncNotificationIfRequired() |
Contributor
There was a problem hiding this comment.
Is the comment at right place there ?
Should we remove the syncNotificationIfRequired() call from this method itself given your comment ?
Contributor
Author
There was a problem hiding this comment.
Sorry, that comment was mistakenly there, removed now 6499668
| public static final String NOTIFICATION_CHANNEL_SERVER_COMMUNICATIONS_ID = "notification-channel-server-communications"; | ||
| public static final String NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID = "notification-channel-push-notifications"; | ||
| public static final String OLD_NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID = "notification-channel-push-notifications"; | ||
| public static final String NEW_HIGH_PRIORITY_NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID = "notification-channel-high-priority-push-notifications"; |
Contributor
There was a problem hiding this comment.
nit: can we rename this to - NOTIFICATION_CHANNEL_GENERAL_PUSH_NOTIFICATIONS_ID to signify that it's a catch all for all push notifications and also be consistent with other variables here.
OrangeAndGreen
approved these changes
Jan 26, 2026
shubham1g5
approved these changes
Jan 26, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Product Description
It doesn't have visible changes on the UI, but the user will be able to see now push notifications whenever sent through the
Send Messagebutton of the opportunity page from web.Technical Summary
https://dimagi.atlassian.net/browse/CI-467
Send Messagefunctionality on Opportunity UI is currently broken on mobile as mobile doesn’t show any notification for it. If the application receives any such generic push notification, it was not able to show due to a bug in the push notification sync manager, which was always returning true, which means that the push notification will be shown after signalling, but there was no signalling present, and that being said, the push notification was never shown on the device tray.Along with the mentioned bug, following things were also changed
After calling the push notification retrieve API, the application was calling the acknowledge API even if the acknowledge list was empty. This change will save the unnecessary call to the server.
The default priority to show push notifications was set to HIGH but is now set to MAX so that push notifications will show interruptively. This will work for devices running Android 7.1 (API level 25) or lower,.
Changed the importance level to HIGH. The importance level cannot be changed once a channel already exists, so delete this channel and create another with a different channel ID. The reason for doing this is that programmatically changing the importance level doesn't work, even by deleting and recreating it with the same ID. The importance level is only modifiable before the channel is submitted to NotificationManager. createNotificationChannel(NotificationChannel).
Safety story
I have tested the above changes with
Send Messageof opportunity page UI from webQA Plan
QA should test general push notifications
Labels and Review