RevenueCat
[Draft] Split logic at Purchases level
| 1 Error | |
|---|---|
| :no_entry_sign: | Label the PR using one of the change type labels |
| Label | Description |
|---|---|
| breaking | Changes that are breaking |
| build | Changes that affect the build system |
| ci | Changes to our CI configuration files and scripts |
| docs | Documentation only changes |
| feat | A new feature |
| fix | A bug fix |
| perf | A code change that improves performance |
| refactor | A code change that neither fixes a bug nor adds a feature |
| style | Changes that don't affect the meaning of the code (white-space, formatting, missing semi-colons, etc |
| test | Adding missing tests or correcting existing tests |
| next_release | Preparing a new release |
| dependencies | Updating a dependency |
Generated by :no_entry_sign: Danger
i'm starting to think we should have useBC5 as a configuration parameter as well as in the dashboard, or in replacement of it. knowing whether we are using BC5 at configuration-time makes all of this logic a lot easier.
we could then consider instantiating an entirely different BIllingWrapper based on that parameter. perhaps the BC5 one overrides the BC4, inheriting most stuff but with its own implementation of querySkuDetails. then we don't have to split logic based on what is returned from getOfferings, and we know whether to use bc5 in the non-offerings flows
we will want to have good error messages if they haven’t updated their subscriptions in the RC dash to have group ID or if they aren't feature flagged on.
we could maybe even pass this parameter on to getOfferings, and it can return the proper ids based on that (rather than trying to be backwards-compatible with every response)
I started my thoughts here: https://github.com/RevenueCat/purchases-android/pull/638 .. I need to sign off for now but hoping I can address the offerings stuff further, I know this doesn’t answer all of your questions yet.
i know we considered the SDK-side toggle before, and i believe i was the one that shut it down, but it sounds like it might make sense now...will dig through my notes to remind myself why i shut it down before though
added a couple more commits with my ideas for how the useBC5 parameter affects things downstream.. definitely still need some thought on the storeproduct, so i just put TODOS for now. stepping through by commit on that PR might make it more consumable
i know this is a total 180 from what you had... was just trying to formulate what my thoughts were and coding it out made the most sense to me. i don't think my solution at this point solves the entire package parsing like yours does. i imagine we can use pieces from both of our approaches.
would still love to do a pairing session to understand your approach, i'm not sure i fully followed the template stuff
@swehner can we close this one? i think it's all been moved to your other poc branch by now...