Galaxy Attribution Fetcher#2917
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## samsung-dev #2917 +/- ##
===============================================
- Coverage 78.36% 78.36% -0.01%
===============================================
Files 337 338 +1
Lines 12986 12989 +3
Branches 1758 1758
===============================================
+ Hits 10177 10179 +2
- Misses 2070 2071 +1
Partials 739 739 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tonidero
left a comment
There was a problem hiding this comment.
Just a simplification that I think would be nice to have
| } | ||
| Store.GALAXY -> { | ||
| try { | ||
| Class.forName("com.revenuecat.purchases.galaxy.attribution.GalaxyDeviceIdentifiersFetcher") |
There was a problem hiding this comment.
Hmm I understand this looks like this for Amazon, but looking at it, this seems to be a leftover from before we refactored the modules to have the amazon code be part of the purchases module as a compile only dependency.
Basically, I don't think we need to use reflection to instantiate this class since it will always be there (it's part of the same module) and we can just do GalaxyDeviceIdentifiersFetcher().
I think the same applies to the amazon one, but we can split that one in a separate PR to main.
There was a problem hiding this comment.
Removed the reflection for the GalaxyDeviceIdentifiersFetcher in 3db7de7
I'll remove it for Amazon in a follow-up PR into main to clean it up :)
There was a problem hiding this comment.
PR to remove reflection from the Amazon instantiation here: #2919 :)
…2919) ### Description Follow-up PR from [this comment thread](#2917 (comment)) on PR #2917. We no longer need to use reflection to instantiate the `AmazonDeviceIdentifiersFetcher` class since it is now in the main module of the SDK, so this PR changes the `AmazonDeviceIdentifiersFetcher`'s instantiation from using reflection to using a normal constructor.
| applicationContext: Application, | ||
| completion: (deviceIdentifiers: Map<String, String>) -> Unit, | ||
| ) { | ||
| // Samsung IAP library doesn't support fetching device identifiers, so this is a no-op |
There was a problem hiding this comment.
For Amazon, we also don't get this from their IAP library, but from the device settings. Is that something that would work for Galaxy devices? Can be done in a separate PR though of course!
There was a problem hiding this comment.
Yeah, I don't see why not! We can come back and do that in another PR, I think for now we've cut the device identifiers from the scope to ship quickly
Description
Without an AttributionFetcher for the Galaxy Store, the SDK will crash at runtime when configured for the Galaxy Store, so we need to create one. Since the Samsung IAP SDK doesn't support fetching device identifiers, this PR creates a no-op
GalaxyDeviceIdentifiersFetcherthat always returns 0 identifiers, and wires it into theAttributionFetcherFactory.