-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
notif: Use live value for app ID on registering APNs token #407
Labels
Milestone
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Abhisheksainii
added a commit
to Abhisheksainii/zulip-flutter
that referenced
this issue
Dec 21, 2024
made changes to access packageName from the PackageInfo class. remove getAppBundleId() from Zulip Binding class Add packageName variable to PackageInfo class Fixes zulip#407
Abhisheksainii
added a commit
to Abhisheksainii/zulip-flutter
that referenced
this issue
Jan 2, 2025
made changes to access packageName from the PackageInfo class. remove getAppBundleId() from Zulip Binding class Add packageName variable to PackageInfo class Fixes zulip#407
Abhisheksainii
added a commit
to Abhisheksainii/zulip-flutter
that referenced
this issue
Jan 30, 2025
… registration To fetch live value of appBundleId through Zulip Binding class, we made use of already establised PackageInfo class by introducing new parameter because using this version directly(final appBundleId = (await PackageInfo.fromPlatform()).packageName;) would break tests as it's directly calling a plugin. Fixes zulip#407 The reason for making packageInfoResult nullable in Zulip Binding class was to test the case where appId fallbacks to default appBundleId (com.zulip.flutter)
AhmedTareek
added a commit
to AhmedTareek/zulip-flutter
that referenced
this issue
Apr 1, 2025
`await ZulipBinding.instance.packageInfo` was added to get the packageName, making `registerToken` take longer to complete. Previously, in "token initially unknown" test in the UpdateMachine.registerNotificationToken `start()` and `_registerNotificationToken()` finished in sync with the test’s expectations. Now, a delay is added to ensure that the listener callback `_registerNotificationToken()` finishes its async work before continuing the test. Fixes zulip#407
AhmedTareek
added a commit
to AhmedTareek/zulip-flutter
that referenced
this issue
Apr 2, 2025
`await ZulipBinding.instance.packageInfo` was added to get the packageName, making `registerToken` take longer to complete. Previously, in "token initially unknown" test in the UpdateMachine.registerNotificationToken `start()` and `_registerNotificationToken()` finished in sync with the test’s expectations. Now, a delay is added to ensure that the listener callback `_registerNotificationToken()` finishes its async work before continuing the test. Fixes zulip#407
AhmedTareek
added a commit
to AhmedTareek/zulip-flutter
that referenced
this issue
Apr 11, 2025
`await ZulipBinding.instance.packageInfo` was added to get the packageName, making `registerToken` take longer to complete. Previously, in "token initially unknown" test in the UpdateMachine.registerNotificationToken `start()` and `_registerNotificationToken()` finished in sync with the test’s expectations. Now, `flushMicrotasks` is added to ensure that the listener callback `_registerNotificationToken()` finishes its async work before continuing the test. Fixes zulip#407
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In my draft for #321 I have a TODO like this:
This will be just fine as long as the app ID is indeed
com.zulip.flutter
. That means it will basically be fine right through the beta period, until we start rolling this app out as the new version oforg.zulip.Zulip
(the app ID currently occupied by the zulip-mobile RN app).The fix is easy, locally:
The one wrinkle is that that version breaks tests, because it's directly calling a plugin. To fix it, we'll want to indirect
PackageInfo.fromPlatform
through a method onZulipBinding
, like we've done for other plugins. This still isn't hard, but given the impending beta, I'm deferring it for now.The text was updated successfully, but these errors were encountered: