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.

Sign up for GitHub

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

Refactor presentedOfferingIdentifier into presentedOfferingContext object #1612

Merged
merged 8 commits into from
Feb 15, 2024
Merged

Refactor presentedOfferingIdentifier into presentedOfferingContext object #1612

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
953a39d
Refacor presentedOfferingIdentifier into presentedOfferingContext object
tonidero Feb 14, 2024
4f00efe
Add API tests
tonidero Feb 14, 2024
b5a2653
Fix tests
tonidero Feb 14, 2024
9eaf8a4
Make PresentedOfferingContext offeringId not nullable and make it nul…
tonidero Feb 15, 2024
ed49688
Remove unused parameter
tonidero Feb 15, 2024
a38dff4
Fix replace with
tonidero Feb 15, 2024
6855a71
Comments from review
tonidero Feb 15, 2024
9b6e897
Fix tests
tonidero Feb 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions purchases/src/defaults/kotlin/com/revenuecat/purchases/Purchases.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ class Purchases internal constructor(
purchasesOrchestrator.startDeprecatedProductChange(
activity,
storeProduct.purchasingData,
null,
PresentedOfferingContext(),
upgradeInfo.oldSku,
GoogleProrationMode.fromPlayBillingClientMode(upgradeInfo.prorationMode)?.asGoogleReplacementMode,
listener,
Expand Down Expand Up @@ -265,7 +265,7 @@ class Purchases internal constructor(
purchasesOrchestrator.startPurchase(
activity,
storeProduct.purchasingData,
null,
PresentedOfferingContext(),
null,
callback,
)
Expand Down Expand Up @@ -302,7 +302,7 @@ class Purchases internal constructor(
purchasesOrchestrator.startDeprecatedProductChange(
activity,
packageToPurchase.product.purchasingData,
packageToPurchase.offering,
packageToPurchase.presentedOfferingContext,
upgradeInfo.oldSku,
GoogleProrationMode.fromPlayBillingClientMode(upgradeInfo.prorationMode)?.asGoogleReplacementMode,
callback,
Expand Down Expand Up @@ -333,7 +333,7 @@ class Purchases internal constructor(
purchasesOrchestrator.startPurchase(
activity,
packageToPurchase.product.purchasingData,
packageToPurchase.offering,
packageToPurchase.presentedOfferingContext,
null,
listener,
)
Expand Down
33 changes: 30 additions & 3 deletions purchases/src/main/kotlin/com/revenuecat/purchases/Package.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,41 @@ import com.revenuecat.purchases.models.StoreProduct
* @property identifier Unique identifier for this package. Can be one a predefined package type or a custom one.
* @property packageType Package type for the product. Will be one of [PackageType].
* @property product [StoreProduct] assigned to this package.
* @property offering offering this package was returned from.
* @property offering offering Id this package was returned from.
* @property presentedOfferingContext [PresentedOfferingContext] from which this package was obtained.
*/
data class Package(
val identifier: String,
val packageType: PackageType,
val product: StoreProduct,
val offering: String,
)
val presentedOfferingContext: PresentedOfferingContext,
) {
@Deprecated(
"Use constructor with presentedOfferingContext instead",
ReplaceWith(
"Package(identifier, packageType, product, offering, " +
tonidero marked this conversation as resolved.
Show resolved Hide resolved
"PresentedOfferingContext(offeringIdentifier = offering))",
),
)
constructor(
identifier: String,
packageType: PackageType,
product: StoreProduct,
offering: String,
) : this(
identifier,
packageType,
product,
PresentedOfferingContext(offeringIdentifier = offering),
)

@Deprecated(
"Use presentedOfferingContext.offeringIdentifier instead",
ReplaceWith("presentedOfferingContext.offeringIdentifier"),
)
val offering: String
get() = presentedOfferingContext.offeringIdentifier ?: ""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the presentedOfferingContext have a nullable offeringIdentifier instead of making the presentedOfferingContext nullable in StoreProduct/SubscriptionOption in case we could have non-offering related context, which is probably not the case right now... But this forces us to default to something here as a fallback. It should never be null right now though in the Package.

Copy link
Member

Choose a reason for hiding this comment

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

I like this! I think this makes sense 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm I think it makes sense for StoreProduct/SubscriptionOption, but with this there's no way to indicate that a package will always have an offering and a package is always going to have an offering. Do we need to replace offering in Package with presentedOfferingContext? I think we can leave this as is offering: String and only replace presentedOfferingIdentifier in StoreProduct and SubscriptionOption? We access the presentedOfferingContext through the products, and not the packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm it's a good point... the only difference is going to be that the offering is not nullable in the Package... However, It feels nicer to me if we have the same format in all 3 places... If we don't have this data at the Package level we will need to access it through the StoreProduct when purchasing, which seems slightly less clean...

What do you think about leaving the offering property undeprecated here in the Package? It would be duplicated with the one in presentedOfferingContext but that way we can keep the non-nullability here

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess my question is if we need a presentedOfferingContext at all in Package. Is it ever going to be different than the offering value? I guess we can add a different way of getting Packages in the future, which would indeed make offering not nullable and create the need of a presentedOfferingContext, but maybe we can add that property whenever we need. I just think adding it right now makes the API confusing and doesn't add value. Maybe I am missing context

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually... What if we make it a sealed class that only has an implementation right now? That way we can keep the original nullabilities. Can you remind me what other type of data we are adding to the PresentedOfferingContext?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joshdholtz I was chatting live with @vegaro about this one... In the end, we believe it would be better to make the offeringIdentifier not nullable inside the PresentedOfferingContext so we can make the PresentedOfferingContext not nullable in Package but nullable in StoreProduct/SubscriptionOption. Do you think we will (now or in the future) have data like this without having an offering id?

My original reason to make it nullable was to provide more flexibility in the future in case we have some of this data without an offering id, but I don't think that would happen for now at least, so it makes sense to better reflect the current model.

Let us know what you think!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 9eaf8a4

}

/**
* Enumeration of all possible Package types.
Expand Down
2 changes: 1 addition & 1 deletion purchases/src/main/kotlin/com/revenuecat/purchases/PostReceiptHelper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ internal class PostReceiptHelper(
) {
val receiptInfo = ReceiptInfo(
productIDs = purchase.productIds,
offeringIdentifier = purchase.presentedOfferingIdentifier,
presentedOfferingContext = purchase.presentedOfferingContext,
storeProduct = storeProduct,
subscriptionOptionId = purchase.subscriptionOptionId,
replacementMode = purchase.replacementMode,
Expand Down
18 changes: 18 additions & 0 deletions purchases/src/main/kotlin/com/revenuecat/purchases/PresentedOfferingContext.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.revenuecat.purchases

import android.os.Parcelable
import kotlinx.parcelize.Parcelize

/**
* Contains data about the context in which an offering was presented.
*/
@Parcelize
data class PresentedOfferingContext(
/**
* The identifier of the offering used to obtain this object.
*
* Null if not using RevenueCat offerings system, if fetched directly via `Purchases.getProducts`,
* or on restores/syncs.
*/
val offeringIdentifier: String? = null,
) : Parcelable
12 changes: 6 additions & 6 deletions purchases/src/main/kotlin/com/revenuecat/purchases/PurchaseParams.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ data class PurchaseParams(val builder: Builder) {
internal val activity: Activity

@get:JvmSynthetic
internal val presentedOfferingIdentifier: String?
internal val presentedOfferingContext: PresentedOfferingContext

init {
this.isPersonalizedPrice = builder.isPersonalizedPrice
this.oldProductId = builder.oldProductId
this.googleReplacementMode = builder.googleReplacementMode
this.purchasingData = builder.purchasingData
this.activity = builder.activity
this.presentedOfferingIdentifier = builder.presentedOfferingIdentifier
this.presentedOfferingContext = builder.presentedOfferingContext
}

/**
Expand All @@ -45,19 +45,19 @@ data class PurchaseParams(val builder: Builder) {
open class Builder private constructor(
@get:JvmSynthetic internal val activity: Activity,
@get:JvmSynthetic internal val purchasingData: PurchasingData,
@get:JvmSynthetic internal val presentedOfferingIdentifier: String? = null,
@get:JvmSynthetic internal val presentedOfferingContext: PresentedOfferingContext,
@get:JvmSynthetic internal val product: StoreProduct?,
) {
constructor(activity: Activity, packageToPurchase: Package) :
this(
activity,
packageToPurchase.product.purchasingData,
packageToPurchase.offering,
packageToPurchase.presentedOfferingContext,
packageToPurchase.product,
)

constructor(activity: Activity, storeProduct: StoreProduct) :
this(activity, storeProduct.purchasingData, storeProduct.presentedOfferingIdentifier, storeProduct)
this(activity, storeProduct.purchasingData, storeProduct.presentedOfferingContext, storeProduct)

private fun ensureNoTestProduct(storeProduct: StoreProduct) {
if (storeProduct is TestStoreProduct) {
Expand All @@ -74,7 +74,7 @@ data class PurchaseParams(val builder: Builder) {
this(
activity,
subscriptionOption.purchasingData,
subscriptionOption.presentedOfferingIdentifier,
subscriptionOption.presentedOfferingContext,
product = null,
)

Expand Down
32 changes: 16 additions & 16 deletions purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesOrchestrator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ internal class PurchasesOrchestrator constructor(
startProductChange(
activity,
purchasingData,
presentedOfferingIdentifier,
presentedOfferingContext,
productId,
googleReplacementMode,
isPersonalizedPrice,
Expand All @@ -333,7 +333,7 @@ internal class PurchasesOrchestrator constructor(
startPurchase(
activity,
purchasingData,
presentedOfferingIdentifier,
presentedOfferingContext,
isPersonalizedPrice,
callback,
)
Expand Down Expand Up @@ -896,16 +896,16 @@ internal class PurchasesOrchestrator constructor(
fun startPurchase(
activity: Activity,
purchasingData: PurchasingData,
presentedOfferingIdentifier: String?,
presentedOfferingContext: PresentedOfferingContext,
isPersonalizedPrice: Boolean?,
listener: PurchaseCallback,
) {
log(
LogIntent.PURCHASE,
PurchaseStrings.PURCHASE_STARTED.format(
" $purchasingData ${
presentedOfferingIdentifier?.let {
PurchaseStrings.OFFERING + "$presentedOfferingIdentifier"
presentedOfferingContext.offeringIdentifier?.let {
PurchaseStrings.OFFERING + "$it"
}
}",
),
Expand All @@ -930,7 +930,7 @@ internal class PurchasesOrchestrator constructor(
appUserID,
purchasingData,
null,
presentedOfferingIdentifier,
presentedOfferingContext,
isPersonalizedPrice,
)
} ?: listener.dispatch(PurchasesError(PurchasesErrorCode.OperationAlreadyInProgressError).also { errorLog(it) })
Expand All @@ -939,7 +939,7 @@ internal class PurchasesOrchestrator constructor(
fun startProductChange(
activity: Activity,
purchasingData: PurchasingData,
offeringIdentifier: String?,
presentedOfferingContext: PresentedOfferingContext,
oldProductId: String,
googleReplacementMode: GoogleReplacementMode,
isPersonalizedPrice: Boolean?,
Expand All @@ -959,8 +959,8 @@ internal class PurchasesOrchestrator constructor(
LogIntent.PURCHASE,
PurchaseStrings.PRODUCT_CHANGE_STARTED.format(
" $purchasingData ${
offeringIdentifier?.let {
PurchaseStrings.OFFERING + "$offeringIdentifier"
presentedOfferingContext.offeringIdentifier?.let {
PurchaseStrings.OFFERING + "$it"
}
} oldProductId: $oldProductId googleReplacementMode $googleReplacementMode",

Expand Down Expand Up @@ -988,7 +988,7 @@ internal class PurchasesOrchestrator constructor(
googleReplacementMode,
activity,
appUserID,
offeringIdentifier,
presentedOfferingContext,
isPersonalizedPrice,
purchaseCallback,
)
Expand All @@ -1003,7 +1003,7 @@ internal class PurchasesOrchestrator constructor(
fun startDeprecatedProductChange(
activity: Activity,
purchasingData: PurchasingData,
offeringIdentifier: String?,
presentedOfferingContext: PresentedOfferingContext,
oldProductId: String,
googleReplacementMode: GoogleReplacementMode?,
listener: ProductChangeCallback,
Expand All @@ -1023,8 +1023,8 @@ internal class PurchasesOrchestrator constructor(
LogIntent.PURCHASE,
PurchaseStrings.PRODUCT_CHANGE_STARTED.format(
" $purchasingData ${
offeringIdentifier?.let {
PurchaseStrings.OFFERING + "$offeringIdentifier"
presentedOfferingContext.offeringIdentifier?.let {
PurchaseStrings.OFFERING + "$it"
}
} oldProductId: $oldProductId googleReplacementMode $googleReplacementMode",

Expand All @@ -1047,7 +1047,7 @@ internal class PurchasesOrchestrator constructor(
googleReplacementMode,
activity,
appUserID,
offeringIdentifier,
presentedOfferingContext,
null,
listener,
)
Expand All @@ -1063,7 +1063,7 @@ internal class PurchasesOrchestrator constructor(
googleReplacementMode: GoogleReplacementMode?,
activity: Activity,
appUserID: String,
presentedOfferingIdentifier: String?,
presentedOfferingContext: PresentedOfferingContext,
isPersonalizedPrice: Boolean?,
listener: PurchaseErrorCallback,
) {
Expand Down Expand Up @@ -1099,7 +1099,7 @@ internal class PurchasesOrchestrator constructor(
appUserID,
purchasingData,
ReplaceProductInfo(purchaseRecord, googleReplacementMode),
presentedOfferingIdentifier,
presentedOfferingContext,
isPersonalizedPrice,
)
},
Expand Down
17 changes: 9 additions & 8 deletions purchases/src/main/kotlin/com/revenuecat/purchases/amazon/AmazonBilling.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import com.amazon.device.iap.model.Receipt
import com.amazon.device.iap.model.UserData
import com.amazon.device.iap.model.UserDataResponse
import com.revenuecat.purchases.PostReceiptInitiationSource
import com.revenuecat.purchases.PresentedOfferingContext
import com.revenuecat.purchases.PurchasesError
import com.revenuecat.purchases.PurchasesErrorCallback
import com.revenuecat.purchases.PurchasesErrorCode
Expand Down Expand Up @@ -52,7 +53,7 @@ import com.revenuecat.purchases.ProductType as RevenueCatProductType
private const val TERM_SKU_JSON_KEY = "termSku"

@SuppressWarnings("LongParameterList", "TooManyFunctions")
internal class AmazonBilling constructor(
internal class AmazonBilling(
private val applicationContext: Context,
private val amazonBackend: AmazonBackend,
private val cache: AmazonCache,
Expand Down Expand Up @@ -244,7 +245,7 @@ internal class AmazonBilling constructor(
appUserID: String,
purchasingData: PurchasingData,
replaceProductInfo: ReplaceProductInfo?,
presentedOfferingIdentifier: String?,
presentedOfferingContext: PresentedOfferingContext,
isPersonalizedPrice: Boolean?,
) {
val amazonPurchaseInfo = purchasingData as? AmazonPurchasingData.Product
Expand Down Expand Up @@ -275,9 +276,9 @@ internal class AmazonBilling constructor(
activity,
appUserID,
storeProduct,
presentedOfferingIdentifier,
presentedOfferingContext,
onSuccess = { receipt, userData ->
handleReceipt(receipt, userData, storeProduct, presentedOfferingIdentifier)
handleReceipt(receipt, userData, storeProduct, presentedOfferingContext)
},
onError = {
onPurchaseError(it)
Expand Down Expand Up @@ -354,7 +355,7 @@ internal class AmazonBilling constructor(
}
val amazonPurchaseWrapper = receipt.toStoreTransaction(
productId = sku,
presentedOfferingIdentifier = null,
presentedOfferingContext = null,
purchaseState = PurchaseState.UNSPECIFIED_STATE,
userData,
)
Expand Down Expand Up @@ -515,7 +516,7 @@ internal class AmazonBilling constructor(
receipt: Receipt,
userData: UserData,
storeProduct: StoreProduct,
presentedOfferingIdentifier: String?,
presentedOfferingContext: PresentedOfferingContext,
) {
if (receipt.productType != ProductType.SUBSCRIPTION) {
/**
Expand All @@ -526,7 +527,7 @@ internal class AmazonBilling constructor(
*/
val amazonPurchaseWrapper = receipt.toStoreTransaction(
productId = storeProduct.id,
presentedOfferingIdentifier = presentedOfferingIdentifier,
presentedOfferingContext = presentedOfferingContext,
purchaseState = PurchaseState.PURCHASED,
userData,
)
Expand All @@ -541,7 +542,7 @@ internal class AmazonBilling constructor(
val termSku = response[TERM_SKU_JSON_KEY] as String
val amazonPurchaseWrapper = receipt.toStoreTransaction(
productId = termSku,
presentedOfferingIdentifier = presentedOfferingIdentifier,
presentedOfferingContext = presentedOfferingContext,
purchaseState = PurchaseState.PURCHASED,
userData,
)
Expand Down
Loading