Skip to content

Commit

Permalink
notif: Handle remove notification message
Browse files Browse the repository at this point in the history 
Fixes: zulip#341
Co-authored-by: Greg Price <greg@zulip.com>
  • Loading branch information
@rajveermalviya @gnprice
rajveermalviya and gnprice committed Aug 11, 2024
1 parent 5f88f0a commit b542c61
Show file tree
Hide file tree
Showing 3 changed files with 238 additions and 16 deletions.
54 changes: 53 additions & 1 deletion lib/notifications/display.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class NotificationDisplayManager {
static void onFcmMessage(FcmMessage data, Map<String, dynamic> dataJson) {
switch (data) {
case MessageFcmMessage(): _onMessageFcmMessage(data, dataJson);
case RemoveFcmMessage(): break; // TODO(#341) handle
case RemoveFcmMessage(): _onRemoveFcmMessage(data);
case UnexpectedFcmMessage(): break; // TODO(log)
}
}
Expand Down Expand Up @@ -155,6 +155,9 @@ class NotificationDisplayManager {

messagingStyle: messagingStyle,
number: messagingStyle.messages.length,
extras: {
kExtraZulipMessageId: data.zulipMessageId.toString(),
},

contentIntent: PendingIntent(
// TODO make intent URLs distinct, instead of requestCode
Expand Down Expand Up @@ -202,6 +205,55 @@ class NotificationDisplayManager {
);
}

static void _onRemoveFcmMessage(RemoveFcmMessage data) async {
assert(debugLog('notif remove zulipMessageIds: ${data.zulipMessageIds}'));

final groupKey = _groupKey(data);

var haveRemaining = false;
final activeNotifications = await ZulipBinding.instance.androidNotificationHost
.getActiveNotifications(desiredExtras: [kExtraZulipMessageId]);
for (final statusBarNotification in activeNotifications) {
if (statusBarNotification == null) continue; // TODO(pigeon) eliminate this case
final notification = statusBarNotification.notification;

// Sadly we don't get toString on Pigeon data classes: flutter#59027
assert(debugLog(' existing notif'
' id: ${statusBarNotification.id}, tag: ${statusBarNotification.tag},'
' notification: (group: ${notification.group}, extras: ${notification.extras}))'));

// Don't act on notifications that are for other Zulip accounts/identities.
if (notification.group != groupKey) continue;

// Don't act on the summary notification for the group.
if (statusBarNotification.tag == groupKey) continue;

final lastMessageIdStr = notification.extras[kExtraZulipMessageId];
if (lastMessageIdStr == null) continue;
final lastMessageId = int.parse(lastMessageIdStr, radix: 10);
if (data.zulipMessageIds.contains(lastMessageId)) {
// The latest Zulip message in this conversation was read.
// That's our cue to cancel the notification for the conversation.
await ZulipBinding.instance.androidNotificationHost
.cancel(tag: statusBarNotification.tag, id: statusBarNotification.id);
} else {
// This notification is for another conversation that's still unread.
// We won't cancel the summary notification.
haveRemaining = true;
}
}

if (!haveRemaining) {
// The notification group is now empty; it had no notifications we didn't
// just cancel, except the summary notification. Cancel that one too.
await ZulipBinding.instance.androidNotificationHost
.cancel(tag: groupKey, id: notificationIdAsHashOf(groupKey));
}
}

@visibleForTesting
static const kExtraZulipMessageId = 'zulipMessageId';

/// A notification ID, derived as a hash of the given string key.
///
/// The result fits in 31 bits, the size of a nonnegative Java `int`,
Expand Down
38 changes: 26 additions & 12 deletions test/model/binding.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -555,10 +555,13 @@ class FakeAndroidNotificationHostApi implements AndroidNotificationHostApi {
}
List<AndroidNotificationHostApiNotifyCall> _notifyCalls = [];

Iterable<StatusBarNotification> get activeNotifications => _activeNotifications.values;
final Map<(int, String?), StatusBarNotification> _activeNotifications = {};

final Map<String, MessagingStyle?> _activeNotificationsMessagingStyle = {};

/// Clears all active notifications that have been created via [notify].
void clearActiveNotifications() {
/// Clears all messaginging style of active notifications that have been created via [notify].
void clearActiveNotificationsMessagingStyle() {
_activeNotificationsMessagingStyle.clear();
}

Expand Down Expand Up @@ -599,6 +602,11 @@ class FakeAndroidNotificationHostApi implements AndroidNotificationHostApi {
));

if (tag != null) {
_activeNotifications[(id, tag)] = StatusBarNotification(
id: id,
notification: Notification(group: groupKey ?? '', extras: extras ?? {}),
tag: tag);

_activeNotificationsMessagingStyle[tag] = messagingStyle == null
? null
: MessagingStyle(
Expand All @@ -613,24 +621,30 @@ class FakeAndroidNotificationHostApi implements AndroidNotificationHostApi {
key: message.person.key,
name: message.person.name,
iconBitmap: null)),
).toList());
).toList(growable: false));
}
}

