Skip to content

Commit cc3d250

Browse files
rajveermalviyagnprice
rajveermalviya
and
gnprice
committed
notif: Handle remove notification message
Fixes zulip#341 Co-authored-by: Greg Price <[email protected]>
1 parent 5f88f0a commit cc3d250

File tree

3 files changed

+238
-16
lines changed

3 files changed

+238
-16
lines changed

‎lib/notifications/display.dart

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class NotificationDisplayManager {
8484
static void onFcmMessage(FcmMessage data, Map<String, dynamic> dataJson) {
8585
switch (data) {
8686
case MessageFcmMessage(): _onMessageFcmMessage(data, dataJson);
87-
case RemoveFcmMessage(): break; // TODO(#341) handle
87+
case RemoveFcmMessage(): _onRemoveFcmMessage(data);
8888
case UnexpectedFcmMessage(): break; // TODO(log)
8989
}
9090
}
@@ -155,6 +155,9 @@ class NotificationDisplayManager {
155155

156156
messagingStyle: messagingStyle,
157157
number: messagingStyle.messages.length,
158+
extras: {
159+
kExtraZulipMessageId: data.zulipMessageId.toString(),
160+
},
158161

159162
contentIntent: PendingIntent(
160163
// TODO make intent URLs distinct, instead of requestCode
@@ -202,6 +205,55 @@ class NotificationDisplayManager {
202205
);
203206
}
204207

208+
static void _onRemoveFcmMessage(RemoveFcmMessage data) async {
209+
assert(debugLog('notif remove zulipMessageIds: ${data.zulipMessageIds}'));
210+
211+
final groupKey = _groupKey(data);
212+
213+
var haveRemaining = false;
214+
final activeNotifications = await ZulipBinding.instance.androidNotificationHost
215+
.getActiveNotifications(desiredExtras: [kExtraZulipMessageId]);
216+
for (final statusBarNotification in activeNotifications) {
217+
if (statusBarNotification == null) continue; // TODO(pigeon) eliminate this case
218+
final notification = statusBarNotification.notification;
219+
220+
// Sadly we don't get toString on Pigeon data classes: flutter#59027
221+
assert(debugLog(' existing notif'
222+
' id: ${statusBarNotification.id}, tag: ${statusBarNotification.tag},'
223+
' notification: (group: ${notification.group}, extras: ${notification.extras}))'));
224+
225+
// Don't act on notifications that are for other Zulip accounts/identities.
226+
if (notification.group != groupKey) continue;
227+
228+
// Don't act on the summary notification for the group.
229+
if (statusBarNotification.tag == groupKey) continue;
230+
231+
final lastMessageIdStr = notification.extras[kExtraZulipMessageId];
232+
if (lastMessageIdStr == null) continue;
233+
final lastMessageId = int.parse(lastMessageIdStr, radix: 10);
234+
if (data.zulipMessageIds.contains(lastMessageId)) {
235+
// The latest Zulip message in this conversation was read.
236+
// That's our cue to cancel the notification for the conversation.
237+
await ZulipBinding.instance.androidNotificationHost
238+
.cancel(tag: statusBarNotification.tag, id: statusBarNotification.id);
239+
} else {
240+
// This notification is for another conversation that's still unread.
241+
// We won't cancel the summary notification.
242+
haveRemaining = true;
243+
}
244+
}
245+
246+
if (!haveRemaining) {
247+
// The notification group is now empty; it had no notifications we didn't
248+
// just cancel, except the summary notification. Cancel that one too.
249+
await ZulipBinding.instance.androidNotificationHost
250+
.cancel(tag: groupKey, id: notificationIdAsHashOf(groupKey));
251+
}
252+
}
253+
254+
@visibleForTesting
255+
static const kExtraZulipMessageId = 'zulipMessageId';
256+
205257
/// A notification ID, derived as a hash of the given string key.
206258
///
207259
/// The result fits in 31 bits, the size of a nonnegative Java `int`,

‎test/model/binding.dart

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -555,10 +555,13 @@ class FakeAndroidNotificationHostApi implements AndroidNotificationHostApi {
555555
}
556556
List<AndroidNotificationHostApiNotifyCall> _notifyCalls = [];
557557

558+
Iterable<StatusBarNotification> get activeNotifications => _activeNotifications.values;
559+
final Map<(int, String?), StatusBarNotification> _activeNotifications = {};
560+
558561
final Map<String, MessagingStyle?> _activeNotificationsMessagingStyle = {};
559562

560-
/// Clears all active notifications that have been created via [notify].
561-
void clearActiveNotifications() {
563+
/// Clears all messaginging style of active notifications that have been created via [notify].
564+
void clearActiveNotificationsMessagingStyle() {
562565
_activeNotificationsMessagingStyle.clear();
563566
}
564567

@@ -599,6 +602,11 @@ class FakeAndroidNotificationHostApi implements AndroidNotificationHostApi {
599602
));
600603

601604
if (tag != null) {
605+
_activeNotifications[(id, tag)] = StatusBarNotification(
606+
id: id,
607+
notification: Notification(group: groupKey ?? '', extras: extras ?? {}),
608+
tag: tag);
609+
602610
_activeNotificationsMessagingStyle[tag] = messagingStyle == null
603611
? null
604612
: MessagingStyle(
@@ -613,24 +621,30 @@ class FakeAndroidNotificationHostApi implements AndroidNotificationHostApi {
613621
key: message.person.key,
614622
name: message.person.name,
615623
iconBitmap: null)),
616-
).toList());
624+
).toList(growable: false));
617625
}
618626
}
619627

620628
@override
621-
Future<MessagingStyle?> getActiveNotificationMessagingStyleByTag(String tag) async =>
622-
_activeNotificationsMessagingStyle[tag];
629+
Future<List<StatusBarNotification?>> getActiveNotifications({required List<String?> desiredExtras}) async {
630+
return _activeNotifications.values.map((statusNotif) {
631+
final notificationExtras = statusNotif.notification.extras;
632+
statusNotif.notification.extras = Map.fromEntries(
633+
desiredExtras
634+
.map((key) => MapEntry(key, notificationExtras[key]))
635+
.where((entry) => entry.value != null)
636+
);
637+
return statusNotif;
638+
}).toList(growable: false);
639+
}
623640

624641
@override
625-
Future<List<StatusBarNotification?>> getActiveNotifications({required List<String?> desiredExtras}) {
626-
// TODO: implement getActiveNotifications
627-
throw UnimplementedError();
628-
}
642+
Future<MessagingStyle?> getActiveNotificationMessagingStyleByTag(String tag) async =>
643+
_activeNotificationsMessagingStyle[tag];
629644

630645
@override
631-
Future<void> cancel({String? tag, required int id}) {
632-
// TODO: implement cancel
633-
throw UnimplementedError();
646+
Future<void> cancel({String? tag, required int id}) async {
647+
_activeNotifications.remove((id, tag));
634648
}
635649
}
636650

‎test/notifications/display_test.dart

Lines changed: 159 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import 'package:checks/checks.dart';
66
import 'package:collection/collection.dart';
77
import 'package:fake_async/fake_async.dart';
88
import 'package:firebase_messaging/firebase_messaging.dart';
9-
import 'package:flutter/widgets.dart';
9+
import 'package:flutter/widgets.dart' hide Notification;
1010
import 'package:flutter_local_notifications/flutter_local_notifications.dart' hide Message, Person;
1111
import 'package:flutter_test/flutter_test.dart';
1212
import 'package:http/http.dart' as http;
@@ -74,6 +74,20 @@ MessageFcmMessage messageFcmMessage(
7474
}) as MessageFcmMessage;
7575
}
7676

