From 499573ccc2dfb719e9fdd46f3a6ed3bf9101ab87 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 31 May 2024 14:39:24 +0530 Subject: [PATCH 1/4] notif: Add support group summary notifications to Pigeon bindings This adds support for creating a group summary notification: https://developer.android.com/develop/ui/views/notifications/group#group-summary --- .../com/zulip/flutter/Notifications.g.kt | 56 ++++++++++++++++--- .../kotlin/com/zulip/flutter/ZulipPlugin.kt | 11 ++++ lib/host/android_notifications.g.dart | 33 ++++++++++- pigeon/notifications.dart | 13 +++++ test/model/binding.dart | 12 ++++ 5 files changed, 114 insertions(+), 11 deletions(-) diff --git a/android/app/src/main/kotlin/com/zulip/flutter/Notifications.g.kt b/android/app/src/main/kotlin/com/zulip/flutter/Notifications.g.kt index 3cca82790c..0e0b640c9c 100644 --- a/android/app/src/main/kotlin/com/zulip/flutter/Notifications.g.kt +++ b/android/app/src/main/kotlin/com/zulip/flutter/Notifications.g.kt @@ -82,6 +82,31 @@ data class PendingIntent ( ) } } + +/** + * Corresponds to `androidx.core.app.NotificationCompat.InboxStyle` + * + * See: https://developer.android.com/reference/androidx/core/app/NotificationCompat.InboxStyle + * + * Generated class from Pigeon that represents data sent in messages. + */ +data class InboxStyle ( + val summaryText: String + +) { + companion object { + @Suppress("LocalVariableName") + fun fromList(__pigeon_list: List): InboxStyle { + val summaryText = __pigeon_list[0] as String + return InboxStyle(summaryText) + } + } + fun toList(): List { + return listOf( + summaryText, + ) + } +} private object NotificationsPigeonCodec : StandardMessageCodec() { override fun readValueOfType(type: Byte, buffer: ByteBuffer): Any? { return when (type) { @@ -90,6 +115,11 @@ private object NotificationsPigeonCodec : StandardMessageCodec() { PendingIntent.fromList(it) } } + 130.toByte() -> { + return (readValue(buffer) as? List)?.let { + InboxStyle.fromList(it) + } + } else -> super.readValueOfType(type, buffer) } } @@ -99,6 +129,10 @@ private object NotificationsPigeonCodec : StandardMessageCodec() { stream.write(129) writeValue(stream, value.toList()) } + is InboxStyle -> { + stream.write(130) + writeValue(stream, value.toList()) + } else -> super.writeValue(stream, value) } } @@ -125,7 +159,7 @@ interface AndroidNotificationHostApi { * https://developer.android.com/reference/kotlin/android/app/NotificationManager.html#notify * https://developer.android.com/reference/androidx/core/app/NotificationCompat.Builder */ - fun notify(tag: String?, id: Long, channelId: String, color: Long?, contentIntent: PendingIntent?, contentText: String?, contentTitle: String?, extras: Map?, smallIconResourceName: String?) + fun notify(tag: String?, id: Long, autoCancel: Boolean?, channelId: String, color: Long?, contentIntent: PendingIntent?, contentText: String?, contentTitle: String?, extras: Map?, groupKey: String?, inboxStyle: InboxStyle?, isGroupSummary: Boolean?, smallIconResourceName: String?) companion object { /** The codec used by AndroidNotificationHostApi. */ @@ -143,15 +177,19 @@ interface AndroidNotificationHostApi { val args = message as List val tagArg = args[0] as String? val idArg = args[1].let { num -> if (num is Int) num.toLong() else num as Long } - val channelIdArg = args[2] as String - val colorArg = args[3].let { num -> if (num is Int) num.toLong() else num as Long? } - val contentIntentArg = args[4] as PendingIntent? - val contentTextArg = args[5] as String? - val contentTitleArg = args[6] as String? - val extrasArg = args[7] as Map? - val smallIconResourceNameArg = args[8] as String? + val autoCancelArg = args[2] as Boolean? + val channelIdArg = args[3] as String + val colorArg = args[4].let { num -> if (num is Int) num.toLong() else num as Long? } + val contentIntentArg = args[5] as PendingIntent? + val contentTextArg = args[6] as String? + val contentTitleArg = args[7] as String? + val extrasArg = args[8] as Map? + val groupKeyArg = args[9] as String? + val inboxStyleArg = args[10] as InboxStyle? + val isGroupSummaryArg = args[11] as Boolean? + val smallIconResourceNameArg = args[12] as String? val wrapped: List = try { - api.notify(tagArg, idArg, channelIdArg, colorArg, contentIntentArg, contentTextArg, contentTitleArg, extrasArg, smallIconResourceNameArg) + api.notify(tagArg, idArg, autoCancelArg, channelIdArg, colorArg, contentIntentArg, contentTextArg, contentTitleArg, extrasArg, groupKeyArg, inboxStyleArg, isGroupSummaryArg, smallIconResourceNameArg) listOf(null) } catch (exception: Throwable) { wrapError(exception) diff --git a/android/app/src/main/kotlin/com/zulip/flutter/ZulipPlugin.kt b/android/app/src/main/kotlin/com/zulip/flutter/ZulipPlugin.kt index 313c402506..03db2333fd 100644 --- a/android/app/src/main/kotlin/com/zulip/flutter/ZulipPlugin.kt +++ b/android/app/src/main/kotlin/com/zulip/flutter/ZulipPlugin.kt @@ -23,15 +23,20 @@ private class AndroidNotificationHost(val context: Context) override fun notify( tag: String?, id: Long, + autoCancel: Boolean?, channelId: String, color: Long?, contentIntent: PendingIntent?, contentText: String?, contentTitle: String?, extras: Map?, + groupKey: String?, + inboxStyle: InboxStyle?, + isGroupSummary: Boolean?, smallIconResourceName: String? ) { val notification = NotificationCompat.Builder(context, channelId).apply { + autoCancel?.let { setAutoCancel(it) } color?.let { setColor(it.toInt()) } contentIntent?.let { setContentIntent( android.app.PendingIntent.getActivity(context, @@ -49,6 +54,12 @@ private class AndroidNotificationHost(val context: Context) contentTitle?.let { setContentTitle(it) } extras?.let { setExtras( Bundle().apply { it.forEach { (k, v) -> putString(k, v) } } ) } + groupKey?.let { setGroup(it) } + inboxStyle?.let { setStyle( + NotificationCompat.InboxStyle() + .setSummaryText(it.summaryText) + ) } + isGroupSummary?.let { setGroupSummary(it) } smallIconResourceName?.let { setSmallIcon(context.resources.getIdentifier( it, "drawable", context.packageName)) } }.build() diff --git a/lib/host/android_notifications.g.dart b/lib/host/android_notifications.g.dart index 4337aa6b4b..ea49034a94 100644 --- a/lib/host/android_notifications.g.dart +++ b/lib/host/android_notifications.g.dart @@ -53,6 +53,30 @@ class PendingIntent { } } +/// Corresponds to `androidx.core.app.NotificationCompat.InboxStyle` +/// +/// See: https://developer.android.com/reference/androidx/core/app/NotificationCompat.InboxStyle +class InboxStyle { + InboxStyle({ + required this.summaryText, + }); + + String summaryText; + + Object encode() { + return [ + summaryText, + ]; + } + + static InboxStyle decode(Object result) { + result as List; + return InboxStyle( + summaryText: result[0]! as String, + ); + } +} + class _PigeonCodec extends StandardMessageCodec { const _PigeonCodec(); @@ -61,6 +85,9 @@ class _PigeonCodec extends StandardMessageCodec { if (value is PendingIntent) { buffer.putUint8(129); writeValue(buffer, value.encode()); + } else if (value is InboxStyle) { + buffer.putUint8(130); + writeValue(buffer, value.encode()); } else { super.writeValue(buffer, value); } @@ -71,6 +98,8 @@ class _PigeonCodec extends StandardMessageCodec { switch (type) { case 129: return PendingIntent.decode(readValue(buffer)!); + case 130: + return InboxStyle.decode(readValue(buffer)!); default: return super.readValueOfType(type, buffer); } @@ -107,7 +136,7 @@ class AndroidNotificationHostApi { /// See: /// https://developer.android.com/reference/kotlin/android/app/NotificationManager.html#notify /// https://developer.android.com/reference/androidx/core/app/NotificationCompat.Builder - Future notify({String? tag, required int id, required String channelId, int? color, PendingIntent? contentIntent, String? contentText, String? contentTitle, Map? extras, String? smallIconResourceName,}) async { + Future notify({String? tag, required int id, bool? autoCancel, required String channelId, int? color, PendingIntent? contentIntent, String? contentText, String? contentTitle, Map? extras, String? groupKey, InboxStyle? inboxStyle, bool? isGroupSummary, String? smallIconResourceName,}) async { final String __pigeon_channelName = 'dev.flutter.pigeon.zulip.AndroidNotificationHostApi.notify$__pigeon_messageChannelSuffix'; final BasicMessageChannel __pigeon_channel = BasicMessageChannel( __pigeon_channelName, @@ -115,7 +144,7 @@ class AndroidNotificationHostApi { binaryMessenger: __pigeon_binaryMessenger, ); final List? __pigeon_replyList = - await __pigeon_channel.send([tag, id, channelId, color, contentIntent, contentText, contentTitle, extras, smallIconResourceName]) as List?; + await __pigeon_channel.send([tag, id, autoCancel, channelId, color, contentIntent, contentText, contentTitle, extras, groupKey, inboxStyle, isGroupSummary, smallIconResourceName]) as List?; if (__pigeon_replyList == null) { throw _createConnectionError(__pigeon_channelName); } else if (__pigeon_replyList.length > 1) { diff --git a/pigeon/notifications.dart b/pigeon/notifications.dart index 4af904da56..2a362bba82 100644 --- a/pigeon/notifications.dart +++ b/pigeon/notifications.dart @@ -27,6 +27,15 @@ class PendingIntent { final int flags; } +/// Corresponds to `androidx.core.app.NotificationCompat.InboxStyle` +/// +/// See: https://developer.android.com/reference/androidx/core/app/NotificationCompat.InboxStyle +class InboxStyle { + InboxStyle({required this.summaryText}); + + final String summaryText; +} + @HostApi() abstract class AndroidNotificationHostApi { /// Corresponds to `android.app.NotificationManager.notify`, @@ -54,12 +63,16 @@ abstract class AndroidNotificationHostApi { required int id, // The remaining arguments go to method calls on NotificationCompat.Builder. + bool? autoCancel, required String channelId, int? color, PendingIntent? contentIntent, String? contentText, String? contentTitle, Map? extras, + String? groupKey, + InboxStyle? inboxStyle, + bool? isGroupSummary, String? smallIconResourceName, // NotificationCompat.Builder has lots more methods; add as needed. // Keep them alphabetized, for easy comparison with that class's docs. diff --git a/test/model/binding.dart b/test/model/binding.dart index 1a561da92d..9f88847e0f 100644 --- a/test/model/binding.dart +++ b/test/model/binding.dart @@ -496,23 +496,31 @@ class FakeAndroidNotificationHostApi implements AndroidNotificationHostApi { Future notify({ String? tag, required int id, + bool? autoCancel, required String channelId, int? color, PendingIntent? contentIntent, String? contentText, String? contentTitle, Map? extras, + String? groupKey, + InboxStyle? inboxStyle, + bool? isGroupSummary, String? smallIconResourceName, }) async { _notifyCalls.add(( tag: tag, id: id, + autoCancel: autoCancel, channelId: channelId, color: color, contentIntent: contentIntent, contentText: contentText, contentTitle: contentTitle, extras: extras, + groupKey: groupKey, + inboxStyle: inboxStyle, + isGroupSummary: isGroupSummary, smallIconResourceName: smallIconResourceName, )); } @@ -521,11 +529,15 @@ class FakeAndroidNotificationHostApi implements AndroidNotificationHostApi { typedef AndroidNotificationHostApiNotifyCall = ({ String? tag, int id, + bool? autoCancel, String channelId, int? color, PendingIntent? contentIntent, String? contentText, String? contentTitle, Map? extras, + String? groupKey, + InboxStyle? inboxStyle, + bool? isGroupSummary, String? smallIconResourceName, }); From c665d6e738a26f2a223c2c91f28a0f3dd893e0cf Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 31 May 2024 16:05:30 +0530 Subject: [PATCH 2/4] notif: Create group summary notification Make use of Group Summary Notifications to group notifications based on different realms and also display the respective group label (which is currently the realm URL). See: https://developer.android.com/develop/ui/views/notifications/group#group-summary This change is a port of implementation in zulip-mobile: https://github.com/zulip/zulip-mobile/blob/6d5d56d175644cd0cdf47f3cd30ffadf6756bbdc/android/app/src/main/java/com/zulipmobile/notifications/NotificationUiManager.kt#L299-L382 Fixes: #569 Fixes: #571 --- lib/notifications/display.dart | 27 ++++++++++--- test/notifications/display_test.dart | 60 +++++++++++++++++++++------- 2 files changed, 68 insertions(+), 19 deletions(-) diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index d89312501e..f23d4ab451 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -89,7 +89,7 @@ class NotificationDisplayManager { } } - static void _onMessageFcmMessage(MessageFcmMessage data, Map dataJson) { + static Future _onMessageFcmMessage(MessageFcmMessage data, Map dataJson) async { assert(debugLog('notif message content: ${data.content}')); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; final title = switch (data.recipient) { @@ -103,13 +103,16 @@ class NotificationDisplayManager { FcmMessageDmRecipient() => data.senderFullName, }; - final conversationKey = _conversationKey(data); - ZulipBinding.instance.androidNotificationHost.notify( + final groupKey = _groupKey(data); + final conversationKey = _conversationKey(data, groupKey); + + await ZulipBinding.instance.androidNotificationHost.notify( // TODO the notification ID can be constant, instead of matching requestCode // (This is a legacy of `flutter_local_notifications`.) id: notificationIdAsHashOf(conversationKey), tag: conversationKey, channelId: NotificationChannelManager.kChannelId, + groupKey: groupKey, contentTitle: title, contentText: data.content, @@ -140,6 +143,21 @@ class NotificationDisplayManager { // (This is a legacy of `flutter_local_notifications`.) ), ); + + await ZulipBinding.instance.androidNotificationHost.notify( + id: notificationIdAsHashOf(groupKey), + tag: groupKey, + channelId: NotificationChannelManager.kChannelId, + groupKey: groupKey, + isGroupSummary: true, + + color: kZulipBrandColor.value, + // TODO vary notification icon for debug + smallIconResourceName: 'zulip_notification', // This name must appear in keep.xml too: https://github.com/zulip/zulip-flutter/issues/528 + inboxStyle: InboxStyle( + // TODO(#570) Show organization name, not URL + summaryText: data.realmUri.toString()), + ); } /// A notification ID, derived as a hash of the given string key. @@ -157,8 +175,7 @@ class NotificationDisplayManager { | ((bytes[3] & 0x7f) << 24); } - static String _conversationKey(MessageFcmMessage data) { - final groupKey = _groupKey(data); + static String _conversationKey(MessageFcmMessage data, String groupKey) { final conversation = switch (data.recipient) { FcmMessageStreamRecipient(:var streamId, :var topic) => 'stream:$streamId:$topic', FcmMessageDmRecipient(:var allRecipientIds) => 'dm:${allRecipientIds.join(',')}', diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index 6502ffe0fb..81a1de360a 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -111,24 +111,48 @@ void main() { required String expectedTagComponent, }) { final expectedTag = '${data.realmUri}|${data.userId}|$expectedTagComponent'; + final expectedGroupKey = '${data.realmUri}|${data.userId}'; final expectedId = NotificationDisplayManager.notificationIdAsHashOf(expectedTag); const expectedIntentFlags = PendingIntentFlag.immutable | PendingIntentFlag.updateCurrent; - check(testBinding.androidNotificationHost.takeNotifyCalls()).single - ..id.equals(expectedId) - ..tag.equals(expectedTag) - ..channelId.equals(NotificationChannelManager.kChannelId) - ..contentTitle.equals(expectedTitle) - ..contentText.equals(data.content) - ..color.equals(kZulipBrandColor.value) - ..smallIconResourceName.equals('zulip_notification') - ..extras.isNull() - ..contentIntent.which((it) => it.isNotNull() - ..requestCode.equals(expectedId) - ..flags.equals(expectedIntentFlags) - ..intentPayload.equals(jsonEncode(data.toJson())) - ); + + check(testBinding.androidNotificationHost.takeNotifyCalls()) + ..length.equals(2) + ..containsInOrder(>[ + (it) => it + ..id.equals(expectedId) + ..tag.equals(expectedTag) + ..channelId.equals(NotificationChannelManager.kChannelId) + ..contentTitle.equals(expectedTitle) + ..contentText.equals(data.content) + ..color.equals(kZulipBrandColor.value) + ..smallIconResourceName.equals('zulip_notification') + ..extras.isNull() + ..groupKey.equals(expectedGroupKey) + ..isGroupSummary.isNull() + ..inboxStyle.isNull() + ..autoCancel.isNull() + ..contentIntent.which((it) => it.isNotNull() + ..requestCode.equals(expectedId) + ..flags.equals(expectedIntentFlags) + ..intentPayload.equals(jsonEncode(data.toJson()))), + (it) => it + ..id.equals(NotificationDisplayManager.notificationIdAsHashOf(expectedGroupKey)) + ..tag.equals(expectedGroupKey) + ..channelId.equals(NotificationChannelManager.kChannelId) + ..contentTitle.isNull() + ..contentText.isNull() + ..color.equals(kZulipBrandColor.value) + ..smallIconResourceName.equals('zulip_notification') + ..extras.isNull() + ..groupKey.equals(expectedGroupKey) + ..isGroupSummary.equals(true) + ..inboxStyle.which((it) => it.isNotNull() + ..summaryText.equals(data.realmUri.toString())) + ..autoCancel.isNull() + ..contentIntent.isNull() + ]); } Future checkNotifications(FakeAsync async, MessageFcmMessage data, { @@ -369,12 +393,16 @@ extension AndroidNotificationChannelChecks on Subject { Subject get tag => has((x) => x.tag, 'tag'); Subject get id => has((x) => x.id, 'id'); + Subject get autoCancel => has((x) => x.autoCancel, 'autoCancel'); Subject get channelId => has((x) => x.channelId, 'channelId'); Subject get color => has((x) => x.color, 'color'); Subject get contentIntent => has((x) => x.contentIntent, 'contentIntent'); Subject get contentText => has((x) => x.contentText, 'contentText'); Subject get contentTitle => has((x) => x.contentTitle, 'contentTitle'); Subject?> get extras => has((x) => x.extras, 'extras'); + Subject get groupKey => has((x) => x.groupKey, 'groupKey'); + Subject get inboxStyle => has((x) => x.inboxStyle, 'inboxStyle'); + Subject get isGroupSummary => has((x) => x.isGroupSummary, 'isGroupSummary'); Subject get smallIconResourceName => has((x) => x.smallIconResourceName, 'smallIconResourceName'); } @@ -383,3 +411,7 @@ extension on Subject { Subject get intentPayload => has((x) => x.intentPayload, 'intentPayload'); Subject get flags => has((x) => x.flags, 'flags'); } + +extension on Subject { + Subject get summaryText => has((x) => x.summaryText, 'summaryText'); +} From c7ee2d32eacc0faab9ee3dd0d7772ee2c6092348 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 14 Jun 2024 21:06:24 +0530 Subject: [PATCH 3/4] notif: Enable autoCancel to dismiss notifications when tapped Set autoCancel flag for notifications, ensuring they automatically disappear from the panel when tapped. See: https://developer.android.com/reference/androidx/core/app/NotificationCompat.Builder#setAutoCancel(boolean) Also, zulip-mobile does this already: https://github.com/zulip/zulip-mobile/blob/e352f563ecf2fa9b09b688d5a65b6bc89b0358bc/android/app/src/main/java/com/zulipmobile/notifications/NotificationUiManager.kt#L353 --- lib/notifications/display.dart | 1 + test/notifications/display_test.dart | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index f23d4ab451..46561ae48d 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -142,6 +142,7 @@ class NotificationDisplayManager { // TODO this doesn't set the Intent flags we set in zulip-mobile; is that OK? // (This is a legacy of `flutter_local_notifications`.) ), + autoCancel: true, ); await ZulipBinding.instance.androidNotificationHost.notify( diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index 81a1de360a..a220be0ce0 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -132,7 +132,7 @@ void main() { ..groupKey.equals(expectedGroupKey) ..isGroupSummary.isNull() ..inboxStyle.isNull() - ..autoCancel.isNull() + ..autoCancel.equals(true) ..contentIntent.which((it) => it.isNotNull() ..requestCode.equals(expectedId) ..flags.equals(expectedIntentFlags) From 365f9da384405fc182f2fd6316d908e64fcf8081 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Fri, 31 May 2024 16:07:46 +0530 Subject: [PATCH 4/4] notif: Prefix stream names with `#` Fixes: #572 --- lib/notifications/display.dart | 4 ++-- test/notifications/display_test.dart | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index 46561ae48d..acbf15012a 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -94,9 +94,9 @@ class NotificationDisplayManager { final zulipLocalizations = GlobalLocalizations.zulipLocalizations; final title = switch (data.recipient) { FcmMessageStreamRecipient(:var streamName?, :var topic) => - '$streamName > $topic', + '#$streamName > $topic', FcmMessageStreamRecipient(:var topic) => - '(unknown channel) > $topic', // TODO get stream name from data + '#(unknown channel) > $topic', // TODO get stream name from data FcmMessageDmRecipient(:var allRecipientIds) when allRecipientIds.length > 2 => zulipLocalizations.notifGroupDmConversationLabel( data.senderFullName, allRecipientIds.length - 2), // TODO use others' names, from data diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index a220be0ce0..907ab3991f 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -181,7 +181,7 @@ void main() { final stream = eg.stream(); final message = eg.streamMessage(stream: stream); await checkNotifications(async, messageFcmMessage(message, streamName: stream.name), - expectedTitle: '${stream.name} > ${message.subject}', + expectedTitle: '#${stream.name} > ${message.subject}', expectedTagComponent: 'stream:${message.streamId}:${message.subject}'); })); @@ -190,7 +190,7 @@ void main() { final stream = eg.stream(); final message = eg.streamMessage(stream: stream); await checkNotifications(async, messageFcmMessage(message, streamName: null), - expectedTitle: '(unknown channel) > ${message.subject}', + expectedTitle: '#(unknown channel) > ${message.subject}', expectedTagComponent: 'stream:${message.streamId}:${message.subject}'); }));