[MBL 873] Implement TriggerThirdPartyEvent Mutation

[scottkicks]

📲 What

Replacing TriggerCapiEventInput calls with TriggerThirdPartyEvent

🤔 Why

We're going to handle Facebook CAPI and Google Analytics calls with a single mutation.
For more details see the engineering plan

🛠 How

Remove TriggerCapiEvent related files and references.

Replace current .triggerCapiEventInput calls with .triggerThirdPartyEvent
We’re currently triggering this mutation in the following ViewModels:

• PledgePaymentMethodsViewModel
• ProjectPageViewModel
• RewardsCollectionViewModel
• ThanksViewModel

Existing parameter differences below:

• externalId → deviceId
• eventName → eventName
• projectId → projectId
• appData → appData (we can keep hardcoding these values to true, just like we do now because externalId is required)
• we do not need to pass anything into firebaseScreen, firebasePreviousScreen
• userId →AppEnvironment.current.currentUser?.id

New Parameters (These are only passed in on the ThanksViewModel):
We can use this existing code…

      if let checkoutDataValues = configData.checkoutData {
        ...
      }

• pledgeAmount → checkoutDataValues.revenueInUsd + checkoutDataValues.bonusAmountInUsd + checkoutDataValues.addOnsMinimumUsd
• shipping → checkoutDataValues.shippingAmountUsd
• transactionId → checkoutDataValues.checkoutId
• clientMutationId → ““

✅ Acceptance criteria

• Use Charles Proxy, or something similar, to verify that these calls are being sent at the expected times with success 200 responses

[codecov]

Codecov Report

Merging #1835 (49e6889) into main (61ffb8b) will increase coverage by 0.05%.
The diff coverage is 79.34%.

@@            Coverage Diff             @@
##             main    #1835      +/-   ##
==========================================
+ Coverage   84.49%   84.54%   +0.05%     
==========================================
  Files        1277     1273       -4     
  Lines      115232   115190      -42     
  Branches    30674    30656      -18     
==========================================
+ Hits        97365    97389      +24     
+ Misses      16799    16732      -67     
- Partials     1068     1069       +1     

Files Changed	Coverage Δ
KsApi/MockService.swift	13.21% <ø> (-0.01%)	⬇️
KsApi/Service.swift	9.39% <ø> (+0.14%)	⬆️
KsApi/ServiceType.swift	76.06% <ø> (ø)
...iewModels/PledgePaymentMethodsViewModelTests.swift	96.79% <ø> (-0.01%)	⬇️
...ary/ViewModels/PledgePaymentMethodsViewModel.swift	88.37% <5.55%> (-1.87%)	⬇️
Library/NSBundleType.swift	38.29% <50.00%> (+0.52%)	⬆️
Library/ViewModels/ThanksViewModel.swift	97.51% <96.87%> (+10.98%)	⬆️
Library/NSBundleType+Tests.swift	100.00% <100.00%> (ø)
Library/TestHelpers/MockBundle.swift	100.00% <100.00%> (ø)
Library/ViewModels/ProjectPageViewModel.swift	97.52% <100.00%> (+5.95%)	⬆️
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[scottkicks]

According to the CAPI documentation all values in extInfo are required and must be in the given indexed order.
I paired with Isa on this and this format is the only way we can get a successful 200 response for this mutation.

IMO we should only have to pass the used values (i2, OS version) to the backend and they should be responsible for formatting the data correctly.

Since the docs require this I chose to implement it this way on our end.

[msadoon]

So here's my recommendation for how this function should written. The big take-away is to process the value sent into the function outside the function, not inline. Keeps things cleaner.

        guard let externalId = AppEnvironment.current.appTrackingTransparency.advertisingIdentifier
        else { return }

        var userId = ""
        
        if let userValue = AppEnvironment.current.currentUser {
          userId = "\(userValue.id)"
        }
        
        let projectId = "\(project.id)"
        
        var extInfo = Array(repeating: "", count: 16)
        extInfo[0] = "i2"
        extInfo[4] = AppEnvironment.current.mainBundle.platformVersion
        
        _ = AppEnvironment
          .current
          .apiService
          .triggerThirdPartyEventInput(
            input: .init(
              deviceId: externalId,
              eventName: ThirdPartyEventInputName.AddNewPaymentMethod.rawValue,
              projectId: projectId,
              pledgeAmount: nil,
              shipping: nil,
              transactionId: nil,
              userId: userId,
              appData: .init(
                advertiserTrackingEnabled: true,
                applicationTrackingEnabled: true,
                extinfo: extInfo
              ),
              clientMutationId: ""
            )
          )

