Skip to content

Commit 27d5ce8

Browse files
tomlin7tomlin7
committed
notif: Ignore notifications for logged out accounts
Fixes: #1264
1 parent a5af8d3 commit 27d5ce8

File tree

4 files changed

+117
-13
lines changed

4 files changed

+117
-13
lines changed

‎lib/model/actions.dart‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import 'dart:async';
22

3+
import 'package:flutter/foundation.dart';
4+
5+
import '../notifications/display.dart';
36
import '../notifications/receive.dart';
47
import 'store.dart';
58

@@ -11,6 +14,10 @@ Future<void> logOutAccount(GlobalStore globalStore, int accountId) async {
1114
// Unawaited, to not block removing the account on this request.
1215
unawaited(unregisterToken(globalStore, accountId));
1316

17+
if (defaultTargetPlatform == TargetPlatform.android){
18+
unawaited(NotificationDisplayManager.removeNotificationsForAccount(account.realmUrl, account.userId));
19+
}
20+
1421
await globalStore.removeAccount(accountId);
1522
}
1623

‎lib/notifications/display.dart‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import 'dart:async';
22
import 'dart:io';
33

4-
import 'package:http/http.dart' as http;
54
import 'package:collection/collection.dart';
65
import 'package:flutter/foundation.dart';
76
import 'package:flutter/widgets.dart' hide Notification;
7+
import 'package:http/http.dart' as http;
88

99
import '../api/model/model.dart';
1010
import '../api/notifications.dart';
@@ -231,9 +231,16 @@ class NotificationDisplayManager {
231231
static Future<void> _onMessageFcmMessage(MessageFcmMessage data, Map<String, dynamic> dataJson) async {
232232
assert(debugLog('notif message content: ${data.content}'));
233233
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
234-
final groupKey = _groupKey(data);
234+
final groupKey = _groupKey(data.realmUrl, data.userId);
235235
final conversationKey = _conversationKey(data, groupKey);
236236

237+
final globalStore = await ZulipBinding.instance.getGlobalStore();
238+
final account = globalStore.accounts.firstWhereOrNull((account) =>
239+
account.realmUrl.origin == data.realmUrl.origin && account.userId == data.userId);
240+
if (account == null) {
241+
return;
242+
}
243+
237244
final oldMessagingStyle = await _androidHost
238245
.getActiveNotificationMessagingStyleByTag(conversationKey);
239246

@@ -365,7 +372,7 @@ class NotificationDisplayManager {
365372
// There may be a lot of messages mentioned here, across a lot of
366373
// conversations. But they'll all be for one account, so they'll
367374
// fall under one notification group.
368-
final groupKey = _groupKey(data);
375+
final groupKey = _groupKey(data.realmUrl, data.userId);
369376

370377
// Find any conversations we can cancel the notification for.
371378
// The API doesn't lend itself to removing individual messages as
@@ -445,10 +452,10 @@ class NotificationDisplayManager {
445452
return '$groupKey|$conversation';
446453
}
447454

448-
static String _groupKey(FcmMessageWithIdentity data) {
455+
static String _groupKey(Uri realmUrl, int userId) {
449456
// The realm URL can't contain a `|`, because `|` is not a URL code point:
450457
// https://url.spec.whatwg.org/#url-code-points
451-
return "${data.realmUrl}|${data.userId}";
458+
return "$realmUrl|$userId";
452459
}
453460

454461
static String _personKey(Uri realmUrl, int userId) => "$realmUrl|$userId";
@@ -518,6 +525,16 @@ class NotificationDisplayManager {
518525
}
519526
return null;
520527
}
528+
529+
static Future<void> removeNotificationsForAccount(Uri realmUri, int userId) async {
530+
final groupKey = _groupKey(realmUri, userId);
531+
final activeNotifications = await _androidHost.getActiveNotifications(desiredExtras: [kExtraLastZulipMessageId]);
532+
for (final statusBarNotification in activeNotifications) {
533+
if (statusBarNotification.notification.group == groupKey) {
534+
await _androidHost.cancel(tag: statusBarNotification.tag, id: statusBarNotification.id);
535+
}
536+
}
537+
}
521538
}
522539

523540
/// The information contained in 'zulip://notification/…' internal

‎test/model/actions_test.dart‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ void main() {
9292
check(connection.isOpen).isFalse();
9393
check(newConnection.isOpen).isTrue(); // still busy with unregister-token
9494

95+
// Check that notifications were removed
96+
final activeNotifications = await testBinding.androidNotificationHost.getActiveNotifications(desiredExtras: []);
97+
check(activeNotifications).isEmpty();
98+
9599
async.elapse(unregisterDelay - TestGlobalStore.removeAccountDuration);
96100
check(newConnection.isOpen).isFalse();
97101
}));
@@ -118,6 +122,10 @@ void main() {
118122
check(connection.isOpen).isFalse();
119123
check(newConnection.isOpen).isTrue(); // for the unregister-token request
120124

125+
// Check that notifications were removed
126+
final activeNotifications = await testBinding.androidNotificationHost.getActiveNotifications(desiredExtras: []);
127+
check(activeNotifications).isEmpty();
128+
121129
async.elapse(unregisterDelay - TestGlobalStore.removeAccountDuration);
122130
check(newConnection.isOpen).isFalse();
123131
}));

