[NT-720] Add ApplePayCapableType to Environment, add Checkout Properties #1015
Conversation
…NT-720-checkout-properties
…NT-720-checkout-properties
Generated by 🚫 Danger |
Library/ApplePayCapable.swift
Outdated
| import KsApi | ||
| import PassKit | ||
|
|
||
| public protocol ApplePayCapableType { |
There was a problem hiding this comment.
Just wondering about the name seeing as it does a little more than just tell us whether the device is capable of Apple Pay. Anyway, no super strong feelings about it, but just ApplePayType would probably be fine too 👍
There was a problem hiding this comment.
Yeah the term I was going for here is capabilities to encompass all Apple-Pay-related capabilities. But ApplePayCapabilitiesType didn't quite have the same ring to it 😅. ApplePayType and then ApplePay seems a little vague to me.
There was a problem hiding this comment.
I personally like ApplePayCababilities, because it's more general. For me, ApplePayCapable refers to something with a boolean type (function, var...).
There was a problem hiding this comment.
Nice job, @ifbarrera . I only left a couple of questions/suggestions.
| @@ -497,7 +497,13 @@ public final class DeprecatedRewardPledgeViewModel: Type, Inputs, Outputs { | |||
| ) | |||
| .map { ($0.0, $0.1, $1, $2, $3) } | |||
| .takeWhen(Signal.merge(applePayEventAfterLogin, loggedInUserTappedApplePayButton)) | |||
| .map(PKPaymentRequest.paymentRequest(for:reward:pledgeAmount:selectedShippingRule:merchantIdentifier:)) | |||
| .map { PKPaymentRequest.paymentRequest( | |||
There was a problem hiding this comment.
Just for curiosity, is there any reason why you decided to pass the values explicitly?
There was a problem hiding this comment.
The compiler needed help 😓
| XCTAssertEqual( | ||
| ["Pledge Screen Viewed", "Pledge Button Clicked"], | ||
| self.trackingClient.events | ||
| ) |
There was a problem hiding this comment.
Hm.. I think the properties are not being tested here.
|
|
||
| self.scheduler.run() | ||
|
|
||
| XCTAssertEqual(["Pledge Screen Viewed"], self.trackingClient.events) |
There was a problem hiding this comment.
Here too... which properties are involved in this test? 🤔
There was a problem hiding this comment.
Ah right, we actually don't need this test anymore because we don't have the Completed Checkout event anymore. Will remove!
Library/ApplePayCapable.swift
Outdated
| import KsApi | ||
| import PassKit | ||
|
|
||
| public protocol ApplePayCapableType { |
There was a problem hiding this comment.
I personally like ApplePayCababilities, because it's more general. For me, ApplePayCapable refers to something with a boolean type (function, var...).
📲 What
In order to better access and better test Apple Pay capabilities, this PR creates
ApplePayCapableTypeand adds this type as a property on theEnvironment. This allows us to easily access Apple Pay capabilities from the environment, and to mock out various Apple Pay capabilities during testing.🤔 Why
Events cleanup.
🛠 How
This refactor made it possible to access
applePayCapableFor(project)from thePledgeViewModelwithout needing to inject this value into the configuration.Additionally, this PR adds the following "Checkout Properties:
checkout_amount// including shippingcheckout_payment_type// APPLE_PAY or CREDIT_CARDcheckout_reward_idcheckout_reward_titlecheckout_revenue_in_usd_centscheckout_shipping_amountcheckout_reward_estimated_delivery_oncheckout_reward_shipping_enabledcheckout_user_has_eligibile_stored_apple_pay_cardTo test these checkout properties, this PR adds the "Checkout Completed" event.
♿️ Accessibility
N/A
🏎 Performance
N/A
✅ Acceptance criteria
⏰ TODO
checkout_revenue_in_usd_cents