-
Notifications
You must be signed in to change notification settings - Fork 249
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
Comments
My approach to solve the issue:
|
I have started working on the issue and would like to claim it. |
made changes to access packageName from the PackageInfo class. remove getAppBundleId() from Zulip Binding class Add packageName variable to PackageInfo class Fixes zulip#407
made changes to access packageName from the PackageInfo class. remove getAppBundleId() from Zulip Binding class Add packageName variable to PackageInfo class Fixes zulip#407
… 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)
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: