Fix Bug where fetchPaymentMethodNonces Return Incorrect Type
#1100
Conversation
|
Will resolve the failing test, need to update the logic slightly to handle when the type is not a valid type |
|
|
||
| let type = json?["type"].asString() | ||
|
|
||
| if type == "CreditCard", let cardType = json?["details"]["cardType"].asString() { |
There was a problem hiding this comment.
The below logic mirrors the behavior of iOS v5 with regards to the type. Though this is with the constraints of not being able to cast to unknown nonce types in Swift. The load method used previously was a very hacky workaround and does not map to Swift - the method (load) in Obj-C is prohibited in Swift. Currently the JS SDK has the method fetchPaymentMethodNonces in Core while Android has this method in the Drop-in. We should consider where this method should live and align across all 3 platforms.
| @@ -1,33 +1,9 @@ | |||
| import XCTest | |||
| @testable import BraintreePayPal | |||
|
|
|||
| class BTPaymentMethodNonceParser_PayPal_Tests: XCTestCase { | |||
| func testSharedParser_whenTypeIsPayPal_returnsPayPalAccountNonce() { | |||
There was a problem hiding this comment.
Moved the parser specific test BTPaymentMethodNonceParser_Tests.swift
| XCTAssertTrue(payPalAccountNonce!.isDefault) | ||
| XCTAssertNil(payPalAccountNonce?.creditFinancing) | ||
| } | ||
| final class BTPayPalAccountNonce_Tests: XCTestCase { |
There was a problem hiding this comment.
Renamed this class and the tests since they were not actually testing the shared parser and the test names were not accurate.
| // NEXT_MAJOR_VERSION: this should move into the Drop-in for parity with Android | ||
| // This will also allow us to return the types directly which we were doing in the +load method | ||
| // previously in Obj-C - this is not available in Swift |
There was a problem hiding this comment.
We've actually gotten multiple requests to move this back into core. We initially moved it to DropIn to get rid of circular dependencies, and we're planning to implement it the correct way in v5.
There was a problem hiding this comment.
Ah interesting... we may have to rethink what we want to return to merchants if we move it back to core. Not sure if Android is capable of what we were doing in v5. We were using the load method to dynamically register types that don't exist in core (BTCardNonce/PayPalNonce/'VenmoNonce/ApplePayNonce`) at compile time. We don't have the ability to do that hacky workaround in Swift and importing those modules into Core would cause a circular dependency. How are y'all planning to correctly implement it in v5?
There was a problem hiding this comment.
Yeah it was a hack in Android v3 as well. We used compileOnly in our Gradle files to basically create weak references in the dependency tree. That allowed Gradle to build successfully, but it caused some issues in the long run.
By "correct" I'm thinking "in a non-hacky way." It's tough because there will probably be tradeoffs. An enum feels like the right way to me, but I'd be up to do some cross-platform pairing on this since we need a solution on Android too.
Summary of changes
Checklist
Authors