@override
Future<MessagingStyle?> getActiveNotificationMessagingStyleByTag(String tag) async =>
_activeNotificationsMessagingStyle[tag];
Future<List<StatusBarNotification?>> getActiveNotifications({required List<String?> desiredExtras}) async {
return _activeNotifications.values.map((statusNotif) {
final notificationExtras = statusNotif.notification.extras;
statusNotif.notification.extras = Map.fromEntries(
desiredExtras
.map((key) => MapEntry(key, notificationExtras[key]))
.where((entry) => entry.value != null)
);
return statusNotif;
}).toList(growable: false);
}

@override
Future<List<StatusBarNotification?>> getActiveNotifications({required List<String?> desiredExtras}) {
// TODO: implement getActiveNotifications
throw UnimplementedError();
}
Future<MessagingStyle?> getActiveNotificationMessagingStyleByTag(String tag) async =>
_activeNotificationsMessagingStyle[tag];

@override
Future<void> cancel({String? tag, required int id}) {
// TODO: implement cancel
throw UnimplementedError();
Future<void> cancel({String? tag, required int id}) async {
_activeNotifications.remove((id, tag));
}
}

Expand Down
162 changes: 159 additions & 3 deletions test/notifications/display_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import 'package:checks/checks.dart';
import 'package:collection/collection.dart';
import 'package:fake_async/fake_async.dart';
import 'package:firebase_messaging/firebase_messaging.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter/widgets.dart' hide Notification;
import 'package:flutter_local_notifications/flutter_local_notifications.dart' hide Message, Person;
import 'package:flutter_test/flutter_test.dart';
import 'package:http/http.dart' as http;
Expand Down Expand Up @@ -74,6 +74,20 @@ MessageFcmMessage messageFcmMessage(
}) as MessageFcmMessage;
}

RemoveFcmMessage removeFcmMessage(List<Message> zulipMessages, {Account? account}) {
account ??= eg.selfAccount;
return FcmMessage.fromJson({
"event": "remove",

"server": "zulip.example.cloud",
"realm_id": "4",
"realm_uri": account.realmUrl.toString(),
"user_id": account.userId.toString(),

"zulip_message_ids": zulipMessages.map((e) => e.id).join(','),
}) as RemoveFcmMessage;
}

void main() {
TestZulipBinding.ensureInitialized();
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
Expand Down Expand Up @@ -171,7 +185,10 @@ void main() {
..number.equals(messageStyleMessages.length)
..color.equals(kZulipBrandColor.value)
..smallIconResourceName.equals('zulip_notification')
..extras.isNull()
..extras.which((it) => it.isNotNull()
..deepEquals(<String, String>{
NotificationDisplayManager.kExtraZulipMessageId: data.zulipMessageId.toString(),
}))
..groupKey.equals(expectedGroupKey)
..isGroupSummary.isNull()
..inboxStyle.isNull()
Expand Down Expand Up @@ -215,7 +232,7 @@ void main() {
expectedIsGroupConversation: expectedIsGroupConversation,
expectedTitle: expectedTitle,
expectedTagComponent: expectedTagComponent);
testBinding.androidNotificationHost.clearActiveNotifications();
testBinding.androidNotificationHost.clearActiveNotificationsMessagingStyle();

testBinding.firebaseMessaging.onBackgroundMessage.add(
RemoteMessage(data: data.toJson()));
Expand All @@ -233,6 +250,28 @@ void main() {
async.flushMicrotasks();
}

Condition<Object?> conditionActiveNotif(MessageFcmMessage data, String tagComponent) {
final expectedGroupKey = '${data.realmUri}|${data.userId}';
final expectedTag = '$expectedGroupKey|$tagComponent';
return (it) => it.isA<StatusBarNotification>()
..id.equals(NotificationDisplayManager.notificationIdAsHashOf(expectedTag))
..notification.which((it) => it
..group.equals(expectedGroupKey)
..extras.deepEquals(<String, String>{
NotificationDisplayManager.kExtraZulipMessageId: data.zulipMessageId.toString(),
}))
..tag.equals(expectedTag);
}

Condition<Object?> conditionSummaryActiveNotif(String expectedGroupKey) {
return (it) => it.isA<StatusBarNotification>()
..id.equals(NotificationDisplayManager.notificationIdAsHashOf(expectedGroupKey))
..notification.which((it) => it
..group.equals(expectedGroupKey)
..extras.isEmpty())
..tag.equals(expectedGroupKey);
}

test('stream message', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final stream = eg.stream();
Expand Down Expand Up @@ -495,6 +534,112 @@ void main() {
expectedTitle: eg.selfUser.fullName,
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
})));

