Skip to content

Commit

Permalink
notif: Make notification IDs distinct by conversation
Browse files Browse the repository at this point in the history
  • Loading branch information
gnprice committed Nov 2, 2023
1 parent a8a6929 commit 35dda32
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 8 deletions.
34 changes: 28 additions & 6 deletions lib/notifications.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import 'dart:convert';

import 'package:crypto/crypto.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_local_notifications/flutter_local_notifications.dart';

Expand Down Expand Up @@ -179,10 +182,6 @@ class NotificationChannelManager {

/// Service for managing the notifications shown to the user.
class NotificationDisplayManager {
// We rely on the tag instead.
@visibleForTesting
static const kNotificationId = 0;

static Future<void> _init() async {
await ZulipBinding.instance.notifications.initialize(
const InitializationSettings(
Expand All @@ -204,8 +203,16 @@ class NotificationDisplayManager {
FcmMessageDmRecipient() =>
data.senderFullName,
};
final conversationKey = _conversationKey(data);
ZulipBinding.instance.notifications.show(
kNotificationId,
// When creating the PendingIntent for the user to open the notification,
// the plugin makes the underlying Intent objects look the same.
// This leaves only PendingIntent.requestCode to distinguish one
// PendingIntent from another; the plugin sets that to the notification ID.
// We need a distinct PendingIntent for each conversation, so that the
// notifications can lead to the right conversations when opened.
// So, use a hash of the conversation key.
notificationIdAsHashOf(conversationKey),
title,
data.content,
NotificationDetails(android: AndroidNotificationDetails(
Expand All @@ -218,13 +225,28 @@ class NotificationDisplayManager {
// But really we should fix flutter_local_notifications to not do that
// (see issue linked below), or replace that package entirely (#351).
'(Zulip internal error)', // TODO never implicitly create channel: https://github.com/MaikuB/flutter_local_notifications/issues/2135
tag: _conversationKey(data),
tag: conversationKey,
color: kZulipBrandColor,
icon: 'zulip_notification', // TODO vary for debug
// TODO(#128) inbox-style
)));
}

/// 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) {
final groupKey = _groupKey(data);
final conversation = switch (data.recipient) {
Expand Down
7 changes: 5 additions & 2 deletions test/notifications_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,16 @@ void main() {
required String expectedTitle,
required String expectedTagComponent,
}) {
final expectedTag = '${data.realmUri}|${data.userId}|$expectedTagComponent';
final expectedId =
NotificationDisplayManager.notificationIdAsHashOf(expectedTag);
check(testBinding.notifications.takeShowCalls()).single
..id.equals(NotificationDisplayManager.kNotificationId)
..id.equals(expectedId)
..title.equals(expectedTitle)
..body.equals(data.content)
..notificationDetails.isNotNull().android.isNotNull().which(it()
..channelId.equals(NotificationChannelManager.kChannelId)
..tag.equals('${data.realmUri}|${data.userId}|$expectedTagComponent')
..tag.equals(expectedTag)
..color.equals(kZulipBrandColor)
..icon.equals('zulip_notification')
);
Expand Down

0 comments on commit 35dda32

Please sign in to comment.