Skip to content

Commit 3bbab90

Browse files
Jignesh-dimagiJignesh-dimagi
authored
Merge pull request #3512 from dimagi/jignesh/hotfix/ci-467
CI-467 - Send message bug solved
2 parents 508b1da + 6499668 commit 3bbab90

File tree

7 files changed

+83
-54
lines changed

7 files changed

+83
-54
lines changed

‎app/src/org/commcare/CommCareNoficationManager.java‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ public class CommCareNoficationManager {
3939
public static final String NOTIFICATION_CHANNEL_ERRORS_ID = "notification-channel-errors";
4040
public static final String NOTIFICATION_CHANNEL_USER_SESSION_ID = "notification-channel-user-session";
4141
public static final String NOTIFICATION_CHANNEL_SERVER_COMMUNICATIONS_ID = "notification-channel-server-communications";
42-
public static final String NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID = "notification-channel-push-notifications";
42+
public static final String OLD_NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID = "notification-channel-push-notifications";
43+
public static final String NOTIFICATION_CHANNEL_GENERAL_PUSH_NOTIFICATIONS_ID = "notification-channel-general-push-notifications";
4344
public static final String NOTIFICATION_CHANNEL_MESSAGING_ID = "notification-channel-messaging";
4445

4546
/**
@@ -191,15 +192,33 @@ public void createNotificationChannels() {
191192
R.string.notification_channel_server_communication_description,
192193
NotificationManager.IMPORTANCE_LOW);
193194

194-
createNotificationChannel(NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID,
195+
deleteIfOldPushNotificationExists();
196+
createNotificationChannel(NOTIFICATION_CHANNEL_GENERAL_PUSH_NOTIFICATIONS_ID,
195197
R.string.notification_channel_push_notfications_title,
196198
R.string.notification_channel_push_notfications_description,
197-
NotificationManager.IMPORTANCE_DEFAULT);
199+
NotificationManager.IMPORTANCE_HIGH);
198200

199201
createNotificationChannel(NOTIFICATION_CHANNEL_MESSAGING_ID,
200202
R.string.notification_channel_messaging_title,
201203
R.string.notification_channel_messaging_description,
202-
NotificationManager.IMPORTANCE_MAX);
204+
NotificationManager.IMPORTANCE_HIGH);
205+
}
206+
207+
/**
208+
* The importance level cannot be changed once a channel already exists,
209+
* so delete this channel and create another with a different channel ID.
210+
* Reason: Programmatically changing the importance level doesn't work,
211+
* even by deleting and recreating it with the same ID.
212+
* Importance level is only modifiable before the channel is submitted to
213+
* NotificationManager. createNotificationChannel(NotificationChannel).
214+
*/
215+
@TargetApi(Build.VERSION_CODES.O)
216+
private void deleteIfOldPushNotificationExists(){
217+
NotificationManager notificationManager = context.getSystemService(NotificationManager.class);
218+
NotificationChannel oldPushNotificationChannel = notificationManager.getNotificationChannel(OLD_NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID);
219+
if(oldPushNotificationChannel!=null){
220+
notificationManager.deleteNotificationChannel(OLD_NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID);
221+
}
203222
}
204223

205224
@TargetApi(Build.VERSION_CODES.O)

‎app/src/org/commcare/activities/connect/viewmodel/PushNotificationViewModel.kt‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import kotlinx.coroutines.launch
1212
import org.commcare.android.database.connect.models.PushNotificationRecord
1313
import org.commcare.connect.database.NotificationRecordDatabaseHelper
1414
import org.commcare.pn.workermanager.NotificationsSyncWorkerManager
15+
import org.commcare.utils.PushNotificationApiHelper
1516
import org.commcare.utils.PushNotificationApiHelper.retrieveLatestPushNotifications
1617

1718
class PushNotificationViewModel(
@@ -51,10 +52,10 @@ class PushNotificationViewModel(
5152
.onSuccess {
5253
val currentNotifications = _allNotifications.value.orEmpty()
5354
NotificationsSyncWorkerManager(
54-
application,
55-
it,
56-
false,
57-
).startPNApiSync()
55+
context = application,
56+
showNotification = false,
57+
syncNotification = false,
58+
).startSyncWorkers(PushNotificationApiHelper.convertPNRecordsToPayload(it))
5859
val updatedNotifications =
5960
(it + currentNotifications)
6061
.distinctBy { it.notificationId }

‎app/src/org/commcare/pn/workermanager/NotificationsSyncWorkerManager.kt‎

Lines changed: 47 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import androidx.work.OneTimeWorkRequestBuilder
1111
import androidx.work.PeriodicWorkRequest
1212
import androidx.work.WorkManager
1313
import com.google.gson.Gson
14-
import org.commcare.android.database.connect.models.PushNotificationRecord
1514
import org.commcare.connect.ConnectConstants.CCC_DEST_DELIVERY_PROGRESS
1615
import org.commcare.connect.ConnectConstants.CCC_DEST_LEARN_PROGRESS
1716
import org.commcare.connect.ConnectConstants.CCC_DEST_OPPORTUNITY_SUMMARY_PAGE
@@ -26,7 +25,6 @@ import org.commcare.pn.workers.NotificationsSyncWorker.Companion.ACTION
2625
import org.commcare.pn.workers.NotificationsSyncWorker.Companion.NOTIFICATION_PAYLOAD
2726
import org.commcare.pn.workers.NotificationsSyncWorker.Companion.SyncAction
2827
import org.commcare.utils.FirebaseMessagingUtil.cccCheckPassed
29-
import org.commcare.utils.PushNotificationApiHelper.convertPNRecordsToPayload
3028
import java.util.concurrent.TimeUnit
3129

3230
/**
@@ -112,91 +110,103 @@ class NotificationsSyncWorkerManager(
112110
delay: Long = 0,
113111
) {
114112
val notificationSyncWorkerManager =
115-
NotificationsSyncWorkerManager(context, null, false, true)
116-
notificationSyncWorkerManager.startPersonalIdNotificationsWorker(
117-
emptyMap(),
118-
false,
119-
delay,
120-
)
113+
NotificationsSyncWorkerManager(
114+
context,
115+
showNotification = false,
116+
syncNotification = true,
117+
)
118+
notificationSyncWorkerManager.startPersonalIdNotificationsWorker(emptyMap(), false, delay)
121119
}
122120
}
123121

124-
lateinit var notificationsPayload: ArrayList<Map<String, String>>
125-
126122
private var showNotification = false
127123
private var syncNotification = false
128-
129-
var signaling = false
124+
private var isNotificationSyncScheduled = false
130125

131126
/**
132127
* This can receive the push notification data payload from FCM and notification API.
128+
* @param context - android context
129+
* @param showNotification - decide whether to show a notification or not after sync is successful.
130+
* @param syncNotification - decide to pull / sync any pending notifications or not.
133131
*/
134132
constructor(
135133
context: Context,
136-
notificationsPayload: ArrayList<Map<String, String>>,
137134
showNotification: Boolean,
138-
syncNotification: Boolean = false,
135+
syncNotification: Boolean,
139136
) : this(context) {
140-
this.notificationsPayload = notificationsPayload
141-
this.showNotification = showNotification
142137
this.syncNotification = syncNotification
143-
}
144-
145-
constructor(
146-
context: Context,
147-
pnsRecords: List<PushNotificationRecord>?,
148-
showNotification: Boolean,
149-
syncNotification: Boolean = false,
150-
) : this(context) {
151-
this.notificationsPayload = convertPNRecordsToPayload(pnsRecords)
152138
this.showNotification = showNotification
153-
this.syncNotification = syncNotification
154139
}
155140

156-
/**
157-
* This method will start Api sync for received PNs either through FCM or notification API
158-
* @return whether a sync was scheduled as part of this call
159-
*/
160-
fun startPNApiSync(): Boolean = startSyncWorker()
161-
162-
private fun startSyncWorker(): Boolean {
163-
var isNotificationSyncScheduled = false
164-
for (notificationPayload in notificationsPayload) {
141+
private fun parseAndStartNotificationWorker(notificationPayload: Map<String, String>?): Boolean {
142+
var notificationHandled = false
143+
notificationPayload?.let {
165144
when (notificationPayload[REDIRECT_ACTION]) {
166145
CCC_MESSAGE -> {
167146
startPersonalIdNotificationsWorker(notificationPayload)
147+
notificationHandled = true
168148
isNotificationSyncScheduled = true
169149
}
170150

171151
CCC_DEST_PAYMENTS -> {
172152
startOpportunitiesSyncWorker(notificationPayload)
173153
startDeliverySyncWorker(notificationPayload)
154+
notificationHandled = true
174155
}
175156

176157
CCC_PAYMENT_INFO_CONFIRMATION -> {
177158
startOpportunitiesSyncWorker(notificationPayload)
159+
notificationHandled = true
178160
}
179161

180162
CCC_DEST_OPPORTUNITY_SUMMARY_PAGE -> {
181163
startOpportunitiesSyncWorker(notificationPayload)
164+
notificationHandled = true
182165
}
183166

184167
CCC_DEST_LEARN_PROGRESS -> {
185168
startOpportunitiesSyncWorker(notificationPayload)
186169
startLearningSyncWorker(notificationPayload)
170+
notificationHandled = true
187171
}
188172

189173
CCC_DEST_DELIVERY_PROGRESS -> {
190174
startOpportunitiesSyncWorker(notificationPayload)
191175
startDeliverySyncWorker(notificationPayload)
176+
notificationHandled = true
192177
}
193178
}
194179
}
180+
return notificationHandled
181+
}
182+
183+
/**
184+
* This method will start sync for received payload from FCM
185+
* @return whether the notification was handled in this call
186+
*/
187+
fun startSyncWorker(notificationPayload: Map<String, String>?): Boolean {
188+
val notificationHandled = parseAndStartNotificationWorker(notificationPayload)
189+
syncNotificationIfRequired()
190+
return notificationHandled
191+
}
192+
193+
/**
194+
* This method will start sync for received payloads from notification API
195+
*/
196+
fun startSyncWorkers(notificationsPayload: ArrayList<Map<String, String>>?) {
197+
notificationsPayload?.let {
198+
for (notificationPayload in notificationsPayload) {
199+
parseAndStartNotificationWorker(notificationPayload)
200+
}
201+
}
202+
syncNotificationIfRequired()
203+
}
204+
205+
// we want to get info on pending notifications irrespective of whether there are notification related FCMs or not
206+
private fun syncNotificationIfRequired() {
195207
if (syncNotification && !isNotificationSyncScheduled) {
196-
// we want to get info on pending notifications irrespective of whether there are notification related FCMs or not
197208
startPersonalIdNotificationsWorker(emptyMap(), false)
198209
}
199-
return signaling
200210
}
201211

202212
private fun startPersonalIdNotificationsWorker(
@@ -281,6 +291,5 @@ class NotificationsSyncWorkerManager(
281291
ExistingWorkPolicy.KEEP,
282292
syncWorkRequest,
283293
)
284-
signaling = true
285294
}
286295
}

‎app/src/org/commcare/services/CommCareFirebaseMessagingService.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,8 @@ public void onMessageReceived(RemoteMessage remoteMessage) {
5656
}
5757

5858
private Boolean startSyncForNotification(RemoteMessage remoteMessage) {
59-
ArrayList<Map<String, String>> pns = new ArrayList<>();
60-
pns.add(remoteMessage.getData());
61-
NotificationsSyncWorkerManager notificationsSyncWorkerManager = new NotificationsSyncWorkerManager(
62-
getApplicationContext(), pns, true, true);
63-
return notificationsSyncWorkerManager.startPNApiSync();
59+
return new NotificationsSyncWorkerManager(getApplicationContext(), true, true).
60+
startSyncWorker(remoteMessage.getData());
6461
}
6562

6663
@Override

‎app/src/org/commcare/services/FCMMessageData.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ public FCMMessageData(Map<String, String> payloadData) {
5959
notificationTitle = payloadData.get(NOTIFICATION_TITLE);
6060
notificationText = payloadData.get(NOTIFICATION_BODY);
6161
action = payloadData.get(REDIRECT_ACTION);
62-
priority = NotificationCompat.PRIORITY_HIGH;
63-
notificationChannel = CommCareNoficationManager.NOTIFICATION_CHANNEL_PUSH_NOTIFICATIONS_ID;
62+
priority = NotificationCompat.PRIORITY_MAX;
63+
notificationChannel = CommCareNoficationManager.NOTIFICATION_CHANNEL_GENERAL_PUSH_NOTIFICATIONS_ID;
6464
smallIcon = R.drawable.commcare_actionbar_logo;
6565
}
6666

‎app/src/org/commcare/utils/FirebaseMessagingUtil.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,6 @@ private static Intent handleGeneralApplicationPushNotification(Context context,
362362
private static Intent handleCCCMessageChannelPushNotification(Context context, FCMMessageData fcmMessageData, boolean showNotification) {
363363
Intent intent = null;
364364
fcmMessageData.setNotificationChannel(CommCareNoficationManager.NOTIFICATION_CHANNEL_MESSAGING_ID);
365-
fcmMessageData.setPriority(NotificationCompat.PRIORITY_MAX);
366365
fcmMessageData.setLargeIcon(BitmapFactory.decodeResource(context.getResources(), R.drawable.ic_connect_message_large));
367366

368367
boolean isMessage = fcmMessageData.getPayloadData().containsKey(ConnectMessagingMessageRecord.META_MESSAGE_ID);

‎app/src/org/commcare/utils/PushNotificationApiHelper.kt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ object PushNotificationApiHelper {
115115
context: Context,
116116
savedNotificationIds: List<String>,
117117
): Boolean {
118+
// don't call server unnecessarily if nothing to update
119+
if (savedNotificationIds.isEmpty()) {
120+
return true
121+
}
118122
val user = ConnectUserDatabaseUtil.getUser(context)
119123
return suspendCoroutine { continuation ->
120124
object : PersonalIdApiHandler<Boolean>() {

0 commit comments

Comments
 (0)