77+
RemoveFcmMessage removeFcmMessage(List<Message> zulipMessages, {Account? account}) {
78+
account ??= eg.selfAccount;
79+
return FcmMessage.fromJson({
80+
"event": "remove",
81+
82+
"server": "zulip.example.cloud",
83+
"realm_id": "4",
84+
"realm_uri": account.realmUrl.toString(),
85+
"user_id": account.userId.toString(),
86+
87+
"zulip_message_ids": zulipMessages.map((e) => e.id).join(','),
88+
}) as RemoveFcmMessage;
89+
}
90+
7791
void main() {
7892
TestZulipBinding.ensureInitialized();
7993
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
@@ -171,7 +185,10 @@ void main() {
171185
..number.equals(messageStyleMessages.length)
172186
..color.equals(kZulipBrandColor.value)
173187
..smallIconResourceName.equals('zulip_notification')
174-
..extras.isNull()
188+
..extras.which((it) => it.isNotNull()
189+
..deepEquals(<String, String>{
190+
NotificationDisplayManager.kExtraZulipMessageId: data.zulipMessageId.toString(),
191+
}))
175192
..groupKey.equals(expectedGroupKey)
176193
..isGroupSummary.isNull()
177194
..inboxStyle.isNull()
@@ -215,7 +232,7 @@ void main() {
215232
expectedIsGroupConversation: expectedIsGroupConversation,
216233
expectedTitle: expectedTitle,
217234
expectedTagComponent: expectedTagComponent);
218-
testBinding.androidNotificationHost.clearActiveNotifications();
235+
testBinding.androidNotificationHost.clearActiveNotificationsMessagingStyle();
219236

220237
testBinding.firebaseMessaging.onBackgroundMessage.add(
221238
RemoteMessage(data: data.toJson()));
@@ -233,6 +250,28 @@ void main() {
233250
async.flushMicrotasks();
234251
}
235252

253+
Condition<Object?> conditionActiveNotif(MessageFcmMessage data, String tagComponent) {
254+
final expectedGroupKey = '${data.realmUri}|${data.userId}';
255+
final expectedTag = '$expectedGroupKey|$tagComponent';
256+
return (it) => it.isA<StatusBarNotification>()
257+
..id.equals(NotificationDisplayManager.notificationIdAsHashOf(expectedTag))
258+
..notification.which((it) => it
259+
..group.equals(expectedGroupKey)
260+
..extras.deepEquals(<String, String>{
261+
NotificationDisplayManager.kExtraZulipMessageId: data.zulipMessageId.toString(),
262+
}))
263+
..tag.equals(expectedTag);
264+
}
265+
266+
Condition<Object?> conditionSummaryActiveNotif(String expectedGroupKey) {
267+
return (it) => it.isA<StatusBarNotification>()
268+
..id.equals(NotificationDisplayManager.notificationIdAsHashOf(expectedGroupKey))
269+
..notification.which((it) => it
270+
..group.equals(expectedGroupKey)
271+
..extras.isEmpty())
272+
..tag.equals(expectedGroupKey);
273+
}
274+
236275
test('stream message', () => runWithHttpClient(() => awaitFakeAsync((async) async {
237276
await init();
238277
final stream = eg.stream();
@@ -495,6 +534,112 @@ void main() {
495534
expectedTitle: eg.selfUser.fullName,
496535
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
497536
})));
537+
538+
test('remove: smoke', () => runWithHttpClient(() => awaitFakeAsync((async) async {
539+
await init();
540+
final message = eg.streamMessage();
541+
final data = messageFcmMessage(message);
542+
final expectedGroupKey = '${data.realmUri}|${data.userId}';
543+
544+
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
545+
await receiveFcmMessage(async, data);
546+
check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
547+
conditionActiveNotif(data, 'stream:${message.streamId}:${message.topic}'),
548+
conditionSummaryActiveNotif(expectedGroupKey),
549+
]);
550+
testBinding.firebaseMessaging.onMessage.add(
551+
RemoteMessage(data: removeFcmMessage([message]).toJson()));
552+
async.flushMicrotasks();
553+
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
554+
555+
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
556+
await receiveFcmMessage(async, data);
557+
check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
558+
conditionActiveNotif(data, 'stream:${message.streamId}:${message.topic}'),
559+
conditionSummaryActiveNotif(expectedGroupKey),
560+
]);
561+
testBinding.firebaseMessaging.onBackgroundMessage.add(
562+
RemoteMessage(data: removeFcmMessage([message]).toJson()));
563+
async.flushMicrotasks();
564+
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
565+
})));
566+
567+
test('remove: clears conversation only if the removal event is for the last message', () => runWithHttpClient(() => awaitFakeAsync((async) async {
568+
await init();
569+
final stream = eg.stream();
570+
const topicA = 'Topic A';
571+
final message1 = eg.streamMessage(stream: stream, topic: topicA);
572+
final data1 = messageFcmMessage(message1, streamName: stream.name);
573+
final message2 = eg.streamMessage(stream: stream, topic: topicA);
574+
final data2 = messageFcmMessage(message2, streamName: stream.name);
575+
final message3 = eg.streamMessage(stream: stream, topic: topicA);
576+
final data3 = messageFcmMessage(message3, streamName: stream.name);
577+
final expectedGroupKey = '${data1.realmUri}|${data1.userId}';
578+
579+
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
580+
581+
await receiveFcmMessage(async, data1);
582+
await receiveFcmMessage(async, data2);
583+
await receiveFcmMessage(async, data3);
584+
585+
check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
586+
conditionActiveNotif(data3, 'stream:${stream.streamId}:$topicA'),
587+
conditionSummaryActiveNotif(expectedGroupKey),
588+
]);
589+
590+
testBinding.firebaseMessaging.onMessage.add(
591+
RemoteMessage(data: removeFcmMessage([message1, message2]).toJson()));
592+
async.flushMicrotasks();
593+
594+
check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
595+
conditionActiveNotif(data3, 'stream:${stream.streamId}:$topicA'),
596+
conditionSummaryActiveNotif(expectedGroupKey),
597+
]);
598+
599+
testBinding.firebaseMessaging.onMessage.add(
600+
RemoteMessage(data: removeFcmMessage([message3]).toJson()));
601+
async.flushMicrotasks();
602+
603+
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
604+
})));
605+
606+
test('remove: clears summary notification only if all conversation notifications are cleared', () => runWithHttpClient(() => awaitFakeAsync((async) async {
607+
await init();
608+
final stream = eg.stream();
609+
const topicA = 'Topic A';
610+
final message1 = eg.streamMessage(stream: stream, topic: topicA);
611+
final data1 = messageFcmMessage(message1, streamName: stream.name);
612+
const topicB = 'Topic B';
613+
final message2 = eg.streamMessage(stream: stream, topic: topicB);
614+
final data2 = messageFcmMessage(message2, streamName: stream.name);
615+
final expectedGroupKey = '${data1.realmUri}|${data1.userId}';
616+
617+
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
618+
619+
await receiveFcmMessage(async, data1);
620+
await receiveFcmMessage(async, data2);
621+
622+
check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
623+
conditionActiveNotif(data1, 'stream:${stream.streamId}:$topicA'),
624+
conditionSummaryActiveNotif(expectedGroupKey),
625+
conditionActiveNotif(data2, 'stream:${stream.streamId}:$topicB'),
626+
]);
627+
628+
testBinding.firebaseMessaging.onMessage.add(
629+
RemoteMessage(data: removeFcmMessage([message1]).toJson()));
630+
async.flushMicrotasks();
631+
632+
check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
633+
conditionSummaryActiveNotif(expectedGroupKey),
634+
conditionActiveNotif(data2, 'stream:${stream.streamId}:$topicB'),
635+
]);
636+
637+
testBinding.firebaseMessaging.onMessage.add(
638+
RemoteMessage(data: removeFcmMessage([message2]).toJson()));
639+
async.flushMicrotasks();
640+
641+
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
642+
})));
498643
});
499644

500645
group('NotificationDisplayManager open', () {
@@ -700,3 +845,14 @@ extension on Subject<MessagingStyleMessage> {
700845
Subject<int> get timestampMs => has((x) => x.timestampMs, 'timestampMs');
701846
Subject<Person> get person => has((x) => x.person, 'person');
702847
}
848+
849+
extension on Subject<Notification> {
850+
Subject<String> get group => has((x) => x.group, 'group');
851+
Subject<Map<String?, String?>> get extras => has((x) => x.extras, 'extras');
852+
}
853+
854+
extension on Subject<StatusBarNotification> {
855+
Subject<int> get id => has((x) => x.id, 'id');
856+
Subject<Notification> get notification => has((x) => x.notification, 'notification');
857+
Subject<String> get tag => has((x) => x.tag, 'tag');
858+
}

0 commit comments

Comments
 (0)