Skip to content

Navigate to conversation on tapping notification #362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Nov 6, 2023
94 changes: 88 additions & 6 deletions lib/notifications.dart
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
import 'dart:convert';

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

import 'api/notifications.dart';
import 'log.dart';
import 'model/binding.dart';
import 'model/narrow.dart';
import 'widgets/app.dart';
import 'widgets/message_list.dart';
import 'widgets/page.dart';
import 'widgets/store.dart';

class NotificationService {
static NotificationService get instance => (_instance ??= NotificationService._());
Expand Down Expand Up @@ -179,16 +188,17 @@ 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(
android: AndroidInitializationSettings('zulip_notification'),
),
onDidReceiveNotificationResponse: _onNotificationOpened,
);
final launchDetails = await ZulipBinding.instance.notifications.getNotificationAppLaunchDetails();
if (launchDetails?.didNotificationLaunchApp ?? false) {
_handleNotificationAppLaunch(launchDetails!.notificationResponse);
}
await NotificationChannelManager._ensureChannel();
}

Expand All @@ -204,10 +214,22 @@ 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.
// They differ in their extras, but that doesn't count:
// https://developer.android.com/reference/android/app/PendingIntent
//
// 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,
payload: jsonEncode(dataJson),
NotificationDetails(android: AndroidNotificationDetails(
NotificationChannelManager.kChannelId,
// This [FlutterLocalNotificationsPlugin.show] call can potentially create
Expand All @@ -218,13 +240,31 @@ 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

// TODO plugin sets PendingIntent.FLAG_UPDATE_CURRENT; is that OK?
// TODO plugin doesn't set our Intent flags; is that OK?
)));
}

/// 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 All @@ -239,4 +279,46 @@ class NotificationDisplayManager {
// https://url.spec.whatwg.org/#url-code-points
return "${data.realmUri}|${data.userId}";
}

static void _onNotificationOpened(NotificationResponse response) async {
final data = MessageFcmMessage.fromJson(jsonDecode(response.payload!));
assert(debugLog('opened notif: message ${data.zulipMessageId}, content ${data.content}'));
_navigateForNotification(data);
}

static void _handleNotificationAppLaunch(NotificationResponse? response) async {
assert(response != null);
if (response == null) return; // TODO(log) seems like a bug in flutter_local_notifications if this can happen

final data = MessageFcmMessage.fromJson(jsonDecode(response.payload!));
assert(debugLog('launched from notif: message ${data.zulipMessageId}, content ${data.content}'));
_navigateForNotification(data);
}

static void _navigateForNotification(MessageFcmMessage data) async {
NavigatorState navigator = await ZulipApp.navigator;
final context = navigator.context;
assert(context.mounted);
if (!context.mounted) return; // TODO(linter): this is impossible as there's no actual async gap, but the use_build_context_synchronously lint doesn't see that

final globalStore = GlobalStoreWidget.of(context);
final account = globalStore.accounts.firstWhereOrNull((account) =>
account.realmUrl == data.realmUri && account.userId == data.userId);
if (account == null) return; // TODO(log)

final narrow = switch (data.recipient) {
FcmMessageStreamRecipient(:var streamId, :var topic) =>
TopicNarrow(streamId, topic),
FcmMessageDmRecipient(:var allRecipientIds) =>
DmNarrow(allRecipientIds: allRecipientIds, selfUserId: account.userId),
};

assert(debugLog(' account: $account, narrow: $narrow'));
// TODO(nav): Better interact with existing nav stack on notif open
navigator.push(MaterialWidgetRoute(
page: PerAccountStoreWidget(accountId: account.id,
// TODO(#82): Open at specific message, not just conversation
child: MessageListPage(narrow: narrow))));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could add a TODO(#82) for opening the message list to the specific message the notification was about:

return;
}
}
71 changes: 70 additions & 1 deletion lib/widgets/app.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import 'dart:async';

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';

import '../model/localizations.dart';
Expand All @@ -11,7 +15,66 @@ import 'recent_dm_conversations.dart';
import 'store.dart';

