[MBL-870] Fix GraphSchema with an Update to AppTrackingTransparency #1832

msadoon · 2023-06-27T02:34:45Z

📲 What and 🤔 Why

Firstly - don't be alarmed at the number of changes - that's mostly due to bin/apollo-schema-download.sh making automatic updates - there's only about 800 actual lines of application code that changed including tests.

There are a few things this PR covers:

triggerThirdPartyEvents triggerCAPIEvents mutation changes from running bin/apollo-schema-download.sh
After consent is allowed via our feature flagging tool - the advertisingIdentifier can be turned off via Settings - so we need to check ATTrackingManager.trackingAuthorizationStatus every time we use triggerCapiEvents or KSRAnalytics (Segment tracking)

🛠 How this was build out

The structure of `AppTrackingTransparency`

It's all basically in AppTrackingTransparency which is now a class not a struct - because we want to set the advertisingIdentifier internally to the object. We don't want to allow it to be set outside its' own instance.

Which makes sense because we have a stack of Environment objects that refreshes every time the app is backgrounded/foregrounded. See didFinishLaunchingWithOptions fromStorage, replaceEnvironment (many places) and pushEnvironment (many places). So if by default we have Environment(appTrackingTransparency: AppTrackingTransparency = AppTrackingTransparency()), then its' easier to set that advertisingIdentifier on init than after the fact which is messy.

This is much easier in a class than struct because a struct requires making functions mutating to change internal properties.

Ensuring we represent the latest permissions for tracking from the system

AppDelegateViewModel requestATTrackingAuthorizationStatus has also changed to check if the app is active after its' launched (which is triggered when the app comes in from a background state).

You'll also see that we call a new updateAdvertisingIdentifier function before checking the advertisingIdentifier wherever it is used. This is way to ensure the permissions are allowed before tracking right before use.

That's probably a lot to absorb and it took me about a week to figure all this out, so feel free to ask questions on any details that aren't clear. Here is the official Apple documentation where I started - https://developer.apple.com/documentation/apptrackingtransparency

Passing around the `AppTrackingTransparency` in `AppEnvironment`

So we might use this class instead of what it was before - just the advertisingIdentifier string when we switch to triggerThirdPartyEvents. We can technically have a setting that can be sent in that mutation applicationTrackingEnabled that we set in-app.

✅ Acceptance criteria

CI Tests are passing
First install launch is the only time system message appears if feature flag is true
Turning off tracking in Settings means the CAPI and Segment events not sent. On means they are sent. Essentially if there is an advertising identifier to send, we track users, if not we don’t.

⏰ TODO

Fix tests

⏰ Developer Testing

Launch app for first time - beta, ensure consent dialog is shown and accepted.
Browse around, go to project page, ensure segment events are being tracked, and CAPI calls are being made (try to get Charles Proxy setup for this)
Background app, go to Settings -> Privacy and Security -> Tracking -> Turn off tracking for KickBeta, go back to the app and ensure that browsing around no longer sends Segment events and if you have Charles setup - that triggerCapiEvents are not being sent.

…/background transitions Also sets up our AppTrackingTransparency for use with `triggerThirdPartyEvents` which is going to be a replacement for `triggerCapiEvents` (once we get the go ahead from Leigh that all the bugs are fixed)

msadoon · 2023-06-27T02:36:49Z

KsApi/GraphAPI.swift

