notif: package info indirection #1143
Conversation
Abhisheksainii
changed the title
|
Hi @Abhisheksainii, thanks for the PR. It will need to follow our Git style before it gets reviewed by a core team member. |
|
@chrisbobbe changes made according to the git style. |
Abhisheksainii
force-pushed
the
fix/package-info-indirection
branch
from
75331d7 to
5970957
Compare
There was a problem hiding this comment.
Thanks for the PR @Abhisheksainii! Looks like there is a simplification to be made, because we already access and store packageInfo. It will probably require you to modify our copy of PackageInfo class by adding packageName to it.
|
@PIG208 changes done as requested, kindly take a look at them. |
|
Thanks for the update. Please review the "commit style guide" referenced in the README and update your commits: https://github.com/zulip/zulip-flutter?tab=readme-ov-file#submitting-a-pull-request
|
@PIG208 I checked the commits and they pretty much explain what they do. Am I supposed to add some more description? |
|
Please read the Zulip commit style guide, which is linked from the README where Zixuan pointed to. |
Abhisheksainii
force-pushed
the
fix/package-info-indirection
branch
from
c119c94 to
2cc9de6
Compare
@PIG208 commits updated according to the style guide. Kindly review them |
Abhisheksainii
force-pushed
the
fix/package-info-indirection
branch
2 times, most recently
from
3a87065 to
ff11675
Compare
|
@PIG208 kindly review the changes. |
|
Thanks for the update @Abhisheksainii! Left some comments. |
Abhisheksainii
force-pushed
the
fix/package-info-indirection
branch
from
ff11675 to
9849d78
Compare
|
@PIG208 made changes as requested. Kindly review. |
made packageName accessible from PackageInfo class. This helps in writing tests more efficiently
made packageName accessible to receive.dart file and test files via (await ZulipBinding.instance.packageInfo)?.packageName ?? "com.zulip.flutter"
enabled test files and other modules to simulate packageInfo being null or set custom values via ZulipBinding.instance.packageInfo
removed non-fallback case for app bundleId as is already covered under store_test file removed unnecessary helper function setPackageInfo
Abhisheksainii
force-pushed
the
fix/package-info-indirection
branch
from
9849d78 to
90a7774
Compare
|
@PIG208 made changes as requested. Kindly review. |
In this PR, I made the changes to indirect PackageInfo.fromPlatform through a method on ZulipBinding.
Steps I followed :
Update the ZulipBinding to provide package info: Add a method in ZulipBinding to return the package name. This method will wrap the call to PackageInfo.fromPlatform() and make it test-friendly.
Override the method in TestZulipBinding: In the TestZulipBinding, override getAppBundleId to return a mock or predefined value for testing.
Refactor the code to use getAppBundleId: Replace the direct calls with the indirection from Zulip Binding class.
Write a test for the above logic under a group called 'tokens'. The allow the grouping of all tests related to tokens, I created a separate group : tokens.
Fixes #407