From aacf52af44481e79b1545ad7e6df05aef8ce44aa Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Thu, 12 Feb 2026 21:22:48 +0530 Subject: [PATCH] notif: On Android, use org name instead of URL as notif group summary Zulip Server 8 (FL 234) started sending realmName in the FCM payload: https://github.com/zulip/zulip/commit/a018f2611 So, we try to use the realm name from the account first, then fallback to notification data, and then finally fallback to displaying the realm URL. Fixes: #570 --- lib/api/notifications.dart | 6 ++ lib/api/notifications.g.dart | 4 + lib/notifications/display.dart | 5 +- test/api/notifications_test.dart | 8 ++ test/notifications/display_test.dart | 131 ++++++++++++++++++++++++++- 5 files changed, 149 insertions(+), 5 deletions(-) diff --git a/lib/api/notifications.dart b/lib/api/notifications.dart index 95068f7834..1dae8bbe93 100644 --- a/lib/api/notifications.dart +++ b/lib/api/notifications.dart @@ -67,6 +67,9 @@ sealed class FcmMessageWithIdentity extends FcmMessage { @JsonKey(readValue: _readRealmUrl) // TODO(server-9) final Uri realmUrl; + /// The realm's name. + final String? realmName; // TODO(server-8) + /// This user's ID within the server. /// /// Useful mainly in the case where the user has multiple accounts in the @@ -76,6 +79,7 @@ sealed class FcmMessageWithIdentity extends FcmMessage { FcmMessageWithIdentity({ required this.realmUrl, + required this.realmName, required this.userId, }); @@ -120,6 +124,7 @@ class MessageFcmMessage extends FcmMessageWithIdentity { MessageFcmMessage({ required super.realmUrl, + required super.realmName, required super.userId, required this.senderId, required this.senderAvatarUrl, @@ -250,6 +255,7 @@ class RemoveFcmMessage extends FcmMessageWithIdentity { RemoveFcmMessage({ required super.realmUrl, + required super.realmName, required super.userId, required this.messageIds, }); diff --git a/lib/api/notifications.g.dart b/lib/api/notifications.g.dart index 7f01b3c420..43e08dc4e8 100644 --- a/lib/api/notifications.g.dart +++ b/lib/api/notifications.g.dart @@ -13,6 +13,7 @@ MessageFcmMessage _$MessageFcmMessageFromJson(Map json) => realmUrl: Uri.parse( FcmMessageWithIdentity._readRealmUrl(json, 'realm_url') as String, ), + realmName: json['realm_name'] as String?, userId: (_readIntOrString(json, 'user_id') as num).toInt(), senderId: (_readIntOrString(json, 'sender_id') as num).toInt(), senderAvatarUrl: Uri.parse(json['sender_avatar_url'] as String), @@ -29,6 +30,7 @@ MessageFcmMessage _$MessageFcmMessageFromJson(Map json) => Map _$MessageFcmMessageToJson(MessageFcmMessage instance) => { 'realm_url': instance.realmUrl.toString(), + 'realm_name': instance.realmName, 'user_id': instance.userId, 'type': instance.type, 'sender_id': instance.senderId, @@ -56,6 +58,7 @@ RemoveFcmMessage _$RemoveFcmMessageFromJson(Map json) => realmUrl: Uri.parse( FcmMessageWithIdentity._readRealmUrl(json, 'realm_url') as String, ), + realmName: json['realm_name'] as String?, userId: (_readIntOrString(json, 'user_id') as num).toInt(), messageIds: (RemoveFcmMessage._readMessageIds(json, 'message_ids') @@ -67,6 +70,7 @@ RemoveFcmMessage _$RemoveFcmMessageFromJson(Map json) => Map _$RemoveFcmMessageToJson(RemoveFcmMessage instance) => { 'realm_url': instance.realmUrl.toString(), + 'realm_name': instance.realmName, 'user_id': instance.userId, 'type': instance.type, 'message_ids': instance.messageIds, diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index e78e89f0cf..1625fbbb5e 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -353,8 +353,9 @@ class NotificationDisplayManager { // 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.realmUrl.toString()), + summaryText: account.realmName + ?? data.realmName + ?? data.realmUrl.toString()), // On Android 11 and lower, if autoCancel is not specified, // the summary notification may linger even after all child diff --git a/test/api/notifications_test.dart b/test/api/notifications_test.dart index 373e3eabc4..f01c5e7ffc 100644 --- a/test/api/notifications_test.dart +++ b/test/api/notifications_test.dart @@ -8,6 +8,7 @@ import '../stdlib_checks.dart'; void main() { final baseBaseJson = { "realm_url": "https://zulip.example.com/", + "realm_name": "Example Organization", "user_id": 234, }; @@ -19,6 +20,7 @@ void main() { "realm_id": "4", "realm_uri": "https://zulip.example.com/", // TODO(server-9) "realm_url": "https://zulip.example.com/", + "realm_name": "Example Organization", "user_id": "234", }; @@ -115,6 +117,7 @@ void main() { check(parse(streamJson)) ..realmUrl.equals(Uri.parse(baseJson['realm_url'] as String)) ..realmUrl.equals(Uri.parse(baseJsonPreE2ee['realm_uri'] as String)) // TODO(server-9) + ..realmName.equals(baseBaseJson['realm_name'] as String) ..userId.equals(234) ..senderId.equals(123) ..senderAvatarUrl.equals(Uri.parse(streamJson['sender_avatar_url'] as String)) @@ -137,6 +140,9 @@ void main() { }); test('optional fields missing cause no error', () { + check(parse({ ...streamJson }..remove('realm_name'))) + .realmName.isNull(); + check(parse({ ...streamJson }..remove('channel_name'))) .recipient.isA().which((it) => it ..channelId.equals(42) @@ -260,6 +266,7 @@ void main() { check(parse(baseJson)) ..realmUrl.equals(Uri.parse(baseJson['realm_url'] as String)) ..realmUrl.equals(Uri.parse(preE2eeJson['realm_uri'] as String)) // TODO(server-9) + ..realmName.equals(baseJson['realm_name'] as String) ..userId.equals(234) ..messageIds.deepEquals([123, 234]); }); @@ -318,6 +325,7 @@ extension UnexpectedFcmMessageChecks on Subject { extension FcmMessageWithIdentityChecks on Subject { Subject get realmUrl => has((x) => x.realmUrl, 'realmUrl'); + Subject get realmName => has((x) => x.realmName, 'realmName'); Subject get userId => has((x) => x.userId, 'userId'); } diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index ea596cde72..7bad05c03d 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -24,14 +24,18 @@ import 'package:zulip/widgets/theme.dart'; import '../example_data.dart' as eg; import '../fake_async.dart'; import '../model/binding.dart'; +import '../model/store_checks.dart'; import '../test_images.dart'; +import '../api/notifications_test.dart'; MessageFcmMessage messageFcmMessage( Message zulipMessage, { String? streamName, + String? realmName, Account? account, }) { account ??= eg.selfAccount; + realmName ??= account.realmName; final narrow = SendableNarrow.ofMessage(zulipMessage, selfUserId: account.userId); return FcmMessage.fromJson({ "event": "message", @@ -40,6 +44,7 @@ MessageFcmMessage messageFcmMessage( "realm_id": "4", "realm_uri": account.realmUrl.toString(), "user_id": account.userId.toString(), + if (realmName != null) "realm_name": realmName, "zulip_message_id": zulipMessage.id.toString(), "time": zulipMessage.timestamp.toString(), @@ -343,13 +348,19 @@ void main() { }); group('NotificationDisplayManager show', () { - void checkNotification(MessageFcmMessage data, { + void checkNotification( + MessageFcmMessage data, { + Account? account, required List messageStyleMessages, required String expectedTitle, required String expectedTagComponent, required bool expectedIsGroupConversation, List? expectedIconBitmap = kSolidBlueAvatar, + String? expectedSummaryText, }) { + account ??= eg.selfAccount; + assert(account.userId == data.userId + && account.realmUrl == data.realmUrl); assert(messageStyleMessages.every((e) => e.userId == data.userId)); assert(messageStyleMessages.every((e) => e.realmUrl == data.realmUrl)); @@ -367,6 +378,9 @@ void main() { FcmMessageDmRecipient(:var allRecipientIds) => DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), }).buildAndroidNotificationUrl(); + expectedSummaryText ??= account.realmName + ?? data.realmName + ?? data.realmUrl.toString(); final messageStyleMessagesChecks = messageStyleMessages.mapIndexed((i, messageData) { @@ -429,13 +443,16 @@ void main() { ..groupKey.equals(expectedGroupKey) ..isGroupSummary.equals(true) ..inboxStyle.which((it) => it.isNotNull() - ..summaryText.equals(data.realmUrl.toString())) + ..summaryText.equals(expectedSummaryText!)) ..autoCancel.equals(true) ..contentIntent.isNull(), ]); } - Future checkNotifications(FakeAsync async, MessageFcmMessage data, { + Future checkNotifications( + FakeAsync async, + MessageFcmMessage data, { + Account? account, required String expectedTitle, required String expectedTagComponent, required bool expectedIsGroupConversation, @@ -448,6 +465,7 @@ void main() { RemoteMessage(data: data.toJson())); async.flushMicrotasks(); checkNotification(data, + account: account, messageStyleMessages: [data], expectedIsGroupConversation: expectedIsGroupConversation, expectedTitle: expectedTitle, @@ -458,6 +476,7 @@ void main() { RemoteMessage(data: data.toJson())); async.flushMicrotasks(); checkNotification(data, + account: account, messageStyleMessages: [data], expectedIsGroupConversation: expectedIsGroupConversation, expectedTitle: expectedTitle, @@ -634,6 +653,112 @@ void main() { expectedTagComponent: 'stream:${message.streamId}:${message.topic}'); }))); + test('stream message: different realms', () => runWithHttpClient(() => awaitFakeAsync((async) async { + await init(addSelfAccount: false); + + final account1 = eg.account( + id: 1001, + user: eg.user(), + realmUrl: Uri.parse('http://realm1.example'), + realmName: 'Realm 1'); + await testBinding.globalStore.add(account1, eg.initialSnapshot()); + final account2 = eg.account( + id: 1002, + user: eg.user(), + realmUrl: Uri.parse('http://realm2.example'), + realmName: 'Realm 2'); + await testBinding.globalStore.add(account2, eg.initialSnapshot()); + + final stream = eg.stream(); + final topic = 'test topic'; + final data1 = messageFcmMessage( + eg.streamMessage(stream: stream, topic: topic), + account: account1, streamName: stream.name); + final data2 = messageFcmMessage( + eg.streamMessage(stream: stream, topic: topic), + account: account2, streamName: stream.name); + + receiveFcmMessage(async, data1); + checkNotification(data1, + account: account1, + messageStyleMessages: [data1], + expectedIsGroupConversation: true, + expectedTitle: '#${stream.name} > $topic', + expectedTagComponent: 'stream:${stream.streamId}:$topic', + expectedSummaryText: account1.realmName); + + receiveFcmMessage(async, data2); + checkNotification(data2, + account: account2, + messageStyleMessages: [data2], + expectedIsGroupConversation: true, + expectedTitle: '#${stream.name} > $topic', + expectedTagComponent: 'stream:${stream.streamId}:$topic', + expectedSummaryText: account2.realmName); + }))); + + test('stream message: realm name absent in account, falls back to notif data', () => runWithHttpClient(() => awaitFakeAsync((async) async { + await init(addSelfAccount: false); + + var account = eg.account( + id: 1001, + user: eg.user(), + realmUrl: Uri.parse('http://realm1.example')); + await testBinding.globalStore.add(account, eg.initialSnapshot()); + // Override the default realmName from eg.account(). + account = await testBinding.globalStore.updateAccount(account.id, + AccountsCompanion(realmName: const Value(null))); + check(account).realmName.isNull(); + + final stream = eg.stream(); + final topic = 'test topic'; + final data = messageFcmMessage( + eg.streamMessage(stream: stream, topic: topic), + account: account, + streamName: stream.name, + realmName: 'Notif realm name'); + check(data).realmName.equals('Notif realm name'); + + receiveFcmMessage(async, data); + checkNotification(data, + account: account, + messageStyleMessages: [data], + expectedIsGroupConversation: true, + expectedTitle: '#${stream.name} > $topic', + expectedTagComponent: 'stream:${stream.streamId}:$topic', + expectedSummaryText: 'Notif realm name'); + }))); + + test('stream message: realm name absent in account and notif data, falls back to realm URL', () => runWithHttpClient(() => awaitFakeAsync((async) async { + await init(addSelfAccount: false); + + var account = eg.account( + id: 1001, + user: eg.user(), + realmUrl: Uri.parse('http://realm1.example')); + await testBinding.globalStore.add(account, eg.initialSnapshot()); + // Override the default realmName from eg.account(). + account = await testBinding.globalStore.updateAccount(account.id, + AccountsCompanion(realmName: const Value(null))); + check(account).realmName.isNull(); + + final stream = eg.stream(); + final topic = 'test topic'; + final data = messageFcmMessage( + eg.streamMessage(stream: stream, topic: topic), + account: account, streamName: stream.name); + check(data).realmName.isNull(); + + receiveFcmMessage(async, data); + checkNotification(data, + account: account, + messageStyleMessages: [data], + expectedIsGroupConversation: true, + expectedTitle: '#${stream.name} > $topic', + expectedTagComponent: 'stream:${stream.streamId}:$topic', + expectedSummaryText: account.realmUrl.toString()); + }))); + test('group DM: 3 users', () => runWithHttpClient(() => awaitFakeAsync((async) async { await init(); final message = eg.dmMessage(from: eg.thirdUser, to: [eg.otherUser, eg.selfUser]);