PR Suggestions

ElementX/Sources/Application/Settings/AppSettings.swift

@@ -48,7 +48,7 @@ final class AppSettings {
         case analyticsConsentState
         case hasRunNotificationPermissionsOnboarding
         case hasRunIdentityConfirmationOnboarding
-        case hasRequestedAlwaysLocationAuthorization
+        case hasRequestedLocationAlwaysLocationAuthorization
         
         case frequentlyUsedSystemEmojis
         
@@ -344,8 +344,8 @@ final class AppSettings {
     @UserPreference(key: UserDefaultsKeys.hasRunIdentityConfirmationOnboarding, defaultValue: false, storageType: .userDefaults(store))
     var hasRunIdentityConfirmationOnboarding
     
-    @UserPreference(key: UserDefaultsKeys.hasRequestedAlwaysLocationAuthorization, defaultValue: false, storageType: .userDefaults(store))
+    @UserPreference(key: UserDefaultsKeys.hasRequestedLocationAlwaysLocationAuthorization, defaultValue: false, storageType: .userDefaults(store))
-    var hasRequestedAlwaysLocationAuthorization
+    var hasRequestedLocationAlwaysLocationAuthorization
     
     @UserPreference(key: UserDefaultsKeys.frequentlyUsedSystemEmojis, defaultValue: [FrequentlyUsedEmoji](), storageType: .userDefaults(store))
     var frequentlyUsedSystemEmojis

ElementX/Sources/Screens/LocationSharing/LocationSharingScreenViewModel.swift

@@ -8,7 +8,7 @@
 
 import Combine
 import Foundation
-import UIKit
+import SwiftUI
 
 typealias LocationSharingScreenViewModelType = StateStoreViewModelV2<LocationSharingScreenViewState, LocationSharingScreenViewAction>
 
@@ -114,8 +114,8 @@ class LocationSharingScreenViewModel: LocationSharingScreenViewModelType, Locati
     
     private func startLiveLocationSharing() {
         authorizationStatusSubscription = nil
-        let authStatus = liveLocationManager.authorizationStatus.value
+        let authorizationStatus = liveLocationManager.authorizationStatus.value
-        switch authStatus {
+        switch authorizationStatus {
         case .authorizedAlways:
             // TODO: Start sending live location updates to the room
             break
@@ -123,7 +123,7 @@ class LocationSharingScreenViewModel: LocationSharingScreenViewModelType, Locati
             // This is to solve a race condition with map libre which always tries first
             // to request the when in use permission, we wait for it and then try again
             authorizationStatusSubscription = liveLocationManager.authorizationStatus
-                .filter { $0 != authStatus } // skip current status
+                .filter { $0 != authorizationStatus } // skip current status
                 .first() // this publisher only fires when there is an actual change, and if the user is done with permissions
                 .sink { [weak self] newValue in
                     guard newValue == .authorizedWhenInUse else { return }
@@ -137,7 +137,7 @@ class LocationSharingScreenViewModel: LocationSharingScreenViewModelType, Locati
             }
             
             authorizationStatusSubscription = liveLocationManager.authorizationStatus
-                .filter { $0 != authStatus } // skip current status
+                .filter { $0 != authorizationStatus } // skip current status
                 .first() // this publisher only fires when there is an actual change, and if the user is done with permissions
                 .sink { newValue in
                     guard newValue == .authorizedAlways else { return }
@@ -149,10 +149,9 @@ class LocationSharingScreenViewModel: LocationSharingScreenViewModelType, Locati
     }
     
     private func showMissingAlwaysAuthorizedAlert() {
-        let action: () -> Void = { [weak self] in self?.actionsSubject.send(.openSystemSettings) }
         state.bindings.alertInfo = .init(locationSharingViewError: .missingAlwaysAuthorization,
                                          primaryButton: .init(title: L10n.actionNotNow, role: .cancel, action: nil),
-                                         secondaryButton: .init(title: L10n.commonSettings, action: action))
+                                         secondaryButton: .init(title: L10n.commonSettings) { [weak self] in self?.actionsSubject.send(.openSystemSettings) })
     }
     
     private func sendLocation(_ geoURI: GeoURI, isUserLocation: Bool) async {

ElementX/Sources/Services/Location/LiveLocationManager.swift

@@ -39,8 +39,8 @@ class LiveLocationManager: NSObject, LiveLocationManagerProtocol, CLLocationMana
     
     @discardableResult
     func requestAlwaysAuthorizationIfPossible() -> Bool {
-        guard !appSettings.hasRequestedAlwaysLocationAuthorization else { return false }
+        guard !appSettings.hasRequestedLocationAlwaysLocationAuthorization else { return false }
-        appSettings.hasRequestedAlwaysLocationAuthorization = true
+        appSettings.hasRequestedLocationAlwaysLocationAuthorization = true
         locationManager.requestAlwaysAuthorization()
         return true
     }
@@ -51,14 +51,8 @@ class LiveLocationManager: NSObject, LiveLocationManagerProtocol, CLLocationMana
         // If the system resets authorization to notDetermined (e.g. after app reinstall or
         // settings reset), clear the flag so we can request again.
         if manager.authorizationStatus == .notDetermined {
-            appSettings.hasRequestedAlwaysLocationAuthorization = false
+            appSettings.hasRequestedLocationAlwaysLocationAuthorization = false
         }
         authorizationStatusSubject.send(manager.authorizationStatus)
     }
-    
-    // MARK: - Private
-    
-    // TODO: Add CLLocationManager location update handling to forward updates to rooms
-    // TODO: Track which rooms are currently sharing live location
-    // TODO: Send location updates to all active rooms via clientProxy
 }

ElementX/Sources/Services/Location/LiveLocationManagerProtocol.swift

@@ -11,7 +11,7 @@ import Foundation
 
 // sourcery: AutoMockable
 protocol LiveLocationManagerProtocol: AnyObject {
-    /// Publishes the current "Always" location authorization status.
+    /// Publishes the current location authorization status.
     var authorizationStatus: CurrentValuePublisher<CLAuthorizationStatus, Never> { get }
 
     /// Requests "Always" location authorization from the user if the system allows it.

UnitTests/Sources/LocationSharingScreenViewModelTests.swift

@@ -12,11 +12,11 @@ import CoreLocation
 import Testing
 
 @MainActor
-struct LocationSharingScreenViewModelTests {
+final class LocationSharingScreenViewModelTests {
-    var timelineProxy: TimelineProxyMock!
+    private var timelineProxy: TimelineProxyMock!
-    var viewModel: LocationSharingScreenViewModel!
+    private var viewModel: LocationSharingScreenViewModel!
     
-    var context: LocationSharingScreenViewModel.Context {
+    private var context: LocationSharingScreenViewModel.Context {
         viewModel.context
     }
     
@@ -24,8 +24,12 @@ struct LocationSharingScreenViewModelTests {
         AppSettings.resetAllSettings()
     }
     
+    deinit {
+        AppSettings.resetAllSettings()
+    }
+    
     @Test
-    mutating func userDidPan() {
+    func userDidPan() {
         setupViewModel()
         #expect(context.viewState.isSharingUserLocation)
         #expect(context.showsUserLocationMode == .showAndFollow)
@@ -35,7 +39,7 @@ struct LocationSharingScreenViewModelTests {
     }
     
     @Test
-    mutating func centerOnUser() {
+    func centerOnUser() {
         setupViewModel()
         #expect(context.viewState.isSharingUserLocation)
         context.showsUserLocationMode = .show
@@ -46,7 +50,7 @@ struct LocationSharingScreenViewModelTests {
     }
     
     @Test
-    mutating func centerOnUserWithoutAuth() {
+    func centerOnUserWithoutAuthorization() {
         setupViewModel()
         context.showsUserLocationMode = .hide
         context.isLocationAuthorized = nil
@@ -55,7 +59,7 @@ struct LocationSharingScreenViewModelTests {
     }
     
     @Test
-    mutating func centerOnUserWithDeniedAuth() {
+    func centerOnUserWithDeniedAuthorization() {
         setupViewModel()
         context.isLocationAuthorized = false
         context.showsUserLocationMode = .hide
@@ -65,18 +69,18 @@ struct LocationSharingScreenViewModelTests {
     }
     
     @Test
-    mutating func errorMapping() {
+    func errorMapping() {
         setupViewModel()
         let mapError = AlertInfo(locationSharingViewError: .mapError(.failedLoadingMap))
         #expect(mapError.title == L10n.errorFailedLoadingMap(InfoPlistReader.main.bundleDisplayName))
         let locationError = AlertInfo(locationSharingViewError: .mapError(.failedLocatingUser))
         #expect(locationError.title == L10n.errorFailedLocatingUser(InfoPlistReader.main.bundleDisplayName))
-        let authorizationError = AlertInfo(locationSharingViewError: .missingAuthorization)
+        let AuthorizationError = AlertInfo(locationSharingViewError: .missingAuthorization)
-        #expect(authorizationError.message == L10n.dialogPermissionLocationDescriptionIos(InfoPlistReader.main.bundleDisplayName))
+        #expect(AuthorizationError.message == L10n.dialogPermissionLocationDescriptionIos(InfoPlistReader.main.bundleDisplayName))
     }
 
     @Test
-    mutating func sendUserLocation() async throws {
+    func sendUserLocation() async throws {
         setupViewModel()
         context.mapCenterLocation = .init(latitude: 0, longitude: 0)
         context.geolocationUncertainty = 10
@@ -98,7 +102,7 @@ struct LocationSharingScreenViewModelTests {
     }
 
     @Test
-    mutating func sendPickedLocation() async throws {
+    func sendPickedLocation() async throws {
         setupViewModel()
         context.mapCenterLocation = .init(latitude: 0, longitude: 0)
         context.isLocationAuthorized = nil
@@ -123,28 +127,28 @@ struct LocationSharingScreenViewModelTests {
     // MARK: - Live Location Authorization Tests
     
     @Test
-    mutating func startLiveLocationWithDeniedAuth() {
+    func startLiveLocationWithDeniedAuthorization() {
         setupViewModel(liveLocationManagerConfiguration: .init(authorizationStatus: .denied))
         context.send(viewAction: .startLiveLocation)
         #expect(context.alertInfo?.id == .missingAlwaysAuthorization)
     }
     
     @Test
-    mutating func startLiveLocationWithRestrictedAuth() {
+    func startLiveLocationWithRestrictedAuthorization() {
         setupViewModel(liveLocationManagerConfiguration: .init(authorizationStatus: .restricted))
         context.send(viewAction: .startLiveLocation)
         #expect(context.alertInfo?.id == .missingAlwaysAuthorization)
     }
     
     @Test
-    mutating func startLiveLocationWithAlwaysAuth() {
+    func startLiveLocationWithAlwaysAuthorization() {
         setupViewModel(liveLocationManagerConfiguration: .init(authorizationStatus: .authorizedAlways))
         context.send(viewAction: .startLiveLocation)
         #expect(context.alertInfo == nil)
     }
     
     @Test
-    mutating func startLiveLocationWithWhenInUseAuthAlreadyRequested() {
+    func startLiveLocationWithWhenInUseAuthorizationAlreadyRequested() {
         setupViewModel(liveLocationManagerConfiguration: .init(authorizationStatus: .authorizedWhenInUse,
                                                                requestAlwaysAuthorizationIfPossibleReturnValue: false))
         context.send(viewAction: .startLiveLocation)
@@ -152,7 +156,7 @@ struct LocationSharingScreenViewModelTests {
     }
     
     @Test
-    mutating func startLiveLocationWithWhenInUseAuthNotYetRequested() {
+    func startLiveLocationWithWhenInUseAuthorizationNotYetRequested() {
         setupViewModel(liveLocationManagerConfiguration: .init(authorizationStatus: .authorizedWhenInUse,
                                                                requestAlwaysAuthorizationIfPossibleReturnValue: true))
         context.send(viewAction: .startLiveLocation)
@@ -161,7 +165,7 @@ struct LocationSharingScreenViewModelTests {
     }
     
     @Test
-    mutating func startLiveLocationWithNotDeterminedAuthTransitionsToWhenInUse() async {
+    func startLiveLocationWithNotDeterminedAuthorizationTransitionsToWhenInUse() async {
         let authorizationStatusSubject = CurrentValueSubject<CLAuthorizationStatus, Never>(.notDetermined)
         let liveLocationManagerMock = LiveLocationManagerMock()
         liveLocationManagerMock.underlyingAuthorizationStatus = .init(authorizationStatusSubject)
@@ -173,7 +177,7 @@ struct LocationSharingScreenViewModelTests {
         // No alert yet — waiting for MapLibre to resolve the status to whenInUse
         #expect(context.alertInfo == nil)
         
-        // Simulate MapLibre resolving the authorization to whenInUse, and confirm that the ViewModel
+        // Simulate MapLibre resolving the Authorization to whenInUse, and confirm that the ViewModel
         // recurses and calls requestAlwaysAuthorizationIfPossible as a result
         await waitForConfirmation { confirmation in
             liveLocationManagerMock.requestAlwaysAuthorizationIfPossibleClosure = {
@@ -183,13 +187,13 @@ struct LocationSharingScreenViewModelTests {
             authorizationStatusSubject.send(.authorizedWhenInUse)
         }
         
-        // The request was made, so no alert — waiting for the always authorization prompt response
+        // The request was made, so no alert — waiting for the always Authorization prompt response
         #expect(context.alertInfo == nil)
     }
     
     // MARK: - Private
     
-    private mutating func setupViewModel(liveLocationManagerConfiguration: LiveLocationManagerMock.Configuration = .init()) {
+    private func setupViewModel(liveLocationManagerConfiguration: LiveLocationManagerMock.Configuration = .init()) {
         timelineProxy = TimelineProxyMock(.init())
         viewModel = LocationSharingScreenViewModel(interactionMode: .picker,
                                                    mapURLBuilder: ServiceLocator.shared.settings.mapTilerConfiguration,
@@ -203,7 +207,7 @@ struct LocationSharingScreenViewModelTests {
         viewModel.state.bindings.isLocationAuthorized = true
     }
     
-    private mutating func setupViewModel(liveLocationManagerMock: LiveLocationManagerMock) {
+    private func setupViewModel(liveLocationManagerMock: LiveLocationManagerMock) {
         timelineProxy = TimelineProxyMock(.init())
         viewModel = LocationSharingScreenViewModel(interactionMode: .picker,
                                                    mapURLBuilder: ServiceLocator.shared.settings.mapTilerConfiguration,