Align BillingAddressMode configuration for CardComponent#2590
Align BillingAddressMode configuration for CardComponent#2590robertdalmeida wants to merge 6 commits into
Conversation
…e lookup configuration from demo
robertdalmeida
added
the
chore
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
ℹ️ No baseline data found for 'develop'.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the billing address configuration by replacing the BillingAddressConfiguration struct with parameters directly on the BillingAddressMode enum cases, allowing the billing address fields to be hidden based on card types. Feedback on these changes highlights a missing supportedCountryCodes parameter in the .lookup case, which breaks country filtering during address lookup. Additionally, the reviewer recommends cleaning up unused methods like updateOptionalStatus and resolving outstanding TODOs regarding backend analytics compatibility and test events before merging.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| case lookup( | ||
| onAddressLookup: (String) async -> [AddressLookupResult], | ||
| onAddressSelected: ((AddressLookupResult) async throws -> PostalAddress)? = nil | ||
| onAddressSelected: ((AddressLookupResult) async throws -> PostalAddress)? = nil, | ||
| hideForCardTypes: Set<CardType> = [] | ||
| ) |
There was a problem hiding this comment.
The .lookup case is missing the supportedCountryCodes parameter. Previously, merchants could configure supported country codes independently of the mode, and these were passed to the address lookup view model. With this change, supportedCountryCodes is only available for the .full case, meaning country filtering is no longer possible when using address lookup. Consider adding supportedCountryCodes: [String] = [] to the .lookup case as well.
case lookup(
supportedCountryCodes: [String] = [],
onAddressLookup: (String) async -> [AddressLookupResult],
onAddressSelected: ((AddressLookupResult) async throws -> PostalAddress)? = nil,
hideForCardTypes: Set<CardType> = []
)| internal var supportedCountryCodes: [String]? { | ||
| switch self { | ||
| case let .full(supportedCountryCodes, _): | ||
| return supportedCountryCodes.isEmpty ? nil : supportedCountryCodes | ||
| case .none, .postalCode, .lookup: | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
Update the supportedCountryCodes helper property to also extract and return the supported country codes for the .lookup case.
| internal var supportedCountryCodes: [String]? { | |
| switch self { | |
| case let .full(supportedCountryCodes, _): | |
| return supportedCountryCodes.isEmpty ? nil : supportedCountryCodes | |
| case .none, .postalCode, .lookup: | |
| return nil | |
| } | |
| } | |
| internal var supportedCountryCodes: [String]? { | |
| switch self { | |
| case let .full(supportedCountryCodes, _), | |
| let .lookup(supportedCountryCodes, _, _, _): | |
| return supportedCountryCodes.isEmpty ? nil : supportedCountryCodes | |
| case .none, .postalCode: | |
| return nil | |
| } | |
| } |
| internal lazy var billingAddressPickerItem: FormAddressPickerItem? = { | ||
| switch addressMode { | ||
| case let .lookup(onLookup, onAddressSelected): | ||
| case let .lookup(onLookup, onAddressSelected, _): |
There was a problem hiding this comment.
If supportedCountryCodes is added to the .lookup case, make sure to update the pattern matching here to account for the new parameter.
| case let .lookup(onLookup, onAddressSelected, _): | |
| case let .lookup(_, onLookup, onAddressSelected, _): |
| // TODO: Robert: BillingAddressModeConfiguration: I don't think this is needed anymore. | ||
| package func updateOptionalStatus(isOptional: Bool) { | ||
| context.isOptional = isOptional | ||
| } |
There was a problem hiding this comment.
Since the billing address optional status is no longer used (replaced by hiding the field entirely), this method and the associated TODO comment can be safely removed to keep the codebase clean.
| } else { | ||
| self.billingAddressRequired = false | ||
| } | ||
| // TODO: Robert: BillingAddressMode: What should we send to the backend for billingAddressRequired? |
There was a problem hiding this comment.
Unresolved TODO: Please resolve what should be sent to the backend for billingAddressRequired before merging. Removing this field entirely might break backend analytics or compatibility if the backend expects this key.
| XCTAssertEqual(configDataDict["socialSecurityNumberVisibility"], "auto") | ||
| XCTAssertEqual(configDataDict["hasInstallmentOptions"], "false") | ||
| XCTAssertEqual(configDataDict["billingAddressRequired"], "true") | ||
| // TODO: Robert: billingAddressRequired removed in favor of hideForCardTypes. What should be the alternate way to receive this event? |
There was a problem hiding this comment.
Unresolved TODO: Please resolve how this event should be handled or what alternative event/parameter should be verified in the test before merging.
|
| 🔀 | 3 Modifications |
| ❌ | 1 Removal |
AdyenCard
BillingAddressMode
🔀 Modified
// From
case full
// To
case full(
supportedCountryCodes: [Swift.String] = [],
hideForCardTypes: Swift.Set<Adyen.CardType> = []
)
/**
Changes:
- Added parameter `hideForCardTypes: Swift.Set<Adyen.CardType> = []`
- Added parameter `supportedCountryCodes: [Swift.String] = []`
*/// From
case lookup(
onAddressLookup: (Swift.String) async -> [Adyen.AddressLookupResult],
onAddressSelected: ((Adyen.AddressLookupResult) async throws -> Adyen.PostalAddress)? = nil
)
// To
case lookup(
onAddressLookup: (Swift.String) async -> [Adyen.AddressLookupResult],
onAddressSelected: ((Adyen.AddressLookupResult) async throws -> Adyen.PostalAddress)? = nil,
hideForCardTypes: Swift.Set<Adyen.CardType> = []
)
/**
Changes:
- Added parameter `hideForCardTypes: Swift.Set<Adyen.CardType> = []`
*/// From
case postalCode
// To
case postalCode(hideForCardTypes: Swift.Set<Adyen.CardType> = [])
/**
Changes:
- Added parameter `hideForCardTypes: Swift.Set<Adyen.CardType> = []`
*/CardConfiguration
❌ Removed
public func billingAddressCountryCodes(_ countryCodes: [Swift.String]) -> AdyenCard.CardConfigurationAnalyzed targets: Adyen, AdyenActions, AdyenCard, AdyenCardScanner, AdyenCashAppPay, AdyenCheckout, AdyenComponents, AdyenDelegatedAuthentication, AdyenDropIn, AdyenEncryption, AdyenSession, AdyenSwiftUI, AdyenTwint, AdyenUI, AdyenWeChatPay
| /// - hideForCardTypes: Card types for which the address form should be hidden | ||
| /// when detected via BIN lookup. | ||
| case full( | ||
| supportedCountryCodes: [String] = [], |
There was a problem hiding this comment.
@erenbesel @nauaros
I find this part strange, when we provide nil then all countries are available.
I would have preferred to have an enum something like below. WDYT?
CountryCode {
case all
case countries([String])
}
| /// When empty, all countries are available. | ||
| /// - hideForCardTypes: Card types for which the address form should be hidden | ||
| /// when detected via BIN lookup. | ||
| case full( |
There was a problem hiding this comment.
@nauaros @erenbesel
I'm not sure what full means here. I personally think that either .form or .manual better describes the option where the user has to manually fill up the address without the search functioality.
Is this something that warrants a change? WDYT?
…nd refactor tests to match optional address policy behavior
… property instead of billingAddress.mode
1 participant
Summary [Required]
Changed the way the Billing Address Configuration is done as per the alignment PR.
Next
Ticket [Optional]
COSDK-1206Checklist [Required]