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
Remove local feature flag for Instant Debits #8418
Remove local feature flag for Instant Debits #8418
Conversation
|
||
override fun valueUpdated(value: Boolean, playgroundSettings: PlaygroundSettings) { | ||
FeatureFlags.instantDebits.setEnabled(value) | ||
checkoutRequestBuilder.supportedPaymentMethods(listOf("card", "link")) |
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.
Allow Instant Debits by only requesting card
and link
, but not us_bank_account
. This still needs a merchant that supports Instant Debits.
Diffuse output:
APK
DEX
|
a864e9c
to
468565f
Compare
@@ -64,8 +63,7 @@ internal enum class AddPaymentMethodRequirement { | |||
val noUsBankAccount = USBankAccount.code !in paymentMethodTypes | |||
val supportsBankAccounts = "bank_account" in metadata.stripeIntent.linkFundingSources | |||
val isDeferred = metadata.stripeIntent.clientSecret == null | |||
val isEnabled = FeatureFlags.instantDebits.isEnabled | |||
return noUsBankAccount && supportsBankAccounts && !isDeferred && isEnabled | |||
return noUsBankAccount && supportsBankAccounts && !isDeferred |
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.
We no longer need a feature flag, as the bank_account
funding source is now only included if the request comes from a supported client version (or from an explicitly-enabled merchant account).
override fun valueUpdated(value: Boolean, playgroundSettings: PlaygroundSettings) { | ||
FeatureFlags.instantDebits.setEnabled(value) | ||
if (value) { | ||
checkoutRequestBuilder.supportedPaymentMethods(listOf("card", "link")) |
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 wondering if we should do this differently.
Should we add this as it's own SettingsDefinition? Otherwise this could clash with another SettingsDefinition, since we set this elsewhere too, right?
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.
As far as I can tell, the SupportedPaymentMethodsSettingsDefinition
is only used in tests, so we wouldn’t have any conflicts right now.
Should we add this as it's own SettingsDefinition?
Can you elaborate what you have in mind?
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 suggesting we remove EnableInstantDebitsSettingsDefinition
, and add SupportedPaymentMethodsSettingsDefinition
to uiSettingDefinitions
.
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.
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.
Rebased onto Carlos’ pull request!
468565f
to
d11cd61
Compare
Summary
This pull request removes the local feature flag for Instant Debits, as we merged the server-side feature flag.
We keep the
EnableInstantDebitsSettingsDefinition
to easily allow testing in the playground.Motivation
Testing
Screenshots
Changelog