-
Notifications
You must be signed in to change notification settings - Fork 6
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
Omnisub 5499 - Upgrade/downgrade subscription #98
base: master
Are you sure you want to change the base?
Conversation
@@ -77,6 +77,23 @@ object CBPurchase { | |||
}) | |||
} | |||
|
|||
fun changeProduct( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add appropriate code comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added!
|
||
private fun changeProduct(changeProductParams: ChangeProductParams, callback: CBCallback.PurchaseCallback<String>) { | ||
isSDKKeyValid({ | ||
log(customer, changeProductParams.purchaseProductParams.product.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The action
being logged in this method is before_purchase_command
.
Can we change this to something appropriate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modified log method to accept name, oldproductId?, action. And pass those values based on where it is being called. Does this make sense?
this.purchaseProductParams = changeProductParams.purchaseProductParams | ||
val offerToken = changeProductParams.purchaseProductParams.offerToken | ||
val oldProductId = changeProductParams.oldProductId | ||
val prorationMode = changeProductParams.prorationMode ?: BillingFlowParams.ProrationMode.IMMEDIATE_WITH_TIME_PRORATION |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's start with support for IMMEDIATE_WITH_TIME_PRORATION
. We would need to build in support for other proration modes in the backend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
data class ChangeProductParams( | ||
val purchaseProductParams: PurchaseProductParams, | ||
val oldProductId: String, | ||
val prorationMode: Int? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the prorationMode
right now, given we only support IMMEDIATE_WITH_TIME_PRORATION
currently?
Does it make sense to compose ChangeProductParams
with PurchaseProductParams
as opposed to using individual parameters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
private fun changeProduct(changeProductParams: ChangeProductParams) { | ||
this.changeProductParams = changeProductParams | ||
this.purchaseProductParams = changeProductParams.purchaseProductParams |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to set this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes its used in acknowledgePurchase method
purchaseCallBack?.onError(throwCBException(billingResult)) | ||
} else { | ||
oneTimePurchaseCallback?.onError(throwCBException(billingResult)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to handle for oneTimePurchaseCallback
? Since OTP cannot be changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed it!!... Removed now.
val purchaseTransactionHistory = mutableListOf<PurchaseTransaction>() | ||
purchaseTransactionHistory.addAll(subscriptionHistory ?: emptyList()) | ||
val prevProduct: PurchaseTransaction? = purchaseTransactionHistory.find { it.productId.first() == oldProductId } | ||
return prevProduct?.purchaseToken ?: "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: return an empty optional instead of empty string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
.setProductDetailsParamsList(productDetailsParamsList) | ||
.setSubscriptionUpdateParams( | ||
BillingFlowParams.SubscriptionUpdateParams.newBuilder() | ||
.setOldPurchaseToken(oldPurchaseToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be the response if the oldPurchaseToken
is empty/null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's loading infinitely. Handled it now with an error message.
oneTimePurchaseCallback?.onError( | ||
billingError | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to handle for oneTimePurchaseCallback? Since OTP cannot be changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed it!!... Removed now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this removed yet?
purchaseCallBack?.onError( | ||
throwCBException(billingResult) | ||
) | ||
if (this.changeProductParams.oldProductId.isNotEmpty() && billingResult.responseCode == ERROR) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do we reset the changeProductParams
variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
README.md
Outdated
### Upgrade or Downgrade Product | ||
Pass the `ChangeProductParams` and `CBCustomer` to the following function when the user chooses the product to purchase. | ||
|
||
`CBCustomer` - **Optional object**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add the full description similar to the previous examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
README.md
Outdated
} | ||
}) | ||
``` | ||
The above function will handle the subscription upgrade or downgrade. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems redundant. Can we rephrase this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
) | ||
} else { | ||
oneTimePurchaseCallback?.onError( | ||
billingError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why have we this in changed the purchase
method?
oneTimePurchaseCallback?.onError( | ||
billingError | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this removed yet?
purchaseCallBack?.onError( | ||
throwCBException(billingResult) | ||
) | ||
if (this::changeProductParams.isInitialized && this.changeProductParams.currentProductId.isNotEmpty() && billingResult.responseCode == ERROR) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is isInitialized
defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is inbuild in kotlin. Returns true if this lateinit property has been assigned a value, and false otherwise.
}) | ||
``` | ||
The above function is designed to manage subscription upgrades or downgrades between base plans within a single subscription or across multiple subscriptions. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be across different subscriptions
?
README.md
Outdated
@@ -161,6 +161,29 @@ CBPurchase.purchaseProduct(purchaseProductParams = purchaseProductParams, custom | |||
``` | |||
The above function will handle the purchase against Google Play Store and send the IAP token for server-side token verification to your Chargebee account. Use the Subscription ID returned by the above function, to check for Subscription status on Chargebee and confirm the access - granted or denied. | |||
|
|||
### Upgrade or Downgrade Product | |||
Pass the `ChangeProductParams` and `CBCustomer` to the following function when the user chooses the product to purchase. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we mention the function name rather than only saying "to the following function..."? It will make the sentence more clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass the ChangeProductParams
and CBCustomer
to the following ChangeProduct
function when the user chooses the product to purchase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cb-sabuj Done
README.md
Outdated
@@ -161,6 +161,29 @@ CBPurchase.purchaseProduct(purchaseProductParams = purchaseProductParams, custom | |||
``` | |||
The above function will handle the purchase against Google Play Store and send the IAP token for server-side token verification to your Chargebee account. Use the Subscription ID returned by the above function, to check for Subscription status on Chargebee and confirm the access - granted or denied. | |||
|
|||
### Upgrade or Downgrade Product | |||
Pass the `ChangeProductParams` and `CBCustomer` to the following function when the user chooses the product to purchase. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass the ChangeProductParams
and CBCustomer
to the following ChangeProduct
function when the user chooses the product to purchase.
Quality Gate passedIssues Measures |
No description provided.