From eff8e9ee81d34716305bd6f3148c4a93c96594e8 Mon Sep 17 00:00:00 2001
From: Nicolas Mauri <nicolas.mauri@gmail.com>
Date: Fri, 21 Apr 2023 17:05:39 +0200
Subject: [PATCH] =?UTF-8?q?Fix:=20forget=20the=20user=E2=80=99s=20consents?=
 =?UTF-8?q?=20for=20analytics=20on=20logout=20(#816)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Fix: forget the user’s consents for analytics on logout

* Fix: change how analytics consent state is stored

* Fix: renaming of AnalyticsConsentState entries
---
 .../Sources/Application/AppCoordinator.swift  |  3 +-
 .../Sources/Application/AppSettings.swift     | 19 ++++-------
 .../AnalyticsSettingsScreenViewModel.swift    |  7 ++--
 .../Services/Analytics/Analytics.swift        | 18 +++++++---
 .../Analytics/AnalyticsConsentState.swift     | 23 +++++++++++++
 ...nalyticsSettingsScreenViewModelTests.swift |  3 +-
 UnitTests/Sources/AnalyticsTests.swift        | 34 ++++++++++++++-----
 changelog.d/pr-816.change                     |  1 +
 8 files changed, 78 insertions(+), 30 deletions(-)
 create mode 100644 ElementX/Sources/Services/Analytics/AnalyticsConsentState.swift
 create mode 100644 changelog.d/pr-816.change

diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift
index ad1cdae23..670bdf5b9 100644
--- a/ElementX/Sources/Application/AppCoordinator.swift
+++ b/ElementX/Sources/Application/AppCoordinator.swift
@@ -295,7 +295,8 @@ class AppCoordinator: AppCoordinatorProtocol {
             tearDownUserSession()
             
             // reset analytics
-            ServiceLocator.shared.analytics.reset()
+            ServiceLocator.shared.analytics.optOut()
+            ServiceLocator.shared.analytics.resetConsentState()
             
             stateMachine.processEvent(.completedSigningOut(isSoft: isSoft))
         }
diff --git a/ElementX/Sources/Application/AppSettings.swift b/ElementX/Sources/Application/AppSettings.swift
index 64d91b920..b362613fb 100644
--- a/ElementX/Sources/Application/AppSettings.swift
+++ b/ElementX/Sources/Application/AppSettings.swift
@@ -23,7 +23,7 @@ final class AppSettings: ObservableObject {
         case lastVersionLaunched
         case seenInvites
         case timelineStyle
-        case enableAnalytics
+        case analyticsConsentState
         case enableInAppNotifications
         case pusherProfileTag
         case shouldCollapseRoomStateEvents
@@ -109,7 +109,7 @@ final class AppSettings: ObservableObject {
     let bugReportUISIId = "element-auto-uisi"
     
     // MARK: - Analytics
-        
+    
     #if DEBUG
     /// The configuration to use for analytics during development. Set `isEnabled` to false to disable analytics in debug builds.
     /// **Note:** Analytics are disabled by default for forks. If you are maintaining a fork, set custom configurations.
@@ -125,16 +125,11 @@ final class AppSettings: ObservableObject {
                                                         apiKey: "phc_Jzsm6DTm6V2705zeU5dcNvQDlonOR68XvX2sh1sEOHO",
                                                         termsURL: URL(staticString: "https://element.io/cookie-policy"))
     #endif
-    
-    /// Whether the user has already been shown the PostHog analytics prompt.
-    var hasSeenAnalyticsPrompt: Bool {
-        Self.store.object(forKey: UserDefaultsKeys.enableAnalytics.rawValue) != nil
-    }
-    
-    /// `true` when the user has opted in to send analytics.
-    @UserPreference(key: UserDefaultsKeys.enableAnalytics, defaultValue: false, storageType: .userDefaults(store))
-    var enableAnalytics
         
+    /// Whether the user has opted in to send analytics.
+    @UserPreference(key: UserDefaultsKeys.analyticsConsentState, defaultValue: AnalyticsConsentState.unknown, storageType: .userDefaults(store))
+    var analyticsConsentState
+    
     // MARK: - Room Screen
     
     @UserPreference(key: UserDefaultsKeys.timelineStyle, defaultValue: TimelineStyle.bubbles, storageType: .userDefaults(store))
@@ -145,7 +140,7 @@ final class AppSettings: ObservableObject {
     
     // MARK: - Notifications
 
-    @UserPreference(key: UserDefaultsKeys.timelineStyle, defaultValue: true, storageType: .userDefaults(store))
+    @UserPreference(key: UserDefaultsKeys.enableInAppNotifications, defaultValue: true, storageType: .userDefaults(store))
     var enableInAppNotifications
 
     /// Tag describing which set of device specific rules a pusher executes.
diff --git a/ElementX/Sources/Screens/AnalyticsSettings/AnalyticsSettingsScreenViewModel.swift b/ElementX/Sources/Screens/AnalyticsSettings/AnalyticsSettingsScreenViewModel.swift
index d1a933ede..c0c3f056b 100644
--- a/ElementX/Sources/Screens/AnalyticsSettings/AnalyticsSettingsScreenViewModel.swift
+++ b/ElementX/Sources/Screens/AnalyticsSettings/AnalyticsSettingsScreenViewModel.swift
@@ -22,12 +22,13 @@ typealias AnalyticsSettingsScreenViewModelType = StateStoreViewModel<AnalyticsSe
 class AnalyticsSettingsScreenViewModel: AnalyticsSettingsScreenViewModelType, AnalyticsSettingsScreenViewModelProtocol {
     init() {
         let strings = AnalyticsSettingsScreenStrings(termsURL: ServiceLocator.shared.settings.analyticsConfiguration.termsURL)
-        let bindings = AnalyticsSettingsScreenViewStateBindings(enableAnalytics: ServiceLocator.shared.settings.enableAnalytics)
+        let bindings = AnalyticsSettingsScreenViewStateBindings(enableAnalytics: ServiceLocator.shared.analytics.isEnabled)
         let state = AnalyticsSettingsScreenViewState(strings: strings, bindings: bindings)
         
         super.init(initialViewState: state)
         
-        ServiceLocator.shared.settings.$enableAnalytics
+        ServiceLocator.shared.settings.$analyticsConsentState
+            .map { $0 == .optedIn }
             .weakAssign(to: \.state.bindings.enableAnalytics, on: self)
             .store(in: &cancellables)
     }
@@ -35,7 +36,7 @@ class AnalyticsSettingsScreenViewModel: AnalyticsSettingsScreenViewModelType, An
     override func process(viewAction: AnalyticsSettingsScreenViewAction) {
         switch viewAction {
         case .toggleAnalytics:
-            if ServiceLocator.shared.settings.enableAnalytics {
+            if ServiceLocator.shared.analytics.isEnabled {
                 ServiceLocator.shared.analytics.optOut()
             } else {
                 ServiceLocator.shared.analytics.optIn()
diff --git a/ElementX/Sources/Services/Analytics/Analytics.swift b/ElementX/Sources/Services/Analytics/Analytics.swift
index 844dc75d7..4701bf93d 100644
--- a/ElementX/Sources/Services/Analytics/Analytics.swift
+++ b/ElementX/Sources/Services/Analytics/Analytics.swift
@@ -44,18 +44,22 @@ class Analytics {
     /// Whether to show the user the analytics opt in prompt.
     var shouldShowAnalyticsPrompt: Bool {
         // Only show the prompt once, and when analytics are enabled in BuildSettings.
-        !ServiceLocator.shared.settings.hasSeenAnalyticsPrompt && ServiceLocator.shared.settings.analyticsConfiguration.isEnabled
+        ServiceLocator.shared.settings.analyticsConsentState == .unknown && ServiceLocator.shared.settings.analyticsConfiguration.isEnabled
+    }
+    
+    var isEnabled: Bool {
+        ServiceLocator.shared.settings.analyticsConsentState == .optedIn
     }
     
     /// Opts in to analytics tracking with the supplied user session.
     func optIn() {
-        ServiceLocator.shared.settings.enableAnalytics = true
+        ServiceLocator.shared.settings.analyticsConsentState = .optedIn
         startIfEnabled()
     }
     
     /// Stops analytics tracking and calls `reset` to clear any IDs and event queues.
     func optOut() {
-        ServiceLocator.shared.settings.enableAnalytics = false
+        ServiceLocator.shared.settings.analyticsConsentState = .optedOut
         
         // The order is important here. PostHog ignores the reset if stopped.
         reset()
@@ -66,7 +70,7 @@ class Analytics {
     
     /// Starts the analytics client if the user has opted in, otherwise does nothing.
     func startIfEnabled() {
-        guard ServiceLocator.shared.settings.enableAnalytics, !isRunning else { return }
+        guard isEnabled, !isRunning else { return }
         
         client.start()
         ServiceLocator.shared.bugReportService.start()
@@ -87,6 +91,12 @@ class Analytics {
         MXLog.info("Reset.")
     }
     
+    /// Reset the consent state for analytics
+    func resetConsentState() {
+        MXLog.warning("Resetting consent state for analytics.")
+        ServiceLocator.shared.settings.analyticsConsentState = .unknown
+    }
+    
     /// Flushes the event queue in the analytics client, uploading all pending events.
     /// Normally events are sent in batches. Call this method when you need an event
     /// to be sent immediately.
diff --git a/ElementX/Sources/Services/Analytics/AnalyticsConsentState.swift b/ElementX/Sources/Services/Analytics/AnalyticsConsentState.swift
new file mode 100644
index 000000000..f8805803e
--- /dev/null
+++ b/ElementX/Sources/Services/Analytics/AnalyticsConsentState.swift
@@ -0,0 +1,23 @@
+//
+// Copyright 2023 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+import Foundation
+
+enum AnalyticsConsentState: String, Codable {
+    case optedOut
+    case optedIn
+    case unknown
+}
diff --git a/UnitTests/Sources/AnalyticsSettingsScreenViewModelTests.swift b/UnitTests/Sources/AnalyticsSettingsScreenViewModelTests.swift
index b576b9cc8..43a4ec308 100644
--- a/UnitTests/Sources/AnalyticsSettingsScreenViewModelTests.swift
+++ b/UnitTests/Sources/AnalyticsSettingsScreenViewModelTests.swift
@@ -42,12 +42,13 @@ class AnalyticsSettingsScreenViewModelTests: XCTestCase {
     }
 
     func testOptIn() {
+        applicationSettings.analyticsConsentState = .optedOut
         context.send(viewAction: .toggleAnalytics)
         XCTAssertTrue(context.enableAnalytics)
     }
     
     func testOptOut() {
-        applicationSettings.enableAnalytics = true
+        applicationSettings.analyticsConsentState = .optedIn
         context.send(viewAction: .toggleAnalytics)
         XCTAssertFalse(context.enableAnalytics)
     }
diff --git a/UnitTests/Sources/AnalyticsTests.swift b/UnitTests/Sources/AnalyticsTests.swift
index bb2f67a06..aed582058 100644
--- a/UnitTests/Sources/AnalyticsTests.swift
+++ b/UnitTests/Sources/AnalyticsTests.swift
@@ -47,7 +47,7 @@ class AnalyticsTests: XCTestCase {
     
     func testAnalyticsPromptUserDeclinedPostHog() {
         // Given an existing install of the app where the user previously declined PostHog
-        applicationSettings.enableAnalytics = false
+        applicationSettings.analyticsConsentState = .optedOut
         
         // When the user is prompted for analytics
         let showPrompt = ServiceLocator.shared.analytics.shouldShowAnalyticsPrompt
@@ -58,7 +58,7 @@ class AnalyticsTests: XCTestCase {
     
     func testAnalyticsPromptUserAcceptedPostHog() {
         // Given an existing install of the app where the user previously accepted PostHog
-        applicationSettings.enableAnalytics = true
+        applicationSettings.analyticsConsentState = .optedIn
         
         // When the user is prompted for analytics
         let showPrompt = ServiceLocator.shared.analytics.shouldShowAnalyticsPrompt
@@ -69,7 +69,8 @@ class AnalyticsTests: XCTestCase {
     
     func testAnalyticsPromptNotDisplayed() {
         // Given a fresh install of the app both Analytics and BugReportService should be disabled
-        XCTAssertFalse(ServiceLocator.shared.settings.enableAnalytics)
+        XCTAssertEqual(ServiceLocator.shared.settings.analyticsConsentState, .unknown)
+        XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled)
         XCTAssertFalse(ServiceLocator.shared.analytics.isRunning)
         XCTAssertFalse(analyticsClient.startCalled)
         XCTAssertFalse(bugReportService.startCalled)
@@ -80,7 +81,8 @@ class AnalyticsTests: XCTestCase {
         // When analytics is opt-out
         ServiceLocator.shared.analytics.optOut()
         // Then analytics should be disabled
-        XCTAssertFalse(applicationSettings.enableAnalytics)
+        XCTAssertEqual(applicationSettings.analyticsConsentState, .optedOut)
+        XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled)
         XCTAssertFalse(ServiceLocator.shared.analytics.isRunning)
         XCTAssertFalse(analyticsClient.isRunning)
         XCTAssertFalse(bugReportService.isRunning)
@@ -94,7 +96,8 @@ class AnalyticsTests: XCTestCase {
         // When analytics is opt-in
         ServiceLocator.shared.analytics.optIn()
         // The analytics should be enabled
-        XCTAssertTrue(applicationSettings.enableAnalytics)
+        XCTAssertEqual(applicationSettings.analyticsConsentState, .optedIn)
+        XCTAssertTrue(ServiceLocator.shared.analytics.isEnabled)
         // Analytics client and the bug report service should have been started
         XCTAssertTrue(analyticsClient.startCalled)
         XCTAssertTrue(bugReportService.startCalled)
@@ -102,20 +105,20 @@ class AnalyticsTests: XCTestCase {
 
     func testAnalyticsStartIfNotEnabled() {
         // Given an existing install of the app where the user previously declined the tracking
-        applicationSettings.enableAnalytics = false
+        applicationSettings.analyticsConsentState = .optedOut
         // Analytics should not start
+        XCTAssertFalse(ServiceLocator.shared.analytics.isEnabled)
         ServiceLocator.shared.analytics.startIfEnabled()
-        XCTAssertFalse(ServiceLocator.shared.settings.enableAnalytics)
         XCTAssertFalse(analyticsClient.startCalled)
         XCTAssertFalse(bugReportService.startCalled)
     }
     
     func testAnalyticsStartIfEnabled() {
         // Given an existing install of the app where the user previously accpeted the tracking
-        applicationSettings.enableAnalytics = true
+        applicationSettings.analyticsConsentState = .optedIn
         // Analytics should start
+        XCTAssertTrue(ServiceLocator.shared.analytics.isEnabled)
         ServiceLocator.shared.analytics.startIfEnabled()
-        XCTAssertTrue(ServiceLocator.shared.settings.enableAnalytics)
         XCTAssertTrue(analyticsClient.startCalled)
         XCTAssertTrue(bugReportService.startCalled)
     }
@@ -182,4 +185,17 @@ class AnalyticsTests: XCTestCase {
         // Then the properties should be cleared
         XCTAssertNil(client.pendingUserProperties, "The user properties should be cleared.")
     }
+    
+    func testResetConsentState() {
+        // Given an existing install of the app where the user previously accpeted the tracking
+        applicationSettings.analyticsConsentState = .optedIn
+        XCTAssertFalse(ServiceLocator.shared.analytics.shouldShowAnalyticsPrompt)
+
+        // When forgetting analytics consents
+        ServiceLocator.shared.analytics.resetConsentState()
+        
+        // Then the analytics prompt should be presented again
+        XCTAssertEqual(applicationSettings.analyticsConsentState, .unknown)
+        XCTAssertTrue(ServiceLocator.shared.analytics.shouldShowAnalyticsPrompt)
+    }
 }
diff --git a/changelog.d/pr-816.change b/changelog.d/pr-816.change
new file mode 100644
index 000000000..4da56a433
--- /dev/null
+++ b/changelog.d/pr-816.change
@@ -0,0 +1 @@
+Analytics: reset user's consents on logout.