@@ -590,9 +590,10 @@ public enum GraphAPI {
    ///   - appData


Do not review this file it's auto generated by bin/apollo-schema-download.sh

msadoon · 2023-06-27T02:37:02Z

KsApi/graphql-schema.json

Do not review this file it's auto generated by bin/apollo-schema-download.sh

msadoon · 2023-06-27T02:38:28Z

Kickstarter.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

I can't really prevent these - they are small automatic SPM framework updates, but I generally check the release tags of the repo - if for example we go up a couple mid versions ie. 7.6.2 --> 7.8.0 (Kingfisher), to make sure nothing obvious broke/changed.

nativeksr · 2023-06-28T16:34:52Z

	1 Warning
⚠️	Big PR

Generated by 🚫 Danger

codecov · 2023-06-28T17:22:55Z

Codecov Report

Merging #1832 (98a3ca6) into main (2c5ef89) will increase coverage by 0.02%.
The diff coverage is 89.03%.

@@            Coverage Diff             @@
##             main    #1832      +/-   ##
==========================================
+ Coverage   84.35%   84.37%   +0.02%     
==========================================
  Files        1276     1276              
  Lines      115457   115679     +222     
  Branches    30751    30764      +13     
==========================================
+ Hits        97395    97606     +211     
- Misses      16994    17007      +13     
+ Partials     1068     1066       -2

Impacted Files	Coverage Δ
...ary/ViewModels/PledgePaymentMethodsViewModel.swift	`91.18% <0.00%> (-1.40%)`	⬇️
Library/ViewModels/ProjectPageViewModel.swift	`91.57% <0.00%> (-2.15%)`	⬇️
...ibrary/ViewModels/RewardsCollectionViewModel.swift	`88.55% <0.00%> (-3.18%)`	⬇️
Library/ViewModels/ThanksViewModel.swift	`86.52% <0.00%> (-3.85%)`	⬇️
Library/Tracking/AppTrackingTransparency.swift	`41.17% <43.75%> (+41.17%)`	⬆️
Library/AppEnvironment.swift	`93.08% <66.66%> (+0.02%)`	⬆️
Library/Tracking/KSRAnalytics.swift	`82.48% <66.66%> (-0.18%)`	⬇️
Kickstarter-iOS/AppDelegateViewModel.swift	`92.94% <100.00%> (+0.73%)`	⬆️
Kickstarter-iOS/AppDelegateViewModelTests.swift	`98.73% <100.00%> (+0.05%)`	⬆️
...gerCapiEventInput+TriggerCapiEventInputTests.swift	`100.00% <100.00%> (ø)`
... and 12 more

... and 1 file with indirect coverage changes

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

msadoon · 2023-06-28T19:56:31Z

@scottkicks Here's a big one! Couldn't be avoided because the schema changes. This PR fix unblocks our test failures on all the other PRs. As soon as this goes in I'll review your outstanding PRs, just assign them to me if ready once tests pass.

scottkicks · 2023-06-29T16:53:19Z

Library/MockRemoteConfigClient.swift

@@ -3,6 +3,8 @@ import Foundation

 private class MockRemoteConfigValue: RemoteConfigValue {
  var bool = false
+
+  override var boolValue: Bool { self.bool }


Even though this change needs to be made, it is more relevant to my creator dashboard ff related changes . Might be worth removing to maintain a clearer context

yea good call out, I noticed that too but some tests need to pass using the MockRemoteConfigClient and under the hood it uses the RemoteConfigValue which in our helper classes references .boolValue.

ah right. ok cool

scottkicks · 2023-06-29T17:07:49Z

Library/ViewModels/PledgePaymentMethodsViewModel.swift

+        guard project.sendMetaCapiEvents else { return }
+
+        AppEnvironment.current.appTrackingTransparency.updateAdvertisingIdentifier()
+


With this pattern we need to always remember to updateAdvertisingIdentifier before accessing the identifier and calling the mutation.
Maybe we can mitigate the risk of forgetting by adding a helper method in AppTrackingTransparency that calls updateAdvertisingIdentifier and then returns the advertisingIdentifier 🤔

Yea that's a good idea - in the cases where to do the check for advertisingIdentifer before calling triggerCapiEvents let's do that. Could you add that once we start on triggerThirdPartyEvents?

sounds good. i'll make a note

scottkicks

Developer testing steps look good and everything is working well 👍

left some comments with suggestions, that are not required for approval, for you to consider.

msadoon self-assigned this Jun 27, 2023

msadoon added the WIP label Jun 27, 2023

msadoon added this to the release-5.9.0 milestone Jun 27, 2023

msadoon commented Jun 27, 2023

View reviewed changes

KsApi/graphql-schema.json Outdated

Copy link

Contributor Author

msadoon Jun 27, 2023

Choose a reason for hiding this comment

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

Do not review this file it's auto generated by bin/apollo-schema-download.sh

msadoon commented Jun 27, 2023

View reviewed changes

scottkicks mentioned this pull request Jun 28, 2023

[No Ticket] Update Localized Strings #1831

Merged

1 task

fixed existing tests to pass under new appTrackingTransparency class

f7e686e

formatting

984a2eb

added new test cases

98a3ca6

msadoon changed the title ~~[Unticketed] Fix GraphSchema with an Update to AppTrackingTransparency~~ [MBL-870] Fix GraphSchema with an Update to AppTrackingTransparency Jun 28, 2023

msadoon added needs review and removed WIP labels Jun 28, 2023

msadoon marked this pull request as ready for review June 28, 2023 19:53

msadoon requested a review from scottkicks June 28, 2023 19:53

scottkicks reviewed Jun 29, 2023

View reviewed changes

scottkicks approved these changes Jun 29, 2023

View reviewed changes

msadoon merged commit e629b80 into main Jun 29, 2023

msadoon deleted the fix/blocking-graph-changes branch June 29, 2023 18:17

msadoon mentioned this pull request Jul 6, 2023

[MBL-866] Remove Add New Card Page #1829

Merged

3 tasks

msadoon mentioned this pull request Jul 25, 2023

[MBL-59] Fix personal data export. #1836

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MBL-870] Fix GraphSchema with an Update to AppTrackingTransparency #1832

[MBL-870] Fix GraphSchema with an Update to AppTrackingTransparency #1832

msadoon commented Jun 27, 2023 •

edited by scottkicks

Loading

msadoon Jun 27, 2023

msadoon Jun 27, 2023

msadoon Jun 27, 2023 •

edited

Loading

nativeksr commented Jun 28, 2023 •

edited

Loading

codecov bot commented Jun 28, 2023 •

edited

Loading

msadoon commented Jun 28, 2023 •

edited

Loading

scottkicks Jun 29, 2023

msadoon Jun 29, 2023

scottkicks Jun 29, 2023

scottkicks Jun 29, 2023

msadoon Jun 29, 2023 •

edited

Loading

scottkicks Jun 29, 2023

scottkicks left a comment •

edited

Loading

		guard project.sendMetaCapiEvents else { return }

		AppEnvironment.current.appTrackingTransparency.updateAdvertisingIdentifier()

[MBL-870] Fix GraphSchema with an Update to AppTrackingTransparency #1832

[MBL-870] Fix GraphSchema with an Update to AppTrackingTransparency #1832

Conversation

msadoon commented Jun 27, 2023 • edited by scottkicks Loading

📲 What and 🤔 Why

🛠 How this was build out

The structure of AppTrackingTransparency

Ensuring we represent the latest permissions for tracking from the system

Passing around the AppTrackingTransparency in AppEnvironment

✅ Acceptance criteria

⏰ TODO

⏰ Developer Testing

msadoon Jun 27, 2023

Choose a reason for hiding this comment

msadoon Jun 27, 2023

Choose a reason for hiding this comment

msadoon Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

nativeksr commented Jun 28, 2023 • edited Loading

codecov bot commented Jun 28, 2023 • edited Loading

Codecov Report

msadoon commented Jun 28, 2023 • edited Loading

scottkicks Jun 29, 2023

Choose a reason for hiding this comment

msadoon Jun 29, 2023

Choose a reason for hiding this comment

scottkicks Jun 29, 2023

Choose a reason for hiding this comment

scottkicks Jun 29, 2023

Choose a reason for hiding this comment

msadoon Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

scottkicks Jun 29, 2023

Choose a reason for hiding this comment

scottkicks left a comment • edited Loading

Choose a reason for hiding this comment

msadoon commented Jun 27, 2023 •

edited by scottkicks

Loading

The structure of `AppTrackingTransparency`

Passing around the `AppTrackingTransparency` in `AppEnvironment`

msadoon Jun 27, 2023 •

edited

Loading

nativeksr commented Jun 28, 2023 •

edited

Loading

codecov bot commented Jun 28, 2023 •

edited

Loading

msadoon commented Jun 28, 2023 •

edited

Loading

msadoon Jun 29, 2023 •

edited

Loading

scottkicks left a comment •

edited

Loading