‎test/notifications/display_test.dart‎

Lines changed: 80 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ import 'package:zulip/widgets/message_list.dart';
2626
import 'package:zulip/widgets/page.dart';
2727
import 'package:zulip/widgets/theme.dart';
2828

29+
import '../example_data.dart' as eg;
2930
import '../fake_async.dart';
3031
import '../model/binding.dart';
31-
import '../example_data.dart' as eg;
3232
import '../model/narrow_checks.dart';
3333
import '../stdlib_checks.dart';
3434
import '../test_images.dart';
@@ -481,7 +481,11 @@ void main() {
481481
await init();
482482
final stream = eg.stream();
483483
final message = eg.streamMessage(stream: stream);
484-
await checkNotifications(async, messageFcmMessage(message, streamName: stream.name),
484+
final data =messageFcmMessage(message, streamName: stream.name);
485+
final account = eg.account(id: data.userId, user: eg.user(userId: data.userId));
486+
await testBinding.globalStore.add(account, eg.initialSnapshot());
487+
488+
await checkNotifications(async, data,
485489
expectedIsGroupConversation: true,
486490
expectedTitle: '#${stream.name} > ${message.topic}',
487491
expectedTagComponent: 'stream:${message.streamId}:${message.topic}');
@@ -493,6 +497,9 @@ void main() {
493497
const topic = 'topic 1';
494498
final message1 = eg.streamMessage(topic: topic, stream: stream);
495499
final data1 = messageFcmMessage(message1, streamName: stream.name);
500+
final account = eg.account(id: data1.userId, user: eg.user(userId: data1.userId));
501+
await testBinding.globalStore.add(account, eg.initialSnapshot());
502+
496503
final message2 = eg.streamMessage(topic: topic, stream: stream);
497504
final data2 = messageFcmMessage(message2, streamName: stream.name);
498505
final message3 = eg.streamMessage(topic: topic, stream: stream);
@@ -530,6 +537,9 @@ void main() {
530537
const topicB = 'topic B';
531538
final message1 = eg.streamMessage(topic: topicA, stream: stream);
532539
final data1 = messageFcmMessage(message1, streamName: stream.name);
540+
final account = eg.account(id: data1.userId, user: eg.user(userId: data1.userId));
541+
await testBinding.globalStore.add(account, eg.initialSnapshot());
542+
533543
final message2 = eg.streamMessage(topic: topicB, stream: stream);
534544
final data2 = messageFcmMessage(message2, streamName: stream.name);
535545
final message3 = eg.streamMessage(topic: topicA, stream: stream);
@@ -564,6 +574,9 @@ void main() {
564574
const topic2 = 'a TOpic';
565575
final message1 = eg.streamMessage(topic: topic1, stream: stream);
566576
final data1 = messageFcmMessage(message1, streamName: stream.name);
577+
final account = eg.account(id: data1.userId, user: eg.user(userId: data1.userId));
578+
await testBinding.globalStore.add(account, eg.initialSnapshot());
579+
567580
final message2 = eg.streamMessage(topic: topic2, stream: stream);
568581
final data2 = messageFcmMessage(message2, streamName: stream.name);
569582

@@ -589,13 +602,15 @@ void main() {
589602
const topic = 'topic';
590603
final message1 = eg.streamMessage(topic: topic, stream: stream);
591604
final data1 = messageFcmMessage(message1, streamName: stream.name);
605+
final account = eg.account(id: data1.userId, user: eg.user(userId: data1.userId));
606+
await testBinding.globalStore.add(account, eg.initialSnapshot());
592607

593608
receiveFcmMessage(async, data1);
594609
checkNotification(data1,
595610
messageStyleMessages: [data1],
596611
expectedIsGroupConversation: true,
597612
expectedTitle: '#Before > $topic',
598-
expectedTagComponent: 'stream:${stream.streamId}:$topic');
613+
expectedTagComponent: 'stream:${stream.streamId}:${topic.toLowerCase()}');
599614

600615
stream = eg.stream(streamId: 1, name: 'After');
601616
final message2 = eg.streamMessage(topic: topic, stream: stream);
@@ -606,13 +621,17 @@ void main() {
606621
messageStyleMessages: [data1, data2],
607622
expectedIsGroupConversation: true,
608623
expectedTitle: '#After > $topic',
609-
expectedTagComponent: 'stream:${stream.streamId}:$topic');
624+
expectedTagComponent: 'stream:${stream.streamId}:${topic.toLowerCase()}');
610625
})));
611626

