Adds subscriptions to CustomerInfo

[vegaro]

We had more information we were not exposing in the CustomerInfo JSON

This PR adds CustomerInfo.subscriptions which has some useful information for devs that don't use the entitlements system

[aboedo]

I think this is fine.

But there's one thing that just feels a bit odd to me in the fact that we're using our own public REST API, but then we abstract it privately, then make it public again.

It's not like it's a very internal REST API either - it's very much public and stable and we wouldn't just modify it out the blue since it would break older SDKs, which makes me feel like we're overcomplicating things a bit

[aboedo]

my first thought was that it's a bit confusing that this is a dict but nonSubscriptions is an array. I get why, but naming-wise I'd maybe have expected both to be the same. Wonder if subscriptionsByProductIdentifier would have helped, but not sure. Or maybe even typealiasing ProductIdentifier = String so that then you have the type [ProductIdentifier: SubscriptionInfo].
Then again you could make the argument that we should have all APIs do that or none ¯\(ツ)/¯

[tonidero]

It's also a bit confusing with activeSubscriptions being only the subs ids and this being full objects... Maybe something like allSubscriptionsInfo? Or maybe too verbose not sure...

[JayShortway]

Maybe we could deprecate activeSubscriptions as part of this change? Not a ton of value in that anymore, and very easy to recreate with subscriptions.

[vegaro]

yes, @fire-at-will made the same comment that maybe subscriptions could be a Set. Imho I think a dictionary makes more sense since most people using this would want to get a subscription by product identifier, and making it a Set forces them to use a filter. Making it a dictionary offers accessing by product id and also offers a .values to access only the values.

Thoughts:

• activeSubscriptions should really be activeSubscriptionProductIds or something like that
• I think I would not want to deprecate activeSubscriptions, maybe in the next major, but I still see activeSubscriptions valuable since it saves folks from having to do logic on subscriptions to get the active ones. I would treat it as syntactic sugar.
• allSubscriptionsInfo also makes sense, but I don't think it cleans up the fact that activeSubscriptions is just the product ids and allSubscriptionsInfo classes. I think it would still missing the informatino that it is a dictionary
• subscriptionsByProductIdentifier makes a lot of sense. And the typealias ProductIdentifier can also be added. I think that would be the most informative and most correct. I think byProductIdentifier is important so we can replicate it in the rest of the SDKs that don't have type aliases

[aboedo]

or rather, I guess what I'm asking is "should we just make the subscriber object public instead"

[JayShortway]

I'm pretty excited about this! 😄 No comments other than Andy's.

[JayShortway]

Just checking that we know this is always available, and having it optional was being too defensive?

[vegaro]

I checked in the backend and it's not optional there

[tonidero]

Agreed this is great! Left some comments but I think this could work as is.

[tonidero]

Small concern but I'm wondering if we should have a VerificationResult (from trusted entitlements) here as well, or maybe move the existing one in EntitlementInfos up to the CustomerInfo entity... otherwise some devs might use this without checking trusted entitlements...

[tonidero]

It's also a bit confusing with activeSubscriptions being only the subs ids and this being full objects... Maybe something like allSubscriptionsInfo? Or maybe too verbose not sure...

[vegaro]

@tonidero @aboedo @JayShortway made some updates in 0d388c5 (#4508), let me know what you think

[tonidero]