[msadoon]

Also add this to NSBundleType

  public var platformVersion: String {
    return self.infoDictionary?["DTPlatformVersion"] as? String ?? "Unknown"
  }

And write a test for it in NSBundleType + Tests

[msadoon]

How did I get the above?

Start with an example from in app ie. line 740 AppDelegateViewModel that leads toNSBundleType for things like appVersionString (similar property).

Build out that “OS version” here. The benefit of keeping it here as opposed to using ProcessInfo in multiple places, is its’ easy to change later and also test.

Why not use process info?

A lot properties in NSBundle+Type use “self.infoDictionary”. It turns out it is a provided property that contains the OS version in the key - "DTPlatformVersion" - at least on simulator this worked.

[msadoon]

I suggest following this pattern below in ProjectPageViewModel, RewardsColletionViewModel and ThanksViewModel.

[msadoon]

Ok, this is mostly fine.

I refactored a bit of the parameter passing to keep the code clean.

Normally it would be good to test these network requests wherever they are used in the view model. Oddly we didn't test the CAPI ones. Will enforce this in the eng. plan in the future, so no need to do it here.

The changes are all laid out so make those quickly and I'll approve tomorrow morning. Good work!

[msadoon]

So here's my recommendation for how this function should written. The big take-away is to process the value sent into the function outside the function, not inline. Keeps things cleaner.

        guard let externalId = AppEnvironment.current.appTrackingTransparency.advertisingIdentifier
        else { return }

        var userId = ""
        
        if let userValue = AppEnvironment.current.currentUser {
          userId = "\(userValue.id)"
        }
        
        let projectId = "\(project.id)"
        
        var extInfo = Array(repeating: "", count: 16)
        extInfo[0] = "i2"
        extInfo[4] = AppEnvironment.current.mainBundle.platformVersion
        
        _ = AppEnvironment
          .current
          .apiService
          .triggerThirdPartyEventInput(
            input: .init(
              deviceId: externalId,
              eventName: ThirdPartyEventInputName.AddNewPaymentMethod.rawValue,
              projectId: projectId,
              pledgeAmount: nil,
              shipping: nil,
              transactionId: nil,
              userId: userId,
              appData: .init(
                advertiserTrackingEnabled: true,
                applicationTrackingEnabled: true,
                extinfo: extInfo
              ),
              clientMutationId: ""
            )
          )

[msadoon]

Also add this to NSBundleType

  public var platformVersion: String {
    return self.infoDictionary?["DTPlatformVersion"] as? String ?? "Unknown"
  }

And write a test for it in NSBundleType + Tests

[msadoon]

How did I get the above?

Start with an example from in app ie. line 740 AppDelegateViewModel that leads toNSBundleType for things like appVersionString (similar property).

Build out that “OS version” here. The benefit of keeping it here as opposed to using ProcessInfo in multiple places, is its’ easy to change later and also test.

Why not use process info?

A lot properties in NSBundle+Type use “self.infoDictionary”. It turns out it is a provided property that contains the OS version in the key - "DTPlatformVersion" - at least on simulator this worked.

[msadoon]

I suggest following this pattern below in ProjectPageViewModel, RewardsColletionViewModel and ThanksViewModel.

[scottkicks]

@msadoon I've addressed your comments. thanks for the feedback. I tried your your suggestion for calculating the pledgeAmount and didn't find that it added more accuracy. I think your solution makes more sense anyway though so I've made that change as well.

[msadoon]

There were mistakes here I found, please correct before we can merge.