-
Notifications
You must be signed in to change notification settings - Fork 21
[V3] Add Networking to Send Analytics #75
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
Changes from 26 commits
d272d2c
304190d
03f9365
52fd81c
6224055
4a4e97f
4f87b20
0d71ffb
060e7e4
ddb944c
37ea28a
05cf278
6310175
b03bdac
d6fc4fc
022530d
c9e846c
292affb
5308717
a268cd0
9734fb3
3ba4d21
700f695
fd33535
d0a8d8d
147dc9b
9a5e980
a658633
70bb98e
c8fd4cf
5911546
7bc9147
39a61a6
ec82e7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import Foundation | ||
|
||
protocol AnalyticsServiceable { | ||
func sendAnalyticsEvent(_ eventName: String, sessionID: String) | ||
richherrera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
final class AnalyticsService: AnalyticsServiceable { | ||
|
||
// MARK: - Private Properties | ||
|
||
/// The FPTI URL to post all analytic events. | ||
private let url = URL(string: "https://api.paypal.com/v1/tracking/batch/events")! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we aren't actually batching, FPTI has a separate endpoint for single event upload To see the structure of each, you can look at the API details in PPAAS (I also have it in a Postman collection) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's the endpoint used in PPCP, however, we would need a custom encode method. I did a POC with those changes and was thinking that it would be better to have the same implementation we have in BT, which is why I opted for this implementation. While working on BT v7, we chose not to use a custom encode method. Do you think we could use it for this repo? |
||
private let networkClient: Networkable | ||
|
||
// MARK: - Initializer | ||
|
||
init(networkClient: Networkable = NetworkClient()) { | ||
self.networkClient = networkClient | ||
} | ||
|
||
// MARK: - Internal Methods | ||
|
||
func sendAnalyticsEvent(_ eventName: String, sessionID: String) { | ||
Task(priority: .background) { | ||
await performEventRequest(eventName, sessionID: sessionID) | ||
} | ||
} | ||
|
||
func performEventRequest(_ eventName: String, sessionID: String) async { | ||
let body = createAnalyticsEvent(eventName: eventName, sessionID: sessionID) | ||
try? await networkClient.post(url: url, body: body) | ||
richherrera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// MARK: - Private Methods | ||
|
||
/// Constructs POST params to be sent to FPTI | ||
private func createAnalyticsEvent(eventName: String, sessionID: String) -> Codable { | ||
let batchMetadata = FPTIBatchData.Metadata(sessionID: sessionID) | ||
let event = FPTIBatchData.Event(eventName: eventName) | ||
return FPTIBatchData(metadata: batchMetadata, events: [event]) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import Foundation | ||
|
||
protocol Networkable { | ||
func post<T: Encodable>(url: URL, body: T) async throws | ||
} | ||
|
||
final class NetworkClient: Networkable { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we know that PopupBridge doesn't do any other networking besides just sending a single POST for analytics, do we think it's worth having this as a separate layer from AnalyticsService? Thinking of the KISS principe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated: 7bc9147 |
||
|
||
// MARK: - Private Properties | ||
|
||
private let session: Sessionable | ||
|
||
// MARK: - Initializer | ||
|
||
init(session: Sessionable = URLSession.shared) { | ||
richherrera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.session = session | ||
} | ||
|
||
// MARK: - Internal Methods | ||
|
||
func post<T: Encodable>(url: URL, body: T) async throws { | ||
richherrera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var request = URLRequest(url: url) | ||
request.httpMethod = "POST" | ||
request.allHTTPHeaderFields = ["Content-Type": "application/json"] | ||
|
||
do { | ||
let encodedBody = try JSONEncoder().encode(body) | ||
request.httpBody = encodedBody | ||
} catch let encodingError { | ||
throw NetworkError.encodingError(encodingError) | ||
} | ||
richherrera marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
let (_, response) = try await session.data(for: request) | ||
|
||
guard let httpResponse = response as? HTTPURLResponse, | ||
(200...299).contains(httpResponse.statusCode) else { | ||
throw NetworkError.invalidResponse | ||
} | ||
} | ||
} |
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.
Can we add all of the new files that were specifically added for analytics to an
Analytics
folder underSources/PopupBridge
? nitpick, but would help navigation of the PopupBridge source filesThere 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.
Updated: c8fd4cf