[WEB-1007] FetchUserEmailQuery #1800
Conversation
…tchUserEmail query
Codecov Report
@@ Coverage Diff @@
## main #1800 +/- ##
==========================================
- Coverage 85.40% 85.36% -0.04%
==========================================
Files 1284 1286 +2
Lines 117604 117624 +20
Branches 31121 31142 +21
==========================================
- Hits 100437 100413 -24
- Misses 16083 16127 +44
Partials 1084 1084
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…tchUserEmail query
… advertising identifier injectable in places where it is created. The benefit is to loosely couple the KSRAnalytics class with our Optimizely feature flags, AppEnvironment struct and AppTrackingTransparency.
| self.applicationWillEnterForegroundProperty.signal, | ||
| self.applicationLaunchOptionsProperty.signal.ignoreValues(), |
There was a problem hiding this comment.
Can you get this signal firing without these signals because they don't exactly correlate to the user logging in.
There was a problem hiding this comment.
@msadoon I wanted to account for the case where users are already logged in when the feature is turned on. This way the next time they open the app we make sure to set their email. As opposed to waiting until they've logged out and then back in.
Does that make sense?
There was a problem hiding this comment.
Yep, good call, didn't think about that.
KsApi/ServiceType.swift
Outdated
| @@ -165,6 +165,10 @@ public protocol ServiceType { | |||
| func fetchGraphUser(withStoredCards: Bool) | |||
| -> SignalProducer<UserEnvelope<GraphUser>, ErrorEnvelope> | |||
|
|
|||
| /// Fetches the email of the currently logged in User using graphQL. | |||
There was a problem hiding this comment.
You don't need to specify that it is GraphQL.
Library/Environment.swift
Outdated
| @@ -48,6 +48,9 @@ public struct Environment { | |||
| /// The currently logged in user. | |||
| public let currentUser: User? | |||
|
|
|||
| /// The currently logged in user's email. Fetched from GraphQL | |||
There was a problem hiding this comment.
no need to say you get it from GQL
| @@ -64,7 +64,7 @@ public protocol PledgeViewModelInputs { | |||
| func riskMessagingViewControllerDismissed(isApplePay: Bool) | |||
| func scaFlowCompleted(with result: StripePaymentHandlerActionStatusType, error: Error?) | |||
| func shippingRuleSelected(_ shippingRule: ShippingRule) | |||
| func storeFacebookCAPIUserEmail(_ email: String?) | |||
|
|
|||
There was a problem hiding this comment.
can probably remove this newline, format might do it, if not don't worry about it.
Generated by 🚫 Danger |
📲 What
In order to reduce the amount/size of network calls to get the currently logged in user's email for our CAPI events, we can add a simpler query that just gets the email and store it globally instead of asking for the entire user object every time.
🤔 Why
reduces the amount of network calls needed and makes accessing the user's email simpler.
🛠 How
Add a small one field query in AppDelegate that will grab the email and make it accessible app-wide in AppEnvironment.currency.loggedInUserEmail