class ZulipApp extends StatelessWidget {
const ZulipApp({super.key});
const ZulipApp({super.key, this.navigatorObservers});

/// Whether the app's widget tree is ready.
///
/// This begins as false. It transitions to true when the
/// [GlobalStore] has been loaded and the [MaterialApp] has been mounted,
/// and then remains true.
static ValueListenable<bool> get ready => _ready;
static ValueNotifier<bool> _ready = ValueNotifier(false);

/// The navigator for the whole app.
///
/// This is always the [GlobalKey.currentState] of [navigatorKey].
/// If [navigatorKey] is already mounted, this future completes immediately.
/// Otherwise, it waits for [ready] to become true and then completes.
static Future<NavigatorState> get navigator {
final state = navigatorKey.currentState;
if (state != null) return Future.value(state);

assert(!ready.value);
final completer = Completer<NavigatorState>();
ready.addListener(() {
assert(ready.value);
completer.complete(navigatorKey.currentState!);
});
return completer.future;
}

/// A key for the navigator for the whole app.
///
/// For code that exists entirely outside the widget tree and has no natural
/// [BuildContext] of its own, this enables interacting with the app's
/// navigation, by calling [GlobalKey.currentState] to get a [NavigatorState].
///
/// During the app's early startup, this key will not yet be mounted.
/// It will always be mounted before [ready] becomes true,
/// and naturally before any widgets are mounted which are part of the
/// app's main UI managed by the navigator.
///
/// See also [navigator], to asynchronously wait for the navigator
/// to be mounted.
static final navigatorKey = GlobalKey<NavigatorState>();

/// Reset the state of [ZulipApp] statics, for testing.
///
/// TODO refactor this better, perhaps unify with ZulipBinding
@visibleForTesting
static void debugReset() {
_ready.dispose();
_ready = ValueNotifier(false);
}

/// A list to pass through to [MaterialApp.navigatorObservers].
/// Useful in tests.
final List<NavigatorObserver>? navigatorObservers;

void _declareReady() {
assert(navigatorKey.currentContext != null);
_ready.value = true;
}

@override
Widget build(BuildContext context) {
Expand Down Expand Up @@ -50,7 +113,13 @@ class ZulipApp extends StatelessWidget {
localizationsDelegates: ZulipLocalizations.localizationsDelegates,
supportedLocales: ZulipLocalizations.supportedLocales,
theme: theme,
navigatorKey: navigatorKey,
navigatorObservers: navigatorObservers ?? const [],
builder: (BuildContext context, Widget? child) {
if (!ready.value) {
SchedulerBinding.instance.addPostFrameCallback(
(_) => _declareReady());
}
GlobalLocalizations.zulipLocalizations = ZulipLocalizations.of(context);
return child!;
},
Expand Down
2 changes: 1 addition & 1 deletion pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ packages:
source: hosted
version: "0.3.3+6"
crypto:
dependency: transitive
dependency: "direct main"
description:
name: crypto
sha256: ff625774173754681d66daaf4a448684fb04b78f902da9cb3d308c19cc5e8bab
Expand Down
1 change: 1 addition & 0 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ dependencies:
firebase_core: ^2.14.0
flutter_local_notifications_platform_interface: ^7.0.0+1
flutter_local_notifications: ^16.1.0
crypto: ^3.0.3

dev_dependencies:
flutter_driver:
Expand Down
37 changes: 23 additions & 14 deletions test/example_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import 'stdlib_checks.dart';
//

final Uri realmUrl = Uri.parse('https://chat.example/');
Uri get _realmUrl => realmUrl;

const String recentZulipVersion = '8.0';
const int recentZulipFeatureLevel = 185;
Expand Down Expand Up @@ -53,28 +54,36 @@ User user({
);
}

Account account({
int? id,
Uri? realmUrl,
required User user,
String? apiKey,
}) {
return Account(
id: id ?? 1000, // TODO generate example IDs
realmUrl: realmUrl ?? _realmUrl,
email: user.email,
apiKey: apiKey ?? 'aeouasdf',
userId: user.userId,
zulipFeatureLevel: recentZulipFeatureLevel,
zulipVersion: recentZulipVersion,
zulipMergeBase: recentZulipVersion,
);
}

final User selfUser = user(fullName: 'Self User', email: 'self@example', userId: 123);
final Account selfAccount = Account(
final Account selfAccount = account(
id: 1001,
realmUrl: realmUrl,
email: selfUser.email,
user: selfUser,
apiKey: 'asdfqwer',
userId: selfUser.userId,
zulipFeatureLevel: recentZulipFeatureLevel,
zulipVersion: recentZulipVersion,
zulipMergeBase: recentZulipVersion,
);

final User otherUser = user(fullName: 'Other User', email: 'other@example', userId: 234);
final Account otherAccount = Account(
final Account otherAccount = account(
id: 1002,
realmUrl: realmUrl,
email: otherUser.email,
user: otherUser,
apiKey: 'sdfgwert',
userId: otherUser.userId,
zulipFeatureLevel: recentZulipFeatureLevel,
zulipVersion: recentZulipVersion,
zulipMergeBase: recentZulipVersion,
);

final User thirdUser = user(fullName: 'Third User', email: 'third@example', userId: 345);
Expand Down
16 changes: 16 additions & 0 deletions test/model/binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:test/fake.dart';
import 'package:url_launcher/url_launcher.dart' as url_launcher;
import 'package:zulip/model/binding.dart';
import 'package:zulip/model/store.dart';
import 'package:zulip/widgets/app.dart';

import 'test_store.dart';

Expand Down Expand Up @@ -61,6 +62,7 @@ class TestZulipBinding extends ZulipBinding {
/// should clean up by calling this method. Typically this is done using
/// [addTearDown], like `addTearDown(testBinding.reset);`.
void reset() {
ZulipApp.debugReset();
_resetStore();
_resetLaunchUrl();
_resetDeviceInfo();
Expand Down Expand Up @@ -317,6 +319,20 @@ class FakeFlutterLocalNotificationsPlugin extends Fake implements FlutterLocalNo
assert(initializationSettings != null);
_showCalls.add((id, title, body, notificationDetails, payload: payload));
}

/// The value to be returned by [getNotificationAppLaunchDetails].
NotificationAppLaunchDetails? appLaunchDetails;

@override
Future<NotificationAppLaunchDetails?> getNotificationAppLaunchDetails() {
return Future.value(appLaunchDetails);
}

void receiveNotificationResponse(NotificationResponse details) {
if (onDidReceiveNotificationResponse != null) {
onDidReceiveNotificationResponse!(details);
}
}
}

typedef FlutterLocalNotificationsPluginShowCall = (
Expand Down
Loading