-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[WEB-970] Add CAPI Events #1798
Conversation
… other view models
Codecov Report
@@ Coverage Diff @@
## main #1798 +/- ##
==========================================
- Coverage 85.45% 85.40% -0.05%
==========================================
Files 1282 1283 +1
Lines 117291 117469 +178
Branches 31042 31073 +31
==========================================
+ Hits 100226 100323 +97
- Misses 15990 16068 +78
- Partials 1075 1078 +3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, good first shot at this.
There's a lot of comments of here, but mostly just following existing conventions and reducing the network requests required.
Let's pair on any of this if you have questions.
…f making a separate network call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're getting there!
A few more suggestions and its' good to go!
f12d8e1
to
5ee34c9
Compare
5ee34c9
to
1447d1a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok very close, just a few more comments and next pass should be mergeable.
...tarter-iOS/Features/PledgePaymentMethods/Controller/PledgePaymentMethodsViewController.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let tests pass and good to go!
📲 What
Triggers our new triggerCapiEvent mutation in the following cases
Event Cases:
Events should only trigger if the following is true:
sendMetaCapiEvents
property on the project is true.🤔 Why
This mutation helps us collect data on those members that have entered the app via a FB ad. Since iOS members can opt out of 3rd party app tracking, our FB pixel could be rendered useless. Facebook's CAPI (Conversions API) allows us to collect tracking data server to server, essentially getting around this issue.
It's important to note that Facebook does some magic on their end to associate the member emails we send to them with ad clicks so it's important that we send this value back whenever possible. There is of course the case where someone has enters from an ad and doesn't not log in/create an account.
🛠 How
Created a new service:
AppTrackingTransparency
: for getting authorizationStatus and advertisingIdentifierThen I just made those calls in the correct places according to the cases listed above
Note: We've been using this project for testing. It has its
sendMetaCapiEvents
property set to true.