test('remove: smoke', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final message = eg.streamMessage();
final data = messageFcmMessage(message);
final expectedGroupKey = '${data.realmUri}|${data.userId}';

check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
await receiveFcmMessage(async, data);
check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
conditionActiveNotif(data, 'stream:${message.streamId}:${message.topic}'),
conditionSummaryActiveNotif(expectedGroupKey),
]);
testBinding.firebaseMessaging.onMessage.add(
RemoteMessage(data: removeFcmMessage([message]).toJson()));
async.flushMicrotasks();
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();

check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
await receiveFcmMessage(async, data);
check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
conditionActiveNotif(data, 'stream:${message.streamId}:${message.topic}'),
conditionSummaryActiveNotif(expectedGroupKey),
]);
testBinding.firebaseMessaging.onBackgroundMessage.add(
RemoteMessage(data: removeFcmMessage([message]).toJson()));
async.flushMicrotasks();
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
})));

test('remove: clears conversation only if the removal event is for the last message', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final stream = eg.stream();
const topicA = 'Topic A';
final message1 = eg.streamMessage(stream: stream, topic: topicA);
final data1 = messageFcmMessage(message1, streamName: stream.name);
final message2 = eg.streamMessage(stream: stream, topic: topicA);
final data2 = messageFcmMessage(message2, streamName: stream.name);
final message3 = eg.streamMessage(stream: stream, topic: topicA);
final data3 = messageFcmMessage(message3, streamName: stream.name);
final expectedGroupKey = '${data1.realmUri}|${data1.userId}';

check(testBinding.androidNotificationHost.activeNotifications).isEmpty();

await receiveFcmMessage(async, data1);
await receiveFcmMessage(async, data2);
await receiveFcmMessage(async, data3);

check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
conditionActiveNotif(data3, 'stream:${stream.streamId}:$topicA'),
conditionSummaryActiveNotif(expectedGroupKey),
]);

testBinding.firebaseMessaging.onMessage.add(
RemoteMessage(data: removeFcmMessage([message1, message2]).toJson()));
async.flushMicrotasks();

check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
conditionActiveNotif(data3, 'stream:${stream.streamId}:$topicA'),
conditionSummaryActiveNotif(expectedGroupKey),
]);

testBinding.firebaseMessaging.onMessage.add(
RemoteMessage(data: removeFcmMessage([message3]).toJson()));
async.flushMicrotasks();

check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
})));

test('remove: clears summary notification only if all conversation notifications are cleared', () => runWithHttpClient(() => awaitFakeAsync((async) async {
await init();
final stream = eg.stream();
const topicA = 'Topic A';
final message1 = eg.streamMessage(stream: stream, topic: topicA);
final data1 = messageFcmMessage(message1, streamName: stream.name);
const topicB = 'Topic B';
final message2 = eg.streamMessage(stream: stream, topic: topicB);
final data2 = messageFcmMessage(message2, streamName: stream.name);
final expectedGroupKey = '${data1.realmUri}|${data1.userId}';

check(testBinding.androidNotificationHost.activeNotifications).isEmpty();

await receiveFcmMessage(async, data1);
await receiveFcmMessage(async, data2);

check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
conditionActiveNotif(data1, 'stream:${stream.streamId}:$topicA'),
conditionSummaryActiveNotif(expectedGroupKey),
conditionActiveNotif(data2, 'stream:${stream.streamId}:$topicB'),
]);

testBinding.firebaseMessaging.onMessage.add(
RemoteMessage(data: removeFcmMessage([message1]).toJson()));
async.flushMicrotasks();

check(testBinding.androidNotificationHost.activeNotifications).deepEquals(<Condition<Object?>>[
conditionSummaryActiveNotif(expectedGroupKey),
conditionActiveNotif(data2, 'stream:${stream.streamId}:$topicB'),
]);

testBinding.firebaseMessaging.onMessage.add(
RemoteMessage(data: removeFcmMessage([message2]).toJson()));
async.flushMicrotasks();

check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
})));
});

group('NotificationDisplayManager open', () {
Expand Down Expand Up @@ -700,3 +845,14 @@ extension on Subject<MessagingStyleMessage> {
Subject<int> get timestampMs => has((x) => x.timestampMs, 'timestampMs');
Subject<Person> get person => has((x) => x.person, 'person');
}

extension on Subject<Notification> {
Subject<String> get group => has((x) => x.group, 'group');
Subject<Map<String?, String?>> get extras => has((x) => x.extras, 'extras');
}

extension on Subject<StatusBarNotification> {
Subject<int> get id => has((x) => x.id, 'id');
Subject<Notification> get notification => has((x) => x.notification, 'notification');
Subject<String> get tag => has((x) => x.tag, 'tag');
}

0 comments on commit b542c61

Please sign in to comment.