Native checkout experiment #801

Scollaco · 2019-08-15T17:42:36Z

📲 What

Implements an A/B experiment that brings users to the new Native Checkout or to the old checkout flow, depending on which bucket they are.

🤔 Why

To investigate the impact the new Native Checkout will have in comparison to the old checkout flow.
And this is the JIRA ticket

🛠 How

The app now receives the native_checkout_v1 on the Config response.
Note The users will only see the checkout if the nativeCheckout feature is enabled AND they are on the experimental bucket.

✅ Acceptance criteria

One way to test this is first go to Config.swift and add a breakpoint to see the values of stringsArray (line 30).

If the value of native_checkout_v1 is experimental:

Go to the Debug menu, turn on the Native Checkout feature flag and select any project. You should see the Native Checkout flow.
Turn off the feature flag and select any project. You should see the old checkout flow.

If the value of native_checkout_v1 is control:

Go to the Debug menu, turn on the Native Checkout feature flag and select any project. You should see the old checkout flow.
Turn off the feature flag and select any project. You should see the old checkout flow.

If you wanna test all the scenarios, simply go to ExperimentName +Helpers.swift and change the value of return Experiment.Variant(rawValue: variant) == .experimental to control. Here we are just changing the logic simulating a different bucket.

dusi

Looks good..had couple minor comments

dusi · 2019-08-15T18:14:30Z

Kickstarter-iOS/DataSources/ProjectPamphletContentDataSourceTests.swift

@@ -64,10 +65,22 @@ final class ProjectPamphletContentDataSourceTests: TestCase {
    }
  }

