Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix Stripe Link remote configuration flag #2061

Merged
merged 2 commits into from
May 15, 2024

Conversation

amy-at-kickstarter
Copy link
Contributor

@amy-at-kickstarter amy-at-kickstarter commented May 15, 2024

📲 What

Fix the Stripe Link remote configuration flag, so that it actually controls whether or not Stripe Link is enabled.

May-15-2024.11-22-32.mp4

🤔 Why

Apparently setting the email address was insufficient to control the appearance of Stripe Link. 🤷‍♀️

@@ -159,10 +159,6 @@ public final class PaymentMethodSettingsViewModel: PaymentMethodsViewModelType,
configuration.merchantDisplayName = Strings.general_accessibility_kickstarter()
configuration.allowsDelayedPaymentMethods = true

if featureStripeLinkEnabled() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per @tedPen, we don't need to show Stripe Link from the payment method settings screen - only in the pledge flow. Removing this for clarity's sake, since it would never show anyways.

@@ -354,6 +354,9 @@ public final class PledgePaymentMethodsViewModel: PledgePaymentMethodsViewModelT

if featureStripeLinkEnabled() {
configuration.defaultBillingDetails.email = AppEnvironment.current.currentUserEmail
configuration.billingDetailsCollectionConfiguration.email = .automatic
} else {
configuration.billingDetailsCollectionConfiguration.email = .always
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ycheng-kickstarter Can you confirm this is what Android is doing? Stripe's documentation is stunningly unclear on this, but it does work for me.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup we match

@@ -354,6 +354,9 @@ public final class PledgePaymentMethodsViewModel: PledgePaymentMethodsViewModelT

if featureStripeLinkEnabled() {
configuration.defaultBillingDetails.email = AppEnvironment.current.currentUserEmail
configuration.billingDetailsCollectionConfiguration.email = .automatic
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.automatic is the default but setting it explicitly for clarity

@amy-at-kickstarter amy-at-kickstarter marked this pull request as ready for review May 15, 2024 15:25
@@ -354,6 +354,9 @@ public final class PledgePaymentMethodsViewModel: PledgePaymentMethodsViewModelT

if featureStripeLinkEnabled() {
configuration.defaultBillingDetails.email = AppEnvironment.current.currentUserEmail
configuration.billingDetailsCollectionConfiguration.email = .automatic
} else {
configuration.billingDetailsCollectionConfiguration.email = .always

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup we match

@amy-at-kickstarter amy-at-kickstarter merged commit 366742e into main May 15, 2024
4 checks passed
@amy-at-kickstarter amy-at-kickstarter deleted the bug/adyer/stripe-link-config-flag branch May 15, 2024 15:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants