From 50610d9def6b04de1fb9d8fd4de4ee966567f70c Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Fri, 28 Feb 2025 20:10:58 +0530 Subject: [PATCH 01/16] pigeon [nfc]: Rename `notifications.dart` to `android_notifications.dart` This makes it clear that these bindings are for Android only. --- ...cations.g.kt => AndroidNotifications.g.kt} | 44 +++++++++---------- ...ations.dart => android_notifications.dart} | 2 +- 2 files changed, 23 insertions(+), 23 deletions(-) rename android/app/src/main/kotlin/com/zulip/flutter/{Notifications.g.kt => AndroidNotifications.g.kt} (94%) rename pigeon/{notifications.dart => android_notifications.dart} (99%) diff --git a/android/app/src/main/kotlin/com/zulip/flutter/Notifications.g.kt b/android/app/src/main/kotlin/com/zulip/flutter/AndroidNotifications.g.kt similarity index 94% rename from android/app/src/main/kotlin/com/zulip/flutter/Notifications.g.kt rename to android/app/src/main/kotlin/com/zulip/flutter/AndroidNotifications.g.kt index f4862e2b0b..39207e3470 100644 --- a/android/app/src/main/kotlin/com/zulip/flutter/Notifications.g.kt +++ b/android/app/src/main/kotlin/com/zulip/flutter/AndroidNotifications.g.kt @@ -13,7 +13,7 @@ import io.flutter.plugin.common.StandardMethodCodec import io.flutter.plugin.common.StandardMessageCodec import java.io.ByteArrayOutputStream import java.nio.ByteBuffer -private object NotificationsPigeonUtils { +private object AndroidNotificationsPigeonUtils { fun wrapResult(result: Any?): List { return listOf(result) @@ -128,7 +128,7 @@ data class NotificationChannel ( if (this === other) { return true } - return NotificationsPigeonUtils.deepEquals(toList(), other.toList()) } + return AndroidNotificationsPigeonUtils.deepEquals(toList(), other.toList()) } override fun hashCode(): Int = toList().hashCode() } @@ -171,7 +171,7 @@ data class AndroidIntent ( if (this === other) { return true } - return NotificationsPigeonUtils.deepEquals(toList(), other.toList()) } + return AndroidNotificationsPigeonUtils.deepEquals(toList(), other.toList()) } override fun hashCode(): Int = toList().hashCode() } @@ -215,7 +215,7 @@ data class PendingIntent ( if (this === other) { return true } - return NotificationsPigeonUtils.deepEquals(toList(), other.toList()) } + return AndroidNotificationsPigeonUtils.deepEquals(toList(), other.toList()) } override fun hashCode(): Int = toList().hashCode() } @@ -249,7 +249,7 @@ data class InboxStyle ( if (this === other) { return true } - return NotificationsPigeonUtils.deepEquals(toList(), other.toList()) } + return AndroidNotificationsPigeonUtils.deepEquals(toList(), other.toList()) } override fun hashCode(): Int = toList().hashCode() } @@ -299,7 +299,7 @@ data class Person ( if (this === other) { return true } - return NotificationsPigeonUtils.deepEquals(toList(), other.toList()) } + return AndroidNotificationsPigeonUtils.deepEquals(toList(), other.toList()) } override fun hashCode(): Int = toList().hashCode() } @@ -339,7 +339,7 @@ data class MessagingStyleMessage ( if (this === other) { return true } - return NotificationsPigeonUtils.deepEquals(toList(), other.toList()) } + return AndroidNotificationsPigeonUtils.deepEquals(toList(), other.toList()) } override fun hashCode(): Int = toList().hashCode() } @@ -382,7 +382,7 @@ data class MessagingStyle ( if (this === other) { return true } - return NotificationsPigeonUtils.deepEquals(toList(), other.toList()) } + return AndroidNotificationsPigeonUtils.deepEquals(toList(), other.toList()) } override fun hashCode(): Int = toList().hashCode() } @@ -419,7 +419,7 @@ data class Notification ( if (this === other) { return true } - return NotificationsPigeonUtils.deepEquals(toList(), other.toList()) } + return AndroidNotificationsPigeonUtils.deepEquals(toList(), other.toList()) } override fun hashCode(): Int = toList().hashCode() } @@ -459,7 +459,7 @@ data class StatusBarNotification ( if (this === other) { return true } - return NotificationsPigeonUtils.deepEquals(toList(), other.toList()) } + return AndroidNotificationsPigeonUtils.deepEquals(toList(), other.toList()) } override fun hashCode(): Int = toList().hashCode() } @@ -509,11 +509,11 @@ data class StoredNotificationSound ( if (this === other) { return true } - return NotificationsPigeonUtils.deepEquals(toList(), other.toList()) } + return AndroidNotificationsPigeonUtils.deepEquals(toList(), other.toList()) } override fun hashCode(): Int = toList().hashCode() } -private open class NotificationsPigeonCodec : StandardMessageCodec() { +private open class AndroidNotificationsPigeonCodec : StandardMessageCodec() { override fun readValueOfType(type: Byte, buffer: ByteBuffer): Any? { return when (type) { 129.toByte() -> { @@ -721,7 +721,7 @@ interface AndroidNotificationHostApi { companion object { /** The codec used by AndroidNotificationHostApi. */ val codec: MessageCodec by lazy { - NotificationsPigeonCodec() + AndroidNotificationsPigeonCodec() } /** Sets up an instance of `AndroidNotificationHostApi` to handle messages through the `binaryMessenger`. */ @JvmOverloads @@ -737,7 +737,7 @@ interface AndroidNotificationHostApi { api.createNotificationChannel(channelArg) listOf(null) } catch (exception: Throwable) { - NotificationsPigeonUtils.wrapError(exception) + AndroidNotificationsPigeonUtils.wrapError(exception) } reply.reply(wrapped) } @@ -752,7 +752,7 @@ interface AndroidNotificationHostApi { val wrapped: List = try { listOf(api.getNotificationChannels()) } catch (exception: Throwable) { - NotificationsPigeonUtils.wrapError(exception) + AndroidNotificationsPigeonUtils.wrapError(exception) } reply.reply(wrapped) } @@ -770,7 +770,7 @@ interface AndroidNotificationHostApi { api.deleteNotificationChannel(channelIdArg) listOf(null) } catch (exception: Throwable) { - NotificationsPigeonUtils.wrapError(exception) + AndroidNotificationsPigeonUtils.wrapError(exception) } reply.reply(wrapped) } @@ -785,7 +785,7 @@ interface AndroidNotificationHostApi { val wrapped: List = try { listOf(api.listStoredSoundsInNotificationsDirectory()) } catch (exception: Throwable) { - NotificationsPigeonUtils.wrapError(exception) + AndroidNotificationsPigeonUtils.wrapError(exception) } reply.reply(wrapped) } @@ -803,7 +803,7 @@ interface AndroidNotificationHostApi { val wrapped: List = try { listOf(api.copySoundResourceToMediaStore(targetFileDisplayNameArg, sourceResourceNameArg)) } catch (exception: Throwable) { - NotificationsPigeonUtils.wrapError(exception) + AndroidNotificationsPigeonUtils.wrapError(exception) } reply.reply(wrapped) } @@ -835,7 +835,7 @@ interface AndroidNotificationHostApi { api.notify(tagArg, idArg, autoCancelArg, channelIdArg, colorArg, contentIntentArg, contentTextArg, contentTitleArg, extrasArg, groupKeyArg, inboxStyleArg, isGroupSummaryArg, messagingStyleArg, numberArg, smallIconResourceNameArg) listOf(null) } catch (exception: Throwable) { - NotificationsPigeonUtils.wrapError(exception) + AndroidNotificationsPigeonUtils.wrapError(exception) } reply.reply(wrapped) } @@ -852,7 +852,7 @@ interface AndroidNotificationHostApi { val wrapped: List = try { listOf(api.getActiveNotificationMessagingStyleByTag(tagArg)) } catch (exception: Throwable) { - NotificationsPigeonUtils.wrapError(exception) + AndroidNotificationsPigeonUtils.wrapError(exception) } reply.reply(wrapped) } @@ -869,7 +869,7 @@ interface AndroidNotificationHostApi { val wrapped: List = try { listOf(api.getActiveNotifications(desiredExtrasArg)) } catch (exception: Throwable) { - NotificationsPigeonUtils.wrapError(exception) + AndroidNotificationsPigeonUtils.wrapError(exception) } reply.reply(wrapped) } @@ -888,7 +888,7 @@ interface AndroidNotificationHostApi { api.cancel(tagArg, idArg) listOf(null) } catch (exception: Throwable) { - NotificationsPigeonUtils.wrapError(exception) + AndroidNotificationsPigeonUtils.wrapError(exception) } reply.reply(wrapped) } diff --git a/pigeon/notifications.dart b/pigeon/android_notifications.dart similarity index 99% rename from pigeon/notifications.dart rename to pigeon/android_notifications.dart index 708ae4efb5..901369001c 100644 --- a/pigeon/notifications.dart +++ b/pigeon/android_notifications.dart @@ -4,7 +4,7 @@ import 'package:pigeon/pigeon.dart'; // run `tools/check pigeon --fix`. @ConfigurePigeon(PigeonOptions( dartOut: 'lib/host/android_notifications.g.dart', - kotlinOut: 'android/app/src/main/kotlin/com/zulip/flutter/Notifications.g.kt', + kotlinOut: 'android/app/src/main/kotlin/com/zulip/flutter/AndroidNotifications.g.kt', kotlinOptions: KotlinOptions(package: 'com.zulip.flutter'), )) From b16d034dd3c31870e9cd7bf1f7034e5b81284d0d Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Thu, 27 Mar 2025 20:22:38 +0530 Subject: [PATCH 02/16] dialog [nfc]: Document required ancestors for BuildContext And fix a typo. --- lib/widgets/dialog.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/widgets/dialog.dart b/lib/widgets/dialog.dart index 08ce8f08c7..4d269cddba 100644 --- a/lib/widgets/dialog.dart +++ b/lib/widgets/dialog.dart @@ -52,10 +52,12 @@ class DialogStatus { /// /// Prose in [message] should have final punctuation: /// https://github.com/zulip/zulip-flutter/pull/1498#issuecomment-2853578577 +/// +/// The context argument should be a descendant of the app's main [Navigator]. // This API is inspired by [ScaffoldManager.showSnackBar]. We wrap // [showDialog]'s return value, a [Future], inside [DialogStatus] // whose documentation can be accessed. This helps avoid confusion when -// intepreting the meaning of the [Future]. +// interpreting the meaning of the [Future]. DialogStatus showErrorDialog({ required BuildContext context, required String title, @@ -86,6 +88,8 @@ DialogStatus showErrorDialog({ /// If the dialog was canceled, /// either with the cancel button or by tapping outside the dialog's area, /// it completes with null. +/// +/// The context argument should be a descendant of the app's main [Navigator]. DialogStatus showSuggestedActionDialog({ required BuildContext context, required String title, From 1b11847c1c2b6357d416cdddd0b86dc1ded67a9a Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Thu, 27 Mar 2025 20:39:21 +0530 Subject: [PATCH 03/16] notif: Fix error message when account not found in store --- assets/l10n/app_en.arb | 6 +++--- assets/l10n/app_pl.arb | 4 ---- assets/l10n/app_ru.arb | 4 ---- lib/generated/l10n/zulip_localizations.dart | 6 +++--- lib/generated/l10n/zulip_localizations_ar.dart | 4 ++-- lib/generated/l10n/zulip_localizations_de.dart | 4 ++-- lib/generated/l10n/zulip_localizations_en.dart | 4 ++-- lib/generated/l10n/zulip_localizations_ja.dart | 4 ++-- lib/generated/l10n/zulip_localizations_nb.dart | 4 ++-- lib/generated/l10n/zulip_localizations_pl.dart | 4 ++-- lib/generated/l10n/zulip_localizations_ru.dart | 4 ++-- lib/generated/l10n/zulip_localizations_sk.dart | 4 ++-- lib/generated/l10n/zulip_localizations_uk.dart | 4 ++-- lib/notifications/display.dart | 2 +- test/notifications/display_test.dart | 4 ++-- 15 files changed, 27 insertions(+), 35 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index 91206fabc6..9aee5a4e7f 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -919,9 +919,9 @@ "@errorNotificationOpenTitle": { "description": "Error title when notification opening fails" }, - "errorNotificationOpenAccountMissing": "The account associated with this notification no longer exists.", - "@errorNotificationOpenAccountMissing": { - "description": "Error message when the account associated with the notification is not found" + "errorNotificationOpenAccountNotFound": "The account associated with this notification could not be found.", + "@errorNotificationOpenAccountNotFound": { + "description": "Error message when the account associated with the notification could not be found" }, "errorReactionAddingFailedTitle": "Adding reaction failed", "@errorReactionAddingFailedTitle": { diff --git a/assets/l10n/app_pl.arb b/assets/l10n/app_pl.arb index 8de9527def..f23e44092e 100644 --- a/assets/l10n/app_pl.arb +++ b/assets/l10n/app_pl.arb @@ -557,10 +557,6 @@ "@errorNotificationOpenTitle": { "description": "Error title when notification opening fails" }, - "errorNotificationOpenAccountMissing": "Konto związane z tym powiadomieniem już nie istnieje.", - "@errorNotificationOpenAccountMissing": { - "description": "Error message when the account associated with the notification is not found" - }, "aboutPageOpenSourceLicenses": "Licencje otwartego źródła", "@aboutPageOpenSourceLicenses": { "description": "Item title in About Zulip page to navigate to Licenses page" diff --git a/assets/l10n/app_ru.arb b/assets/l10n/app_ru.arb index 57cf48e3e0..aceefe65c6 100644 --- a/assets/l10n/app_ru.arb +++ b/assets/l10n/app_ru.arb @@ -287,10 +287,6 @@ "@errorNotificationOpenTitle": { "description": "Error title when notification opening fails" }, - "errorNotificationOpenAccountMissing": "Учетной записи, связанной с этим оповещением, больше нет.", - "@errorNotificationOpenAccountMissing": { - "description": "Error message when the account associated with the notification is not found" - }, "switchAccountButton": "Сменить учетную запись", "@switchAccountButton": { "description": "Label for main-menu button leading to the choose-account page." diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index 675407f68d..a6d6f16fbc 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -1355,11 +1355,11 @@ abstract class ZulipLocalizations { /// **'Failed to open notification'** String get errorNotificationOpenTitle; - /// Error message when the account associated with the notification is not found + /// Error message when the account associated with the notification could not be found /// /// In en, this message translates to: - /// **'The account associated with this notification no longer exists.'** - String get errorNotificationOpenAccountMissing; + /// **'The account associated with this notification could not be found.'** + String get errorNotificationOpenAccountNotFound; /// Error title when adding a message reaction fails /// diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index febbb2c585..c3cd0da784 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -747,8 +747,8 @@ class ZulipLocalizationsAr extends ZulipLocalizations { String get errorNotificationOpenTitle => 'Failed to open notification'; @override - String get errorNotificationOpenAccountMissing => - 'The account associated with this notification no longer exists.'; + String get errorNotificationOpenAccountNotFound => + 'The account associated with this notification could not be found.'; @override String get errorReactionAddingFailedTitle => 'Adding reaction failed'; diff --git a/lib/generated/l10n/zulip_localizations_de.dart b/lib/generated/l10n/zulip_localizations_de.dart index a9188773c0..e052825d6c 100644 --- a/lib/generated/l10n/zulip_localizations_de.dart +++ b/lib/generated/l10n/zulip_localizations_de.dart @@ -747,8 +747,8 @@ class ZulipLocalizationsDe extends ZulipLocalizations { String get errorNotificationOpenTitle => 'Failed to open notification'; @override - String get errorNotificationOpenAccountMissing => - 'The account associated with this notification no longer exists.'; + String get errorNotificationOpenAccountNotFound => + 'The account associated with this notification could not be found.'; @override String get errorReactionAddingFailedTitle => 'Adding reaction failed'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index 73a4212ee3..51d68f9c4a 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -747,8 +747,8 @@ class ZulipLocalizationsEn extends ZulipLocalizations { String get errorNotificationOpenTitle => 'Failed to open notification'; @override - String get errorNotificationOpenAccountMissing => - 'The account associated with this notification no longer exists.'; + String get errorNotificationOpenAccountNotFound => + 'The account associated with this notification could not be found.'; @override String get errorReactionAddingFailedTitle => 'Adding reaction failed'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index b614f71cbb..b880fdf3b6 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -747,8 +747,8 @@ class ZulipLocalizationsJa extends ZulipLocalizations { String get errorNotificationOpenTitle => 'Failed to open notification'; @override - String get errorNotificationOpenAccountMissing => - 'The account associated with this notification no longer exists.'; + String get errorNotificationOpenAccountNotFound => + 'The account associated with this notification could not be found.'; @override String get errorReactionAddingFailedTitle => 'Adding reaction failed'; diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index 4929eae453..73461db29b 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -747,8 +747,8 @@ class ZulipLocalizationsNb extends ZulipLocalizations { String get errorNotificationOpenTitle => 'Failed to open notification'; @override - String get errorNotificationOpenAccountMissing => - 'The account associated with this notification no longer exists.'; + String get errorNotificationOpenAccountNotFound => + 'The account associated with this notification could not be found.'; @override String get errorReactionAddingFailedTitle => 'Adding reaction failed'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index a5ee696797..570100b0c6 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -757,8 +757,8 @@ class ZulipLocalizationsPl extends ZulipLocalizations { 'Otwieranie powiadomienia bez powodzenia'; @override - String get errorNotificationOpenAccountMissing => - 'Konto związane z tym powiadomieniem już nie istnieje.'; + String get errorNotificationOpenAccountNotFound => + 'The account associated with this notification could not be found.'; @override String get errorReactionAddingFailedTitle => 'Dodanie reakcji bez powodzenia'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 1fcde0d7a2..508b8eec09 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -761,8 +761,8 @@ class ZulipLocalizationsRu extends ZulipLocalizations { String get errorNotificationOpenTitle => 'Не удалось открыть оповещения'; @override - String get errorNotificationOpenAccountMissing => - 'Учетной записи, связанной с этим оповещением, больше нет.'; + String get errorNotificationOpenAccountNotFound => + 'The account associated with this notification could not be found.'; @override String get errorReactionAddingFailedTitle => 'Не удалось добавить реакцию'; diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index 30cd4c89f8..a1e835bbfc 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -749,8 +749,8 @@ class ZulipLocalizationsSk extends ZulipLocalizations { String get errorNotificationOpenTitle => 'Nepodarilo sa otvoriť oznámenie'; @override - String get errorNotificationOpenAccountMissing => - 'The account associated with this notification no longer exists.'; + String get errorNotificationOpenAccountNotFound => + 'The account associated with this notification could not be found.'; @override String get errorReactionAddingFailedTitle => 'Nepodarilo sa pridať reakciu'; diff --git a/lib/generated/l10n/zulip_localizations_uk.dart b/lib/generated/l10n/zulip_localizations_uk.dart index a77d28aeff..d0cac54a56 100644 --- a/lib/generated/l10n/zulip_localizations_uk.dart +++ b/lib/generated/l10n/zulip_localizations_uk.dart @@ -761,8 +761,8 @@ class ZulipLocalizationsUk extends ZulipLocalizations { String get errorNotificationOpenTitle => 'Не вдалося відкрити сповіщення'; @override - String get errorNotificationOpenAccountMissing => - 'Обліковий запис, пов’язаний із цим сповіщенням, більше не існує.'; + String get errorNotificationOpenAccountNotFound => + 'The account associated with this notification could not be found.'; @override String get errorReactionAddingFailedTitle => 'Не вдалося додати реакцію'; diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index 00a5f6fda5..081a2cf633 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -507,7 +507,7 @@ class NotificationDisplayManager { final zulipLocalizations = ZulipLocalizations.of(context); showErrorDialog(context: context, title: zulipLocalizations.errorNotificationOpenTitle, - message: zulipLocalizations.errorNotificationOpenAccountMissing); + message: zulipLocalizations.errorNotificationOpenAccountNotFound); return null; } diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index ccba7e24cc..ffb5d345d8 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -1137,7 +1137,7 @@ void main() { check(pushedRoutes.single).isA>(); await tester.tap(find.byWidget(checkErrorDialog(tester, expectedTitle: zulipLocalizations.errorNotificationOpenTitle, - expectedMessage: zulipLocalizations.errorNotificationOpenAccountMissing))); + expectedMessage: zulipLocalizations.errorNotificationOpenAccountNotFound))); }); testWidgets('mismatching account', (tester) async { @@ -1149,7 +1149,7 @@ void main() { check(pushedRoutes.single).isA>(); await tester.tap(find.byWidget(checkErrorDialog(tester, expectedTitle: zulipLocalizations.errorNotificationOpenTitle, - expectedMessage: zulipLocalizations.errorNotificationOpenAccountMissing))); + expectedMessage: zulipLocalizations.errorNotificationOpenAccountNotFound))); }); testWidgets('find account among several', (tester) async { From d96617967f8d574372763198ae9be50c9234db08 Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Fri, 28 Mar 2025 19:32:30 +0530 Subject: [PATCH 04/16] binding test [nfc]: Reorder androidNotificationHost getter --- test/model/binding.dart | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/model/binding.dart b/test/model/binding.dart index 2c70b68826..6b4de26608 100644 --- a/test/model/binding.dart +++ b/test/model/binding.dart @@ -313,12 +313,10 @@ class TestZulipBinding extends ZulipBinding { _androidNotificationHostApi = null; } - FakeAndroidNotificationHostApi? _androidNotificationHostApi; - @override - FakeAndroidNotificationHostApi get androidNotificationHost { - return (_androidNotificationHostApi ??= FakeAndroidNotificationHostApi()); - } + FakeAndroidNotificationHostApi get androidNotificationHost => + (_androidNotificationHostApi ??= FakeAndroidNotificationHostApi()); + FakeAndroidNotificationHostApi? _androidNotificationHostApi; /// The value that `ZulipBinding.instance.pickFiles()` should return. /// From 8288b10aa948c3bedfa15638a7ca290e37759288 Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Mon, 10 Mar 2025 16:34:32 +0530 Subject: [PATCH 05/16] docs: Document testing push notifications on iOS Simulator --- .../howto/push-notifications-ios-simulator.md | 281 ++++++++++++++++++ 1 file changed, 281 insertions(+) create mode 100644 docs/howto/push-notifications-ios-simulator.md diff --git a/docs/howto/push-notifications-ios-simulator.md b/docs/howto/push-notifications-ios-simulator.md new file mode 100644 index 0000000000..ff9fd3b370 --- /dev/null +++ b/docs/howto/push-notifications-ios-simulator.md @@ -0,0 +1,281 @@ +# Testing Push Notifications on iOS Simulator + +For documentation on testing push notifications on Android or a real +iOS Device, see https://github.com/zulip/zulip-mobile/blob/main/docs/howto/push-notifications.md + +This doc describes how to test client side changes on iOS Simulator. +It will demonstrate how to use APNs payloads the server sends to +Apple's Push Notification service to show notifications on iOS +Simulator. + +
+ +## Receive a notification on the iOS Simulator + +Follow the following steps if you already have a valid APNs payload. + +Otherwise, you can either use one of the pre-canned payloads +provided [here](#pre-canned-payloads), or you can retrieve the APNs +payload generated by the latest dev server by following the steps +[here](#retrieve-apns-payload). + + +### 1. Determine the device ID of the iOS Simulator + +To receive a notification on the iOS Simulator, we need to first +determine the device ID of the iOS Simulator, to specify which +Simulator instance we want to push the payload to. + +```shell-session +$ xcrun simctl list devices booted +``` + +
+Example output: + +```shell-session +$ xcrun simctl list devices booted +== Devices == +-- iOS 18.3 -- + iPhone 16 Pro (90CC33B2-679B-4053-B380-7B986A29F28C) (Booted) +``` + +
+ + +### 2. Trigger a notification by pushing the payload to the iOS Simulator + +By running the following command with a valid APNs payload, you should +receive a notification on the iOS Simulator for the zulip-flutter app, +and tapping on it should route to the respective conversation. + +```shell-session +$ xcrun simctl push [device-id] com.zulip.flutter [payload json path] +``` + +
+Example output: + +```shell-session +$ xcrun simctl push 90CC33B2-679B-4053-B380-7B986A29F28C com.zulip.flutter ./dm.json +Notification sent to 'com.zulip.flutter' +``` + +
+ + +
+ +## Pre-canned APNs payloads + +The following pre-canned APNs payloads can be used in case you don't +have one. + +The following pre-canned payloads were generated from +Zulip Server 11.0-dev+git 8fd04b0f0, API Feature Level 377, +in April 2025. + +Also, they assume that EXTERNAL_HOST has its default value for the dev +server. If you've [set EXTERNAL_HOST to use an IP address](https://github.com/zulip/zulip-mobile/blob/main/docs/howto/dev-server.md#4-set-external_host) +in order to enable your device to connect to the dev server, you'll +need to adjust the `realm_url` fields. You can do this by a +find-and-replace for `localhost`; for example, +`perl -i -0pe s/localhost/10.0.2.2/g tmp/*.json` after saving the +canned payloads to files `tmp/*.json`. + +
+Payload: dm.json + +```json +{ + "aps": { + "alert": { + "title": "Zoe", + "subtitle": "", + "body": "But wouldn't that show you contextually who is in the audience before you have to open the compose box?" + }, + "sound": "default", + "badge": 0, + }, + "zulip": { + "server": "zulipdev.com:9991", + "realm_id": 2, + "realm_uri": "http://localhost:9991", + "realm_url": "http://localhost:9991", + "realm_name": "Zulip Dev", + "user_id": 11, + "sender_id": 7, + "sender_email": "user7@zulipdev.com", + "time": 1740890583, + "recipient_type": "private", + "message_ids": [ + 87 + ] + } +} +``` + +
+ +
+Payload: group_dm.json + +```json +{ + "aps": { + "alert": { + "title": "Othello, the Moor of Venice, Polonius (guest), Iago", + "subtitle": "Othello, the Moor of Venice:", + "body": "Sit down awhile; And let us once again assail your ears, That are so fortified against our story What we have two nights seen." + }, + "sound": "default", + "badge": 0, + }, + "zulip": { + "server": "zulipdev.com:9991", + "realm_id": 2, + "realm_uri": "http://localhost:9991", + "realm_url": "http://localhost:9991", + "realm_name": "Zulip Dev", + "user_id": 11, + "sender_id": 12, + "sender_email": "user12@zulipdev.com", + "time": 1740533641, + "recipient_type": "private", + "pm_users": "11,12,13", + "message_ids": [ + 17 + ] + } +} +``` + +
+ +
+Payload: stream.json + +```json +{ + "aps": { + "alert": { + "title": "#devel > plotter", + "subtitle": "Desdemona:", + "body": "Despite the fact that such a claim at first glance seems counterintuitive, it is derived from known results. Electrical engineering follows a cycle of four phases: location, refinement, visualization, and evaluation." + }, + "sound": "default", + "badge": 0, + }, + "zulip": { + "server": "zulipdev.com:9991", + "realm_id": 2, + "realm_uri": "http://localhost:9991", + "realm_url": "http://localhost:9991", + "realm_name": "Zulip Dev", + "user_id": 11, + "sender_id": 9, + "sender_email": "user9@zulipdev.com", + "time": 1740558997, + "recipient_type": "stream", + "stream": "devel", + "stream_id": 11, + "topic": "plotter", + "message_ids": [ + 40 + ] + } +} +``` + +
+ + +
+ +## Retrieve an APNs payload from dev server + +### 1. Set up dev server + +Follow +[this setup tutorial](https://zulip.readthedocs.io/en/latest/development/setup-recommended.html) +to setup and run the dev server on same the Mac machine that hosts +the iOS Simulator. + +If you want to run the dev server on a different machine than the Mac +host, you'll need to follow extra steps +[documented here](https://github.com/zulip/zulip-mobile/blob/main/docs/howto/dev-server.md) +to make it possible for the app running on the iOS Simulator to +connect to the dev server. + + +### 2. Set up the dev user to receive mobile notifications. + +We'll use the devlogin user `iago@zulip.com` to test notifications, +log in to that user by going to `/devlogin` on that server on Web. + +And then follow the steps [here](https://zulip.com/help/mobile-notifications) +to enable Mobile Notifications for "Channels". + + +### 3. Log in as the dev user on zulip-flutter. + + + +To login to this user in the Flutter app, you'll need the password +that was generated by the development server. You can print the +password by running this command inside your `vagrant ssh` shell: +``` +$ ./manage.py print_initial_password iago@zulip.com +``` + +Then run the app on the iOS Simulator, accept the permission to +receive push notifications, and then login to the dev user +(`iago@zulip.com`). + + +### 4. Edit the server code to log the notification payload. + +We need to retrieve the APNs payload the server generates and sends +to the bouncer. To do that we can add a log statement after the +server completes generating the APNs in `zerver/lib/push_notifications.py`: + +```diff + apns_payload = get_message_payload_apns( + user_profile, + message, + trigger, + mentioned_user_group_id, + mentioned_user_group_name, + can_access_sender, + ) + gcm_payload, gcm_options = get_message_payload_gcm( + user_profile, message, mentioned_user_group_id, mentioned_user_group_name, can_access_sender + ) + logger.info("Sending push notifications to mobile clients for user %s", user_profile_id) ++ logger.info("APNS payload %s", orjson.dumps(apns_payload).decode()) + + android_devices = list( + PushDeviceToken.objects.filter(user=user_profile, kind=PushDeviceToken.FCM).order_by("id") +``` + + +### 5. Send messages to the dev user + +To generate notifications to the dev user `iago@zulip.com` we need to +send messages from another user. For a variety of different types of +payloads try sending a message in a topic, a message in a group DM, +and one in one-one DM. Then look for the payloads in the server logs +by searching for "APNS payload". + + +### 6. Transform and save the payload to a file + +The logged payload JSON will have different structure than what an +iOS device actually receives, to fix that and save the result to a +file, run the payload through the following command: + +```shell-session +$ echo '{"alert":{"title": ...' \ + | jq '{aps: {alert: .alert, sound: .sound, badge: .badge}, zulip: .custom.zulip}' \ + > payload.json +``` From 8f81702689293113a4417884c28eee4ad8709d84 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Sat, 17 May 2025 21:20:02 -0700 Subject: [PATCH 06/16] docs: Clarify and expand a few spots in the iOS simulator notif doc Also make use of a handy shorthand within `jq`. --- .../howto/push-notifications-ios-simulator.md | 85 ++++++++++++------- 1 file changed, 52 insertions(+), 33 deletions(-) diff --git a/docs/howto/push-notifications-ios-simulator.md b/docs/howto/push-notifications-ios-simulator.md index ff9fd3b370..d54bb20491 100644 --- a/docs/howto/push-notifications-ios-simulator.md +++ b/docs/howto/push-notifications-ios-simulator.md @@ -1,23 +1,37 @@ # Testing Push Notifications on iOS Simulator For documentation on testing push notifications on Android or a real -iOS Device, see https://github.com/zulip/zulip-mobile/blob/main/docs/howto/push-notifications.md +iOS device, see https://github.com/zulip/zulip-mobile/blob/main/docs/howto/push-notifications.md -This doc describes how to test client side changes on iOS Simulator. +This doc describes how to test client-side changes on iOS Simulator. It will demonstrate how to use APNs payloads the server sends to -Apple's Push Notification service to show notifications on iOS +the Apple Push Notification service to show notifications on iOS Simulator. -
-## Receive a notification on the iOS Simulator +### Contents -Follow the following steps if you already have a valid APNs payload. +* [Trigger a notification on the iOS Simulator](#trigger-notification) +* [Canned APNs payloads](#canned-payloads) +* [Produce sample APNs payloads](#produce-payload) -Otherwise, you can either use one of the pre-canned payloads -provided [here](#pre-canned-payloads), or you can retrieve the APNs -payload generated by the latest dev server by following the steps -[here](#retrieve-apns-payload). + +
+ +## Trigger a notification on the iOS Simulator + +The iOS Simulator permits delivering a notification payload +artificially, as if APNs had delivered it to the device, +but without actually involving APNs or any other server. + +As input for this operation, you'll need an APNs payload, +i.e. a JSON blob representing what APNs might deliver to the app +for a notification. + +To get an APNs payload, you can generate one from a Zulip dev server +by following the [instructions in a section below](#produce-payload), +or you can use one of the payloads included +in this document [below](#canned-payloads). ### 1. Determine the device ID of the iOS Simulator @@ -46,8 +60,8 @@ $ xcrun simctl list devices booted ### 2. Trigger a notification by pushing the payload to the iOS Simulator By running the following command with a valid APNs payload, you should -receive a notification on the iOS Simulator for the zulip-flutter app, -and tapping on it should route to the respective conversation. +receive a notification on the iOS Simulator for the zulip-flutter app. +Tapping on the notification should route to the respective conversation. ```shell-session $ xcrun simctl push [device-id] com.zulip.flutter [payload json path] @@ -64,19 +78,21 @@ Notification sent to 'com.zulip.flutter' -
+
-## Pre-canned APNs payloads +## Canned APNs payloads The following pre-canned APNs payloads can be used in case you don't have one. -The following pre-canned payloads were generated from +These canned payloads were generated from Zulip Server 11.0-dev+git 8fd04b0f0, API Feature Level 377, in April 2025. +The `user_id` is that of `iago@zulip.com` in the Zulip dev environment. -Also, they assume that EXTERNAL_HOST has its default value for the dev -server. If you've [set EXTERNAL_HOST to use an IP address](https://github.com/zulip/zulip-mobile/blob/main/docs/howto/dev-server.md#4-set-external_host) +These canned payloads assume that EXTERNAL_HOST has its default value +for the dev server. If you've +[set EXTERNAL_HOST to use an IP address](https://github.com/zulip/zulip-mobile/blob/main/docs/howto/dev-server.md#4-set-external_host) in order to enable your device to connect to the dev server, you'll need to adjust the `realm_url` fields. You can do this by a find-and-replace for `localhost`; for example, @@ -190,16 +206,16 @@ canned payloads to files `tmp/*.json`. -
+
-## Retrieve an APNs payload from dev server +## Produce sample APNs payloads ### 1. Set up dev server -Follow -[this setup tutorial](https://zulip.readthedocs.io/en/latest/development/setup-recommended.html) -to setup and run the dev server on same the Mac machine that hosts -the iOS Simulator. +To set up and run the dev server on the same Mac machine that hosts +the iOS Simulator, follow Zulip's +[standard instructions](https://zulip.readthedocs.io/en/latest/development/setup-recommended.html) +for setting up a dev server. If you want to run the dev server on a different machine than the Mac host, you'll need to follow extra steps @@ -210,10 +226,10 @@ connect to the dev server. ### 2. Set up the dev user to receive mobile notifications. -We'll use the devlogin user `iago@zulip.com` to test notifications, -log in to that user by going to `/devlogin` on that server on Web. +We'll use the devlogin user `iago@zulip.com` to test notifications. +Log in to that user by going to `/devlogin` on that server on Web. -And then follow the steps [here](https://zulip.com/help/mobile-notifications) +Then follow the steps [here](https://zulip.com/help/mobile-notifications) to enable Mobile Notifications for "Channels". @@ -221,7 +237,7 @@ to enable Mobile Notifications for "Channels". -To login to this user in the Flutter app, you'll need the password +To log in as this user in the Flutter app, you'll need the password that was generated by the development server. You can print the password by running this command inside your `vagrant ssh` shell: ``` @@ -229,7 +245,7 @@ $ ./manage.py print_initial_password iago@zulip.com ``` Then run the app on the iOS Simulator, accept the permission to -receive push notifications, and then login to the dev user +receive push notifications, and then log in as the dev user (`iago@zulip.com`). @@ -237,7 +253,7 @@ receive push notifications, and then login to the dev user We need to retrieve the APNs payload the server generates and sends to the bouncer. To do that we can add a log statement after the -server completes generating the APNs in `zerver/lib/push_notifications.py`: +server completes generating the payload in `zerver/lib/push_notifications.py`: ```diff apns_payload = get_message_payload_apns( @@ -270,12 +286,15 @@ by searching for "APNS payload". ### 6. Transform and save the payload to a file -The logged payload JSON will have different structure than what an -iOS device actually receives, to fix that and save the result to a -file, run the payload through the following command: +The payload JSON recorded in the steps above is in the form the +Zulip server sends to the bouncer. The bouncer restructures this +slightly to produce the actual payload which it sends to APNs, +and which APNs delivers to the app on the device. +To apply the same restructuring, run the payload through +the following `jq` command: ```shell-session $ echo '{"alert":{"title": ...' \ - | jq '{aps: {alert: .alert, sound: .sound, badge: .badge}, zulip: .custom.zulip}' \ + | jq '{aps: {alert, sound, badge}, zulip: .custom.zulip}' \ > payload.json ``` From a4252fe3bae0e6e59472c03d51647e8ebca1bd31 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 11 Apr 2025 19:55:12 -0700 Subject: [PATCH 07/16] store: Add "blocking future" option on GlobalStoreWidget --- lib/widgets/store.dart | 11 ++++++++ test/widgets/store_test.dart | 54 ++++++++++++++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/lib/widgets/store.dart b/lib/widgets/store.dart index ab287b745a..f5fe4d3cc6 100644 --- a/lib/widgets/store.dart +++ b/lib/widgets/store.dart @@ -18,10 +18,18 @@ import 'page.dart'; class GlobalStoreWidget extends StatefulWidget { const GlobalStoreWidget({ super.key, + this.blockingFuture, this.placeholder = const LoadingPlaceholder(), required this.child, }); + /// An additional future to await before showing the child. + /// + /// If [blockingFuture] is non-null, then this widget will build [child] + /// only after the future completes. This widget's behavior is not affected + /// by whether the future's completion is with a value or with an error. + final Future? blockingFuture; + final Widget placeholder; final Widget child; @@ -87,6 +95,9 @@ class _GlobalStoreWidgetState extends State { super.initState(); (() async { final store = await ZulipBinding.instance.getGlobalStoreUniquely(); + if (widget.blockingFuture != null) { + await widget.blockingFuture!.catchError((_) {}); + } setState(() { this.store = store; }); diff --git a/test/widgets/store_test.dart b/test/widgets/store_test.dart index f8da5e24a0..54490ede93 100644 --- a/test/widgets/store_test.dart +++ b/test/widgets/store_test.dart @@ -70,12 +70,12 @@ void main() { return const SizedBox.shrink(); }))); // First, shows a loading page instead of child. - check(tester.any(find.byType(CircularProgressIndicator))).isTrue(); + check(find.byType(CircularProgressIndicator)).findsOne(); check(globalStore).isNull(); await tester.pump(); // Then after loading, mounts child instead, with provided store. - check(tester.any(find.byType(CircularProgressIndicator))).isFalse(); + check(find.byType(CircularProgressIndicator)).findsNothing(); check(globalStore).identicalTo(testBinding.globalStore); await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); @@ -84,6 +84,56 @@ void main() { .equals((accountId: eg.selfAccount.id, account: eg.selfAccount)); }); + testWidgets('GlobalStoreWidget awaits blockingFuture', (tester) async { + addTearDown(testBinding.reset); + + final completer = Completer(); + await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, + child: GlobalStoreWidget( + blockingFuture: completer.future, + child: Text('done')))); + + await tester.pump(); + await tester.pump(); + await tester.pump(); + // Even after the store must have loaded, + // still shows loading page while blockingFuture is pending. + check(find.byType(CircularProgressIndicator)).findsOne(); + check(find.text('done')).findsNothing(); + + // Once blockingFuture completes… + completer.complete(); + await tester.pump(); + // … mounts child instead of the loading page. + check(find.byType(CircularProgressIndicator)).findsNothing(); + check(find.text('done')).findsOne(); + }); + + testWidgets('GlobalStoreWidget handles failed blockingFuture like success', (tester) async { + addTearDown(testBinding.reset); + + final completer = Completer(); + await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, + child: GlobalStoreWidget( + blockingFuture: completer.future, + child: Text('done')))); + + await tester.pump(); + await tester.pump(); + await tester.pump(); + // Even after the store must have loaded, + // still shows loading page while blockingFuture is pending. + check(find.byType(CircularProgressIndicator)).findsOne(); + check(find.text('done')).findsNothing(); + + // Once blockingFuture completes, even with an error… + completer.completeError(Exception('oops')); + await tester.pump(); + // … mounts child instead of the loading page. + check(find.byType(CircularProgressIndicator)).findsNothing(); + check(find.text('done')).findsOne(); + }); + testWidgets('GlobalStoreWidget.of updates dependents', (tester) async { addTearDown(testBinding.reset); From 6e51473f9b27231d6e2a741997257b3dc5efdff2 Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Wed, 28 May 2025 04:11:48 +0530 Subject: [PATCH 08/16] notif [nfc]: Move NotificationOpenPayload to a separate file --- lib/notifications/display.dart | 84 +---------- lib/notifications/open.dart | 85 +++++++++++ test/notifications/display_test.dart | 205 +------------------------- test/notifications/open_test.dart | 212 +++++++++++++++++++++++++++ 4 files changed, 299 insertions(+), 287 deletions(-) create mode 100644 lib/notifications/open.dart create mode 100644 test/notifications/open_test.dart diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index 081a2cf633..ab0f88138c 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -21,6 +21,7 @@ import '../widgets/message_list.dart'; import '../widgets/page.dart'; import '../widgets/store.dart'; import '../widgets/theme.dart'; +import 'open.dart'; AndroidNotificationHostApi get _androidHost => ZulipBinding.instance.androidNotificationHost; @@ -550,86 +551,3 @@ class NotificationDisplayManager { return null; } } - -/// The information contained in 'zulip://notification/…' internal -/// Android intent data URL, used for notification-open flow. -class NotificationOpenPayload { - final Uri realmUrl; - final int userId; - final Narrow narrow; - - NotificationOpenPayload({ - required this.realmUrl, - required this.userId, - required this.narrow, - }); - - factory NotificationOpenPayload.parseUrl(Uri url) { - if (url case Uri( - scheme: 'zulip', - host: 'notification', - queryParameters: { - 'realm_url': var realmUrlStr, - 'user_id': var userIdStr, - 'narrow_type': var narrowType, - // In case of narrowType == 'topic': - // 'channel_id' and 'topic' handled below. - - // In case of narrowType == 'dm': - // 'all_recipient_ids' handled below. - }, - )) { - final realmUrl = Uri.parse(realmUrlStr); - final userId = int.parse(userIdStr, radix: 10); - - final Narrow narrow; - switch (narrowType) { - case 'topic': - final channelIdStr = url.queryParameters['channel_id']!; - final channelId = int.parse(channelIdStr, radix: 10); - final topicStr = url.queryParameters['topic']!; - narrow = TopicNarrow(channelId, TopicName(topicStr)); - case 'dm': - final allRecipientIdsStr = url.queryParameters['all_recipient_ids']!; - final allRecipientIds = allRecipientIdsStr.split(',') - .map((idStr) => int.parse(idStr, radix: 10)) - .toList(growable: false); - narrow = DmNarrow(allRecipientIds: allRecipientIds, selfUserId: userId); - default: - throw const FormatException(); - } - - return NotificationOpenPayload( - realmUrl: realmUrl, - userId: userId, - narrow: narrow, - ); - } else { - // TODO(dart): simplify after https://github.com/dart-lang/language/issues/2537 - throw const FormatException(); - } - } - - Uri buildUrl() { - return Uri( - scheme: 'zulip', - host: 'notification', - queryParameters: { - 'realm_url': realmUrl.toString(), - 'user_id': userId.toString(), - ...(switch (narrow) { - TopicNarrow(streamId: var channelId, :var topic) => { - 'narrow_type': 'topic', - 'channel_id': channelId.toString(), - 'topic': topic.apiName, - }, - DmNarrow(:var allRecipientIds) => { - 'narrow_type': 'dm', - 'all_recipient_ids': allRecipientIds.join(','), - }, - _ => throw UnsupportedError('Found an unexpected Narrow of type ${narrow.runtimeType}.'), - }) - }, - ); - } -} diff --git a/lib/notifications/open.dart b/lib/notifications/open.dart new file mode 100644 index 0000000000..d087109d17 --- /dev/null +++ b/lib/notifications/open.dart @@ -0,0 +1,85 @@ +import '../api/model/model.dart'; +import '../model/narrow.dart'; + +/// The information contained in 'zulip://notification/…' internal +/// Android intent data URL, used for notification-open flow. +class NotificationOpenPayload { + final Uri realmUrl; + final int userId; + final Narrow narrow; + + NotificationOpenPayload({ + required this.realmUrl, + required this.userId, + required this.narrow, + }); + + factory NotificationOpenPayload.parseUrl(Uri url) { + if (url case Uri( + scheme: 'zulip', + host: 'notification', + queryParameters: { + 'realm_url': var realmUrlStr, + 'user_id': var userIdStr, + 'narrow_type': var narrowType, + // In case of narrowType == 'topic': + // 'channel_id' and 'topic' handled below. + + // In case of narrowType == 'dm': + // 'all_recipient_ids' handled below. + }, + )) { + final realmUrl = Uri.parse(realmUrlStr); + final userId = int.parse(userIdStr, radix: 10); + + final Narrow narrow; + switch (narrowType) { + case 'topic': + final channelIdStr = url.queryParameters['channel_id']!; + final channelId = int.parse(channelIdStr, radix: 10); + final topicStr = url.queryParameters['topic']!; + narrow = TopicNarrow(channelId, TopicName(topicStr)); + case 'dm': + final allRecipientIdsStr = url.queryParameters['all_recipient_ids']!; + final allRecipientIds = allRecipientIdsStr.split(',') + .map((idStr) => int.parse(idStr, radix: 10)) + .toList(growable: false); + narrow = DmNarrow(allRecipientIds: allRecipientIds, selfUserId: userId); + default: + throw const FormatException(); + } + + return NotificationOpenPayload( + realmUrl: realmUrl, + userId: userId, + narrow: narrow, + ); + } else { + // TODO(dart): simplify after https://github.com/dart-lang/language/issues/2537 + throw const FormatException(); + } + } + + Uri buildUrl() { + return Uri( + scheme: 'zulip', + host: 'notification', + queryParameters: { + 'realm_url': realmUrl.toString(), + 'user_id': userId.toString(), + ...(switch (narrow) { + TopicNarrow(streamId: var channelId, :var topic) => { + 'narrow_type': 'topic', + 'channel_id': channelId.toString(), + 'topic': topic.apiName, + }, + DmNarrow(:var allRecipientIds) => { + 'narrow_type': 'dm', + 'all_recipient_ids': allRecipientIds.join(','), + }, + _ => throw UnsupportedError('Found an unexpected Narrow of type ${narrow.runtimeType}.'), + }) + }, + ); + } +} diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index ffb5d345d8..b991d8bfec 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -18,6 +18,7 @@ import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/notifications/display.dart'; +import 'package:zulip/notifications/open.dart'; import 'package:zulip/notifications/receive.dart'; import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/color.dart'; @@ -29,8 +30,6 @@ import 'package:zulip/widgets/theme.dart'; import '../example_data.dart' as eg; import '../fake_async.dart'; import '../model/binding.dart'; -import '../model/narrow_checks.dart'; -import '../stdlib_checks.dart'; import '../test_images.dart'; import '../test_navigation.dart'; import '../widgets/dialog_checks.dart'; @@ -1255,202 +1254,6 @@ void main() { matchesNavigation(check(pushedRoutes).single, accountB, message); }); }); - - group('NotificationOpenPayload', () { - test('smoke round-trip', () { - // DM narrow - var payload = NotificationOpenPayload( - realmUrl: Uri.parse('http://chat.example'), - userId: 1001, - narrow: DmNarrow(allRecipientIds: [1001, 1002], selfUserId: 1001), - ); - var url = payload.buildUrl(); - check(NotificationOpenPayload.parseUrl(url)) - ..realmUrl.equals(payload.realmUrl) - ..userId.equals(payload.userId) - ..narrow.equals(payload.narrow); - - // Topic narrow - payload = NotificationOpenPayload( - realmUrl: Uri.parse('http://chat.example'), - userId: 1001, - narrow: eg.topicNarrow(1, 'topic A'), - ); - url = payload.buildUrl(); - check(NotificationOpenPayload.parseUrl(url)) - ..realmUrl.equals(payload.realmUrl) - ..userId.equals(payload.userId) - ..narrow.equals(payload.narrow); - }); - - test('buildUrl: smoke DM', () { - final url = NotificationOpenPayload( - realmUrl: Uri.parse('http://chat.example'), - userId: 1001, - narrow: DmNarrow(allRecipientIds: [1001, 1002], selfUserId: 1001), - ).buildUrl(); - check(url) - ..scheme.equals('zulip') - ..host.equals('notification') - ..queryParameters.deepEquals({ - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'dm', - 'all_recipient_ids': '1001,1002', - }); - }); - - test('buildUrl: smoke topic', () { - final url = NotificationOpenPayload( - realmUrl: Uri.parse('http://chat.example'), - userId: 1001, - narrow: eg.topicNarrow(1, 'topic A'), - ).buildUrl(); - check(url) - ..scheme.equals('zulip') - ..host.equals('notification') - ..queryParameters.deepEquals({ - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }); - }); - - test('parse: smoke DM', () { - final url = Uri( - scheme: 'zulip', - host: 'notification', - queryParameters: { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'dm', - 'all_recipient_ids': '1001,1002', - }); - check(NotificationOpenPayload.parseUrl(url)) - ..realmUrl.equals(Uri.parse('http://chat.example')) - ..userId.equals(1001) - ..narrow.which((it) => it.isA() - ..allRecipientIds.deepEquals([1001, 1002]) - ..otherRecipientIds.deepEquals([1002])); - }); - - test('parse: smoke topic', () { - final url = Uri( - scheme: 'zulip', - host: 'notification', - queryParameters: { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }); - check(NotificationOpenPayload.parseUrl(url)) - ..realmUrl.equals(Uri.parse('http://chat.example')) - ..userId.equals(1001) - ..narrow.which((it) => it.isA() - ..streamId.equals(1) - ..topic.equals(eg.t('topic A'))); - }); - - test('parse: fails when missing any expected query parameters', () { - final testCases = >[ - { - // 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }, - { - 'realm_url': 'http://chat.example', - // 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }, - { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - // 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }, - { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - // 'channel_id': '1', - 'topic': 'topic A', - }, - { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - // 'topic': 'topic A', - }, - { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - // 'narrow_type': 'dm', - 'all_recipient_ids': '1001,1002', - }, - { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'dm', - // 'all_recipient_ids': '1001,1002', - }, - ]; - for (final params in testCases) { - check(() => NotificationOpenPayload.parseUrl(Uri( - scheme: 'zulip', - host: 'notification', - queryParameters: params, - ))) - // Missing 'realm_url', 'user_id' and 'narrow_type' - // throws 'FormatException'. - // Missing 'channel_id', 'topic', when narrow_type == 'topic' - // throws 'TypeError'. - // Missing 'all_recipient_ids', when narrow_type == 'dm' - // throws 'TypeError'. - .throws(); - } - }); - - test('parse: fails when scheme is not "zulip"', () { - final url = Uri( - scheme: 'http', - host: 'notification', - queryParameters: { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }); - check(() => NotificationOpenPayload.parseUrl(url)) - .throws(); - }); - - test('parse: fails when host is not "notification"', () { - final url = Uri( - scheme: 'zulip', - host: 'example', - queryParameters: { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }); - check(() => NotificationOpenPayload.parseUrl(url)) - .throws(); - }); - }); } extension on Subject { @@ -1530,9 +1333,3 @@ extension on Subject { Subject get notification => has((x) => x.notification, 'notification'); Subject get tag => has((x) => x.tag, 'tag'); } - -extension on Subject { - Subject get realmUrl => has((x) => x.realmUrl, 'realmUrl'); - Subject get userId => has((x) => x.userId, 'userId'); - Subject get narrow => has((x) => x.narrow, 'narrow'); -} diff --git a/test/notifications/open_test.dart b/test/notifications/open_test.dart new file mode 100644 index 0000000000..c9960118b6 --- /dev/null +++ b/test/notifications/open_test.dart @@ -0,0 +1,212 @@ +import 'package:checks/checks.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/model/narrow.dart'; +import 'package:zulip/notifications/open.dart'; + +import '../example_data.dart' as eg; +import '../model/narrow_checks.dart'; +import '../stdlib_checks.dart'; + +void main() { + group('NotificationOpenPayload', () { + test('smoke round-trip', () { + // DM narrow + var payload = NotificationOpenPayload( + realmUrl: Uri.parse('http://chat.example'), + userId: 1001, + narrow: DmNarrow(allRecipientIds: [1001, 1002], selfUserId: 1001), + ); + var url = payload.buildUrl(); + check(NotificationOpenPayload.parseUrl(url)) + ..realmUrl.equals(payload.realmUrl) + ..userId.equals(payload.userId) + ..narrow.equals(payload.narrow); + + // Topic narrow + payload = NotificationOpenPayload( + realmUrl: Uri.parse('http://chat.example'), + userId: 1001, + narrow: eg.topicNarrow(1, 'topic A'), + ); + url = payload.buildUrl(); + check(NotificationOpenPayload.parseUrl(url)) + ..realmUrl.equals(payload.realmUrl) + ..userId.equals(payload.userId) + ..narrow.equals(payload.narrow); + }); + + test('buildUrl: smoke DM', () { + final url = NotificationOpenPayload( + realmUrl: Uri.parse('http://chat.example'), + userId: 1001, + narrow: DmNarrow(allRecipientIds: [1001, 1002], selfUserId: 1001), + ).buildUrl(); + check(url) + ..scheme.equals('zulip') + ..host.equals('notification') + ..queryParameters.deepEquals({ + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'dm', + 'all_recipient_ids': '1001,1002', + }); + }); + + test('buildUrl: smoke topic', () { + final url = NotificationOpenPayload( + realmUrl: Uri.parse('http://chat.example'), + userId: 1001, + narrow: eg.topicNarrow(1, 'topic A'), + ).buildUrl(); + check(url) + ..scheme.equals('zulip') + ..host.equals('notification') + ..queryParameters.deepEquals({ + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }); + }); + + test('parse: smoke DM', () { + final url = Uri( + scheme: 'zulip', + host: 'notification', + queryParameters: { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'dm', + 'all_recipient_ids': '1001,1002', + }); + check(NotificationOpenPayload.parseUrl(url)) + ..realmUrl.equals(Uri.parse('http://chat.example')) + ..userId.equals(1001) + ..narrow.which((it) => it.isA() + ..allRecipientIds.deepEquals([1001, 1002]) + ..otherRecipientIds.deepEquals([1002])); + }); + + test('parse: smoke topic', () { + final url = Uri( + scheme: 'zulip', + host: 'notification', + queryParameters: { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }); + check(NotificationOpenPayload.parseUrl(url)) + ..realmUrl.equals(Uri.parse('http://chat.example')) + ..userId.equals(1001) + ..narrow.which((it) => it.isA() + ..streamId.equals(1) + ..topic.equals(eg.t('topic A'))); + }); + + test('parse: fails when missing any expected query parameters', () { + final testCases = >[ + { + // 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }, + { + 'realm_url': 'http://chat.example', + // 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }, + { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + // 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }, + { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + // 'channel_id': '1', + 'topic': 'topic A', + }, + { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + // 'topic': 'topic A', + }, + { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + // 'narrow_type': 'dm', + 'all_recipient_ids': '1001,1002', + }, + { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'dm', + // 'all_recipient_ids': '1001,1002', + }, + ]; + for (final params in testCases) { + check(() => NotificationOpenPayload.parseUrl(Uri( + scheme: 'zulip', + host: 'notification', + queryParameters: params, + ))) + // Missing 'realm_url', 'user_id' and 'narrow_type' + // throws 'FormatException'. + // Missing 'channel_id', 'topic', when narrow_type == 'topic' + // throws 'TypeError'. + // Missing 'all_recipient_ids', when narrow_type == 'dm' + // throws 'TypeError'. + .throws(); + } + }); + + test('parse: fails when scheme is not "zulip"', () { + final url = Uri( + scheme: 'http', + host: 'notification', + queryParameters: { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }); + check(() => NotificationOpenPayload.parseUrl(url)) + .throws(); + }); + + test('parse: fails when host is not "notification"', () { + final url = Uri( + scheme: 'zulip', + host: 'example', + queryParameters: { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }); + check(() => NotificationOpenPayload.parseUrl(url)) + .throws(); + }); + }); +} + +extension on Subject { + Subject get realmUrl => has((x) => x.realmUrl, 'realmUrl'); + Subject get userId => has((x) => x.userId, 'userId'); + Subject get narrow => has((x) => x.narrow, 'narrow'); +} From cd147ec58d8806cd1c5ab9540404f89c30a80a9e Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Wed, 28 May 2025 04:32:08 +0530 Subject: [PATCH 09/16] notif [nfc]: Introduce NotificationOpenService And move the notification navigation data parsing utilities to the new class. --- lib/notifications/display.dart | 63 ------- lib/notifications/open.dart | 75 ++++++++ lib/widgets/app.dart | 6 +- test/notifications/display_test.dart | 231 ------------------------ test/notifications/open_test.dart | 253 +++++++++++++++++++++++++++ 5 files changed, 331 insertions(+), 297 deletions(-) diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index ab0f88138c..5b73c0b05c 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -3,23 +3,16 @@ import 'dart:io'; import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; -import 'package:flutter/widgets.dart' hide Notification; import 'package:http/http.dart' as http; import '../api/model/model.dart'; import '../api/notifications.dart'; -import '../generated/l10n/zulip_localizations.dart'; import '../host/android_notifications.dart'; import '../log.dart'; import '../model/binding.dart'; import '../model/localizations.dart'; import '../model/narrow.dart'; -import '../widgets/app.dart'; import '../widgets/color.dart'; -import '../widgets/dialog.dart'; -import '../widgets/message_list.dart'; -import '../widgets/page.dart'; -import '../widgets/store.dart'; import '../widgets/theme.dart'; import 'open.dart'; @@ -482,62 +475,6 @@ class NotificationDisplayManager { static String _personKey(Uri realmUrl, int userId) => "$realmUrl|$userId"; - /// Provides the route and the account ID by parsing the notification URL. - /// - /// The URL must have been generated using [NotificationOpenPayload.buildUrl] - /// while creating the notification. - /// - /// Returns null and shows an error dialog if the associated account is not - /// found in the global store. - static AccountRoute? routeForNotification({ - required BuildContext context, - required Uri url, - }) { - assert(defaultTargetPlatform == TargetPlatform.android); - - final globalStore = GlobalStoreWidget.of(context); - - assert(debugLog('got notif: url: $url')); - assert(url.scheme == 'zulip' && url.host == 'notification'); - final payload = NotificationOpenPayload.parseUrl(url); - - final account = globalStore.accounts.firstWhereOrNull( - (account) => account.realmUrl.origin == payload.realmUrl.origin - && account.userId == payload.userId); - if (account == null) { // TODO(log) - final zulipLocalizations = ZulipLocalizations.of(context); - showErrorDialog(context: context, - title: zulipLocalizations.errorNotificationOpenTitle, - message: zulipLocalizations.errorNotificationOpenAccountNotFound); - return null; - } - - return MessageListPage.buildRoute( - accountId: account.id, - // TODO(#82): Open at specific message, not just conversation - narrow: payload.narrow); - } - - /// Navigates to the [MessageListPage] of the specific conversation - /// given the `zulip://notification/…` Android intent data URL, - /// generated with [NotificationOpenPayload.buildUrl] while creating - /// the notification. - static Future navigateForNotification(Uri url) async { - assert(defaultTargetPlatform == TargetPlatform.android); - assert(debugLog('opened notif: url: $url')); - - 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 route = routeForNotification(context: context, url: url); - if (route == null) return; // TODO(log) - - // TODO(nav): Better interact with existing nav stack on notif open - unawaited(navigator.push(route)); - } - static Future _fetchBitmap(Uri url) async { try { // TODO timeout to prevent waiting indefinitely diff --git a/lib/notifications/open.dart b/lib/notifications/open.dart index d087109d17..bb3796512a 100644 --- a/lib/notifications/open.dart +++ b/lib/notifications/open.dart @@ -1,5 +1,80 @@ +import 'dart:async'; + +import 'package:collection/collection.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/widgets.dart'; + import '../api/model/model.dart'; +import '../generated/l10n/zulip_localizations.dart'; +import '../log.dart'; import '../model/narrow.dart'; +import '../widgets/app.dart'; +import '../widgets/dialog.dart'; +import '../widgets/message_list.dart'; +import '../widgets/page.dart'; +import '../widgets/store.dart'; + +/// Responds to the user opening a notification. +class NotificationOpenService { + + /// Provides the route and the account ID by parsing the notification URL. + /// + /// The URL must have been generated using [NotificationOpenPayload.buildUrl] + /// while creating the notification. + /// + /// Returns null and shows an error dialog if the associated account is not + /// found in the global store. + /// + /// The context argument should be a descendant of the app's main [Navigator]. + static AccountRoute? routeForNotification({ + required BuildContext context, + required Uri url, + }) { + assert(defaultTargetPlatform == TargetPlatform.android); + + final globalStore = GlobalStoreWidget.of(context); + + assert(debugLog('got notif: url: $url')); + assert(url.scheme == 'zulip' && url.host == 'notification'); + final payload = NotificationOpenPayload.parseUrl(url); + + final account = globalStore.accounts.firstWhereOrNull( + (account) => account.realmUrl.origin == payload.realmUrl.origin + && account.userId == payload.userId); + if (account == null) { // TODO(log) + final zulipLocalizations = ZulipLocalizations.of(context); + showErrorDialog(context: context, + title: zulipLocalizations.errorNotificationOpenTitle, + message: zulipLocalizations.errorNotificationOpenAccountNotFound); + return null; + } + + return MessageListPage.buildRoute( + accountId: account.id, + // TODO(#82): Open at specific message, not just conversation + narrow: payload.narrow); + } + + /// Navigates to the [MessageListPage] of the specific conversation + /// given the `zulip://notification/…` Android intent data URL, + /// generated with [NotificationOpenPayload.buildUrl] while creating + /// the notification. + static Future navigateForNotification(Uri url) async { + assert(defaultTargetPlatform == TargetPlatform.android); + assert(debugLog('opened notif: url: $url')); + + 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 route = routeForNotification(context: context, url: url); + if (route == null) return; // TODO(log) + + // TODO(nav): Better interact with existing nav stack on notif open + unawaited(navigator.push(route)); + } +} /// The information contained in 'zulip://notification/…' internal /// Android intent data URL, used for notification-open flow. diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 54ba92588b..8017bf67b0 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -9,7 +9,7 @@ import '../log.dart'; import '../model/actions.dart'; import '../model/localizations.dart'; import '../model/store.dart'; -import '../notifications/display.dart'; +import '../notifications/open.dart'; import 'about_zulip.dart'; import 'dialog.dart'; import 'home.dart'; @@ -176,7 +176,7 @@ class _ZulipAppState extends State with WidgetsBindingObserver { final initialRouteUrl = Uri.tryParse(initialRoute); if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) { - final route = NotificationDisplayManager.routeForNotification( + final route = NotificationOpenService.routeForNotification( context: context, url: initialRouteUrl); @@ -209,7 +209,7 @@ class _ZulipAppState extends State with WidgetsBindingObserver { await LoginPage.handleWebAuthUrl(url); return true; case Uri(scheme: 'zulip', host: 'notification') && var url: - await NotificationDisplayManager.navigateForNotification(url); + await NotificationOpenService.navigateForNotification(url); return true; } return super.didPushRouteInformation(routeInformation); diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index b991d8bfec..3cbb96308c 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -1,4 +1,3 @@ -import 'dart:async'; import 'dart:io'; import 'dart:typed_data'; @@ -6,7 +5,6 @@ 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/material.dart' hide Notification; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart' as http_testing; @@ -20,21 +18,13 @@ import 'package:zulip/model/store.dart'; import 'package:zulip/notifications/display.dart'; import 'package:zulip/notifications/open.dart'; import 'package:zulip/notifications/receive.dart'; -import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/color.dart'; -import 'package:zulip/widgets/home.dart'; -import 'package:zulip/widgets/message_list.dart'; -import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/theme.dart'; import '../example_data.dart' as eg; import '../fake_async.dart'; import '../model/binding.dart'; import '../test_images.dart'; -import '../test_navigation.dart'; -import '../widgets/dialog_checks.dart'; -import '../widgets/message_list_checks.dart'; -import '../widgets/page_checks.dart'; MessageFcmMessage messageFcmMessage( Message zulipMessage, { @@ -1033,227 +1023,6 @@ void main() { check(testBinding.androidNotificationHost.activeNotifications).isEmpty(); }))); }); - - group('NotificationDisplayManager open', () { - late List> pushedRoutes; - - void takeStartingRoutes({Account? account, bool withAccount = true}) { - account ??= eg.selfAccount; - final expected = >[ - if (withAccount) - (it) => it.isA() - ..accountId.equals(account!.id) - ..page.isA() - else - (it) => it.isA().page.isA(), - ]; - check(pushedRoutes.take(expected.length)).deepEquals(expected); - pushedRoutes.removeRange(0, expected.length); - } - - Future prepare(WidgetTester tester, - {bool early = false, bool withAccount = true}) async { - await init(addSelfAccount: false); - pushedRoutes = []; - final testNavObserver = TestNavigatorObserver() - ..onPushed = (route, prevRoute) => pushedRoutes.add(route); - // This uses [ZulipApp] instead of [TestZulipApp] because notification - // logic uses `await ZulipApp.navigator`. - await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); - if (early) { - check(pushedRoutes).isEmpty(); - return; - } - await tester.pump(); - takeStartingRoutes(withAccount: withAccount); - check(pushedRoutes).isEmpty(); - } - - Future openNotification(WidgetTester tester, Account account, Message message) async { - final data = messageFcmMessage(message, account: account); - final intentDataUrl = NotificationOpenPayload( - realmUrl: data.realmUrl, - userId: data.userId, - narrow: switch (data.recipient) { - FcmMessageChannelRecipient(:var streamId, :var topic) => - TopicNarrow(streamId, topic), - FcmMessageDmRecipient(:var allRecipientIds) => - DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildUrl(); - unawaited( - WidgetsBinding.instance.handlePushRoute(intentDataUrl.toString())); - await tester.idle(); // let navigateForNotification find navigator - } - - void matchesNavigation(Subject> route, Account account, Message message) { - route.isA() - ..accountId.equals(account.id) - ..page.isA() - .initNarrow.equals(SendableNarrow.ofMessage(message, - selfUserId: account.userId)); - } - - Future checkOpenNotification(WidgetTester tester, Account account, Message message) async { - await openNotification(tester, account, message); - matchesNavigation(check(pushedRoutes).single, account, message); - pushedRoutes.clear(); - } - - testWidgets('stream message', (tester) async { - addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - await prepare(tester); - await checkOpenNotification(tester, eg.selfAccount, eg.streamMessage()); - }); - - testWidgets('direct message', (tester) async { - addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - await prepare(tester); - await checkOpenNotification(tester, eg.selfAccount, - eg.dmMessage(from: eg.otherUser, to: [eg.selfUser])); - }); - - testWidgets('account queried by realmUrl origin component', (tester) async { - addTearDown(testBinding.reset); - await testBinding.globalStore.add( - eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example')), - eg.initialSnapshot()); - await prepare(tester); - - await checkOpenNotification(tester, - eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example/')), - eg.streamMessage()); - await checkOpenNotification(tester, - eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example')), - eg.streamMessage()); - }); - - testWidgets('no accounts', (tester) async { - await prepare(tester, withAccount: false); - await openNotification(tester, eg.selfAccount, eg.streamMessage()); - await tester.pump(); - check(pushedRoutes.single).isA>(); - await tester.tap(find.byWidget(checkErrorDialog(tester, - expectedTitle: zulipLocalizations.errorNotificationOpenTitle, - expectedMessage: zulipLocalizations.errorNotificationOpenAccountNotFound))); - }); - - testWidgets('mismatching account', (tester) async { - addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - await prepare(tester); - await openNotification(tester, eg.otherAccount, eg.streamMessage()); - await tester.pump(); - check(pushedRoutes.single).isA>(); - await tester.tap(find.byWidget(checkErrorDialog(tester, - expectedTitle: zulipLocalizations.errorNotificationOpenTitle, - expectedMessage: zulipLocalizations.errorNotificationOpenAccountNotFound))); - }); - - testWidgets('find account among several', (tester) async { - addTearDown(testBinding.reset); - final realmUrlA = Uri.parse('https://a-chat.example/'); - final realmUrlB = Uri.parse('https://chat-b.example/'); - final user1 = eg.user(); - final user2 = eg.user(); - final accounts = [ - eg.account(id: 1001, realmUrl: realmUrlA, user: user1), - eg.account(id: 1002, realmUrl: realmUrlA, user: user2), - eg.account(id: 1003, realmUrl: realmUrlB, user: user1), - eg.account(id: 1004, realmUrl: realmUrlB, user: user2), - ]; - for (final account in accounts) { - await testBinding.globalStore.add(account, eg.initialSnapshot()); - } - await prepare(tester); - - await checkOpenNotification(tester, accounts[0], eg.streamMessage()); - await checkOpenNotification(tester, accounts[1], eg.streamMessage()); - await checkOpenNotification(tester, accounts[2], eg.streamMessage()); - await checkOpenNotification(tester, accounts[3], eg.streamMessage()); - }); - - testWidgets('wait for app to become ready', (tester) async { - addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - await prepare(tester, early: true); - final message = eg.streamMessage(); - await openNotification(tester, eg.selfAccount, message); - // The app should still not be ready (or else this test won't work right). - check(ZulipApp.ready.value).isFalse(); - check(ZulipApp.navigatorKey.currentState).isNull(); - // And the openNotification hasn't caused any navigation yet. - check(pushedRoutes).isEmpty(); - - // Now let the GlobalStore get loaded and the app's main UI get mounted. - await tester.pump(); - // The navigator first pushes the starting routes… - takeStartingRoutes(); - // … and then the one the notification leads to. - matchesNavigation(check(pushedRoutes).single, eg.selfAccount, message); - }); - - testWidgets('at app launch', (tester) async { - addTearDown(testBinding.reset); - // Set up a value for `PlatformDispatcher.defaultRouteName` to return, - // for determining the intial route. - final account = eg.selfAccount; - final message = eg.streamMessage(); - final data = messageFcmMessage(message, account: account); - final intentDataUrl = NotificationOpenPayload( - realmUrl: data.realmUrl, - userId: data.userId, - narrow: switch (data.recipient) { - FcmMessageChannelRecipient(:var streamId, :var topic) => - TopicNarrow(streamId, topic), - FcmMessageDmRecipient(:var allRecipientIds) => - DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildUrl(); - addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); - tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); - - // Now start the app. - await testBinding.globalStore.add(account, eg.initialSnapshot()); - await prepare(tester, early: true); - check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet - - // Once the app is ready, we navigate to the conversation. - await tester.pump(); - takeStartingRoutes(); - matchesNavigation(check(pushedRoutes).single, account, message); - }); - - testWidgets('uses associated account as initial account; if initial route', (tester) async { - addTearDown(testBinding.reset); - - final accountA = eg.selfAccount; - final accountB = eg.otherAccount; - final message = eg.streamMessage(); - final data = messageFcmMessage(message, account: accountB); - await testBinding.globalStore.add(accountA, eg.initialSnapshot()); - await testBinding.globalStore.add(accountB, eg.initialSnapshot()); - - final intentDataUrl = NotificationOpenPayload( - realmUrl: data.realmUrl, - userId: data.userId, - narrow: switch (data.recipient) { - FcmMessageChannelRecipient(:var streamId, :var topic) => - TopicNarrow(streamId, topic), - FcmMessageDmRecipient(:var allRecipientIds) => - DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildUrl(); - addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); - tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); - - await prepare(tester, early: true); - check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet - - await tester.pump(); - takeStartingRoutes(account: accountB); - matchesNavigation(check(pushedRoutes).single, accountB, message); - }); - }); } extension on Subject { diff --git a/test/notifications/open_test.dart b/test/notifications/open_test.dart index c9960118b6..a08e999c1e 100644 --- a/test/notifications/open_test.dart +++ b/test/notifications/open_test.dart @@ -1,13 +1,266 @@ +import 'dart:async'; + import 'package:checks/checks.dart'; +import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/notifications.dart'; +import 'package:zulip/model/database.dart'; +import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/notifications/open.dart'; +import 'package:zulip/notifications/receive.dart'; +import 'package:zulip/widgets/app.dart'; +import 'package:zulip/widgets/home.dart'; +import 'package:zulip/widgets/message_list.dart'; +import 'package:zulip/widgets/page.dart'; import '../example_data.dart' as eg; +import '../model/binding.dart'; import '../model/narrow_checks.dart'; import '../stdlib_checks.dart'; +import '../test_navigation.dart'; +import '../widgets/dialog_checks.dart'; +import '../widgets/message_list_checks.dart'; +import '../widgets/page_checks.dart'; +import 'display_test.dart'; void main() { + TestZulipBinding.ensureInitialized(); + final zulipLocalizations = GlobalLocalizations.zulipLocalizations; + + Future init({bool addSelfAccount = true}) async { + if (addSelfAccount) { + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + } + addTearDown(testBinding.reset); + testBinding.firebaseMessagingInitialToken = '012abc'; + addTearDown(NotificationService.debugReset); + NotificationService.debugBackgroundIsolateIsLive = false; + await NotificationService.instance.start(); + } + + group('NotificationOpenService', () { + late List> pushedRoutes; + + void takeStartingRoutes({Account? account, bool withAccount = true}) { + account ??= eg.selfAccount; + final expected = >[ + if (withAccount) + (it) => it.isA() + ..accountId.equals(account!.id) + ..page.isA() + else + (it) => it.isA().page.isA(), + ]; + check(pushedRoutes.take(expected.length)).deepEquals(expected); + pushedRoutes.removeRange(0, expected.length); + } + + Future prepare(WidgetTester tester, + {bool early = false, bool withAccount = true}) async { + await init(addSelfAccount: false); + pushedRoutes = []; + final testNavObserver = TestNavigatorObserver() + ..onPushed = (route, prevRoute) => pushedRoutes.add(route); + // This uses [ZulipApp] instead of [TestZulipApp] because notification + // logic uses `await ZulipApp.navigator`. + await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); + if (early) { + check(pushedRoutes).isEmpty(); + return; + } + await tester.pump(); + takeStartingRoutes(withAccount: withAccount); + check(pushedRoutes).isEmpty(); + } + + Future openNotification(WidgetTester tester, Account account, Message message) async { + final data = messageFcmMessage(message, account: account); + final intentDataUrl = NotificationOpenPayload( + realmUrl: data.realmUrl, + userId: data.userId, + narrow: switch (data.recipient) { + FcmMessageChannelRecipient(:var streamId, :var topic) => + TopicNarrow(streamId, topic), + FcmMessageDmRecipient(:var allRecipientIds) => + DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), + }).buildUrl(); + unawaited( + WidgetsBinding.instance.handlePushRoute(intentDataUrl.toString())); + await tester.idle(); // let navigateForNotification find navigator + } + + void matchesNavigation(Subject> route, Account account, Message message) { + route.isA() + ..accountId.equals(account.id) + ..page.isA() + .initNarrow.equals(SendableNarrow.ofMessage(message, + selfUserId: account.userId)); + } + + Future checkOpenNotification(WidgetTester tester, Account account, Message message) async { + await openNotification(tester, account, message); + matchesNavigation(check(pushedRoutes).single, account, message); + pushedRoutes.clear(); + } + + testWidgets('stream message', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + await prepare(tester); + await checkOpenNotification(tester, eg.selfAccount, eg.streamMessage()); + }); + + testWidgets('direct message', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + await prepare(tester); + await checkOpenNotification(tester, eg.selfAccount, + eg.dmMessage(from: eg.otherUser, to: [eg.selfUser])); + }); + + testWidgets('account queried by realmUrl origin component', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add( + eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example')), + eg.initialSnapshot()); + await prepare(tester); + + await checkOpenNotification(tester, + eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example/')), + eg.streamMessage()); + await checkOpenNotification(tester, + eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example')), + eg.streamMessage()); + }); + + testWidgets('no accounts', (tester) async { + await prepare(tester, withAccount: false); + await openNotification(tester, eg.selfAccount, eg.streamMessage()); + await tester.pump(); + check(pushedRoutes.single).isA>(); + await tester.tap(find.byWidget(checkErrorDialog(tester, + expectedTitle: zulipLocalizations.errorNotificationOpenTitle, + expectedMessage: zulipLocalizations.errorNotificationOpenAccountNotFound))); + }); + + testWidgets('mismatching account', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + await prepare(tester); + await openNotification(tester, eg.otherAccount, eg.streamMessage()); + await tester.pump(); + check(pushedRoutes.single).isA>(); + await tester.tap(find.byWidget(checkErrorDialog(tester, + expectedTitle: zulipLocalizations.errorNotificationOpenTitle, + expectedMessage: zulipLocalizations.errorNotificationOpenAccountNotFound))); + }); + + testWidgets('find account among several', (tester) async { + addTearDown(testBinding.reset); + final realmUrlA = Uri.parse('https://a-chat.example/'); + final realmUrlB = Uri.parse('https://chat-b.example/'); + final user1 = eg.user(); + final user2 = eg.user(); + final accounts = [ + eg.account(id: 1001, realmUrl: realmUrlA, user: user1), + eg.account(id: 1002, realmUrl: realmUrlA, user: user2), + eg.account(id: 1003, realmUrl: realmUrlB, user: user1), + eg.account(id: 1004, realmUrl: realmUrlB, user: user2), + ]; + for (final account in accounts) { + await testBinding.globalStore.add(account, eg.initialSnapshot()); + } + await prepare(tester); + + await checkOpenNotification(tester, accounts[0], eg.streamMessage()); + await checkOpenNotification(tester, accounts[1], eg.streamMessage()); + await checkOpenNotification(tester, accounts[2], eg.streamMessage()); + await checkOpenNotification(tester, accounts[3], eg.streamMessage()); + }); + + testWidgets('wait for app to become ready', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + await prepare(tester, early: true); + final message = eg.streamMessage(); + await openNotification(tester, eg.selfAccount, message); + // The app should still not be ready (or else this test won't work right). + check(ZulipApp.ready.value).isFalse(); + check(ZulipApp.navigatorKey.currentState).isNull(); + // And the openNotification hasn't caused any navigation yet. + check(pushedRoutes).isEmpty(); + + // Now let the GlobalStore get loaded and the app's main UI get mounted. + await tester.pump(); + // The navigator first pushes the starting routes… + takeStartingRoutes(); + // … and then the one the notification leads to. + matchesNavigation(check(pushedRoutes).single, eg.selfAccount, message); + }); + + testWidgets('at app launch', (tester) async { + addTearDown(testBinding.reset); + // Set up a value for `PlatformDispatcher.defaultRouteName` to return, + // for determining the intial route. + final account = eg.selfAccount; + final message = eg.streamMessage(); + final data = messageFcmMessage(message, account: account); + final intentDataUrl = NotificationOpenPayload( + realmUrl: data.realmUrl, + userId: data.userId, + narrow: switch (data.recipient) { + FcmMessageChannelRecipient(:var streamId, :var topic) => + TopicNarrow(streamId, topic), + FcmMessageDmRecipient(:var allRecipientIds) => + DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), + }).buildUrl(); + addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); + tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); + + // Now start the app. + await testBinding.globalStore.add(account, eg.initialSnapshot()); + await prepare(tester, early: true); + check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet + + // Once the app is ready, we navigate to the conversation. + await tester.pump(); + takeStartingRoutes(); + matchesNavigation(check(pushedRoutes).single, account, message); + }); + + testWidgets('uses associated account as initial account; if initial route', (tester) async { + addTearDown(testBinding.reset); + + final accountA = eg.selfAccount; + final accountB = eg.otherAccount; + final message = eg.streamMessage(); + final data = messageFcmMessage(message, account: accountB); + await testBinding.globalStore.add(accountA, eg.initialSnapshot()); + await testBinding.globalStore.add(accountB, eg.initialSnapshot()); + + final intentDataUrl = NotificationOpenPayload( + realmUrl: data.realmUrl, + userId: data.userId, + narrow: switch (data.recipient) { + FcmMessageChannelRecipient(:var streamId, :var topic) => + TopicNarrow(streamId, topic), + FcmMessageDmRecipient(:var allRecipientIds) => + DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), + }).buildUrl(); + addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); + tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); + + await prepare(tester, early: true); + check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet + + await tester.pump(); + takeStartingRoutes(account: accountB); + matchesNavigation(check(pushedRoutes).single, accountB, message); + }); + }); + group('NotificationOpenPayload', () { test('smoke round-trip', () { // DM narrow From 529265367baaa794451cde2522181051ef3234be Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Wed, 28 May 2025 04:41:42 +0530 Subject: [PATCH 10/16] notif [nfc]: Rename NotificationOpenPayload methods To make it clear that they are Android specific. --- lib/notifications/display.dart | 2 +- lib/notifications/open.dart | 22 +- test/notifications/display_test.dart | 2 +- test/notifications/open_test.dart | 342 ++++++++++++++------------- 4 files changed, 188 insertions(+), 180 deletions(-) diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index 5b73c0b05c..0a3de1689a 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -296,7 +296,7 @@ class NotificationDisplayManager { TopicNarrow(streamId, topic), FcmMessageDmRecipient(:var allRecipientIds) => DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildUrl(); + }).buildAndroidNotificationUrl(); await _androidHost.notify( id: kNotificationId, diff --git a/lib/notifications/open.dart b/lib/notifications/open.dart index bb3796512a..40465de3c6 100644 --- a/lib/notifications/open.dart +++ b/lib/notifications/open.dart @@ -19,8 +19,9 @@ class NotificationOpenService { /// Provides the route and the account ID by parsing the notification URL. /// - /// The URL must have been generated using [NotificationOpenPayload.buildUrl] - /// while creating the notification. + /// The URL must have been generated using + /// [NotificationOpenPayload.buildAndroidNotificationUrl] while creating the + /// notification. /// /// Returns null and shows an error dialog if the associated account is not /// found in the global store. @@ -36,7 +37,7 @@ class NotificationOpenService { assert(debugLog('got notif: url: $url')); assert(url.scheme == 'zulip' && url.host == 'notification'); - final payload = NotificationOpenPayload.parseUrl(url); + final payload = NotificationOpenPayload.parseAndroidNotificationUrl(url); final account = globalStore.accounts.firstWhereOrNull( (account) => account.realmUrl.origin == payload.realmUrl.origin @@ -57,8 +58,8 @@ class NotificationOpenService { /// Navigates to the [MessageListPage] of the specific conversation /// given the `zulip://notification/…` Android intent data URL, - /// generated with [NotificationOpenPayload.buildUrl] while creating - /// the notification. + /// generated with [NotificationOpenPayload.buildAndroidNotificationUrl] + /// while creating the notification. static Future navigateForNotification(Uri url) async { assert(defaultTargetPlatform == TargetPlatform.android); assert(debugLog('opened notif: url: $url')); @@ -76,8 +77,8 @@ class NotificationOpenService { } } -/// The information contained in 'zulip://notification/…' internal -/// Android intent data URL, used for notification-open flow. +/// The data from a notification that describes what to do +/// when the user opens the notification. class NotificationOpenPayload { final Uri realmUrl; final int userId; @@ -89,7 +90,10 @@ class NotificationOpenPayload { required this.narrow, }); - factory NotificationOpenPayload.parseUrl(Uri url) { + /// Parses the internal Android notification url, that was created using + /// [buildAndroidNotificationUrl], and retrieves the information required + /// for navigation. + factory NotificationOpenPayload.parseAndroidNotificationUrl(Uri url) { if (url case Uri( scheme: 'zulip', host: 'notification', @@ -135,7 +139,7 @@ class NotificationOpenPayload { } } - Uri buildUrl() { + Uri buildAndroidNotificationUrl() { return Uri( scheme: 'zulip', host: 'notification', diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index 3cbb96308c..7fe04c73ee 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -343,7 +343,7 @@ void main() { TopicNarrow(streamId, topic), FcmMessageDmRecipient(:var allRecipientIds) => DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildUrl(); + }).buildAndroidNotificationUrl(); final messageStyleMessagesChecks = messageStyleMessages.mapIndexed((i, messageData) { diff --git a/test/notifications/open_test.dart b/test/notifications/open_test.dart index a08e999c1e..495b6b8d34 100644 --- a/test/notifications/open_test.dart +++ b/test/notifications/open_test.dart @@ -85,7 +85,7 @@ void main() { TopicNarrow(streamId, topic), FcmMessageDmRecipient(:var allRecipientIds) => DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildUrl(); + }).buildAndroidNotificationUrl(); unawaited( WidgetsBinding.instance.handlePushRoute(intentDataUrl.toString())); await tester.idle(); // let navigateForNotification find navigator @@ -215,7 +215,7 @@ void main() { TopicNarrow(streamId, topic), FcmMessageDmRecipient(:var allRecipientIds) => DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildUrl(); + }).buildAndroidNotificationUrl(); addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); @@ -248,7 +248,7 @@ void main() { TopicNarrow(streamId, topic), FcmMessageDmRecipient(:var allRecipientIds) => DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildUrl(); + }).buildAndroidNotificationUrl(); addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); @@ -269,8 +269,8 @@ void main() { userId: 1001, narrow: DmNarrow(allRecipientIds: [1001, 1002], selfUserId: 1001), ); - var url = payload.buildUrl(); - check(NotificationOpenPayload.parseUrl(url)) + var url = payload.buildAndroidNotificationUrl(); + check(NotificationOpenPayload.parseAndroidNotificationUrl(url)) ..realmUrl.equals(payload.realmUrl) ..userId.equals(payload.userId) ..narrow.equals(payload.narrow); @@ -281,179 +281,183 @@ void main() { userId: 1001, narrow: eg.topicNarrow(1, 'topic A'), ); - url = payload.buildUrl(); - check(NotificationOpenPayload.parseUrl(url)) + url = payload.buildAndroidNotificationUrl(); + check(NotificationOpenPayload.parseAndroidNotificationUrl(url)) ..realmUrl.equals(payload.realmUrl) ..userId.equals(payload.userId) ..narrow.equals(payload.narrow); }); - test('buildUrl: smoke DM', () { - final url = NotificationOpenPayload( - realmUrl: Uri.parse('http://chat.example'), - userId: 1001, - narrow: DmNarrow(allRecipientIds: [1001, 1002], selfUserId: 1001), - ).buildUrl(); - check(url) - ..scheme.equals('zulip') - ..host.equals('notification') - ..queryParameters.deepEquals({ - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'dm', - 'all_recipient_ids': '1001,1002', - }); - }); - - test('buildUrl: smoke topic', () { - final url = NotificationOpenPayload( - realmUrl: Uri.parse('http://chat.example'), - userId: 1001, - narrow: eg.topicNarrow(1, 'topic A'), - ).buildUrl(); - check(url) - ..scheme.equals('zulip') - ..host.equals('notification') - ..queryParameters.deepEquals({ - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }); - }); - - test('parse: smoke DM', () { - final url = Uri( - scheme: 'zulip', - host: 'notification', - queryParameters: { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'dm', - 'all_recipient_ids': '1001,1002', - }); - check(NotificationOpenPayload.parseUrl(url)) - ..realmUrl.equals(Uri.parse('http://chat.example')) - ..userId.equals(1001) - ..narrow.which((it) => it.isA() - ..allRecipientIds.deepEquals([1001, 1002]) - ..otherRecipientIds.deepEquals([1002])); - }); - - test('parse: smoke topic', () { - final url = Uri( - scheme: 'zulip', - host: 'notification', - queryParameters: { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }); - check(NotificationOpenPayload.parseUrl(url)) - ..realmUrl.equals(Uri.parse('http://chat.example')) - ..userId.equals(1001) - ..narrow.which((it) => it.isA() - ..streamId.equals(1) - ..topic.equals(eg.t('topic A'))); + group('buildAndroidNotificationUrl', () { + test('smoke DM', () { + final url = NotificationOpenPayload( + realmUrl: Uri.parse('http://chat.example'), + userId: 1001, + narrow: DmNarrow(allRecipientIds: [1001, 1002], selfUserId: 1001), + ).buildAndroidNotificationUrl(); + check(url) + ..scheme.equals('zulip') + ..host.equals('notification') + ..queryParameters.deepEquals({ + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'dm', + 'all_recipient_ids': '1001,1002', + }); + }); + + test('smoke topic', () { + final url = NotificationOpenPayload( + realmUrl: Uri.parse('http://chat.example'), + userId: 1001, + narrow: eg.topicNarrow(1, 'topic A'), + ).buildAndroidNotificationUrl(); + check(url) + ..scheme.equals('zulip') + ..host.equals('notification') + ..queryParameters.deepEquals({ + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }); + }); }); - test('parse: fails when missing any expected query parameters', () { - final testCases = >[ - { - // 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }, - { - 'realm_url': 'http://chat.example', - // 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }, - { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - // 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }, - { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - // 'channel_id': '1', - 'topic': 'topic A', - }, - { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - // 'topic': 'topic A', - }, - { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - // 'narrow_type': 'dm', - 'all_recipient_ids': '1001,1002', - }, - { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'dm', - // 'all_recipient_ids': '1001,1002', - }, - ]; - for (final params in testCases) { - check(() => NotificationOpenPayload.parseUrl(Uri( + group('parseAndroidNotificationUrl', () { + test('smoke DM', () { + final url = Uri( scheme: 'zulip', host: 'notification', - queryParameters: params, - ))) - // Missing 'realm_url', 'user_id' and 'narrow_type' - // throws 'FormatException'. - // Missing 'channel_id', 'topic', when narrow_type == 'topic' - // throws 'TypeError'. - // Missing 'all_recipient_ids', when narrow_type == 'dm' - // throws 'TypeError'. - .throws(); - } - }); - - test('parse: fails when scheme is not "zulip"', () { - final url = Uri( - scheme: 'http', - host: 'notification', - queryParameters: { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }); - check(() => NotificationOpenPayload.parseUrl(url)) - .throws(); - }); - - test('parse: fails when host is not "notification"', () { - final url = Uri( - scheme: 'zulip', - host: 'example', - queryParameters: { - 'realm_url': 'http://chat.example', - 'user_id': '1001', - 'narrow_type': 'topic', - 'channel_id': '1', - 'topic': 'topic A', - }); - check(() => NotificationOpenPayload.parseUrl(url)) - .throws(); + queryParameters: { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'dm', + 'all_recipient_ids': '1001,1002', + }); + check(NotificationOpenPayload.parseAndroidNotificationUrl(url)) + ..realmUrl.equals(Uri.parse('http://chat.example')) + ..userId.equals(1001) + ..narrow.which((it) => it.isA() + ..allRecipientIds.deepEquals([1001, 1002]) + ..otherRecipientIds.deepEquals([1002])); + }); + + test('smoke topic', () { + final url = Uri( + scheme: 'zulip', + host: 'notification', + queryParameters: { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }); + check(NotificationOpenPayload.parseAndroidNotificationUrl(url)) + ..realmUrl.equals(Uri.parse('http://chat.example')) + ..userId.equals(1001) + ..narrow.which((it) => it.isA() + ..streamId.equals(1) + ..topic.equals(eg.t('topic A'))); + }); + + test('fails when missing any expected query parameters', () { + final testCases = >[ + { + // 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }, + { + 'realm_url': 'http://chat.example', + // 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }, + { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + // 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }, + { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + // 'channel_id': '1', + 'topic': 'topic A', + }, + { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + // 'topic': 'topic A', + }, + { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + // 'narrow_type': 'dm', + 'all_recipient_ids': '1001,1002', + }, + { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'dm', + // 'all_recipient_ids': '1001,1002', + }, + ]; + for (final params in testCases) { + check(() => NotificationOpenPayload.parseAndroidNotificationUrl(Uri( + scheme: 'zulip', + host: 'notification', + queryParameters: params, + ))) + // Missing 'realm_url', 'user_id' and 'narrow_type' + // throws 'FormatException'. + // Missing 'channel_id', 'topic', when narrow_type == 'topic' + // throws 'TypeError'. + // Missing 'all_recipient_ids', when narrow_type == 'dm' + // throws 'TypeError'. + .throws(); + } + }); + + test('fails when scheme is not "zulip"', () { + final url = Uri( + scheme: 'http', + host: 'notification', + queryParameters: { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }); + check(() => NotificationOpenPayload.parseAndroidNotificationUrl(url)) + .throws(); + }); + + test('fails when host is not "notification"', () { + final url = Uri( + scheme: 'zulip', + host: 'example', + queryParameters: { + 'realm_url': 'http://chat.example', + 'user_id': '1001', + 'narrow_type': 'topic', + 'channel_id': '1', + 'topic': 'topic A', + }); + check(() => NotificationOpenPayload.parseAndroidNotificationUrl(url)) + .throws(); + }); }); }); } From d130717dbab2ab7c26d34d398062889dcebc209a Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Wed, 28 May 2025 05:04:36 +0530 Subject: [PATCH 11/16] notif [nfc]: Refactor NotificationOpenService.routeForNotification Update it to receive `NotificationOpenPayload` as an argument instead of the Android Notification URL. Also rename `navigateForNotification` to `navigateForAndroidNotificationUrl`, making it more explicit. --- lib/notifications/open.dart | 24 +++++++++------------- lib/widgets/app.dart | 40 ++++++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/lib/notifications/open.dart b/lib/notifications/open.dart index 40465de3c6..2a518d912f 100644 --- a/lib/notifications/open.dart +++ b/lib/notifications/open.dart @@ -17,11 +17,7 @@ import '../widgets/store.dart'; /// Responds to the user opening a notification. class NotificationOpenService { - /// Provides the route and the account ID by parsing the notification URL. - /// - /// The URL must have been generated using - /// [NotificationOpenPayload.buildAndroidNotificationUrl] while creating the - /// notification. + /// Provides the route to open by parsing the notification payload. /// /// Returns null and shows an error dialog if the associated account is not /// found in the global store. @@ -29,19 +25,15 @@ class NotificationOpenService { /// The context argument should be a descendant of the app's main [Navigator]. static AccountRoute? routeForNotification({ required BuildContext context, - required Uri url, + required NotificationOpenPayload data, }) { assert(defaultTargetPlatform == TargetPlatform.android); final globalStore = GlobalStoreWidget.of(context); - assert(debugLog('got notif: url: $url')); - assert(url.scheme == 'zulip' && url.host == 'notification'); - final payload = NotificationOpenPayload.parseAndroidNotificationUrl(url); - final account = globalStore.accounts.firstWhereOrNull( - (account) => account.realmUrl.origin == payload.realmUrl.origin - && account.userId == payload.userId); + (account) => account.realmUrl.origin == data.realmUrl.origin + && account.userId == data.userId); if (account == null) { // TODO(log) final zulipLocalizations = ZulipLocalizations.of(context); showErrorDialog(context: context, @@ -53,14 +45,14 @@ class NotificationOpenService { return MessageListPage.buildRoute( accountId: account.id, // TODO(#82): Open at specific message, not just conversation - narrow: payload.narrow); + narrow: data.narrow); } /// Navigates to the [MessageListPage] of the specific conversation /// given the `zulip://notification/…` Android intent data URL, /// generated with [NotificationOpenPayload.buildAndroidNotificationUrl] /// while creating the notification. - static Future navigateForNotification(Uri url) async { + static Future navigateForAndroidNotificationUrl(Uri url) async { assert(defaultTargetPlatform == TargetPlatform.android); assert(debugLog('opened notif: url: $url')); @@ -69,7 +61,9 @@ class NotificationOpenService { 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 route = routeForNotification(context: context, url: url); + assert(url.scheme == 'zulip' && url.host == 'notification'); + final data = NotificationOpenPayload.parseAndroidNotificationUrl(url); + final route = routeForNotification(context: context, data: data); if (route == null) return; // TODO(log) // TODO(nav): Better interact with existing nav stack on notif open diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 8017bf67b0..22ebadba03 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -168,27 +168,35 @@ class _ZulipAppState extends State with WidgetsBindingObserver { super.dispose(); } + AccountRoute? _initialRouteAndroid( + BuildContext context, + String initialRoute, + ) { + final initialRouteUrl = Uri.tryParse(initialRoute); + if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) { + assert(debugLog('got notif: url: $initialRouteUrl')); + final data = + NotificationOpenPayload.parseAndroidNotificationUrl(initialRouteUrl); + return NotificationOpenService.routeForNotification( + context: context, + data: data); + } + + return null; + } + List> _handleGenerateInitialRoutes(String initialRoute) { // The `_ZulipAppState.context` lacks the required ancestors. Instead // we use the Navigator which should be available when this callback is // called and it's context should have the required ancestors. final context = ZulipApp.navigatorKey.currentContext!; - final initialRouteUrl = Uri.tryParse(initialRoute); - if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) { - final route = NotificationOpenService.routeForNotification( - context: context, - url: initialRouteUrl); - - if (route != null) { - return [ - HomePage.buildRoute(accountId: route.accountId), - route, - ]; - } else { - // The account didn't match any existing accounts, - // fall through to show the default route below. - } + final route = _initialRouteAndroid(context, initialRoute); + if (route != null) { + return [ + HomePage.buildRoute(accountId: route.accountId), + route, + ]; } final globalStore = GlobalStoreWidget.of(context); @@ -209,7 +217,7 @@ class _ZulipAppState extends State with WidgetsBindingObserver { await LoginPage.handleWebAuthUrl(url); return true; case Uri(scheme: 'zulip', host: 'notification') && var url: - await NotificationOpenService.navigateForNotification(url); + await NotificationOpenService.navigateForAndroidNotificationUrl(url); return true; } return super.didPushRouteInformation(routeInformation); From b2e0ed07f0138e3168a9db4ff3fbe7213a8459b0 Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Wed, 28 May 2025 05:10:25 +0530 Subject: [PATCH 12/16] notif: Show a dialog if received malformed Android Notification URL --- lib/notifications/open.dart | 18 +++++++++++++++++- lib/widgets/app.dart | 6 ++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/notifications/open.dart b/lib/notifications/open.dart index 2a518d912f..5bf0f0f772 100644 --- a/lib/notifications/open.dart +++ b/lib/notifications/open.dart @@ -62,13 +62,29 @@ class NotificationOpenService { 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 assert(url.scheme == 'zulip' && url.host == 'notification'); - final data = NotificationOpenPayload.parseAndroidNotificationUrl(url); + final data = tryParseAndroidNotificationUrl(context: context, url: url); + if (data == null) return; // TODO(log) final route = routeForNotification(context: context, data: data); if (route == null) return; // TODO(log) // TODO(nav): Better interact with existing nav stack on notif open unawaited(navigator.push(route)); } + + static NotificationOpenPayload? tryParseAndroidNotificationUrl({ + required BuildContext context, + required Uri url, + }) { + try { + return NotificationOpenPayload.parseAndroidNotificationUrl(url); + } on FormatException catch (e, st) { + assert(debugLog('$e\n$st')); + final zulipLocalizations = ZulipLocalizations.of(context); + showErrorDialog(context: context, + title: zulipLocalizations.errorNotificationOpenTitle); + return null; + } + } } /// The data from a notification that describes what to do diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 22ebadba03..96c546bd3f 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -175,8 +175,10 @@ class _ZulipAppState extends State with WidgetsBindingObserver { final initialRouteUrl = Uri.tryParse(initialRoute); if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) { assert(debugLog('got notif: url: $initialRouteUrl')); - final data = - NotificationOpenPayload.parseAndroidNotificationUrl(initialRouteUrl); + final data = NotificationOpenService.tryParseAndroidNotificationUrl( + context: context, + url: initialRouteUrl); + if (data == null) return null; // TODO(log) return NotificationOpenService.routeForNotification( context: context, data: data); From f12f91199d9f65e47dd7669e991218ea22f0f203 Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Wed, 28 May 2025 23:21:45 +0530 Subject: [PATCH 13/16] notif test: Refactor tests for NotificationOpenService --- test/notifications/open_test.dart | 103 +++++++++++++----------------- 1 file changed, 45 insertions(+), 58 deletions(-) diff --git a/test/notifications/open_test.dart b/test/notifications/open_test.dart index 495b6b8d34..3122fc154a 100644 --- a/test/notifications/open_test.dart +++ b/test/notifications/open_test.dart @@ -29,10 +29,7 @@ void main() { TestZulipBinding.ensureInitialized(); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; - Future init({bool addSelfAccount = true}) async { - if (addSelfAccount) { - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - } + Future init() async { addTearDown(testBinding.reset); testBinding.firebaseMessagingInitialToken = '012abc'; addTearDown(NotificationService.debugReset); @@ -43,12 +40,11 @@ void main() { group('NotificationOpenService', () { late List> pushedRoutes; - void takeStartingRoutes({Account? account, bool withAccount = true}) { - account ??= eg.selfAccount; + void takeStartingRoutes({Account? account}) { final expected = >[ - if (withAccount) + if (account != null) (it) => it.isA() - ..accountId.equals(account!.id) + ..accountId.equals(account.id) ..page.isA() else (it) => it.isA().page.isA(), @@ -57,27 +53,35 @@ void main() { pushedRoutes.removeRange(0, expected.length); } - Future prepare(WidgetTester tester, - {bool early = false, bool withAccount = true}) async { - await init(addSelfAccount: false); + Future prepare( + WidgetTester tester, { + bool dropStartingRoutes = true, + Account? account, + bool withAccount = true, + }) async { + if (withAccount) { + account ??= eg.selfAccount; + await testBinding.globalStore.add(account, eg.initialSnapshot()); + } + await init(); pushedRoutes = []; final testNavObserver = TestNavigatorObserver() ..onPushed = (route, prevRoute) => pushedRoutes.add(route); // This uses [ZulipApp] instead of [TestZulipApp] because notification // logic uses `await ZulipApp.navigator`. await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); - if (early) { + if (!dropStartingRoutes) { check(pushedRoutes).isEmpty(); return; } await tester.pump(); - takeStartingRoutes(withAccount: withAccount); + takeStartingRoutes(account: account); check(pushedRoutes).isEmpty(); } - Future openNotification(WidgetTester tester, Account account, Message message) async { + Uri androidNotificationUrlForMessage(Account account, Message message) { final data = messageFcmMessage(message, account: account); - final intentDataUrl = NotificationOpenPayload( + return NotificationOpenPayload( realmUrl: data.realmUrl, userId: data.userId, narrow: switch (data.recipient) { @@ -86,11 +90,23 @@ void main() { FcmMessageDmRecipient(:var allRecipientIds) => DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), }).buildAndroidNotificationUrl(); + } + + Future openNotification(WidgetTester tester, Account account, Message message) async { + final intentDataUrl = androidNotificationUrlForMessage(account, message); unawaited( WidgetsBinding.instance.handlePushRoute(intentDataUrl.toString())); await tester.idle(); // let navigateForNotification find navigator } + void setupNotificationDataForLaunch(WidgetTester tester, Account account, Message message) { + // Set up a value for `PlatformDispatcher.defaultRouteName` to return, + // for determining the initial route. + final intentDataUrl = androidNotificationUrlForMessage(account, message); + addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); + tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); + } + void matchesNavigation(Subject> route, Account account, Message message) { route.isA() ..accountId.equals(account.id) @@ -107,14 +123,12 @@ void main() { testWidgets('stream message', (tester) async { addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); await prepare(tester); await checkOpenNotification(tester, eg.selfAccount, eg.streamMessage()); }); testWidgets('direct message', (tester) async { addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); await prepare(tester); await checkOpenNotification(tester, eg.selfAccount, eg.dmMessage(from: eg.otherUser, to: [eg.selfUser])); @@ -122,10 +136,8 @@ void main() { testWidgets('account queried by realmUrl origin component', (tester) async { addTearDown(testBinding.reset); - await testBinding.globalStore.add( - eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example')), - eg.initialSnapshot()); - await prepare(tester); + await prepare(tester, + account: eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example'))); await checkOpenNotification(tester, eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example/')), @@ -147,7 +159,6 @@ void main() { testWidgets('mismatching account', (tester) async { addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); await prepare(tester); await openNotification(tester, eg.otherAccount, eg.streamMessage()); await tester.pump(); @@ -172,7 +183,10 @@ void main() { for (final account in accounts) { await testBinding.globalStore.add(account, eg.initialSnapshot()); } - await prepare(tester); + await prepare(tester, dropStartingRoutes: false, withAccount: false); + check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet + await tester.pump(); + takeStartingRoutes(account: accounts[0]); await checkOpenNotification(tester, accounts[0], eg.streamMessage()); await checkOpenNotification(tester, accounts[1], eg.streamMessage()); @@ -182,8 +196,7 @@ void main() { testWidgets('wait for app to become ready', (tester) async { addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - await prepare(tester, early: true); + await prepare(tester, dropStartingRoutes: false); final message = eg.streamMessage(); await openNotification(tester, eg.selfAccount, message); // The app should still not be ready (or else this test won't work right). @@ -195,64 +208,38 @@ void main() { // Now let the GlobalStore get loaded and the app's main UI get mounted. await tester.pump(); // The navigator first pushes the starting routes… - takeStartingRoutes(); + takeStartingRoutes(account: eg.selfAccount); // … and then the one the notification leads to. matchesNavigation(check(pushedRoutes).single, eg.selfAccount, message); }); testWidgets('at app launch', (tester) async { addTearDown(testBinding.reset); - // Set up a value for `PlatformDispatcher.defaultRouteName` to return, - // for determining the intial route. final account = eg.selfAccount; final message = eg.streamMessage(); - final data = messageFcmMessage(message, account: account); - final intentDataUrl = NotificationOpenPayload( - realmUrl: data.realmUrl, - userId: data.userId, - narrow: switch (data.recipient) { - FcmMessageChannelRecipient(:var streamId, :var topic) => - TopicNarrow(streamId, topic), - FcmMessageDmRecipient(:var allRecipientIds) => - DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildAndroidNotificationUrl(); - addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); - tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); + setupNotificationDataForLaunch(tester, account, message); // Now start the app. - await testBinding.globalStore.add(account, eg.initialSnapshot()); - await prepare(tester, early: true); + await prepare(tester, dropStartingRoutes: false); check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet // Once the app is ready, we navigate to the conversation. await tester.pump(); - takeStartingRoutes(); + takeStartingRoutes(account: account); matchesNavigation(check(pushedRoutes).single, account, message); }); - testWidgets('uses associated account as initial account; if initial route', (tester) async { + testWidgets('uses associated account as initial account, if initial route', (tester) async { addTearDown(testBinding.reset); final accountA = eg.selfAccount; final accountB = eg.otherAccount; final message = eg.streamMessage(); - final data = messageFcmMessage(message, account: accountB); await testBinding.globalStore.add(accountA, eg.initialSnapshot()); await testBinding.globalStore.add(accountB, eg.initialSnapshot()); + setupNotificationDataForLaunch(tester, accountB, message); - final intentDataUrl = NotificationOpenPayload( - realmUrl: data.realmUrl, - userId: data.userId, - narrow: switch (data.recipient) { - FcmMessageChannelRecipient(:var streamId, :var topic) => - TopicNarrow(streamId, topic), - FcmMessageDmRecipient(:var allRecipientIds) => - DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildAndroidNotificationUrl(); - addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); - tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); - - await prepare(tester, early: true); + await prepare(tester, dropStartingRoutes: false, withAccount: false); check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet await tester.pump(); From cead8bb79f9e5c3103881ca3163fc5e61defd169 Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Wed, 28 May 2025 21:24:14 +0530 Subject: [PATCH 14/16] notif ios: Add parser for iOS APNs payload Introduces NotificationOpenPayload.parseIosApnsPayload which can parse the payload that Apple push notification service delivers to the app for displaying a notification. It retrieves the navigation data for the specific message notification. --- lib/notifications/open.dart | 66 +++++++++++++++++++++ test/notifications/open_test.dart | 96 ++++++++++++++++++++++++++++++- 2 files changed, 161 insertions(+), 1 deletion(-) diff --git a/lib/notifications/open.dart b/lib/notifications/open.dart index 5bf0f0f772..b8ba4068bd 100644 --- a/lib/notifications/open.dart +++ b/lib/notifications/open.dart @@ -100,6 +100,72 @@ class NotificationOpenPayload { required this.narrow, }); + /// Parses the iOS APNs payload and retrieves the information + /// required for navigation. + factory NotificationOpenPayload.parseIosApnsPayload(Map payload) { + if (payload case { + 'zulip': { + 'user_id': final int userId, + 'sender_id': final int senderId, + } && final zulipData, + }) { + final eventType = zulipData['event']; + if (eventType != null && eventType != 'message') { + // On Android, we also receive "remove" notification messages, tagged + // with an `event` field with value 'remove'. As of Zulip Server 10, + // however, these are not yet sent to iOS devices, and we don't have a + // way to handle them even if they were. + // + // The messages we currently do receive, and can handle, are analogous + // to Android notification messages of event type 'message'. On the + // assumption that some future version of the Zulip server will send + // explicit event types in APNs messages, accept messages with that + // `event` value, but no other. + throw const FormatException(); + } + + final realmUrl = switch (zulipData) { + {'realm_url': final String value} => value, + {'realm_uri': final String value} => value, + _ => throw const FormatException(), + }; + + final narrow = switch (zulipData) { + { + 'recipient_type': 'stream', + // TODO(server-5) remove this comment. + // We require 'stream_id' here but that is new from Server 5.0, + // resulting in failure on pre-5.0 servers. + 'stream_id': final int streamId, + 'topic': final String topic, + } => + TopicNarrow(streamId, TopicName(topic)), + + {'recipient_type': 'private', 'pm_users': final String pmUsers} => + DmNarrow( + allRecipientIds: pmUsers + .split(',') + .map((e) => int.parse(e, radix: 10)) + .toList(growable: false) + ..sort(), + selfUserId: userId), + + {'recipient_type': 'private'} => + DmNarrow.withUser(senderId, selfUserId: userId), + + _ => throw const FormatException(), + }; + + return NotificationOpenPayload( + realmUrl: Uri.parse(realmUrl), + userId: userId, + narrow: narrow); + } else { + // TODO(dart): simplify after https://github.com/dart-lang/language/issues/2537 + throw const FormatException(); + } + } + /// Parses the internal Android notification url, that was created using /// [buildAndroidNotificationUrl], and retrieves the information required /// for navigation. diff --git a/test/notifications/open_test.dart b/test/notifications/open_test.dart index 3122fc154a..3d7cc7c531 100644 --- a/test/notifications/open_test.dart +++ b/test/notifications/open_test.dart @@ -25,6 +25,50 @@ import '../widgets/message_list_checks.dart'; import '../widgets/page_checks.dart'; import 'display_test.dart'; +Map messageApnsPayload( + Message zulipMessage, { + String? streamName, + Account? account, +}) { + account ??= eg.selfAccount; + return { + "aps": { + "alert": { + "title": "test", + "subtitle": "test", + "body": zulipMessage.content, + }, + "sound": "default", + "badge": 0, + }, + "zulip": { + "server": "zulip.example.cloud", + "realm_id": 4, + "realm_uri": account.realmUrl.toString(), + "realm_url": account.realmUrl.toString(), + "realm_name": "Test", + "user_id": account.userId, + "sender_id": zulipMessage.senderId, + "sender_email": zulipMessage.senderEmail, + "time": zulipMessage.timestamp, + "message_ids": [zulipMessage.id], + ...(switch (zulipMessage) { + StreamMessage(:var streamId, :var topic) => { + "recipient_type": "stream", + "stream_id": streamId, + if (streamName != null) "stream": streamName, + "topic": topic, + }, + DmMessage(allRecipientIds: [_, _, _, ...]) => { + "recipient_type": "private", + "pm_users": zulipMessage.allRecipientIds.join(","), + }, + DmMessage() => {"recipient_type": "private"}, + }), + }, + }; +} + void main() { TestZulipBinding.ensureInitialized(); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; @@ -249,7 +293,7 @@ void main() { }); group('NotificationOpenPayload', () { - test('smoke round-trip', () { + test('android: smoke round-trip', () { // DM narrow var payload = NotificationOpenPayload( realmUrl: Uri.parse('http://chat.example'), @@ -275,6 +319,56 @@ void main() { ..narrow.equals(payload.narrow); }); + group('parseIosApnsPayload', () { + test('smoke one-one DM', () { + final userA = eg.user(userId: 1001); + final userB = eg.user(userId: 1002); + final account = eg.account( + realmUrl: Uri.parse('http://chat.example'), + user: userA); + final payload = messageApnsPayload(eg.dmMessage(from: userB, to: [userA]), + account: account); + check(NotificationOpenPayload.parseIosApnsPayload(payload)) + ..realmUrl.equals(Uri.parse('http://chat.example')) + ..userId.equals(1001) + ..narrow.which((it) => it.isA() + ..otherRecipientIds.deepEquals([1002])); + }); + + test('smoke group DM', () { + final userA = eg.user(userId: 1001); + final userB = eg.user(userId: 1002); + final userC = eg.user(userId: 1003); + final account = eg.account( + realmUrl: Uri.parse('http://chat.example'), + user: userA); + final payload = messageApnsPayload(eg.dmMessage(from: userC, to: [userA, userB]), + account: account); + check(NotificationOpenPayload.parseIosApnsPayload(payload)) + ..realmUrl.equals(Uri.parse('http://chat.example')) + ..userId.equals(1001) + ..narrow.which((it) => it.isA() + ..otherRecipientIds.deepEquals([1002, 1003])); + }); + + test('smoke topic message', () { + final userA = eg.user(userId: 1001); + final account = eg.account( + realmUrl: Uri.parse('http://chat.example'), + user: userA); + final payload = messageApnsPayload(eg.streamMessage( + stream: eg.stream(streamId: 1), + topic: 'topic A'), + account: account); + check(NotificationOpenPayload.parseIosApnsPayload(payload)) + ..realmUrl.equals(Uri.parse('http://chat.example')) + ..userId.equals(1001) + ..narrow.which((it) => it.isA() + ..streamId.equals(1) + ..topic.equals(TopicName('topic A'))); + }); + }); + group('buildAndroidNotificationUrl', () { test('smoke DM', () { final url = NotificationOpenPayload( From c08a799a4739cae15dbe76c8778a06ec54b1cb00 Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Fri, 28 Feb 2025 21:20:42 +0530 Subject: [PATCH 15/16] notif ios: Navigate when app launched from notification Introduces a new Pigeon API file, and adds the corresponding bindings in Swift. Unlike the `pigeon/android_notifications.dart` API this doesn't use the ZulipPlugin hack, as that is only needed when we want the Pigeon functions to be available inside a background isolate (see doc in `zulip_plugin/pubspec.yaml`). Since the notification tap will trigger an app launch first (if not running already) anyway, we can be sure that these new functions won't be running on a Dart background isolate, thus not needing the ZulipPlugin hack. --- ios/Runner.xcodeproj/project.pbxproj | 4 + ios/Runner/AppDelegate.swift | 20 +++ ios/Runner/Notifications.g.swift | 235 +++++++++++++++++++++++++++ lib/host/notifications.dart | 1 + lib/host/notifications.g.dart | 146 +++++++++++++++++ lib/model/binding.dart | 14 ++ lib/notifications/open.dart | 87 +++++++++- lib/notifications/receive.dart | 4 + lib/widgets/app.dart | 13 +- pigeon/notifications.dart | 30 ++++ test/model/binding.dart | 21 +++ test/model/store_test.dart | 4 +- test/notifications/open_test.dart | 57 +++++-- 13 files changed, 612 insertions(+), 24 deletions(-) create mode 100644 ios/Runner/Notifications.g.swift create mode 100644 lib/host/notifications.dart create mode 100644 lib/host/notifications.g.dart create mode 100644 pigeon/notifications.dart diff --git a/ios/Runner.xcodeproj/project.pbxproj b/ios/Runner.xcodeproj/project.pbxproj index b4928e2220..7df051a142 100644 --- a/ios/Runner.xcodeproj/project.pbxproj +++ b/ios/Runner.xcodeproj/project.pbxproj @@ -13,6 +13,7 @@ 97C146FC1CF9000F007C117D /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 97C146FA1CF9000F007C117D /* Main.storyboard */; }; 97C146FE1CF9000F007C117D /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 97C146FD1CF9000F007C117D /* Assets.xcassets */; }; 97C147011CF9000F007C117D /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 97C146FF1CF9000F007C117D /* LaunchScreen.storyboard */; }; + B34E9F092D776BEB0009AED2 /* Notifications.g.swift in Sources */ = {isa = PBXBuildFile; fileRef = B34E9F082D776BEB0009AED2 /* Notifications.g.swift */; }; F311C174AF9C005CE4AADD72 /* Pods_Runner.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 3EAE3F3F518B95B7BFEB4FE7 /* Pods_Runner.framework */; }; /* End PBXBuildFile section */ @@ -48,6 +49,7 @@ 97C146FD1CF9000F007C117D /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = ""; }; 97C147001CF9000F007C117D /* Base */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; name = Base; path = Base.lproj/LaunchScreen.storyboard; sourceTree = ""; }; 97C147021CF9000F007C117D /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; + B34E9F082D776BEB0009AED2 /* Notifications.g.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Notifications.g.swift; sourceTree = ""; }; B3AF53A72CA20BD10039801D /* Zulip.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = Zulip.xcconfig; path = Flutter/Zulip.xcconfig; sourceTree = ""; }; /* End PBXFileReference section */ @@ -115,6 +117,7 @@ 1498D2321E8E86230040F4C2 /* GeneratedPluginRegistrant.h */, 1498D2331E8E89220040F4C2 /* GeneratedPluginRegistrant.m */, 74858FAE1ED2DC5600515810 /* AppDelegate.swift */, + B34E9F082D776BEB0009AED2 /* Notifications.g.swift */, 74858FAD1ED2DC5600515810 /* Runner-Bridging-Header.h */, ); path = Runner; @@ -297,6 +300,7 @@ buildActionMask = 2147483647; files = ( 74858FAF1ED2DC5600515810 /* AppDelegate.swift in Sources */, + B34E9F092D776BEB0009AED2 /* Notifications.g.swift in Sources */, 1498D2341E8E89220040F4C2 /* GeneratedPluginRegistrant.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/ios/Runner/AppDelegate.swift b/ios/Runner/AppDelegate.swift index b636303481..33a0fe72cb 100644 --- a/ios/Runner/AppDelegate.swift +++ b/ios/Runner/AppDelegate.swift @@ -8,6 +8,26 @@ import Flutter didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? ) -> Bool { GeneratedPluginRegistrant.register(with: self) + let controller = window?.rootViewController as! FlutterViewController + + // Retrieve the remote notification payload from launch options; + // this will be null if the launch wasn't triggered by a notification. + let notificationPayload = launchOptions?[.remoteNotification] as? [AnyHashable : Any] + let api = NotificationHostApiImpl(notificationPayload.map { NotificationDataFromLaunch(payload: $0) }) + NotificationHostApiSetup.setUp(binaryMessenger: controller.binaryMessenger, api: api) + return super.application(application, didFinishLaunchingWithOptions: launchOptions) } } + +private class NotificationHostApiImpl: NotificationHostApi { + private let maybeDataFromLaunch: NotificationDataFromLaunch? + + init(_ maybeDataFromLaunch: NotificationDataFromLaunch?) { + self.maybeDataFromLaunch = maybeDataFromLaunch + } + + func getNotificationDataFromLaunch() -> NotificationDataFromLaunch? { + maybeDataFromLaunch + } +} diff --git a/ios/Runner/Notifications.g.swift b/ios/Runner/Notifications.g.swift new file mode 100644 index 0000000000..342953fbad --- /dev/null +++ b/ios/Runner/Notifications.g.swift @@ -0,0 +1,235 @@ +// Autogenerated from Pigeon (v25.3.1), do not edit directly. +// See also: https://pub.dev/packages/pigeon + +import Foundation + +#if os(iOS) + import Flutter +#elseif os(macOS) + import FlutterMacOS +#else + #error("Unsupported platform.") +#endif + +/// Error class for passing custom error details to Dart side. +final class PigeonError: Error { + let code: String + let message: String? + let details: Sendable? + + init(code: String, message: String?, details: Sendable?) { + self.code = code + self.message = message + self.details = details + } + + var localizedDescription: String { + return + "PigeonError(code: \(code), message: \(message ?? ""), details: \(details ?? "")" + } +} + +private func wrapResult(_ result: Any?) -> [Any?] { + return [result] +} + +private func wrapError(_ error: Any) -> [Any?] { + if let pigeonError = error as? PigeonError { + return [ + pigeonError.code, + pigeonError.message, + pigeonError.details, + ] + } + if let flutterError = error as? FlutterError { + return [ + flutterError.code, + flutterError.message, + flutterError.details, + ] + } + return [ + "\(error)", + "\(type(of: error))", + "Stacktrace: \(Thread.callStackSymbols)", + ] +} + +private func isNullish(_ value: Any?) -> Bool { + return value is NSNull || value == nil +} + +private func nilOrValue(_ value: Any?) -> T? { + if value is NSNull { return nil } + return value as! T? +} + +func deepEqualsNotifications(_ lhs: Any?, _ rhs: Any?) -> Bool { + let cleanLhs = nilOrValue(lhs) as Any? + let cleanRhs = nilOrValue(rhs) as Any? + switch (cleanLhs, cleanRhs) { + case (nil, nil): + return true + + case (nil, _), (_, nil): + return false + + case is (Void, Void): + return true + + case let (cleanLhsHashable, cleanRhsHashable) as (AnyHashable, AnyHashable): + return cleanLhsHashable == cleanRhsHashable + + case let (cleanLhsArray, cleanRhsArray) as ([Any?], [Any?]): + guard cleanLhsArray.count == cleanRhsArray.count else { return false } + for (index, element) in cleanLhsArray.enumerated() { + if !deepEqualsNotifications(element, cleanRhsArray[index]) { + return false + } + } + return true + + case let (cleanLhsDictionary, cleanRhsDictionary) as ([AnyHashable: Any?], [AnyHashable: Any?]): + guard cleanLhsDictionary.count == cleanRhsDictionary.count else { return false } + for (key, cleanLhsValue) in cleanLhsDictionary { + guard cleanRhsDictionary.index(forKey: key) != nil else { return false } + if !deepEqualsNotifications(cleanLhsValue, cleanRhsDictionary[key]!) { + return false + } + } + return true + + default: + // Any other type shouldn't be able to be used with pigeon. File an issue if you find this to be untrue. + return false + } +} + +func deepHashNotifications(value: Any?, hasher: inout Hasher) { + if let valueList = value as? [AnyHashable] { + for item in valueList { deepHashNotifications(value: item, hasher: &hasher) } + return + } + + if let valueDict = value as? [AnyHashable: AnyHashable] { + for key in valueDict.keys { + hasher.combine(key) + deepHashNotifications(value: valueDict[key]!, hasher: &hasher) + } + return + } + + if let hashableValue = value as? AnyHashable { + hasher.combine(hashableValue.hashValue) + } + + return hasher.combine(String(describing: value)) +} + + + +/// Generated class from Pigeon that represents data sent in messages. +struct NotificationDataFromLaunch: Hashable { + /// The raw payload that is attached to the notification, + /// holding the information required to carry out the navigation. + /// + /// See [NotificationHostApi.getNotificationDataFromLaunch]. + var payload: [AnyHashable?: Any?] + + + // swift-format-ignore: AlwaysUseLowerCamelCase + static func fromList(_ pigeonVar_list: [Any?]) -> NotificationDataFromLaunch? { + let payload = pigeonVar_list[0] as! [AnyHashable?: Any?] + + return NotificationDataFromLaunch( + payload: payload + ) + } + func toList() -> [Any?] { + return [ + payload + ] + } + static func == (lhs: NotificationDataFromLaunch, rhs: NotificationDataFromLaunch) -> Bool { + return deepEqualsNotifications(lhs.toList(), rhs.toList()) } + func hash(into hasher: inout Hasher) { + deepHashNotifications(value: toList(), hasher: &hasher) + } +} + +private class NotificationsPigeonCodecReader: FlutterStandardReader { + override func readValue(ofType type: UInt8) -> Any? { + switch type { + case 129: + return NotificationDataFromLaunch.fromList(self.readValue() as! [Any?]) + default: + return super.readValue(ofType: type) + } + } +} + +private class NotificationsPigeonCodecWriter: FlutterStandardWriter { + override func writeValue(_ value: Any) { + if let value = value as? NotificationDataFromLaunch { + super.writeByte(129) + super.writeValue(value.toList()) + } else { + super.writeValue(value) + } + } +} + +private class NotificationsPigeonCodecReaderWriter: FlutterStandardReaderWriter { + override func reader(with data: Data) -> FlutterStandardReader { + return NotificationsPigeonCodecReader(data: data) + } + + override func writer(with data: NSMutableData) -> FlutterStandardWriter { + return NotificationsPigeonCodecWriter(data: data) + } +} + +class NotificationsPigeonCodec: FlutterStandardMessageCodec, @unchecked Sendable { + static let shared = NotificationsPigeonCodec(readerWriter: NotificationsPigeonCodecReaderWriter()) +} + +/// Generated protocol from Pigeon that represents a handler of messages from Flutter. +protocol NotificationHostApi { + /// Retrieves notification data if the app was launched by tapping on a notification. + /// + /// Returns `launchOptions.remoteNotification`, + /// which is the raw APNs data dictionary + /// if the app launch was opened by a notification tap, + /// else null. See Apple doc: + /// https://developer.apple.com/documentation/uikit/uiapplication/launchoptionskey/remotenotification + func getNotificationDataFromLaunch() throws -> NotificationDataFromLaunch? +} + +/// Generated setup class from Pigeon to handle messages through the `binaryMessenger`. +class NotificationHostApiSetup { + static var codec: FlutterStandardMessageCodec { NotificationsPigeonCodec.shared } + /// Sets up an instance of `NotificationHostApi` to handle messages through the `binaryMessenger`. + static func setUp(binaryMessenger: FlutterBinaryMessenger, api: NotificationHostApi?, messageChannelSuffix: String = "") { + let channelSuffix = messageChannelSuffix.count > 0 ? ".\(messageChannelSuffix)" : "" + /// Retrieves notification data if the app was launched by tapping on a notification. + /// + /// Returns `launchOptions.remoteNotification`, + /// which is the raw APNs data dictionary + /// if the app launch was opened by a notification tap, + /// else null. See Apple doc: + /// https://developer.apple.com/documentation/uikit/uiapplication/launchoptionskey/remotenotification + let getNotificationDataFromLaunchChannel = FlutterBasicMessageChannel(name: "dev.flutter.pigeon.zulip.NotificationHostApi.getNotificationDataFromLaunch\(channelSuffix)", binaryMessenger: binaryMessenger, codec: codec) + if let api = api { + getNotificationDataFromLaunchChannel.setMessageHandler { _, reply in + do { + let result = try api.getNotificationDataFromLaunch() + reply(wrapResult(result)) + } catch { + reply(wrapError(error)) + } + } + } else { + getNotificationDataFromLaunchChannel.setMessageHandler(nil) + } + } +} diff --git a/lib/host/notifications.dart b/lib/host/notifications.dart new file mode 100644 index 0000000000..6c3e593e2c --- /dev/null +++ b/lib/host/notifications.dart @@ -0,0 +1 @@ +export './notifications.g.dart'; diff --git a/lib/host/notifications.g.dart b/lib/host/notifications.g.dart new file mode 100644 index 0000000000..d8448d60b8 --- /dev/null +++ b/lib/host/notifications.g.dart @@ -0,0 +1,146 @@ +// Autogenerated from Pigeon (v25.3.1), do not edit directly. +// See also: https://pub.dev/packages/pigeon +// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import, no_leading_underscores_for_local_identifiers + +import 'dart:async'; +import 'dart:typed_data' show Float64List, Int32List, Int64List, Uint8List; + +import 'package:flutter/foundation.dart' show ReadBuffer, WriteBuffer; +import 'package:flutter/services.dart'; + +PlatformException _createConnectionError(String channelName) { + return PlatformException( + code: 'channel-error', + message: 'Unable to establish connection on channel: "$channelName".', + ); +} +bool _deepEquals(Object? a, Object? b) { + if (a is List && b is List) { + return a.length == b.length && + a.indexed + .every(((int, dynamic) item) => _deepEquals(item.$2, b[item.$1])); + } + if (a is Map && b is Map) { + return a.length == b.length && a.entries.every((MapEntry entry) => + (b as Map).containsKey(entry.key) && + _deepEquals(entry.value, b[entry.key])); + } + return a == b; +} + + +class NotificationDataFromLaunch { + NotificationDataFromLaunch({ + required this.payload, + }); + + /// The raw payload that is attached to the notification, + /// holding the information required to carry out the navigation. + /// + /// See [NotificationHostApi.getNotificationDataFromLaunch]. + Map payload; + + List _toList() { + return [ + payload, + ]; + } + + Object encode() { + return _toList(); } + + static NotificationDataFromLaunch decode(Object result) { + result as List; + return NotificationDataFromLaunch( + payload: (result[0] as Map?)!.cast(), + ); + } + + @override + // ignore: avoid_equals_and_hash_code_on_mutable_classes + bool operator ==(Object other) { + if (other is! NotificationDataFromLaunch || other.runtimeType != runtimeType) { + return false; + } + if (identical(this, other)) { + return true; + } + return _deepEquals(encode(), other.encode()); + } + + @override + // ignore: avoid_equals_and_hash_code_on_mutable_classes + int get hashCode => Object.hashAll(_toList()) +; +} + + +class _PigeonCodec extends StandardMessageCodec { + const _PigeonCodec(); + @override + void writeValue(WriteBuffer buffer, Object? value) { + if (value is int) { + buffer.putUint8(4); + buffer.putInt64(value); + } else if (value is NotificationDataFromLaunch) { + buffer.putUint8(129); + writeValue(buffer, value.encode()); + } else { + super.writeValue(buffer, value); + } + } + + @override + Object? readValueOfType(int type, ReadBuffer buffer) { + switch (type) { + case 129: + return NotificationDataFromLaunch.decode(readValue(buffer)!); + default: + return super.readValueOfType(type, buffer); + } + } +} + +class NotificationHostApi { + /// Constructor for [NotificationHostApi]. The [binaryMessenger] named argument is + /// available for dependency injection. If it is left null, the default + /// BinaryMessenger will be used which routes to the host platform. + NotificationHostApi({BinaryMessenger? binaryMessenger, String messageChannelSuffix = ''}) + : pigeonVar_binaryMessenger = binaryMessenger, + pigeonVar_messageChannelSuffix = messageChannelSuffix.isNotEmpty ? '.$messageChannelSuffix' : ''; + final BinaryMessenger? pigeonVar_binaryMessenger; + + static const MessageCodec pigeonChannelCodec = _PigeonCodec(); + + final String pigeonVar_messageChannelSuffix; + + /// Retrieves notification data if the app was launched by tapping on a notification. + /// + /// Returns `launchOptions.remoteNotification`, + /// which is the raw APNs data dictionary + /// if the app launch was opened by a notification tap, + /// else null. See Apple doc: + /// https://developer.apple.com/documentation/uikit/uiapplication/launchoptionskey/remotenotification + Future getNotificationDataFromLaunch() async { + final String pigeonVar_channelName = 'dev.flutter.pigeon.zulip.NotificationHostApi.getNotificationDataFromLaunch$pigeonVar_messageChannelSuffix'; + final BasicMessageChannel pigeonVar_channel = BasicMessageChannel( + pigeonVar_channelName, + pigeonChannelCodec, + binaryMessenger: pigeonVar_binaryMessenger, + ); + final Future pigeonVar_sendFuture = pigeonVar_channel.send(null); + final List? pigeonVar_replyList = + await pigeonVar_sendFuture as List?; + if (pigeonVar_replyList == null) { + throw _createConnectionError(pigeonVar_channelName); + } else if (pigeonVar_replyList.length > 1) { + throw PlatformException( + code: pigeonVar_replyList[0]! as String, + message: pigeonVar_replyList[1] as String?, + details: pigeonVar_replyList[2], + ); + } else { + return (pigeonVar_replyList[0] as NotificationDataFromLaunch?); + } + } +} diff --git a/lib/model/binding.dart b/lib/model/binding.dart index fb3add46da..7f9b0f6fd1 100644 --- a/lib/model/binding.dart +++ b/lib/model/binding.dart @@ -11,6 +11,7 @@ import 'package:url_launcher/url_launcher.dart' as url_launcher; import 'package:wakelock_plus/wakelock_plus.dart' as wakelock_plus; import '../host/android_notifications.dart'; +import '../host/notifications.dart' as notif_pigeon; import '../log.dart'; import '../widgets/store.dart'; import 'store.dart'; @@ -180,6 +181,9 @@ abstract class ZulipBinding { /// Wraps the [AndroidNotificationHostApi] constructor. AndroidNotificationHostApi get androidNotificationHost; + /// Wraps the [notif_pigeon.NotificationHostApi] class. + NotificationPigeonApi get notificationPigeonApi; + /// Pick files from the media library, via package:file_picker. /// /// This wraps [file_picker.pickFiles]. @@ -324,6 +328,13 @@ class PackageInfo { }); } +class NotificationPigeonApi { + final _hostApi = notif_pigeon.NotificationHostApi(); + + Future getNotificationDataFromLaunch() => + _hostApi.getNotificationDataFromLaunch(); +} + /// A concrete binding for use in the live application. /// /// The global store returned by [getGlobalStore], and consequently by @@ -469,6 +480,9 @@ class LiveZulipBinding extends ZulipBinding { @override AndroidNotificationHostApi get androidNotificationHost => AndroidNotificationHostApi(); + @override + NotificationPigeonApi get notificationPigeonApi => NotificationPigeonApi(); + @override Future pickFiles({ bool allowMultiple = false, diff --git a/lib/notifications/open.dart b/lib/notifications/open.dart index b8ba4068bd..3f136f695c 100644 --- a/lib/notifications/open.dart +++ b/lib/notifications/open.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:convert'; import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; @@ -6,7 +7,9 @@ import 'package:flutter/widgets.dart'; import '../api/model/model.dart'; import '../generated/l10n/zulip_localizations.dart'; +import '../host/notifications.dart'; import '../log.dart'; +import '../model/binding.dart'; import '../model/narrow.dart'; import '../widgets/app.dart'; import '../widgets/dialog.dart'; @@ -14,8 +17,75 @@ import '../widgets/message_list.dart'; import '../widgets/page.dart'; import '../widgets/store.dart'; +NotificationPigeonApi get _notifPigeonApi => ZulipBinding.instance.notificationPigeonApi; + /// Responds to the user opening a notification. class NotificationOpenService { + static NotificationOpenService get instance => (_instance ??= NotificationOpenService._()); + static NotificationOpenService? _instance; + + NotificationOpenService._(); + + /// Reset the state of the [NotificationNavigationService], for testing. + static void debugReset() { + _instance = null; + } + + NotificationDataFromLaunch? _notifDataFromLaunch; + + /// A [Future] that completes to signal that the initialization of + /// [NotificationNavigationService] has completed + /// (with either success or failure). + /// + /// Null if [start] hasn't been called. + Future? get initialized => _initializedSignal?.future; + + Completer? _initializedSignal; + + Future start() async { + assert(_initializedSignal == null); + _initializedSignal = Completer(); + try { + switch (defaultTargetPlatform) { + case TargetPlatform.iOS: + _notifDataFromLaunch = await _notifPigeonApi.getNotificationDataFromLaunch(); + + case TargetPlatform.android: + // Do nothing; we do notification routing differently on Android. + // TODO migrate Android to use the new Pigeon API. + break; + + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.macOS: + case TargetPlatform.windows: + // Do nothing; we don't offer notifications on these platforms. + break; + } + } finally { + _initializedSignal!.complete(); + } + } + + /// Provides the route to open if the app was launched through a tap on + /// a notification. + /// + /// Returns null if app launch wasn't triggered by a notification, or if + /// an error occurs while determining the route for the notification. + /// In the latter case an error dialog is also shown. + /// + /// The context argument should be a descendant of the app's main [Navigator]. + AccountRoute? routeForNotificationFromLaunch({required BuildContext context}) { + assert(defaultTargetPlatform == TargetPlatform.iOS); + final data = _notifDataFromLaunch; + if (data == null) return null; + assert(debugLog('opened notif: ${jsonEncode(data.payload)}')); + + final notifNavData = _tryParseIosApnsPayload(context, data.payload); + if (notifNavData == null) return null; // TODO(log) + + return routeForNotification(context: context, data: notifNavData); + } /// Provides the route to open by parsing the notification payload. /// @@ -27,8 +97,6 @@ class NotificationOpenService { required BuildContext context, required NotificationOpenPayload data, }) { - assert(defaultTargetPlatform == TargetPlatform.android); - final globalStore = GlobalStoreWidget.of(context); final account = globalStore.accounts.firstWhereOrNull( @@ -71,6 +139,21 @@ class NotificationOpenService { unawaited(navigator.push(route)); } + static NotificationOpenPayload? _tryParseIosApnsPayload( + BuildContext context, + Map payload, + ) { + try { + return NotificationOpenPayload.parseIosApnsPayload(payload); + } on FormatException catch (e, st) { + assert(debugLog('$e\n$st')); + final zulipLocalizations = ZulipLocalizations.of(context); + showErrorDialog(context: context, + title: zulipLocalizations.errorNotificationOpenTitle); + return null; + } + } + static NotificationOpenPayload? tryParseAndroidNotificationUrl({ required BuildContext context, required Uri url, diff --git a/lib/notifications/receive.dart b/lib/notifications/receive.dart index d60469ff30..212b0f5f0d 100644 --- a/lib/notifications/receive.dart +++ b/lib/notifications/receive.dart @@ -8,6 +8,7 @@ import '../firebase_options.dart'; import '../log.dart'; import '../model/binding.dart'; import 'display.dart'; +import 'open.dart'; @pragma('vm:entry-point') class NotificationService { @@ -24,6 +25,7 @@ class NotificationService { instance.token.dispose(); _instance = null; assert(debugBackgroundIsolateIsLive = true); + NotificationOpenService.debugReset(); } /// Whether a background isolate should initialize [LiveZulipBinding]. @@ -77,6 +79,8 @@ class NotificationService { await _getFcmToken(); case TargetPlatform.iOS: // TODO(#324): defer requesting notif permission + await NotificationOpenService.instance.start(); + await ZulipBinding.instance.firebaseInitializeApp( options: kFirebaseOptionsIos); diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 96c546bd3f..b1aa763ac8 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -168,6 +168,12 @@ class _ZulipAppState extends State with WidgetsBindingObserver { super.dispose(); } + AccountRoute? _initialRouteIos(BuildContext context) { + return NotificationOpenService.instance + .routeForNotificationFromLaunch(context: context); + } + + // TODO migrate Android's notification navigation to use the new Pigeon API. AccountRoute? _initialRouteAndroid( BuildContext context, String initialRoute, @@ -190,10 +196,12 @@ class _ZulipAppState extends State with WidgetsBindingObserver { List> _handleGenerateInitialRoutes(String initialRoute) { // The `_ZulipAppState.context` lacks the required ancestors. Instead // we use the Navigator which should be available when this callback is - // called and it's context should have the required ancestors. + // called and its context should have the required ancestors. final context = ZulipApp.navigatorKey.currentContext!; - final route = _initialRouteAndroid(context, initialRoute); + final route = defaultTargetPlatform == TargetPlatform.iOS + ? _initialRouteIos(context) + : _initialRouteAndroid(context, initialRoute); if (route != null) { return [ HomePage.buildRoute(accountId: route.accountId), @@ -228,6 +236,7 @@ class _ZulipAppState extends State with WidgetsBindingObserver { @override Widget build(BuildContext context) { return GlobalStoreWidget( + blockingFuture: NotificationOpenService.instance.initialized, child: Builder(builder: (context) { return MaterialApp( onGenerateTitle: (BuildContext context) { diff --git a/pigeon/notifications.dart b/pigeon/notifications.dart new file mode 100644 index 0000000000..efea52d9a6 --- /dev/null +++ b/pigeon/notifications.dart @@ -0,0 +1,30 @@ +import 'package:pigeon/pigeon.dart'; + +// To rebuild this pigeon's output after editing this file, +// run `tools/check pigeon --fix`. +@ConfigurePigeon(PigeonOptions( + dartOut: 'lib/host/notifications.g.dart', + swiftOut: 'ios/Runner/Notifications.g.swift', +)) + +class NotificationDataFromLaunch { + const NotificationDataFromLaunch({required this.payload}); + + /// The raw payload that is attached to the notification, + /// holding the information required to carry out the navigation. + /// + /// See [NotificationHostApi.getNotificationDataFromLaunch]. + final Map payload; +} + +@HostApi() +abstract class NotificationHostApi { + /// Retrieves notification data if the app was launched by tapping on a notification. + /// + /// Returns `launchOptions.remoteNotification`, + /// which is the raw APNs data dictionary + /// if the app launch was opened by a notification tap, + /// else null. See Apple doc: + /// https://developer.apple.com/documentation/uikit/uiapplication/launchoptionskey/remotenotification + NotificationDataFromLaunch? getNotificationDataFromLaunch(); +} diff --git a/test/model/binding.dart b/test/model/binding.dart index 6b4de26608..afeed2f266 100644 --- a/test/model/binding.dart +++ b/test/model/binding.dart @@ -8,6 +8,7 @@ import 'package:flutter/services.dart'; import 'package:test/fake.dart'; import 'package:url_launcher/url_launcher.dart' as url_launcher; import 'package:zulip/host/android_notifications.dart'; +import 'package:zulip/host/notifications.dart'; import 'package:zulip/model/binding.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/app.dart'; @@ -311,6 +312,7 @@ class TestZulipBinding extends ZulipBinding { void _resetNotifications() { _androidNotificationHostApi = null; + _notificationPigeonApi = null; } @override @@ -318,6 +320,11 @@ class TestZulipBinding extends ZulipBinding { (_androidNotificationHostApi ??= FakeAndroidNotificationHostApi()); FakeAndroidNotificationHostApi? _androidNotificationHostApi; + @override + FakeNotificationPigeonApi get notificationPigeonApi => + (_notificationPigeonApi ??= FakeNotificationPigeonApi()); + FakeNotificationPigeonApi? _notificationPigeonApi; + /// The value that `ZulipBinding.instance.pickFiles()` should return. /// /// See also [takePickFilesCalls]. @@ -754,6 +761,20 @@ class FakeAndroidNotificationHostApi implements AndroidNotificationHostApi { } } +class FakeNotificationPigeonApi implements NotificationPigeonApi { + NotificationDataFromLaunch? _notificationDataFromLaunch; + + /// Populates the notification data for launch to be returned + /// by [getNotificationDataFromLaunch]. + void setNotificationDataFromLaunch(NotificationDataFromLaunch? data) { + _notificationDataFromLaunch = data; + } + + @override + Future getNotificationDataFromLaunch() async => + _notificationDataFromLaunch; +} + typedef AndroidNotificationHostApiNotifyCall = ({ String? tag, int id, diff --git a/test/model/store_test.dart b/test/model/store_test.dart index 0b303b53e2..c3aadb1171 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -1293,8 +1293,8 @@ void main() { // (This is probably the common case.) addTearDown(testBinding.reset); testBinding.firebaseMessagingInitialToken = '012abc'; - addTearDown(NotificationService.debugReset); testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.zulip.flutter'); + addTearDown(NotificationService.debugReset); await NotificationService.instance.start(); // On store startup, send the token. @@ -1321,8 +1321,8 @@ void main() { // request for the token is still pending. addTearDown(testBinding.reset); testBinding.firebaseMessagingInitialToken = '012abc'; - addTearDown(NotificationService.debugReset); testBinding.packageInfoResult = eg.packageInfo(packageName: 'com.zulip.flutter'); + addTearDown(NotificationService.debugReset); final startFuture = NotificationService.instance.start(); // TODO this test is a bit brittle in its interaction with asynchrony; diff --git a/test/notifications/open_test.dart b/test/notifications/open_test.dart index 3d7cc7c531..67f816c86a 100644 --- a/test/notifications/open_test.dart +++ b/test/notifications/open_test.dart @@ -1,10 +1,12 @@ import 'dart:async'; import 'package:checks/checks.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/notifications.dart'; +import 'package:zulip/host/notifications.dart'; import 'package:zulip/model/database.dart'; import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; @@ -137,18 +139,37 @@ void main() { } Future openNotification(WidgetTester tester, Account account, Message message) async { - final intentDataUrl = androidNotificationUrlForMessage(account, message); - unawaited( - WidgetsBinding.instance.handlePushRoute(intentDataUrl.toString())); - await tester.idle(); // let navigateForNotification find navigator + switch (defaultTargetPlatform) { + case TargetPlatform.android: + final intentDataUrl = androidNotificationUrlForMessage(account, message); + unawaited( + WidgetsBinding.instance.handlePushRoute(intentDataUrl.toString())); + await tester.idle(); // let navigateForNotification find navigator + + default: + throw UnsupportedError('Unsupported target platform: "$defaultTargetPlatform"'); + } } void setupNotificationDataForLaunch(WidgetTester tester, Account account, Message message) { - // Set up a value for `PlatformDispatcher.defaultRouteName` to return, - // for determining the initial route. - final intentDataUrl = androidNotificationUrlForMessage(account, message); - addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); - tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); + switch (defaultTargetPlatform) { + case TargetPlatform.android: + // Set up a value for `PlatformDispatcher.defaultRouteName` to return, + // for determining the initial route. + final intentDataUrl = androidNotificationUrlForMessage(account, message); + addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); + tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); + + case TargetPlatform.iOS: + // Set up a value to return for + // `notificationPigeonApi.getNotificationDataFromLaunch`. + final payload = messageApnsPayload(message, account: account); + testBinding.notificationPigeonApi.setNotificationDataFromLaunch( + NotificationDataFromLaunch(payload: payload)); + + default: + throw UnsupportedError('Unsupported target platform: "$defaultTargetPlatform"'); + } } void matchesNavigation(Subject> route, Account account, Message message) { @@ -169,14 +190,14 @@ void main() { addTearDown(testBinding.reset); await prepare(tester); await checkOpenNotification(tester, eg.selfAccount, eg.streamMessage()); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android})); testWidgets('direct message', (tester) async { addTearDown(testBinding.reset); await prepare(tester); await checkOpenNotification(tester, eg.selfAccount, eg.dmMessage(from: eg.otherUser, to: [eg.selfUser])); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android})); testWidgets('account queried by realmUrl origin component', (tester) async { addTearDown(testBinding.reset); @@ -189,7 +210,7 @@ void main() { await checkOpenNotification(tester, eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example')), eg.streamMessage()); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android})); testWidgets('no accounts', (tester) async { await prepare(tester, withAccount: false); @@ -199,7 +220,7 @@ void main() { await tester.tap(find.byWidget(checkErrorDialog(tester, expectedTitle: zulipLocalizations.errorNotificationOpenTitle, expectedMessage: zulipLocalizations.errorNotificationOpenAccountNotFound))); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android})); testWidgets('mismatching account', (tester) async { addTearDown(testBinding.reset); @@ -210,7 +231,7 @@ void main() { await tester.tap(find.byWidget(checkErrorDialog(tester, expectedTitle: zulipLocalizations.errorNotificationOpenTitle, expectedMessage: zulipLocalizations.errorNotificationOpenAccountNotFound))); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android})); testWidgets('find account among several', (tester) async { addTearDown(testBinding.reset); @@ -236,7 +257,7 @@ void main() { await checkOpenNotification(tester, accounts[1], eg.streamMessage()); await checkOpenNotification(tester, accounts[2], eg.streamMessage()); await checkOpenNotification(tester, accounts[3], eg.streamMessage()); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android})); testWidgets('wait for app to become ready', (tester) async { addTearDown(testBinding.reset); @@ -255,7 +276,7 @@ void main() { takeStartingRoutes(account: eg.selfAccount); // … and then the one the notification leads to. matchesNavigation(check(pushedRoutes).single, eg.selfAccount, message); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android})); testWidgets('at app launch', (tester) async { addTearDown(testBinding.reset); @@ -271,7 +292,7 @@ void main() { await tester.pump(); takeStartingRoutes(account: account); matchesNavigation(check(pushedRoutes).single, account, message); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); testWidgets('uses associated account as initial account, if initial route', (tester) async { addTearDown(testBinding.reset); @@ -289,7 +310,7 @@ void main() { await tester.pump(); takeStartingRoutes(account: accountB); matchesNavigation(check(pushedRoutes).single, accountB, message); - }); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); }); group('NotificationOpenPayload', () { From 02506ba1b9b261bb3a7f7e027b1a4d51cf57bb16 Mon Sep 17 00:00:00 2001 From: Rajesh Malviya Date: Mon, 3 Mar 2025 20:57:00 +0530 Subject: [PATCH 16/16] notif ios: Navigate when app running but in background --- ios/Runner/AppDelegate.swift | 33 ++++++++++ ios/Runner/Notifications.g.swift | 100 ++++++++++++++++++++++++++++++ lib/host/notifications.g.dart | 64 +++++++++++++++++++ lib/model/binding.dart | 6 ++ lib/notifications/open.dart | 23 +++++++ pigeon/notifications.dart | 23 +++++++ test/model/binding.dart | 12 ++++ test/notifications/open_test.dart | 20 +++--- 8 files changed, 274 insertions(+), 7 deletions(-) diff --git a/ios/Runner/AppDelegate.swift b/ios/Runner/AppDelegate.swift index 33a0fe72cb..eefed07cd6 100644 --- a/ios/Runner/AppDelegate.swift +++ b/ios/Runner/AppDelegate.swift @@ -3,6 +3,8 @@ import Flutter @main @objc class AppDelegate: FlutterAppDelegate { + private var notificationTapEventListener: NotificationTapEventListener? + override func application( _ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? @@ -16,8 +18,25 @@ import Flutter let api = NotificationHostApiImpl(notificationPayload.map { NotificationDataFromLaunch(payload: $0) }) NotificationHostApiSetup.setUp(binaryMessenger: controller.binaryMessenger, api: api) + notificationTapEventListener = NotificationTapEventListener() + NotificationTapEventsStreamHandler.register(with: controller.binaryMessenger, streamHandler: notificationTapEventListener!) + + UNUserNotificationCenter.current().delegate = self + return super.application(application, didFinishLaunchingWithOptions: launchOptions) } + + override func userNotificationCenter( + _ center: UNUserNotificationCenter, + didReceive response: UNNotificationResponse, + withCompletionHandler completionHandler: @escaping () -> Void + ) { + if response.actionIdentifier == UNNotificationDefaultActionIdentifier { + let userInfo = response.notification.request.content.userInfo + notificationTapEventListener!.onNotificationTapEvent(payload: userInfo) + } + completionHandler() + } } private class NotificationHostApiImpl: NotificationHostApi { @@ -31,3 +50,17 @@ private class NotificationHostApiImpl: NotificationHostApi { maybeDataFromLaunch } } + +// Adapted from Pigeon's Swift example for @EventChannelApi: +// https://github.com/flutter/packages/blob/2dff6213a/packages/pigeon/example/app/ios/Runner/AppDelegate.swift#L49-L74 +class NotificationTapEventListener: NotificationTapEventsStreamHandler { + var eventSink: PigeonEventSink? + + override func onListen(withArguments arguments: Any?, sink: PigeonEventSink) { + eventSink = sink + } + + func onNotificationTapEvent(payload: [AnyHashable : Any]) { + eventSink?.success(NotificationTapEvent(payload: payload)) + } +} diff --git a/ios/Runner/Notifications.g.swift b/ios/Runner/Notifications.g.swift index 342953fbad..40db818d33 100644 --- a/ios/Runner/Notifications.g.swift +++ b/ios/Runner/Notifications.g.swift @@ -157,11 +157,42 @@ struct NotificationDataFromLaunch: Hashable { } } +/// Generated class from Pigeon that represents data sent in messages. +struct NotificationTapEvent: Hashable { + /// The raw payload that is attached to the notification, + /// holding the information required to carry out the navigation. + /// + /// See [notificationTapEvents]. + var payload: [AnyHashable?: Any?] + + + // swift-format-ignore: AlwaysUseLowerCamelCase + static func fromList(_ pigeonVar_list: [Any?]) -> NotificationTapEvent? { + let payload = pigeonVar_list[0] as! [AnyHashable?: Any?] + + return NotificationTapEvent( + payload: payload + ) + } + func toList() -> [Any?] { + return [ + payload + ] + } + static func == (lhs: NotificationTapEvent, rhs: NotificationTapEvent) -> Bool { + return deepEqualsNotifications(lhs.toList(), rhs.toList()) } + func hash(into hasher: inout Hasher) { + deepHashNotifications(value: toList(), hasher: &hasher) + } +} + private class NotificationsPigeonCodecReader: FlutterStandardReader { override func readValue(ofType type: UInt8) -> Any? { switch type { case 129: return NotificationDataFromLaunch.fromList(self.readValue() as! [Any?]) + case 130: + return NotificationTapEvent.fromList(self.readValue() as! [Any?]) default: return super.readValue(ofType: type) } @@ -173,6 +204,9 @@ private class NotificationsPigeonCodecWriter: FlutterStandardWriter { if let value = value as? NotificationDataFromLaunch { super.writeByte(129) super.writeValue(value.toList()) + } else if let value = value as? NotificationTapEvent { + super.writeByte(130) + super.writeValue(value.toList()) } else { super.writeValue(value) } @@ -193,6 +227,8 @@ class NotificationsPigeonCodec: FlutterStandardMessageCodec, @unchecked Sendable static let shared = NotificationsPigeonCodec(readerWriter: NotificationsPigeonCodecReaderWriter()) } +var notificationsPigeonMethodCodec = FlutterStandardMethodCodec(readerWriter: NotificationsPigeonCodecReaderWriter()); + /// Generated protocol from Pigeon that represents a handler of messages from Flutter. protocol NotificationHostApi { /// Retrieves notification data if the app was launched by tapping on a notification. @@ -233,3 +269,67 @@ class NotificationHostApiSetup { } } } + +private class PigeonStreamHandler: NSObject, FlutterStreamHandler { + private let wrapper: PigeonEventChannelWrapper + private var pigeonSink: PigeonEventSink? = nil + + init(wrapper: PigeonEventChannelWrapper) { + self.wrapper = wrapper + } + + func onListen(withArguments arguments: Any?, eventSink events: @escaping FlutterEventSink) + -> FlutterError? + { + pigeonSink = PigeonEventSink(events) + wrapper.onListen(withArguments: arguments, sink: pigeonSink!) + return nil + } + + func onCancel(withArguments arguments: Any?) -> FlutterError? { + pigeonSink = nil + wrapper.onCancel(withArguments: arguments) + return nil + } +} + +class PigeonEventChannelWrapper { + func onListen(withArguments arguments: Any?, sink: PigeonEventSink) {} + func onCancel(withArguments arguments: Any?) {} +} + +class PigeonEventSink { + private let sink: FlutterEventSink + + init(_ sink: @escaping FlutterEventSink) { + self.sink = sink + } + + func success(_ value: ReturnType) { + sink(value) + } + + func error(code: String, message: String?, details: Any?) { + sink(FlutterError(code: code, message: message, details: details)) + } + + func endOfStream() { + sink(FlutterEndOfEventStream) + } + +} + +class NotificationTapEventsStreamHandler: PigeonEventChannelWrapper { + static func register(with messenger: FlutterBinaryMessenger, + instanceName: String = "", + streamHandler: NotificationTapEventsStreamHandler) { + var channelName = "dev.flutter.pigeon.zulip.NotificationEventChannelApi.notificationTapEvents" + if !instanceName.isEmpty { + channelName += ".\(instanceName)" + } + let internalStreamHandler = PigeonStreamHandler(wrapper: streamHandler) + let channel = FlutterEventChannel(name: channelName, binaryMessenger: messenger, codec: notificationsPigeonMethodCodec) + channel.setStreamHandler(internalStreamHandler) + } +} + diff --git a/lib/host/notifications.g.dart b/lib/host/notifications.g.dart index d8448d60b8..a83b67c804 100644 --- a/lib/host/notifications.g.dart +++ b/lib/host/notifications.g.dart @@ -74,6 +74,51 @@ class NotificationDataFromLaunch { ; } +class NotificationTapEvent { + NotificationTapEvent({ + required this.payload, + }); + + /// The raw payload that is attached to the notification, + /// holding the information required to carry out the navigation. + /// + /// See [notificationTapEvents]. + Map payload; + + List _toList() { + return [ + payload, + ]; + } + + Object encode() { + return _toList(); } + + static NotificationTapEvent decode(Object result) { + result as List; + return NotificationTapEvent( + payload: (result[0] as Map?)!.cast(), + ); + } + + @override + // ignore: avoid_equals_and_hash_code_on_mutable_classes + bool operator ==(Object other) { + if (other is! NotificationTapEvent || other.runtimeType != runtimeType) { + return false; + } + if (identical(this, other)) { + return true; + } + return _deepEquals(encode(), other.encode()); + } + + @override + // ignore: avoid_equals_and_hash_code_on_mutable_classes + int get hashCode => Object.hashAll(_toList()) +; +} + class _PigeonCodec extends StandardMessageCodec { const _PigeonCodec(); @@ -85,6 +130,9 @@ class _PigeonCodec extends StandardMessageCodec { } else if (value is NotificationDataFromLaunch) { buffer.putUint8(129); writeValue(buffer, value.encode()); + } else if (value is NotificationTapEvent) { + buffer.putUint8(130); + writeValue(buffer, value.encode()); } else { super.writeValue(buffer, value); } @@ -95,12 +143,16 @@ class _PigeonCodec extends StandardMessageCodec { switch (type) { case 129: return NotificationDataFromLaunch.decode(readValue(buffer)!); + case 130: + return NotificationTapEvent.decode(readValue(buffer)!); default: return super.readValueOfType(type, buffer); } } } +const StandardMethodCodec pigeonMethodCodec = StandardMethodCodec(_PigeonCodec()); + class NotificationHostApi { /// Constructor for [NotificationHostApi]. The [binaryMessenger] named argument is /// available for dependency injection. If it is left null, the default @@ -144,3 +196,15 @@ class NotificationHostApi { } } } + +Stream notificationTapEvents( {String instanceName = ''}) { + if (instanceName.isNotEmpty) { + instanceName = '.$instanceName'; + } + final EventChannel notificationTapEventsChannel = + EventChannel('dev.flutter.pigeon.zulip.NotificationEventChannelApi.notificationTapEvents$instanceName', pigeonMethodCodec); + return notificationTapEventsChannel.receiveBroadcastStream().map((dynamic event) { + return event as NotificationTapEvent; + }); +} + diff --git a/lib/model/binding.dart b/lib/model/binding.dart index 7f9b0f6fd1..4d1a0adaac 100644 --- a/lib/model/binding.dart +++ b/lib/model/binding.dart @@ -328,11 +328,17 @@ class PackageInfo { }); } +// Pigeon generates methods under `@EventChannelApi` annotated classes +// in global scope of the generated file. This is a helper class to +// namespace the notification related Pigeon API under a single class. class NotificationPigeonApi { final _hostApi = notif_pigeon.NotificationHostApi(); Future getNotificationDataFromLaunch() => _hostApi.getNotificationDataFromLaunch(); + + Stream notificationTapEventsStream() => + notif_pigeon.notificationTapEvents(); } /// A concrete binding for use in the live application. diff --git a/lib/notifications/open.dart b/lib/notifications/open.dart index 3f136f695c..fc915592d3 100644 --- a/lib/notifications/open.dart +++ b/lib/notifications/open.dart @@ -49,6 +49,8 @@ class NotificationOpenService { switch (defaultTargetPlatform) { case TargetPlatform.iOS: _notifDataFromLaunch = await _notifPigeonApi.getNotificationDataFromLaunch(); + _notifPigeonApi.notificationTapEventsStream() + .listen(_navigateForNotification); case TargetPlatform.android: // Do nothing; we do notification routing differently on Android. @@ -116,6 +118,27 @@ class NotificationOpenService { narrow: data.narrow); } + /// Navigates to the [MessageListPage] of the specific conversation + /// for the provided payload that was attached while creating the + /// notification. + static Future _navigateForNotification(NotificationTapEvent event) async { + assert(defaultTargetPlatform == TargetPlatform.iOS); + assert(debugLog('opened notif: ${jsonEncode(event.payload)}')); + + 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 notifNavData = _tryParseIosApnsPayload(context, event.payload); + if (notifNavData == null) return; // TODO(log) + final route = routeForNotification(context: context, data: notifNavData); + if (route == null) return; // TODO(log) + + // TODO(nav): Better interact with existing nav stack on notif open + unawaited(navigator.push(route)); + } + /// Navigates to the [MessageListPage] of the specific conversation /// given the `zulip://notification/…` Android intent data URL, /// generated with [NotificationOpenPayload.buildAndroidNotificationUrl] diff --git a/pigeon/notifications.dart b/pigeon/notifications.dart index efea52d9a6..66c1bd2e71 100644 --- a/pigeon/notifications.dart +++ b/pigeon/notifications.dart @@ -17,6 +17,16 @@ class NotificationDataFromLaunch { final Map payload; } +class NotificationTapEvent { + const NotificationTapEvent({required this.payload}); + + /// The raw payload that is attached to the notification, + /// holding the information required to carry out the navigation. + /// + /// See [notificationTapEvents]. + final Map payload; +} + @HostApi() abstract class NotificationHostApi { /// Retrieves notification data if the app was launched by tapping on a notification. @@ -28,3 +38,16 @@ abstract class NotificationHostApi { /// https://developer.apple.com/documentation/uikit/uiapplication/launchoptionskey/remotenotification NotificationDataFromLaunch? getNotificationDataFromLaunch(); } + +@EventChannelApi() +abstract class NotificationEventChannelApi { + /// An event stream that emits a notification payload when the app + /// encounters a notification tap, while the app is running. + /// + /// Emits an event when + /// `userNotificationCenter(_:didReceive:withCompletionHandler:)` gets + /// called, indicating that the user has tapped on a notification. The + /// emitted payload will be the raw APNs data dictionary from the + /// `UNNotificationResponse` passed to that method. + NotificationTapEvent notificationTapEvents(); +} diff --git a/test/model/binding.dart b/test/model/binding.dart index afeed2f266..839242c1ce 100644 --- a/test/model/binding.dart +++ b/test/model/binding.dart @@ -773,6 +773,18 @@ class FakeNotificationPigeonApi implements NotificationPigeonApi { @override Future getNotificationDataFromLaunch() async => _notificationDataFromLaunch; + + StreamController? _notificationTapEventsStreamController; + + void addNotificationTapEvent(NotificationTapEvent event) { + _notificationTapEventsStreamController!.add(event); + } + + @override + Stream notificationTapEventsStream() { + _notificationTapEventsStreamController ??= StreamController(); + return _notificationTapEventsStreamController!.stream; + } } typedef AndroidNotificationHostApiNotifyCall = ({ diff --git a/test/notifications/open_test.dart b/test/notifications/open_test.dart index 67f816c86a..6c778a11e9 100644 --- a/test/notifications/open_test.dart +++ b/test/notifications/open_test.dart @@ -146,6 +146,12 @@ void main() { WidgetsBinding.instance.handlePushRoute(intentDataUrl.toString())); await tester.idle(); // let navigateForNotification find navigator + case TargetPlatform.iOS: + final payload = messageApnsPayload(message, account: account); + testBinding.notificationPigeonApi.addNotificationTapEvent( + NotificationTapEvent(payload: payload)); + await tester.idle(); // let navigateForNotification find navigator + default: throw UnsupportedError('Unsupported target platform: "$defaultTargetPlatform"'); } @@ -190,14 +196,14 @@ void main() { addTearDown(testBinding.reset); await prepare(tester); await checkOpenNotification(tester, eg.selfAccount, eg.streamMessage()); - }, variant: const TargetPlatformVariant({TargetPlatform.android})); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); testWidgets('direct message', (tester) async { addTearDown(testBinding.reset); await prepare(tester); await checkOpenNotification(tester, eg.selfAccount, eg.dmMessage(from: eg.otherUser, to: [eg.selfUser])); - }, variant: const TargetPlatformVariant({TargetPlatform.android})); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); testWidgets('account queried by realmUrl origin component', (tester) async { addTearDown(testBinding.reset); @@ -210,7 +216,7 @@ void main() { await checkOpenNotification(tester, eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example')), eg.streamMessage()); - }, variant: const TargetPlatformVariant({TargetPlatform.android})); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); testWidgets('no accounts', (tester) async { await prepare(tester, withAccount: false); @@ -220,7 +226,7 @@ void main() { await tester.tap(find.byWidget(checkErrorDialog(tester, expectedTitle: zulipLocalizations.errorNotificationOpenTitle, expectedMessage: zulipLocalizations.errorNotificationOpenAccountNotFound))); - }, variant: const TargetPlatformVariant({TargetPlatform.android})); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); testWidgets('mismatching account', (tester) async { addTearDown(testBinding.reset); @@ -231,7 +237,7 @@ void main() { await tester.tap(find.byWidget(checkErrorDialog(tester, expectedTitle: zulipLocalizations.errorNotificationOpenTitle, expectedMessage: zulipLocalizations.errorNotificationOpenAccountNotFound))); - }, variant: const TargetPlatformVariant({TargetPlatform.android})); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); testWidgets('find account among several', (tester) async { addTearDown(testBinding.reset); @@ -257,7 +263,7 @@ void main() { await checkOpenNotification(tester, accounts[1], eg.streamMessage()); await checkOpenNotification(tester, accounts[2], eg.streamMessage()); await checkOpenNotification(tester, accounts[3], eg.streamMessage()); - }, variant: const TargetPlatformVariant({TargetPlatform.android})); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); testWidgets('wait for app to become ready', (tester) async { addTearDown(testBinding.reset); @@ -276,7 +282,7 @@ void main() { takeStartingRoutes(account: eg.selfAccount); // … and then the one the notification leads to. matchesNavigation(check(pushedRoutes).single, eg.selfAccount, message); - }, variant: const TargetPlatformVariant({TargetPlatform.android})); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); testWidgets('at app launch', (tester) async { addTearDown(testBinding.reset);