-
Notifications
You must be signed in to change notification settings - Fork 18
[#701] Add foundation for analytics tracking #702
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
base: develop
Are you sure you want to change the base?
Changes from all commits
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,76 @@ | ||
| public final class Analytics: AnalyticsProtocol { | ||
|
|
||
| public static let shared: AnalyticsProtocol = Analytics() | ||
|
|
||
| private var trackers: [AnalyticsTracker] = [] | ||
|
|
||
| private init() {} | ||
|
|
||
| // MARK: - Setup | ||
|
|
||
| public func configure(trackers: [AnalyticsTracker], additionalParameters: [String: Any]? = nil) { | ||
| self.trackers = trackers | ||
| trackers.forEach { $0.setUp(additionalParameters: additionalParameters) } | ||
| } | ||
|
|
||
| public func addTracker(_ tracker: AnalyticsTracker, additionalParameters: [String: Any]? = nil) { | ||
| trackers.append(tracker) | ||
| tracker.setUp(additionalParameters: additionalParameters) | ||
| } | ||
|
|
||
| // MARK: - Event Tracking | ||
|
|
||
| public func trackEvent(name: String, parameters: [String: Any]?) { | ||
| trackEvent(name: name, parameters: parameters, on: AnalyticsTrackerType.allCases) | ||
| } | ||
|
|
||
| public func trackEvent(name: String, parameters: [String: Any]?, on trackerTypes: [AnalyticsTrackerType]) { | ||
| let targetTrackers = trackers.filter { trackerTypes.contains($0.type) } | ||
| targetTrackers.forEach { $0.trackEvent(name: name, parameters: parameters) } | ||
| } | ||
|
|
||
| public func trackEvent(_ event: AnalyticsEvent) { | ||
| trackEvent(event, on: AnalyticsTrackerType.allCases) | ||
| } | ||
|
|
||
| public func trackEvent(_ event: AnalyticsEvent, on trackerTypes: [AnalyticsTrackerType]) { | ||
| trackEvent(name: event.name, parameters: event.parameters, on: trackerTypes) | ||
| } | ||
|
|
||
| // MARK: - Screen Tracking | ||
|
|
||
| public func trackScreen(name: String, screenClass: String?) { | ||
| trackScreen(name: name, screenClass: screenClass, on: AnalyticsTrackerType.allCases) | ||
| } | ||
|
|
||
| public func trackScreen(name: String, screenClass: String?, on trackerTypes: [AnalyticsTrackerType]) { | ||
| let targetTrackers = trackers.filter { trackerTypes.contains($0.type) } | ||
| targetTrackers.forEach { $0.trackScreen(name: name, screenClass: screenClass) } | ||
| } | ||
|
|
||
| // MARK: - User Properties | ||
|
|
||
| public func setUserProperty(key: String, value: String) { | ||
| setUserProperty(key: key, value: value, on: AnalyticsTrackerType.allCases) | ||
| } | ||
|
|
||
| public func setUserProperty(key: String, value: String, on trackerTypes: [AnalyticsTrackerType]) { | ||
| let targetTrackers = trackers.filter { trackerTypes.contains($0.type) } | ||
| targetTrackers.forEach { $0.setUserProperty(key: key, value: value) } | ||
| } | ||
|
|
||
| public func setUserId(_ userId: String?) { | ||
| setUserId(userId, on: AnalyticsTrackerType.allCases) | ||
| } | ||
|
|
||
| public func setUserId(_ userId: String?, on trackerTypes: [AnalyticsTrackerType]) { | ||
| let targetTrackers = trackers.filter { trackerTypes.contains($0.type) } | ||
| targetTrackers.forEach { $0.setUserId(userId) } | ||
| } | ||
|
|
||
| // MARK: - Utility | ||
|
|
||
| public func tracker(for type: AnalyticsTrackerType) -> AnalyticsTracker? { | ||
| trackers.first { $0.type == type } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| public protocol AnalyticsEvent { | ||
|
|
||
| var name: String { get } | ||
| var parameters: [String: Any]? { get } | ||
| } | ||
|
Comment on lines
+1
to
+5
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. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Confirm no AnalyticsParameter type exists in the module.
rg -nP --type=swift -C2 '\bAnalyticsParameter\b'Repository: nimblehq/ios-templates Length of output: 281 🏁 Script executed: #!/bin/bash
# Search for AnalyticsParameter in Swift files only, ignoring gitignore parsing
cd template && rg -nP --type=swift '\bAnalyticsParameter\b' 2>/dev/null || echo "No matches found"Repository: nimblehq/ios-templates Length of output: 83 🏁 Script executed: #!/bin/bash
# Check the full AnalyticsEvent.swift file
cd template && find . -name "AnalyticsEvent.swift" -type f -exec cat -n {} \;Repository: nimblehq/ios-templates Length of output: 213 🏁 Script executed: #!/bin/bash
# Look for any parameter-related enums or protocols in the Analytics module
cd template && find ./Modules/Analytics -name "*.swift" -type f | head -20Repository: nimblehq/ios-templates Length of output: 684 🏁 Script executed: #!/bin/bash
# Check if there are any other analytics-related files that might define types
cd template && ls -la Modules/Analytics/Sources/ 2>/dev/null || echo "Directory not found"Repository: nimblehq/ios-templates Length of output: 745 🏁 Script executed: #!/bin/bash
# Check AnalyticsProtocol.swift to understand the broader API
cd template && cat -n Modules/Analytics/Sources/AnalyticsProtocol.swiftRepository: nimblehq/ios-templates Length of output: 2114 🏁 Script executed: #!/bin/bash
# Check ConsoleAnalyticsTracker to see how parameters are actually handled
cd template && cat -n Modules/Analytics/Sources/ConsoleAnalyticsTracker.swiftRepository: nimblehq/ios-templates Length of output: 2063 🏁 Script executed: #!/bin/bash
# Check Analytics.swift for usage context
cd template && cat -n Modules/Analytics/Sources/Analytics.swiftRepository: nimblehq/ios-templates Length of output: 3548 🏁 Script executed: #!/bin/bash
# Look for any references to issue `#701` or PR objectives in the codebase
cd template && grep -r "701\|AnalyticsParameter" . --include="*.md" --include="*.swift" 2>/dev/null | head -20Repository: nimblehq/ios-templates Length of output: 48
The protocol currently exposes parameters as
Consider introducing an 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| public protocol AnalyticsProtocol { | ||
|
|
||
| // MARK: - Setup | ||
|
|
||
| func configure(trackers: [AnalyticsTracker], additionalParameters: [String: Any]?) | ||
| func addTracker(_ tracker: AnalyticsTracker, additionalParameters: [String: Any]?) | ||
|
|
||
| // MARK: - Event Tracking | ||
|
|
||
| func trackEvent(name: String, parameters: [String: Any]?) | ||
| func trackEvent(name: String, parameters: [String: Any]?, on trackerTypes: [AnalyticsTrackerType]) | ||
| func trackEvent(_ event: AnalyticsEvent) | ||
| func trackEvent(_ event: AnalyticsEvent, on trackerTypes: [AnalyticsTrackerType]) | ||
|
|
||
| // MARK: - Screen Tracking | ||
|
|
||
| func trackScreen(name: String, screenClass: String?) | ||
| func trackScreen(name: String, screenClass: String?, on trackerTypes: [AnalyticsTrackerType]) | ||
|
|
||
| // MARK: - User Properties | ||
|
|
||
| func setUserProperty(key: String, value: String) | ||
| func setUserProperty(key: String, value: String, on trackerTypes: [AnalyticsTrackerType]) | ||
| func setUserId(_ userId: String?) | ||
| func setUserId(_ userId: String?, on trackerTypes: [AnalyticsTrackerType]) | ||
|
|
||
| // MARK: - Utility | ||
|
|
||
| func tracker(for type: AnalyticsTrackerType) -> AnalyticsTracker? | ||
| } | ||
|
|
||
| // MARK: - Default Parameters | ||
|
|
||
| public extension AnalyticsProtocol { | ||
|
|
||
| func configure(trackers: [AnalyticsTracker]) { | ||
| configure(trackers: trackers, additionalParameters: nil) | ||
| } | ||
|
|
||
| func addTracker(_ tracker: AnalyticsTracker) { | ||
| addTracker(tracker, additionalParameters: nil) | ||
| } | ||
|
|
||
| func trackEvent(name: String) { | ||
| trackEvent(name: name, parameters: nil) | ||
| } | ||
|
|
||
| func trackScreen(name: String) { | ||
| trackScreen(name: name, screenClass: nil) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| public protocol AnalyticsTracker { | ||
|
|
||
| var type: AnalyticsTrackerType { get } | ||
|
|
||
| func setUp(additionalParameters: [String: Any]?) | ||
| func trackEvent(name: String, parameters: [String: Any]?) | ||
| func trackScreen(name: String, screenClass: String?) | ||
| func setUserProperty(key: String, value: String) | ||
| func setUserId(_ userId: String?) | ||
| } | ||
|
|
||
| // MARK: - Extensions | ||
|
|
||
| public extension AnalyticsTracker { | ||
|
|
||
| func setUp() { | ||
| setUp(additionalParameters: nil) | ||
| } | ||
|
|
||
| func trackEvent(name: String) { | ||
| trackEvent(name: name, parameters: nil) | ||
| } | ||
|
|
||
| func trackEvent(_ event: AnalyticsEvent) { | ||
| trackEvent(name: event.name, parameters: event.parameters) | ||
| } | ||
|
|
||
| func trackScreen(name: String) { | ||
| trackScreen(name: name, screenClass: nil) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,4 @@ | ||||||||||||||||||||||||
| public enum AnalyticsTrackerType: String, CaseIterable { | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| case console | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+1
to
+4
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. Enum only defines
Decide on one of two fixes and apply it consistently:
🛠 Suggested extension if you want to keep the `.firebase` test references public enum AnalyticsTrackerType: String, CaseIterable {
-
- case console
+
+ case console
+ case firebase
+ case appsFlyer
+ case facebook
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import Foundation | ||
|
|
||
| /// Console tracker that logs analytics events to the console for debugging | ||
| public final class ConsoleAnalyticsTracker: AnalyticsTracker { | ||
|
|
||
| public let type: AnalyticsTrackerType | ||
| private let prefix: String | ||
|
|
||
| public init(type: AnalyticsTrackerType, logPrefix: String? = nil) { | ||
| self.type = type | ||
| self.prefix = logPrefix ?? "[\(type.rawValue.uppercased())]" | ||
| } | ||
|
|
||
| // MARK: - AnalyticsTracker Implementation | ||
|
|
||
| public func setUp(additionalParameters: [String: Any]?) { | ||
| let paramsString = additionalParameters?.description ?? "none" | ||
| print("\(prefix) SETUP - Additional parameters: \(paramsString)") | ||
| } | ||
|
|
||
| public func trackEvent(name: String, parameters: [String: Any]?) { | ||
| var message = "\(prefix) EVENT - \(name)" | ||
|
|
||
| if let parameters = parameters, !parameters.isEmpty { | ||
| let paramsString = parameters.map { "\($0.key): \($0.value)" }.joined(separator: ", ") | ||
| message += " | Parameters: {\(paramsString)}" | ||
| } | ||
|
|
||
| print(message) | ||
| } | ||
|
|
||
| public func trackScreen(name: String, screenClass: String?) { | ||
| var message = "\(prefix) SCREEN - \(name)" | ||
|
|
||
| if let screenClass = screenClass { | ||
| message += " | Class: \(screenClass)" | ||
| } | ||
|
|
||
| print(message) | ||
| } | ||
|
|
||
| public func setUserProperty(key: String, value: String) { | ||
| print("\(prefix) USER_PROPERTY - \(key): \(value)") | ||
| } | ||
|
|
||
| public func setUserId(_ userId: String?) { | ||
| let userIdString = userId ?? "null" | ||
| print("\(prefix) USER_ID - \(userIdString)") | ||
| } | ||
| } | ||
|
thinh2k1310 marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| /// Example analytics event for user login | ||
| public struct UserLoginEvent: AnalyticsEvent { | ||
|
|
||
| public let name = "user_login" | ||
|
|
||
| public let loginMethod: String | ||
| public let isSuccessful: Bool | ||
|
|
||
| public init(loginMethod: String, isSuccessful: Bool = true) { | ||
| self.loginMethod = loginMethod | ||
| self.isSuccessful = isSuccessful | ||
| } | ||
|
|
||
| public var parameters: [String: Any]? { | ||
| return [ | ||
| "login_method": loginMethod, | ||
| "is_successful": isSuccessful | ||
| ] | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| /// Mock tracker for testing purposes | ||
| public final class MockAnalyticsTracker: AnalyticsTracker { | ||
|
|
||
| public let type: AnalyticsTrackerType | ||
|
|
||
| // MARK: - Tracked Data | ||
|
|
||
| public private(set) var isSetUp = false | ||
| public private(set) var setupParameters: [String: Any]? | ||
| public private(set) var trackedEvents: [(name: String, parameters: [String: Any]?)] = [] | ||
| public private(set) var trackedScreens: [(name: String, screenClass: String?)] = [] | ||
| public private(set) var userProperties: [String: String] = [:] | ||
| public private(set) var userId: String? | ||
|
|
||
| public init(type: AnalyticsTrackerType) { | ||
| self.type = type | ||
| } | ||
|
|
||
| // MARK: - AnalyticsTracker Implementation | ||
|
|
||
| public func setUp(additionalParameters: [String: Any]?) { | ||
| isSetUp = true | ||
| setupParameters = additionalParameters | ||
| } | ||
|
|
||
| public func trackEvent(name: String, parameters: [String: Any]?) { | ||
| trackedEvents.append((name: name, parameters: parameters)) | ||
| } | ||
|
|
||
| public func trackScreen(name: String, screenClass: String?) { | ||
| trackedScreens.append((name: name, screenClass: screenClass)) | ||
| } | ||
|
|
||
| public func setUserProperty(key: String, value: String) { | ||
| userProperties[key] = value | ||
| } | ||
|
|
||
| public func setUserId(_ userId: String?) { | ||
| self.userId = userId | ||
| } | ||
|
|
||
| // MARK: - Test Helpers | ||
|
|
||
| public func reset() { | ||
| isSetUp = false | ||
| setupParameters = nil | ||
| trackedEvents.removeAll() | ||
| trackedScreens.removeAll() | ||
| userProperties.removeAll() | ||
| userId = nil | ||
| } | ||
|
|
||
| public func hasTrackedEvent(name: String) -> Bool { | ||
| trackedEvents.contains { $0.name == name } | ||
| } | ||
|
|
||
| public func hasTrackedScreen(name: String) -> Bool { | ||
| trackedScreens.contains { $0.name == name } | ||
| } | ||
|
|
||
| public func eventCount(for name: String) -> Int { | ||
| trackedEvents.filter { $0.name == name }.count | ||
| } | ||
| } | ||
|
Comment on lines
+1
to
+64
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. 🛠️ Refactor suggestion | 🟠 Major
This file is under Recommended options:
🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import Testing | ||
| @testable import Analytics | ||
|
|
||
| @Suite("UserLoginEvent Tests") | ||
| struct UserLoginEventTests { | ||
|
|
||
| @Test("Create event with correct parameters") | ||
| func createEventWithCorrectParameters() { | ||
| let event = UserLoginEvent(loginMethod: "email", isSuccessful: true) | ||
|
|
||
| #expect(event.name == "user_login") | ||
| #expect(event.parameters?["login_method"] as? String == "email") | ||
| #expect(event.parameters?["is_successful"] as? Bool == true) | ||
| } | ||
|
|
||
| @Test("Handle failed login") | ||
| func handleFailedLogin() { | ||
| let event = UserLoginEvent(loginMethod: "social", isSuccessful: false) | ||
|
|
||
| #expect(event.parameters?["is_successful"] as? Bool == false) | ||
| #expect(event.parameters?["login_method"] as? String == "social") | ||
| } | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the complete Analytics.swift file cat -n template/Modules/Analytics/Sources/Analytics.swiftRepository: nimblehq/ios-templates
Length of output: 3548
🏁 Script executed:
Repository: nimblehq/ios-templates
Length of output: 2279
🏁 Script executed:
Repository: nimblehq/ios-templates
Length of output: 315
🏁 Script executed:
Repository: nimblehq/ios-templates
Length of output: 761
🏁 Script executed:
Repository: nimblehq/ios-templates
Length of output: 4348
🏁 Script executed:
Repository: nimblehq/ios-templates
Length of output: 684
🏁 Script executed:
Repository: nimblehq/ios-templates
Length of output: 14836
Thread safety:
trackersis mutated and read without synchronization, and this becomes critical ifconfigure()/addTracker()are called from multiple threads.Analytics.sharedis a process-wide singleton invoked from many threads (main thread for UI, background threads from network/Combine callbacks). Thetrackersarray is mutated byconfigure()/addTracker()(lines 12, 17) and read bytrackEvent/trackScreen/setUserProperty/setUserId/tracker(for:)(lines 28, 47, 58, 67, 74). SwiftArrayis not thread-safe: concurrentappendduring.filter/.firstread causes a data race and can crash.In this template's current usage,
configure()is called once inApp.init()on the main thread, after which only reads occur—so the practical risk is low. However, ifaddTracker()orconfigure()are called dynamically from a background thread (e.g., switching analytics providers), the race becomes critical.Option A (recommended): Guard with a serial dispatch queue:
public final class Analytics: AnalyticsProtocol { public static let shared: AnalyticsProtocol = Analytics() private var trackers: [AnalyticsTracker] = [] + private let queue = DispatchQueue(label: "co.nimblehq.analytics", qos: .utility) private init() {} public func configure(trackers: [AnalyticsTracker], additionalParameters: [String: Any]? = nil) { - self.trackers = trackers + queue.sync { self.trackers = trackers } trackers.forEach { $0.setUp(additionalParameters: additionalParameters) } } public func addTracker(_ tracker: AnalyticsTracker, additionalParameters: [String: Any]? = nil) { - trackers.append(tracker) + queue.sync { trackers.append(tracker) } tracker.setUp(additionalParameters: additionalParameters) }Then wrap each read:
let targetTrackers = queue.sync { trackers.filter { ... } }(lines 28, 47, 58, 67, 74).Option B: Document main-thread-only usage: Add a
// MARK: - Thread Safetycomment clarifying thatconfigure()andaddTracker()must be called only from the main thread during app initialization to prevent future regressions.🤖 Prompt for AI Agents