-  func testRewardsSection_nativeCheckoutFeature_showsWithTurnedOff() {
+  func testRewardsSection_showsWhen_nativeCheckoutFeatureEnabled_checkoutExperimentDisabled() {


Should these tests cover the following combinations? (I think we might be missing one)

feature enable + experiment disabled

feature disabled + experiment enabled

feature disabled + experiment disabled (I think we're missing this one)

dusi · 2019-08-15T18:20:21Z

KsApi/models/Config.swift

@@ -1,6 +1,7 @@
 public enum Experiment {
  public enum Name: String {
    case creatorsNameDiscovery = "show_created_by_discovery"
+    case nativeCheckoutV1 = "native_checkout_v1"


Just wondering about the naming of this ... is this named native after our team (hence it's a native team feature) or is this named after the feature native checkout?

If it's the first then I think it's fine.
If it's the later then V1 is technically web checkout so not sure if it makes sense but don't feel strongly about it (just wanted to bring this up).

This name starts with native so it becomes clear on the Kickstarter app that it's a native experiment (so no one deletes it by accident).

dusi · 2019-08-15T18:25:43Z

Library/Config+Helpers.swift

@@ -2,8 +2,8 @@ import Foundation
 import KsApi

 extension Experiment.Name {
-  public func isEnabled(in _: Environment) -> Bool {
-    guard let experiments = AppEnvironment.current.config?.abExperiments else { return false }
+  public func isEnabled(in environment: Environment = AppEnvironment.current) -> Bool {


I've noticed this function is not tested...can we add appropriate tests?

This function stayed untested for over a year! 👀 I will add a test for it.

dusi · 2019-08-15T18:30:11Z

Library/Feature.swift

@@ -5,6 +5,14 @@ public enum Feature: String {
  case nativeCheckoutPledgeView = "ios_native_checkout_pledge_view"
 }

+extension Feature {
+  public func isEnabled(in environment: Environment = AppEnvironment.current) -> Bool {


Also looks untested..

Library/ViewModels/ProjectPamphletViewModelTests.swift

Co-Authored-By: Pavel Dusatko <pavel.dusatko@gmail.com>

ksr-ci-bot · 2019-08-15T19:59:53Z

	1 Warning
⚠️	Big PR

Generated by 🚫 Danger

…tarter/ios-oss into native_checkout_experiment

dusi

Had couple questions and noticed some issues

ExperimentName+Helpers & ExperimentName+HelpersTests are not ordered alphabetically within their enclosing folder
SwiftFormat seems to have some issues 😅 (just run bin/format .)

dusi · 2019-08-16T19:31:24Z

Library/ExperimentName+HelpersTests.swift

+    }
+  }
+
+  func testIsEnabled_Experimental() {


Can we name these 2 tests similarly?

dusi · 2019-08-16T19:31:55Z

Library/ExperimentName+HelpersTests.swift

+
+  func testIsEnabled_Experimental() {
+    let config = Config.template
+      |> \.abExperiments .~ [Experiment.Name.nativeCheckoutV1.rawValue: "experimental"]


I'd also suggest we test when abExperiments is empty or does not include nativeCheckoutV1. What do you think?

dusi · 2019-08-16T19:34:46Z

Library/ProjectPamphlet+Helpers.swift

+import KsApi
+
+public func userCanSeeNativeCheckout() -> Bool {
+  return nativeCheckoutExperimentIsEnabled() && featureNativeCheckoutEnabled()


I completely missed the naming the first time around...

What do you say we make this more consistent so that we:

use isEnabled for both

use the same pattern (either: nativeCheckoutExperimentIsEnabled & nativeCheckoutFeatureIsEnabled or experimentNativeCheckoutIsEnabled & featureNativeCheckoutIsEnabled)?

…tarter/ios-oss into native_checkout_experiment

dusi · 2019-08-16T21:56:37Z

Library/ViewModels/ProjectPamphletViewModel.swift

@@ -80,10 +80,10 @@ public final class ProjectPamphletViewModel: ProjectPamphletViewModelType, Proje
      }

    self.goToRewards = goToRewards
-      .filter { _ in featureNativeCheckoutPledgeViewEnabled() }


Make sure that when merging master this is actually using the correct feature flag Native Checkout not Native Checkout Pledge View ... @justinswart has recently changed it since it was causing issue with deep links 😅

Yeah, I noticed that. Actually we now will use userCanSeeNativeCheckout because of the experiment 😄

…ment

dusi

🚢 🚢 🚢
🚢 🚢 🚢
🚢 🚢 🚢

Tested on iPad Air 2 (Cher)

Scollaco added 4 commits August 15, 2019 13:40

Added logic to show native checkout if feature && experiment are enabled

9a5d71e

Tests

300cee6

More tests, some refactor

1b80df9

Swiftformat

073c4e4

Scollaco requested review from dusi and cdolm92 August 15, 2019 17:42

Scollaco assigned dusi Aug 15, 2019

dusi reviewed Aug 15, 2019

View reviewed changes

Update Library/ViewModels/ProjectPamphletViewModelTests.swift

7b8242f

Co-Authored-By: Pavel Dusatko <pavel.dusatko@gmail.com>

Scollaco added 3 commits August 16, 2019 12:47

Added tests for the isEnabled helper functions

46f529a

Merge branch 'native_checkout_experiment' of https://github.com/kicks…

78d6836

…tarter/ios-oss into native_checkout_experiment

Pavels feedback

2d35db0

dusi suggested changes Aug 16, 2019

View reviewed changes

Scollaco added 4 commits August 16, 2019 16:40

Merge branch 'master' into native_checkout_experiment

16a90f0

More tests and Feedback

f7a814a

SwiftFormat

1d90e55

Merge branch 'native_checkout_experiment' of https://github.com/kicks…

776e556

…tarter/ios-oss into native_checkout_experiment

dusi reviewed Aug 16, 2019

View reviewed changes

Scollaco added 2 commits August 16, 2019 18:13

Merge remote-tracking branch 'oss/master' into native_checkout_experi…

a1064ff

…ment

Merge branch 'master' into native_checkout_experiment

487733b

dusi approved these changes Aug 16, 2019

View reviewed changes

Scollaco merged commit 0b3efb7 into master Aug 19, 2019

Scollaco deleted the native_checkout_experiment branch August 19, 2019 13:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Native checkout experiment #801

Native checkout experiment #801

Scollaco commented Aug 15, 2019 •

edited by dusi

Loading

dusi left a comment

dusi Aug 15, 2019

dusi Aug 15, 2019

Scollaco Aug 15, 2019

dusi Aug 15, 2019

Scollaco Aug 15, 2019

dusi Aug 15, 2019

ksr-ci-bot commented Aug 15, 2019 •

edited

Loading

dusi left a comment

dusi Aug 16, 2019

Scollaco Aug 16, 2019

dusi Aug 16, 2019

Scollaco Aug 16, 2019

dusi Aug 16, 2019

dusi Aug 16, 2019

Scollaco Aug 16, 2019

dusi left a comment

Native checkout experiment #801

Native checkout experiment #801

Conversation

Scollaco commented Aug 15, 2019 • edited by dusi Loading

📲 What

🤔 Why

🛠 How

✅ Acceptance criteria

dusi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksr-ci-bot commented Aug 15, 2019 • edited Loading

dusi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dusi left a comment

Choose a reason for hiding this comment

Scollaco commented Aug 15, 2019 •

edited by dusi

Loading

ksr-ci-bot commented Aug 15, 2019 •

edited

Loading