Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[NT-647] "Thanks Page Viewed" event #1035

Merged
merged 10 commits into from
Jan 16, 2020
Merged

[NT-647] "Thanks Page Viewed" event #1035

merged 10 commits into from
Jan 16, 2020

Conversation

ifbarrera
Copy link
Contributor

@ifbarrera ifbarrera commented Jan 14, 2020

📲 What

Adds the Thanks Page Viewed event.

🤔 Why

Tracking events cleanup.

🛠 How

Adds the Thanks Page Viewed event which includes projectProperties, pledgeProperties and checkoutProperties. Because the checkout properties are mostly defined in PledgeViewModel, they're passed through to the ThanksViewModel via the configuration function.

✅ Acceptance criteria

With the KOALA_ENABLED environment variable set to true, test the event by backing any project. You should see the Thanks Page Viewed event fire when you get to the thanks page, with the following properties:

  • checkout_amount // pledge total including shipping
  • checkout_payment_type // either CREDIT_CARD or APPLE_PAY
  • checkout_reward_id
  • checkout_reward_title
  • checkout_shipping_amount
  • checkout_revenue_in_usd_cents
  • checkout_reward_estimated_delivery_on
  • checkout_reward_shipping_enabled
  • checkout_user_has_eligible_stored_apple_pay_card

as well as all projectProperties and pledgeProperties.

Note that you'll only see checkoutProperties when backing a project through the native pledge view experience, although the Thanks Page Viewed event will also fire through the deprecated checkout experience.

…NT-647-thank-you-event

# Conflicts:
#	Library/Tracking/Koala.swift
@nativeksr
Copy link
Collaborator

1 Warning
⚠️ Big PR

Generated by 🚫 Danger

Copy link
Contributor

@Scollaco Scollaco left a comment

Choose a reason for hiding this comment

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

It looks pretty legit! I just had a question regarding some tests and other minor things. Good job!


self.scheduler.run()

self.goToThanks.assertValues([project])
self.goToThanksProject.assertValues([project])
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we assert the values of goToThanksReward and goToThanksCheckoutData here too? 🤔

self.showErrorBannerWithMessage.assertDidNotEmitValue()

self.scheduler.run()

self.beginSCAFlowWithClientSecret.assertDidNotEmitValue()
self.submitButtonEnabled.assertValues([false, true, false, true])
self.submitButtonIsLoading.assertValues([true, false])
self.goToThanks.assertValues([.template])
self.goToThanksProject.assertValues([.template])
Copy link
Contributor

Choose a reason for hiding this comment

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

And here too

@@ -3095,7 +3114,7 @@ final class PledgeViewModelTests: TestCase {

self.beginSCAFlowWithClientSecret.assertValues(["client-secret"])
self.submitButtonEnabled.assertValues([false, true, false, true])
self.goToThanks.assertValues([project])
self.goToThanksProject.assertValues([project])
Copy link
Contributor

Choose a reason for hiding this comment

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

and here

fileprivate let goToThanks = TestObserver<Project, Never>()
fileprivate let goToThanksProject = TestObserver<Project, Never>()
fileprivate let goToThanksReward = TestObserver<Reward, Never>()
fileprivate let goToThanksCheckoutData = TestObserver<Koala.CheckoutPropertiesData?, Never>()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you alpha here? 🙈

fileprivate let goToThanks = TestObserver<Project, Never>()
fileprivate let goToThanksProject = TestObserver<Project, Never>()
fileprivate let goToThanksReward = TestObserver<Reward, Never>()
fileprivate let goToThanksCheckoutData = TestObserver<Koala.CheckoutPropertiesData?, Never>()
Copy link
Contributor

Choose a reason for hiding this comment

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

Alpha here too

private let goToThanks = TestObserver<Project, Never>()
private let goToThanksProject = TestObserver<Project, Never>()
private let goToThanksReward = TestObserver<Reward, Never>()
private let goToThanksCheckoutData = TestObserver<Koala.CheckoutPropertiesData?, Never>()
Copy link
Contributor

Choose a reason for hiding this comment

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

And here

Copy link
Contributor

@Scollaco Scollaco left a comment

Choose a reason for hiding this comment

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

🚢 🚢 🚢 🚢
🚢 🚢 🚢 🚢
🚢 🚢 🚢 🚢
🚢 🚢 🚢 🚢

@ifbarrera ifbarrera merged commit 4aba0cf into master Jan 16, 2020
@ifbarrera ifbarrera deleted the NT-647-thank-you-event branch January 16, 2020 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants