-
Notifications
You must be signed in to change notification settings - Fork 664
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
Actually update default payment method when set as default checkbox checked on manage PM screen #10249
Conversation
@@ -560,6 +560,12 @@ internal class CustomerSheetViewModel( | |||
workContext = workContext, | |||
// This checkbox is never displayed in CustomerSheet. | |||
shouldShowSetAsDefaultCheckbox = false, | |||
// Should never be called from CustomerSheet, because we don't enable the set as default checkbox. | |||
setDefaultPaymentMethodExecutor = { | |||
Result.failure( |
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.
I'm gonna follow up an add an unexpected error event here
updateCardBrandResult.isSuccess -> UpdateResult.Success | ||
else -> UpdateResult.NoUpdatesMade | ||
if (updateCardBrandResult == null && setDefaultPaymentMethodResult == null) { | ||
return UpdateResult.NoUpdatesMade |
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.
I'm gonna follow up an add an unexpected error event here -- this should never happen. We should always update either the card brand or the default PM
UpdateResult.Error(setDefaultPaymentMethodErrorMessage) | ||
} else if (updateCardBrandResult?.isFailure == true) { | ||
UpdateResult.Error(updateCardBrandResult.exceptionOrNull()?.stripeErrorMessage()) | ||
} else if (setDefaultPaymentMethodResult?.isFailure == true) { | ||
UpdateResult.Error(updatesFailedErrorMessage) | ||
} else { | ||
UpdateResult.Success |
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 error messages here aren't finalized -- I've scheduled some time with JJ to figure out how we should handle these error states, but I'm just trying to merge something reasonable until then!
Diffuse output:
APK
|
Summary
Actually update default payment method when set as default checkbox checked on manage PM screen
Motivation
https://jira.corp.stripe.com/browse/MOBILESDK-2690
This PR sets the default PM in the backend. There's still a little more work to do here, because the other manage PM screens don't update their selection properly after the new default is set. I'm going to follow up with the remaining behavior changes to make this 💯
Testing
Screen recording
set.default.in.backend.webm