Refactor / align payment (upgrade) integration flows#565
Refactor / align payment (upgrade) integration flows#565ChristiaanScheermeijer wants to merge 1 commit intodevelopfrom
Conversation
ChristiaanScheermeijer
requested review from
AntonLantukh,
MelissaDTH,
budzworthy,
langemike and
royschut
|
Visit the preview URL for this PR (updated for commit fb0d284): https://ottwebapp--pr565-refactor-payment-con-dffj8f27.web.app (expires Wed, 31 Jul 2024 10:43:50 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: c198f8a3a199ba8747819f7f1e45cf602b777529 |
ChristiaanScheermeijer
added
JWP Audit
|
|
||
| await this.accountService.updateCustomer({ | ||
| metadata: { | ||
| [`${subscription.subscriptionId}_pending_downgrade`]: toOfferId, |
There was a problem hiding this comment.
Oh...this part was in the hook previously. Makes sense to have it here.
|
I see the Renew subscription button on JWP side. This is not aligned wit our design where we should only have cancel subscription you can see in the link: https://www.figma.com/design/qLl8KxBOOr640h18ICx0H6/Web-app?t=UrJZI5RpFBxrWajc-0 I have this #565 pr that solves the problem without impacting the cleeng flow simply by using feature flag.I think we should merge this and then if we need we can refractor it later |
| canSupportEmptyFullName: false, | ||
| canChangePasswordWithOldPassword: true, | ||
| canRenewSubscription: false, | ||
| canRenewSubscription: true, |
There was a problem hiding this comment.
It looks like it can't be done for JWP now.
There was a problem hiding this comment.
Yes, this should be reverted and solved with a canSwitchSubscription flag instead.
| getOrder = undefined; | ||
|
|
||
| switchSubscription = undefined; | ||
| switchSubscription: SwitchSubscription = async (payload) => { |
There was a problem hiding this comment.
Can we also remove changeSubscription method from subscription service?
There was a problem hiding this comment.
Yes!
| } | ||
|
|
||
| const showChangeSubscriptionButton = (!isExternalPaymentProvider && offerSwitchesAvailable) || (!isChangingOffer && !canRenewSubscription); | ||
| const showChangeSubscriptionButton = !isExternalPaymentProvider && offerSwitchesAvailable; |
There was a problem hiding this comment.
Maybe we should also add canChangeSubscription feature flag to hide the button even though we get some offers?
There was a problem hiding this comment.
Yes, that would even be better. I think the canRenewSubscription flag is used were it shouldn't and this can fix that as well.
|
@ChristiaanScheermeijer looks great, thank you! Just some small comments. Probably some e2e tests also need changes. |
|
@AntonLantukh thanks, this is still pretty POC'ish. Do you think it's worth to finish this PR? Perhaps we should do that when #563 is merged? |
| !isGrantedSubscription && | ||
| !isChangingOffer && | ||
| canRenewSubscription ? ( | ||
| {(activeSubscription.status === 'active' || activeSubscription.status === 'active_trial') && !isGrantedSubscription && canRenewSubscription ? ( |
There was a problem hiding this comment.
This condition should be checked. I think the canRenewSubscription should be removed for the cancel subscription button. So:
- has subscription and is active -> Show "Cancel Subscription" button
- has subscription and is not active and canRenewSubscription -> show "Renew Subscription" button
@ChristiaanScheermeijer I find this approach cleaner than what we have when we try to build additional complexity on top of current logic. You can finish it if you have some time. |
3 participants
Description
This proposal PR is my attempt to align the subscription switch logic discrepancy between the JWP and Cleeng integration. These two features (doing the same) were implemented differently.
Motivated by #562 and #555 I'm hoping we can simplify flows and eliminate differences between providers. To disable features, we should be using the integration feature flags to keep the code intact for other providers (as long we don't have alternatives).