From 08743be6f8620c5a31ea0f3b59120146f4e9f99b Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Tue, 22 Oct 2024 00:20:16 +0530 Subject: [PATCH] notif: Use the same notification ID for every notification Matching the `requestCode` is no longer necessary due to the removal of `pakcage:flutter_local_notifications`, which required it. --- lib/notifications/display.dart | 34 +++++++++------------------- test/notifications/display_test.dart | 10 ++++---- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index f36dd3d3b7..64b183e6dd 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -1,10 +1,8 @@ import 'dart:async'; -import 'dart:convert'; import 'dart:io'; import 'package:http/http.dart' as http; import 'package:collection/collection.dart'; -import 'package:crypto/crypto.dart'; import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart' hide Notification; @@ -170,9 +168,7 @@ class NotificationDisplayManager { }).buildUrl(); await _androidHost.notify( - // TODO the notification ID can be constant, instead of matching requestCode - // (This is a legacy of `flutter_local_notifications`.) - id: notificationIdAsHashOf(conversationKey), + id: kNotificationId, tag: conversationKey, channelId: NotificationChannelManager.kChannelId, groupKey: groupKey, @@ -215,7 +211,7 @@ class NotificationDisplayManager { ); await _androidHost.notify( - id: notificationIdAsHashOf(groupKey), + id: kNotificationId, tag: groupKey, channelId: NotificationChannelManager.kChannelId, groupKey: groupKey, @@ -300,11 +296,18 @@ class NotificationDisplayManager { // Even though we enable the `autoCancel` flag for summary notification // during creation, the summary notification doesn't get auto canceled if // child notifications are canceled programatically as done above. - await _androidHost.cancel( - tag: groupKey, id: notificationIdAsHashOf(groupKey)); + await _androidHost.cancel(tag: groupKey, id: kNotificationId); } } + /// The constant numeric "ID" we use for all non-test notifications, + /// along with unique tags. + /// + /// Because we construct a unique string "tag" for each distinct + /// notification, and Android notifications are identified by the + /// pair (tag, ID), it's simplest to leave these numeric IDs all the same. + static const kNotificationId = 0x00C0FFEE; + /// A key we use in [Notification.extras] for the [Message.id] of the /// latest Zulip message in the notification's conversation. /// @@ -313,21 +316,6 @@ class NotificationDisplayManager { @visibleForTesting static const kExtraLastZulipMessageId = 'lastZulipMessageId'; - /// 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`, - /// so that it can be used as an Android notification ID. (It's possible - /// negative values would work too, which would add one bit.) - /// - /// This is a cryptographic hash, meaning that collisions are about as - /// unlikely as one could hope for given the size of the hash. - @visibleForTesting - static int notificationIdAsHashOf(String key) { - final bytes = sha256.convert(utf8.encode(key)).bytes; - return bytes[0] | (bytes[1] << 8) | (bytes[2] << 16) - | ((bytes[3] & 0x7f) << 24); - } - static String _conversationKey(MessageFcmMessage data, String groupKey) { final conversation = switch (data.recipient) { FcmMessageChannelRecipient(:var streamId, :var topic) => 'stream:$streamId:$topic', diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index e3c62ec259..d7f47acc1c 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -223,8 +223,6 @@ void main() { final expectedTag = '${data.realmUrl}|${data.userId}|$expectedTagComponent'; final expectedGroupKey = '${data.realmUrl}|${data.userId}'; - final expectedId = - NotificationDisplayManager.notificationIdAsHashOf(expectedTag); const expectedPendingIntentFlags = PendingIntentFlag.immutable; const expectedIntentFlags = IntentFlag.activityClearTop | IntentFlag.activityNewTask; final expectedSelfUserKey = '${data.realmUrl}|${data.userId}'; @@ -256,7 +254,7 @@ void main() { check(testBinding.androidNotificationHost.takeNotifyCalls()) .deepEquals(>[ (it) => it.isA() - ..id.equals(expectedId) + ..id.equals(NotificationDisplayManager.kNotificationId) ..tag.equals(expectedTag) ..channelId.equals(NotificationChannelManager.kChannelId) ..contentTitle.isNull() @@ -288,7 +286,7 @@ void main() { ..dataUrl.equals(expectedIntentDataUrl.toString()) ..flags.equals(expectedIntentFlags))), (it) => it.isA() - ..id.equals(NotificationDisplayManager.notificationIdAsHashOf(expectedGroupKey)) + ..id.equals(NotificationDisplayManager.kNotificationId) ..tag.equals(expectedGroupKey) ..channelId.equals(NotificationChannelManager.kChannelId) ..contentTitle.isNull() @@ -344,7 +342,7 @@ void main() { final expectedGroupKey = '${data.realmUrl}|${data.userId}'; final expectedTag = '$expectedGroupKey|$tagComponent'; return (it) => it.isA() - ..id.equals(NotificationDisplayManager.notificationIdAsHashOf(expectedTag)) + ..id.equals(NotificationDisplayManager.kNotificationId) ..notification.which((it) => it ..group.equals(expectedGroupKey) ..extras.deepEquals({ @@ -355,7 +353,7 @@ void main() { Condition conditionSummaryActiveNotif(String expectedGroupKey) { return (it) => it.isA() - ..id.equals(NotificationDisplayManager.notificationIdAsHashOf(expectedGroupKey)) + ..id.equals(NotificationDisplayManager.kNotificationId) ..notification.which((it) => it ..group.equals(expectedGroupKey) ..extras.isEmpty())