Decouple the bug report service from the analytics one

This commit is contained in:
Stefan Ceriu
2024-05-30 14:10:51 +03:00
committed by Stefan Ceriu
parent c3571f4f4e
commit a4c9135b58
12 changed files with 58 additions and 94 deletions
@@ -93,6 +93,17 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
Self.setupServiceLocator(appSettings: appSettings) Self.setupServiceLocator(appSettings: appSettings)
ServiceLocator.shared.analytics.isRunningPublisher
.removeDuplicates()
.sink { isRunning in
if isRunning {
ServiceLocator.shared.bugReportService.start()
} else {
ServiceLocator.shared.bugReportService.stop()
}
}
.store(in: &cancellables)
ServiceLocator.shared.analytics.startIfEnabled() ServiceLocator.shared.analytics.startIfEnabled()
stateMachine = AppCoordinatorStateMachine() stateMachine = AppCoordinatorStateMachine()
@@ -337,10 +348,9 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
sdkGitSHA: sdkGitSha(), sdkGitSHA: sdkGitSha(),
maxUploadSize: appSettings.bugReportMaxUploadSize)) maxUploadSize: appSettings.bugReportMaxUploadSize))
let posthogAnalyticsClient = PostHogAnalyticsClient() let posthogAnalyticsClient = PostHogAnalyticsClient()
posthogAnalyticsClient.updateSuperProperties(AnalyticsEvent.SuperProperties(appPlatform: nil, cryptoSDK: .Rust, cryptoSDKVersion: sdkGitSha())) posthogAnalyticsClient.updateSuperProperties(AnalyticsEvent.SuperProperties(appPlatform: .EXI, cryptoSDK: .Rust, cryptoSDKVersion: sdkGitSha()))
ServiceLocator.shared.register(analytics: AnalyticsService(client: posthogAnalyticsClient, ServiceLocator.shared.register(analytics: AnalyticsService(client: posthogAnalyticsClient,
appSettings: appSettings, appSettings: appSettings))
bugReportService: ServiceLocator.shared.bugReportService))
} }
/// Perform any required migrations for the app to function correctly. /// Perform any required migrations for the app to function correctly.
@@ -1856,41 +1856,6 @@ class BugReportServiceMock: BugReportServiceProtocol {
stopCallsCount += 1 stopCallsCount += 1
stopClosure?() stopClosure?()
} }
//MARK: - reset
var resetUnderlyingCallsCount = 0
var resetCallsCount: Int {
get {
if Thread.isMainThread {
return resetUnderlyingCallsCount
} else {
var returnValue: Int? = nil
DispatchQueue.main.sync {
returnValue = resetUnderlyingCallsCount
}
return returnValue!
}
}
set {
if Thread.isMainThread {
resetUnderlyingCallsCount = newValue
} else {
DispatchQueue.main.sync {
resetUnderlyingCallsCount = newValue
}
}
}
}
var resetCalled: Bool {
return resetCallsCount > 0
}
var resetClosure: (() -> Void)?
func reset() {
resetCallsCount += 1
resetClosure?()
}
//MARK: - submitBugReport //MARK: - submitBugReport
var submitBugReportProgressListenerUnderlyingCallsCount = 0 var submitBugReportProgressListenerUnderlyingCallsCount = 0
@@ -49,9 +49,7 @@ struct AnalyticsSettingsScreen_Previews: PreviewProvider, TestablePreview {
static var previews: some View { static var previews: some View {
let appSettings = AppSettings() let appSettings = AppSettings()
let viewModel = AnalyticsSettingsScreenViewModel(appSettings: appSettings, let viewModel = AnalyticsSettingsScreenViewModel(appSettings: appSettings,
analytics: AnalyticsService(client: AnalyticsClientMock(), analytics: ServiceLocator.shared.analytics)
appSettings: appSettings,
bugReportService: BugReportServiceMock()))
AnalyticsSettingsScreen(context: viewModel.context) AnalyticsSettingsScreen(context: viewModel.context)
} }
} }
@@ -15,6 +15,7 @@
// //
import AnalyticsEvents import AnalyticsEvents
import Combine
import PostHog import PostHog
/// A class responsible for managing a variety of analytics clients /// A class responsible for managing a variety of analytics clients
@@ -34,20 +35,23 @@ class AnalyticsService {
/// The analytics client to send events with. /// The analytics client to send events with.
private let client: AnalyticsClientProtocol private let client: AnalyticsClientProtocol
private let appSettings: AppSettings private let appSettings: AppSettings
private let bugReportService: BugReportServiceProtocol
/// A signpost client for performance testing the app. This client doesn't respect the /// A signpost client for performance testing the app. This client doesn't respect the
/// `isRunning` state or behave any differently when `start`/`reset` are called. /// `isRunning` state or behave any differently when `start`/`reset` are called.
let signpost = Signposter() let signpost = Signposter()
/// Whether or not the object is enabled and sending events to the server.
private let isRunningSubject: CurrentValueSubject<Bool, Never> = .init(false)
var isRunningPublisher: CurrentValuePublisher<Bool, Never> {
isRunningSubject.asCurrentValuePublisher()
}
init(client: AnalyticsClientProtocol, appSettings: AppSettings, bugReportService: BugReportServiceProtocol) { init(client: AnalyticsClientProtocol, appSettings: AppSettings) {
self.client = client self.client = client
self.appSettings = appSettings self.appSettings = appSettings
self.bugReportService = bugReportService
}
/// Whether or not the object is enabled and sending events to the server. isRunningSubject.send(client.isRunning)
var isRunning: Bool { client.isRunning } }
/// Whether to show the user the analytics opt in prompt. /// Whether to show the user the analytics opt in prompt.
var shouldShowAnalyticsPrompt: Bool { var shouldShowAnalyticsPrompt: Bool {
@@ -72,20 +76,21 @@ class AnalyticsService {
// The order is important here. PostHog ignores the reset if stopped. // The order is important here. PostHog ignores the reset if stopped.
reset() reset()
client.stop() client.stop()
bugReportService.stop()
isRunningSubject.send(false)
MXLog.info("Stopped.") MXLog.info("Stopped.")
} }
/// Starts the analytics client if the user has opted in, otherwise does nothing. /// Starts the analytics client if the user has opted in, otherwise does nothing.
func startIfEnabled() { func startIfEnabled() {
guard isEnabled, !isRunning else { return } guard isEnabled, !isRunningPublisher.value else { return }
client.start(analyticsConfiguration: appSettings.analyticsConfiguration) client.start(analyticsConfiguration: appSettings.analyticsConfiguration)
bugReportService.start()
// Sanity check in case something went wrong. // Sanity check in case something went wrong.
guard client.isRunning else { return } guard client.isRunning else { return }
isRunningSubject.send(true)
MXLog.info("Started.") MXLog.info("Started.")
} }
@@ -95,7 +100,6 @@ class AnalyticsService {
/// Note: **MUST** be called before stopping PostHog or the reset is ignored. /// Note: **MUST** be called before stopping PostHog or the reset is ignored.
func reset() { func reset() {
client.reset() client.reset()
bugReportService.reset()
MXLog.info("Reset.") MXLog.info("Reset.")
} }
@@ -53,7 +53,7 @@ class PostHogAnalyticsClient: AnalyticsClientProtocol {
// Add super property cryptoSDK to the captured events, to allow easy // Add super property cryptoSDK to the captured events, to allow easy
// filtering of events across different client by using same filter. // filtering of events across different client by using same filter.
superProperties = AnalyticsEvent.SuperProperties(appPlatform: nil, cryptoSDK: .Rust, cryptoSDKVersion: nil) superProperties = AnalyticsEvent.SuperProperties(appPlatform: .EXI, cryptoSDK: .Rust, cryptoSDKVersion: nil)
postHog?.optIn() postHog?.optIn()
} }
@@ -105,12 +105,7 @@ class BugReportService: NSObject, BugReportServiceProtocol {
SentrySDK.close() SentrySDK.close()
MXLog.info("Stopped.") MXLog.info("Stopped.")
} }
func reset() {
lastCrashEventId = nil
MXLog.info("Reset.")
}
// swiftlint:disable:next cyclomatic_complexity // swiftlint:disable:next cyclomatic_complexity
func submitBugReport(_ bugReport: BugReport, func submitBugReport(_ bugReport: BugReport,
progressListener: CurrentValueSubject<Double, Never>) async -> Result<SubmitBugReportResponse, BugReportServiceError> { progressListener: CurrentValueSubject<Double, Never>) async -> Result<SubmitBugReportResponse, BugReportServiceError> {
@@ -62,8 +62,6 @@ protocol BugReportServiceProtocol {
func stop() func stop()
func reset()
func submitBugReport(_ bugReport: BugReport, func submitBugReport(_ bugReport: BugReport,
progressListener: CurrentValueSubject<Double, Never>) async -> Result<SubmitBugReportResponse, BugReportServiceError> progressListener: CurrentValueSubject<Double, Never>) async -> Result<SubmitBugReportResponse, BugReportServiceError>
} }
@@ -45,9 +45,11 @@ class UITestsAppCoordinator: AppCoordinatorProtocol, SecureWindowManagerDelegate
AppSettings.resetAllSettings() AppSettings.resetAllSettings()
ServiceLocator.shared.register(appSettings: AppSettings()) ServiceLocator.shared.register(appSettings: AppSettings())
ServiceLocator.shared.register(bugReportService: BugReportServiceMock()) ServiceLocator.shared.register(bugReportService: BugReportServiceMock())
ServiceLocator.shared.register(analytics: AnalyticsService(client: AnalyticsClientMock(),
appSettings: ServiceLocator.shared.settings, let analyticsClient = AnalyticsClientMock()
bugReportService: ServiceLocator.shared.bugReportService)) analyticsClient.isRunning = false
ServiceLocator.shared.register(analytics: AnalyticsService(client: analyticsClient,
appSettings: ServiceLocator.shared.settings))
} }
func start() { func start() {
@@ -27,9 +27,11 @@ class UnitTestsAppCoordinator: AppCoordinatorProtocol {
AppSettings.resetAllSettings() AppSettings.resetAllSettings()
ServiceLocator.shared.register(appSettings: AppSettings()) ServiceLocator.shared.register(appSettings: AppSettings())
ServiceLocator.shared.register(bugReportService: BugReportServiceMock()) ServiceLocator.shared.register(bugReportService: BugReportServiceMock())
ServiceLocator.shared.register(analytics: AnalyticsService(client: AnalyticsClientMock(),
appSettings: ServiceLocator.shared.settings, let analyticsClient = AnalyticsClientMock()
bugReportService: ServiceLocator.shared.bugReportService)) analyticsClient.isRunning = false
ServiceLocator.shared.register(analytics: AnalyticsService(client: analyticsClient,
appSettings: ServiceLocator.shared.settings))
} }
func start() { } func start() { }
@@ -34,8 +34,7 @@ class AnalyticsSettingsScreenViewModelTests: XCTestCase {
let analyticsClient = AnalyticsClientMock() let analyticsClient = AnalyticsClientMock()
analyticsClient.isRunning = false analyticsClient.isRunning = false
ServiceLocator.shared.register(analytics: AnalyticsService(client: analyticsClient, ServiceLocator.shared.register(analytics: AnalyticsService(client: analyticsClient,
appSettings: appSettings, appSettings: appSettings))
bugReportService: ServiceLocator.shared.bugReportService))
viewModel = AnalyticsSettingsScreenViewModel(appSettings: appSettings, viewModel = AnalyticsSettingsScreenViewModel(appSettings: appSettings,
analytics: ServiceLocator.shared.analytics) analytics: ServiceLocator.shared.analytics)
+13 -24
View File
@@ -22,21 +22,16 @@ import XCTest
class AnalyticsTests: XCTestCase { class AnalyticsTests: XCTestCase {
private var appSettings: AppSettings! private var appSettings: AppSettings!
private var analyticsClient: AnalyticsClientMock! private var analyticsClient: AnalyticsClientMock!
private var bugReportService: BugReportServiceMock!
private var posthogMock: PHGPostHogMock! private var posthogMock: PHGPostHogMock!
override func setUp() { override func setUp() {
AppSettings.resetAllSettings() AppSettings.resetAllSettings()
appSettings = AppSettings() appSettings = AppSettings()
bugReportService = BugReportServiceMock()
bugReportService.isRunning = false
ServiceLocator.shared.register(bugReportService: bugReportService)
analyticsClient = AnalyticsClientMock() analyticsClient = AnalyticsClientMock()
analyticsClient.isRunning = false analyticsClient.isRunning = false
ServiceLocator.shared.register(analytics: AnalyticsService(client: analyticsClient, ServiceLocator.shared.register(analytics: AnalyticsService(client: analyticsClient,
appSettings: appSettings, appSettings: appSettings))
bugReportService: ServiceLocator.shared.bugReportService))
posthogMock = PHGPostHogMock() posthogMock = PHGPostHogMock()
posthogMock.configureMockBehavior() posthogMock.configureMockBehavior()
@@ -78,12 +73,11 @@ class AnalyticsTests: XCTestCase {
} }
func testAnalyticsPromptNotDisplayed() { func testAnalyticsPromptNotDisplayed() {
// Given a fresh install of the app both Analytics and BugReportService should be disabled // Given a fresh install of the app Analytics should be disabled
XCTAssertEqual(appSettings.analyticsConsentState, .unknown) XCTAssertEqual(appSettings.analyticsConsentState, .unknown)
XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled) XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled)
XCTAssertFalse(ServiceLocator.shared.analytics.isRunning) XCTAssertFalse(ServiceLocator.shared.analytics.isRunningPublisher.value)
XCTAssertFalse(analyticsClient.startAnalyticsConfigurationCalled) XCTAssertFalse(analyticsClient.startAnalyticsConfigurationCalled)
XCTAssertFalse(bugReportService.startCalled)
} }
func testAnalyticsOptOut() { func testAnalyticsOptOut() {
@@ -93,12 +87,10 @@ class AnalyticsTests: XCTestCase {
// Then analytics should be disabled // Then analytics should be disabled
XCTAssertEqual(appSettings.analyticsConsentState, .optedOut) XCTAssertEqual(appSettings.analyticsConsentState, .optedOut)
XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled) XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled)
XCTAssertFalse(ServiceLocator.shared.analytics.isRunning) XCTAssertFalse(ServiceLocator.shared.analytics.isRunningPublisher.value)
XCTAssertFalse(analyticsClient.isRunning) XCTAssertFalse(analyticsClient.isRunning)
XCTAssertFalse(bugReportService.isRunning) // Analytics client should have been stopped
// Analytics client and the bug report service should have been stopped
XCTAssertTrue(analyticsClient.stopCalled) XCTAssertTrue(analyticsClient.stopCalled)
XCTAssertTrue(bugReportService.stopCalled)
} }
func testAnalyticsOptIn() { func testAnalyticsOptIn() {
@@ -108,9 +100,8 @@ class AnalyticsTests: XCTestCase {
// The analytics should be enabled // The analytics should be enabled
XCTAssertEqual(appSettings.analyticsConsentState, .optedIn) XCTAssertEqual(appSettings.analyticsConsentState, .optedIn)
XCTAssertTrue(ServiceLocator.shared.analytics.isEnabled) XCTAssertTrue(ServiceLocator.shared.analytics.isEnabled)
// Analytics client and the bug report service should have been started // Analytics client should have been started
XCTAssertTrue(analyticsClient.startAnalyticsConfigurationCalled) XCTAssertTrue(analyticsClient.startAnalyticsConfigurationCalled)
XCTAssertTrue(bugReportService.startCalled)
} }
func testAnalyticsStartIfNotEnabled() { func testAnalyticsStartIfNotEnabled() {
@@ -120,17 +111,15 @@ class AnalyticsTests: XCTestCase {
XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled) XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled)
ServiceLocator.shared.analytics.startIfEnabled() ServiceLocator.shared.analytics.startIfEnabled()
XCTAssertFalse(analyticsClient.startAnalyticsConfigurationCalled) XCTAssertFalse(analyticsClient.startAnalyticsConfigurationCalled)
XCTAssertFalse(bugReportService.startCalled)
} }
func testAnalyticsStartIfEnabled() { func testAnalyticsStartIfEnabled() {
// Given an existing install of the app where the user previously accpeted the tracking // Given an existing install of the app where the user previously accepted the tracking
appSettings.analyticsConsentState = .optedIn appSettings.analyticsConsentState = .optedIn
// Analytics should start // Analytics should start
XCTAssertTrue(ServiceLocator.shared.analytics.isEnabled) XCTAssertTrue(ServiceLocator.shared.analytics.isEnabled)
ServiceLocator.shared.analytics.startIfEnabled() ServiceLocator.shared.analytics.startIfEnabled()
XCTAssertTrue(analyticsClient.startAnalyticsConfigurationCalled) XCTAssertTrue(analyticsClient.startAnalyticsConfigurationCalled)
XCTAssertTrue(bugReportService.startCalled)
} }
func testAddingUserProperties() { func testAddingUserProperties() {
@@ -232,7 +221,7 @@ class AnalyticsTests: XCTestCase {
client.start(analyticsConfiguration: appSettings.analyticsConfiguration) client.start(analyticsConfiguration: appSettings.analyticsConfiguration)
client.updateSuperProperties( client.updateSuperProperties(
AnalyticsEvent.SuperProperties(appPlatform: "A thing", AnalyticsEvent.SuperProperties(appPlatform: .EXI,
cryptoSDK: .Rust, cryptoSDK: .Rust,
cryptoSDKVersion: "000") cryptoSDKVersion: "000")
) )
@@ -246,7 +235,7 @@ class AnalyticsTests: XCTestCase {
// All the super properties should have been added // All the super properties should have been added
XCTAssertEqual(screenEvent?.properties?["cryptoSDK"] as? String, AnalyticsEvent.SuperProperties.CryptoSDK.Rust.rawValue) XCTAssertEqual(screenEvent?.properties?["cryptoSDK"] as? String, AnalyticsEvent.SuperProperties.CryptoSDK.Rust.rawValue)
XCTAssertEqual(screenEvent?.properties?["appPlatform"] as? String, "A thing") XCTAssertEqual(screenEvent?.properties?["appPlatform"] as? String, "EXI")
XCTAssertEqual(screenEvent?.properties?["cryptoSDKVersion"] as? String, "000") XCTAssertEqual(screenEvent?.properties?["cryptoSDKVersion"] as? String, "000")
// It should be the same for any event // It should be the same for any event
@@ -267,13 +256,13 @@ class AnalyticsTests: XCTestCase {
// All the super properties should have been added // All the super properties should have been added
XCTAssertEqual(capturedEvent?.properties?["cryptoSDK"] as? String, AnalyticsEvent.SuperProperties.CryptoSDK.Rust.rawValue) XCTAssertEqual(capturedEvent?.properties?["cryptoSDK"] as? String, AnalyticsEvent.SuperProperties.CryptoSDK.Rust.rawValue)
XCTAssertEqual(capturedEvent?.properties?["appPlatform"] as? String, "A thing") XCTAssertEqual(capturedEvent?.properties?["appPlatform"] as? String, "EXI")
XCTAssertEqual(capturedEvent?.properties?["cryptoSDKVersion"] as? String, "000") XCTAssertEqual(capturedEvent?.properties?["cryptoSDKVersion"] as? String, "000")
// Updating should keep the previously set properties // Updating should keep the previously set properties
client.updateSuperProperties( client.updateSuperProperties(
AnalyticsEvent.SuperProperties(appPlatform: nil, AnalyticsEvent.SuperProperties(appPlatform: .EXI,
cryptoSDK: nil, cryptoSDK: .Rust,
cryptoSDKVersion: "001") cryptoSDKVersion: "001")
) )
@@ -282,7 +271,7 @@ class AnalyticsTests: XCTestCase {
// All the super properties should have been added, with the one udpated // All the super properties should have been added, with the one udpated
XCTAssertEqual(capturedEvent2?.properties?["cryptoSDK"] as? String, AnalyticsEvent.SuperProperties.CryptoSDK.Rust.rawValue) XCTAssertEqual(capturedEvent2?.properties?["cryptoSDK"] as? String, AnalyticsEvent.SuperProperties.CryptoSDK.Rust.rawValue)
XCTAssertEqual(capturedEvent2?.properties?["appPlatform"] as? String, "A thing") XCTAssertEqual(capturedEvent2?.properties?["appPlatform"] as? String, "EXI")
XCTAssertEqual(capturedEvent2?.properties?["cryptoSDKVersion"] as? String, "001") XCTAssertEqual(capturedEvent2?.properties?["cryptoSDKVersion"] as? String, "001")
} }
@@ -37,7 +37,8 @@ class ComposerToolbarViewModelTests: XCTestCase {
completionSuggestionService: completionSuggestionServiceMock, completionSuggestionService: completionSuggestionServiceMock,
mediaProvider: MockMediaProvider(), mediaProvider: MockMediaProvider(),
appSettings: appSettings, appSettings: appSettings,
mentionDisplayHelper: ComposerMentionDisplayHelper.mock) mentionDisplayHelper: ComposerMentionDisplayHelper.mock,
analyticsService: ServiceLocator.shared.analytics)
viewModel.context.composerFormattingEnabled = true viewModel.context.composerFormattingEnabled = true
} }
@@ -113,7 +114,8 @@ class ComposerToolbarViewModelTests: XCTestCase {
completionSuggestionService: mockCompletionSuggestionService, completionSuggestionService: mockCompletionSuggestionService,
mediaProvider: MockMediaProvider(), mediaProvider: MockMediaProvider(),
appSettings: ServiceLocator.shared.settings, appSettings: ServiceLocator.shared.settings,
mentionDisplayHelper: ComposerMentionDisplayHelper.mock) mentionDisplayHelper: ComposerMentionDisplayHelper.mock,
analyticsService: ServiceLocator.shared.analytics)
XCTAssertEqual(viewModel.state.suggestions, suggestions) XCTAssertEqual(viewModel.state.suggestions, suggestions)
} }