Render Unknown Payment Methods in Link#6342
Conversation
StripeCorePublic API+public protocol SafeParsedEnumCodable : Swift.CaseIterable, Swift.Hashable, Swift.RawRepresentable where Self.RawValue == Swift.String {
+}If you are adding a new public API consider the following:
If you are modifying or removing a public API:
If you confirm these APIs need to be added/updated and have undergone necessary review, add the label ℹ️ If this comment appears to be left in error, make sure your branch is up-to-date with |
|
✅ Dead code has been resolved in this PR. [find-dead-code] |
c62b96c to
56c02b2
Compare
a6e009b to
075af8d
Compare
e11e1e1 to
8923c66
Compare
367b991 to
98e36be
Compare
8923c66 to
a75478f
Compare
98e36be to
edd1d5e
Compare
1 build increased size, 8 builds had no size change
StripeSize 1.0 (1)
|
| Item | Install Size Change |
|---|---|
| 📝 StripePaymentSheet.ConsumerPaymentDetails.DisplayMetadata.value w... | ⬆️ 2.2 kB |
| StripePaymentSheet.LinkPaymentMethodPicker.CellContentView.paymen... | ⬆️ 1.5 kB |
| 📝 StripePaymentSheet.ConsumerPaymentDetails.DisplayMetadata.Icon.va... | ⬆️ 1.0 kB |
| StripePaymentSheet.LinkPaymentMethodPicker.CellContentView.iconCo... | ⬆️ 996 B |
| 🗑 StripePaymentSheet.LinkPaymentMethodPicker.CellContentView.create... | ⬇️ -872 B |
StripeIdentitySize 1.0 (1)
com.stripe.StripeIdentitySize
No changes to report
StripeIssuingSize 1.0 (1)
com.stripe.StripeIssuingSize
No changes to report
StripeApplePaySize 1.0 (1)
com.stripe.StripeApplePaySize
No changes to report
StripeFinancialConnectionsSize 1.0 (1)
com.stripe.StripeFinancialConnectionsSize
No changes to report
StripeConnectSize 1.0 (1)
com.stripe.StripeConnectSize
No changes to report
🛸 Powered by Emerge Tools
ab62edc to
b64f9c3
Compare
e6791f1 to
bb95070
Compare
b64f9c3 to
5fc3cf3
Compare
bb95070 to
7fcad72
Compare
5fc3cf3 to
cf24967
Compare
7fcad72 to
819ab2d
Compare
| case .unparsable: | ||
| return false | ||
| // Unknown types don't have country info; allow them if display metadata is present. | ||
| return display != nil |
There was a problem hiding this comment.
My thought process here is that if they're not allowed, they would have been filtered by the backend (eg. for brl currencies, Pix is presented and not Crypto).
cf24967 to
e2a4dbb
Compare
b352511 to
dd51024
Compare
## Summary Modifies the default value of `supportedPaymentMethodTypes` ## Motivation In #5309, we introduced parametric filtering of supported payment methods. We used a default value of `allCases`, which works for our current feature set. However, in #6342, we'll want to remove the `intersection` on these filtered payment methods if none are provided to allow for unknown PMs to be included in the list. In this PR, we'll keep the same intersection functionality of all cases, but modify the piping of these values to use `nil` instead of `allCases` as the default value. In #6342, we [remove the allCases intersection](https://github.com/stripe/stripe-ios/pull/6342/changes#diff-7d5339851903df687dc841f3deb388ba566a2f0b319c57c38671cb529b80e7a4R119) entirely if `nil` filtered payment methods are passed in. ## Demos | Test Harness | MPE | | ------------- | ------------- | | <video src="https://github.com/user-attachments/assets/3a09ccb5-59c1-4cfb-aa7f-273776a8e6f8" /> | <video src="https://github.com/user-attachments/assets/8f798fe5-ecc1-4ee9-9d7a-ba532421d354" /> |
e2a4dbb to
cae01f1
Compare
cd317f0 to
40bffda
Compare
d4b11e8 to
9f53197
Compare
9f53197 to
c2fbdba
Compare
tillh-stripe
left a comment
There was a problem hiding this comment.
I haven't yet had the time to take a look myself, but a few things that Codex has been flagging:
- Billing details recollection (when a merchant requests full billing details) might be triggered for an unknown type, but
createUpdateDetailswould fail for unknown types. - The
setLinkPaymentDetailsmethod inElementsCustomer.swiftwouldn't be able to render non-card and non-bank payment methods. - Do we need to update the logic in
getSupportedPaymentDetailsTypesnow? - The delete dialogs in
removePaymentMethoduses an empty title for unknown types, which we don't want.
A quick scan of the codebase suggests to me that these are true — can you confirm that?
c2fbdba to
c071b93
Compare
|
@tillh-stripe Thanks for flagging these!
|
|
%@ will no longer be saved to your wallet., Remove bank? If it's okay for these strings to be unlocalized in master (e.g. this is for an unshipped feature), add the label New strings are localized on a weekly basis and are downloaded as part of the release process. For more details on how to localize a string, you can refer to this link. |
b7c94fa to
cf827ea
Compare
|
|
||
| /* Title for a button that when tapped removes a payment method. | ||
| Title for confirmation prompt when removing a saved payment method without a display label. */ | ||
| "Remove payment method" = "Remove payment method"; |
There was a problem hiding this comment.
I wonder if we should just show Remove in the buttons, as the available space is limited. Not immediately required, but let's follow up with that?
| let billingAddress: BillingAddress? | ||
| let billingEmailAddress: String? | ||
| let nickname: String? | ||
| let display: DisplayMetadata? |
There was a problem hiding this comment.
If I remember correctly, we talked about this becoming non-nullable? Is that still the plan for later on?
There was a problem hiding this comment.
@tillh-stripe Yep! The backend FF is still there so want to make sure that is removed before we remove nullability here.
| // Unknown types don't have country info; allow them if display metadata is present. | ||
| return display != nil |
There was a problem hiding this comment.
I'm not sure what the right logic is here, but I don't think this one is it. Maybe it needs to be similar to the card case and just look at billing address?
There was a problem hiding this comment.
@tillh-stripe Resolved in cce6779. Really good catch!
| Before | After |
|---|---|
before.mov |
after.mov |
| @@ -375,7 +394,9 @@ extension ConsumerPaymentDetails { | |||
| case .bankAccount(let bankAccount): | |||
| return bankAccount.displayName(with: nickname) | |||
| case .unparsable: | |||
There was a problem hiding this comment.
We should rename unparsable, but that's for a later pull request.
| ### PaymentSheet | ||
| * [Added] Added forward-compatiblity for backend-provided payment methods in the native Link payment sheet. |
There was a problem hiding this comment.
We should change this by either writing something that feels more relevant to the merchant (“Added support for new and upcoming payment methods in Link, such as UPI and Pix“) or by removing it entirely. I find that this is an implementation detail, so in favor of the latter.
cf827ea to
cce6779
Compare
|
@tillh-stripe found a bug with billing address collection not include name for non-cards. Resolved in 7de1bce See in the before that the billing address for a Pix PD created on web has a name and when we pay on MPE with billing address collection, we wipe the name and replace with the provided address. See in the after video, this is resolved.
|
7de1bce to
ddc59e3
Compare
|
@tillh-stripe This is ready for review! Added back the changelog entry for this upcoming release to flag that a release is needed. |




Summary
Display unknown payment methods in native Link by parsing the
displaymetadata included in eachredacted_payment_detailsentry. Expected structure includes looks like:Demos
Connecting a New PM
Note: The payment method previously existed (from creation on web), but is being used on a different device, so reconnection is needed.
New.Wallet.mp4
Using an Existing PM
Existing.Wallet.mp4