-
Notifications
You must be signed in to change notification settings - Fork 645
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 fakes for data sources in CustomerSheetViewModel
#9296
Conversation
1e36f9b
to
a15a5c7
Compare
540c281
to
920f2e9
Compare
CustomerSheetViewModel
Diffuse output:
APK
|
920f2e9
to
0c432df
Compare
0c432df
to
a3785df
Compare
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.
nice!! Thanks for making these updates
CustomerSheetDataResult.success(findPaymentMethod(paymentMethods, id)) | ||
}, | ||
private val onDetachPaymentMethod: DetachPaymentMethodOperation = { id -> | ||
CustomerSheetDataResult.success(findPaymentMethod(paymentMethods, 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.
For vertical mode, Jay and I made the default impls of fake functions a not implemented exception. This was useful for ensuring that functions were only called in the tests we expected them to be called in. Just an FYI in case you think this would be useful for customer sheet too!
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.
Yeah some of the tests were actually expecting values which is why I have them like this. I'll follow up by updating those tests and making these all NotImplemented
onSetupIntentClientSecretForCustomerAttach = { | ||
CustomerAdapter.Result.success("invalid setup intent") | ||
onRetrieveSetupIntentClientSecret = { | ||
CustomerSheetDataResult.success("invalid setup intent") |
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.
not necessary to fix in this PR, but should this set up intent be something like si_12345? I feel like that's more consistent with what we usually do for PMs + PIs
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.
or seti_123 which is used below
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.
Will follow-up!
Summary
Add fakes for data sources in
CustomerSheetViewModel
Motivation
Manage fakes for individual data source types.
Testing