612627
test('stream message: stream name omitted', () => runWithHttpClient(() => awaitFakeAsync((async) async {
613628
await init();
614629
final stream = eg.stream();
615630
final message = eg.streamMessage(stream: stream);
631+
final data = messageFcmMessage(message, streamName: null);
632+
final account = eg.account(id: data.userId, user: eg.user(userId: data.userId));
633+
await testBinding.globalStore.add(account, eg.initialSnapshot());
634+
616635
await checkNotifications(async, messageFcmMessage(message, streamName: null),
617636
expectedIsGroupConversation: true,
618637
expectedTitle: '#(unknown channel) > ${message.topic}',
@@ -622,7 +641,11 @@ void main() {
622641
test('group DM: 3 users', () => runWithHttpClient(() => awaitFakeAsync((async) async {
623642
await init();
624643
final message = eg.dmMessage(from: eg.thirdUser, to: [eg.otherUser, eg.selfUser]);
625-
await checkNotifications(async, messageFcmMessage(message),
644+
final data = messageFcmMessage(message);
645+
final account = eg.account(id: data.userId, user: eg.user(userId: data.userId));
646+
await testBinding.globalStore.add(account, eg.initialSnapshot());
647+
648+
await checkNotifications(async, data,
626649
expectedIsGroupConversation: true,
627650
expectedTitle: "${eg.thirdUser.fullName} to you and 1 other",
628651
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
@@ -632,7 +655,11 @@ void main() {
632655
await init();
633656
final message = eg.dmMessage(from: eg.thirdUser,
634657
to: [eg.otherUser, eg.selfUser, eg.fourthUser]);
635-
await checkNotifications(async, messageFcmMessage(message),
658+
final data = messageFcmMessage(message);
659+
final account = eg.account(id: data.userId, user: eg.user(userId: data.userId));
660+
await testBinding.globalStore.add(account, eg.initialSnapshot());
661+
662+
await checkNotifications(async, data,
636663
expectedIsGroupConversation: true,
637664
expectedTitle: "${eg.thirdUser.fullName} to you and 2 others",
638665
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
@@ -642,6 +669,8 @@ void main() {
642669
await init();
643670
final message1 = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser, eg.thirdUser]);
644671
final data1 = messageFcmMessage(message1);
672+
final account = eg.account(id: data1.userId, user: eg.user(userId: data1.userId), realmUrl: data1.realmUrl);
673+
await testBinding.globalStore.add(account, eg.initialSnapshot());
645674
final message2 = eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser, eg.otherUser]);
646675
final data2 = messageFcmMessage(message2);
647676

@@ -665,7 +694,11 @@ void main() {
665694
test('1:1 DM', () => runWithHttpClient(() => awaitFakeAsync((async) async {
666695
await init();
667696
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
668-
await checkNotifications(async, messageFcmMessage(message),
697+
final data = messageFcmMessage(message);
698+
final account = eg.account(id: data.userId, user: eg.user(userId: data.userId), realmUrl: data.realmUrl);
699+
await testBinding.globalStore.add(account, eg.initialSnapshot());
700+
701+
await checkNotifications(async, data,
669702
expectedIsGroupConversation: false,
670703
expectedTitle: eg.otherUser.fullName,
671704
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
@@ -676,6 +709,8 @@ void main() {
676709
final otherUser = eg.user(fullName: 'Before');
677710
final message1 = eg.dmMessage(from: otherUser, to: [eg.selfUser]);
678711
final data1 = messageFcmMessage(message1);
712+
final account = eg.account(id: data1.userId, user: eg.user(userId: data1.userId), realmUrl: data1.realmUrl);
713+
await testBinding.globalStore.add(account, eg.initialSnapshot());
679714

680715
final expectedTagComponent = 'dm:${message1.allRecipientIds.join(",")}';
681716

@@ -703,6 +738,8 @@ void main() {
703738
final otherUser = eg.user(email: 'before@example.com');
704739
final message1 = eg.dmMessage(from: otherUser, to: [eg.selfUser]);
705740
final data1 = messageFcmMessage(message1);
741+
final account = eg.account(id: data1.userId, user: eg.user(userId: data1.userId), realmUrl: data1.realmUrl);
742+
await testBinding.globalStore.add(account, eg.initialSnapshot());
706743

707744
final expectedTagComponent = 'dm:${message1.allRecipientIds.join(",")}';
708745

@@ -730,6 +767,9 @@ void main() {
730767
await init();
731768
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
732769
final data = messageFcmMessage(message);
770+
final account = eg.account(id: data.userId, user: eg.user(userId: data.userId), realmUrl: data.realmUrl);
771+
await testBinding.globalStore.add(account, eg.initialSnapshot());
772+
733773
receiveFcmMessage(async, data);
734774
checkNotification(data,
735775
messageStyleMessages: [data],
@@ -746,6 +786,9 @@ void main() {
746786
await init();
747787
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
748788
final data = messageFcmMessage(message);
789+
final account = eg.account(id: data.userId, user: eg.user(userId: data.userId), realmUrl: data.realmUrl);
790+
await testBinding.globalStore.add(account, eg.initialSnapshot());
791+
749792
receiveFcmMessage(async, data);
750793
checkNotification(data,
751794
messageStyleMessages: [data],
@@ -760,7 +803,11 @@ void main() {
760803
test('self-DM', () => runWithHttpClient(() => awaitFakeAsync((async) async {
761804
await init();
762805
final message = eg.dmMessage(from: eg.selfUser, to: []);
763-
await checkNotifications(async, messageFcmMessage(message),
806+
final data = messageFcmMessage(message);
807+
final account = eg.account(id: data.userId, user: eg.user(userId: data.userId), realmUrl: data.realmUrl);
808+
await testBinding.globalStore.add(account, eg.initialSnapshot());
809+
810+
await checkNotifications(async, data,
764811
expectedIsGroupConversation: false,
765812
expectedTitle: eg.selfUser.fullName,
766813
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
@@ -770,6 +817,8 @@ void main() {
770817
await init();
771818
final message = eg.streamMessage();
772819
final data = messageFcmMessage(message);
820+
final account = eg.account(id: data.userId, user: eg.user(userId: data.userId), realmUrl: data.realmUrl);
821+
await testBinding.globalStore.add(account, eg.initialSnapshot());
773822
final expectedGroupKey = '${data.realmUrl}|${data.userId}';
774823

775824
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
@@ -803,6 +852,8 @@ void main() {
803852
const topicA = 'Topic A';
804853
final message1 = eg.streamMessage(stream: stream, topic: topicA);
805854
final data1 = messageFcmMessage(message1, streamName: stream.name);
855+
final account = eg.account(id: data1.userId, user: eg.user(userId: data1.userId), realmUrl: data1.realmUrl);
856+
await testBinding.globalStore.add(account, eg.initialSnapshot());
806857
final message2 = eg.streamMessage(stream: stream, topic: topicA);
807858
final data2 = messageFcmMessage(message2, streamName: stream.name);
808859
final message3 = eg.streamMessage(stream: stream, topic: topicA);
@@ -839,6 +890,8 @@ void main() {
839890
const topicA = 'Topic A';
840891
final message1 = eg.streamMessage(stream: stream, topic: topicA);
841892
final data1 = messageFcmMessage(message1, streamName: stream.name);
893+
final account = eg.account(id: data1.userId, user: eg.user(userId: data1.userId), realmUrl: data1.realmUrl);
894+
await testBinding.globalStore.add(account, eg.initialSnapshot());
842895
final conversationKey1 = 'stream:${stream.streamId}:${topicA.toLowerCase()}';
843896
final expectedGroupKey = '${data1.realmUrl}|${data1.userId}';
844897

@@ -881,6 +934,7 @@ void main() {
881934
realmUrl: Uri.parse('https://1.chat.example'),
882935
id: 1001,
883936
user: eg.user(userId: 1001));
937+
await testBinding.globalStore.add(account1, eg.initialSnapshot());
884938
final message1 = eg.streamMessage(id: 1000, stream: stream, topic: topic);
885939
final data1 =
886940
messageFcmMessage(message1, account: account1, streamName: stream.name);
@@ -890,6 +944,7 @@ void main() {
890944
realmUrl: Uri.parse('https://2.chat.example'),
891945
id: 1002,
892946
user: eg.user(userId: 1001));
947+
await testBinding.globalStore.add(account2, eg.initialSnapshot());
893948
final message2 = eg.streamMessage(id: 1000, stream: stream, topic: topic);
894949
final data2 =
895950
messageFcmMessage(message2, account: account2, streamName: stream.name);
@@ -924,12 +979,14 @@ void main() {
924979
final conversationKey = 'stream:${stream.streamId}:some topic';
925980

926981
final account1 = eg.account(id: 1001, user: eg.user(userId: 1001), realmUrl: realmUrl);
982+
await testBinding.globalStore.add(account1, eg.initialSnapshot());
927983
final message1 = eg.streamMessage(id: 1000, stream: stream, topic: topic);
928984
final data1 =
929985
messageFcmMessage(message1, account: account1, streamName: stream.name);
930986
final groupKey1 = '${account1.realmUrl}|${account1.userId}';
931987

932988
final account2 = eg.account(id: 1002, user: eg.user(userId: 1002), realmUrl: realmUrl);
989+
await testBinding.globalStore.add(account2, eg.initialSnapshot());
933990
final message2 = eg.streamMessage(id: 1000, stream: stream, topic: topic);
934991
final data2 =
935992
messageFcmMessage(message2, account: account2, streamName: stream.name);
@@ -955,6 +1012,21 @@ void main() {
9551012
receiveFcmMessage(async, removeFcmMessage([message2], account: account2));
9561013
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
9571014
})));
1015+
1016+
test('removeNotificationsForAccount removes notifications', () => runWithHttpClient(() => awaitFakeAsync((async) async {
1017+
await init();
1018+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
1019+
final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]);
1020+
1021+
await checkNotifications(async, messageFcmMessage(message),
1022+
expectedIsGroupConversation: false,
1023+
expectedTitle: eg.otherUser.fullName,
1024+
expectedTagComponent: 'dm:${message.allRecipientIds.join(",")}');
1025+
1026+
check(testBinding.androidNotificationHost.activeNotifications).isNotEmpty();
1027+
await NotificationDisplayManager.removeNotificationsForAccount(eg.selfAccount.realmUrl, eg.selfAccount.userId);
1028+
check(testBinding.androidNotificationHost.activeNotifications).isEmpty();
1029+
})));
9581030
});
9591031

9601032
group('NotificationDisplayManager open', () {

0 commit comments

Comments
 (0)