Skip to content
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

Add ownershipType to entitlementInfo #278

Merged
merged 4 commits into from Dec 2, 2021
Merged

Conversation

daentech
Copy link
Contributor

@daentech daentech commented Nov 29, 2021

Description:

This PR exposes ownershipType on EntitlementInfo as, while it had been added to purchases-hybrid-common, it was unavailable in the flutter SDK itself.

purchases-android PR: RevenueCat/purchases-android#382
purchases-hybrid-common PR: RevenueCat/purchases-hybrid-common#103

  • The issue number(s) or PR number(s) in the description if you are contributing in response to those.

Related to Zendesk ticket: 12668

  • If applicable, unit tests.

Copy link
Contributor

@vegaro vegaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for opening this. I am not sure how it happened that we released 3.7.0 without the actual Dart changes 🤦

CHANGELOG.md Outdated
@@ -1,3 +1,5 @@
- Adds ownershipType to the EntitlementInfo wrapper

Copy link
Contributor

@vegaro vegaro Nov 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this here. We can take care of the Changelog when releasing the newer version, so no need to add this in the Changelog for now 😄

case "PURCHASED":
ownershipType = OwnershipType.purchased;
break;
case "FAMILY_SHARING":
Copy link
Contributor

@vegaro vegaro Nov 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be FAMILY_SHARED as commented in the android PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

purchased,

/// For ownership when a user has been shared the purchase via family sharing
familySharing,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be familyShared

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

/// Enum of ownership types
enum OwnershipType {
/// For ownership when a user made the purchase themselves
purchased,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These docs look great, but maybe we can reuse the docs in the iOS enum? https://github.com/RevenueCat/purchases-ios/blob/main/Purchases/Public/PurchaseOwnershipType.swift#L20

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used the iOS docs and also updated the purchases-android ones to match. I included the UNKNOWN because it looks like the linter wants all public fields documented.

entitlementInfo = EntitlementInfo.fromJson(entitlementInfoJson);

expect(entitlementInfo.ownershipType, OwnershipType.unknown);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! Thanks so much for writing tests!

@taquitos taquitos merged commit 2c85e2b into RevenueCat:main Dec 2, 2021
@taquitos
Copy link
Contributor

taquitos commented Dec 2, 2021

Thank you @daentech!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants