diff --git a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/LocationConstraintsCheck.kt b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/LocationConstraintsCheck.kt index f90793b775..72add1ea14 100644 --- a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/LocationConstraintsCheck.kt +++ b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/LocationConstraintsCheck.kt @@ -14,6 +14,7 @@ import io.element.android.features.location.impl.common.ui.LocationConstraintsDi sealed interface LocationConstraintsCheck { data object Success : LocationConstraintsCheck data object PermissionRationale : LocationConstraintsCheck + data object PermissionShouldBeRequested : LocationConstraintsCheck data object PermissionDenied : LocationConstraintsCheck data object LocationServiceDisabled : LocationConstraintsCheck data object NotEnoughPowerLevel : LocationConstraintsCheck @@ -34,6 +35,7 @@ fun checkLocationConstraints( } } permissionsState.shouldShowRationale -> LocationConstraintsCheck.PermissionRationale + !permissionsState.permissionsAlreadyRequested -> LocationConstraintsCheck.PermissionShouldBeRequested else -> LocationConstraintsCheck.PermissionDenied } } @@ -41,6 +43,7 @@ fun checkLocationConstraints( fun LocationConstraintsCheck.toDialogState(): LocationConstraintsDialogState { return when (this) { LocationConstraintsCheck.Success -> LocationConstraintsDialogState.None + LocationConstraintsCheck.PermissionShouldBeRequested -> LocationConstraintsDialogState.None LocationConstraintsCheck.PermissionRationale -> LocationConstraintsDialogState.PermissionRationale LocationConstraintsCheck.PermissionDenied -> LocationConstraintsDialogState.PermissionDenied LocationConstraintsCheck.LocationServiceDisabled -> LocationConstraintsDialogState.LocationServiceDisabled diff --git a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/permissions/DefaultPermissionsPresenter.kt b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/permissions/DefaultPermissionsPresenter.kt index 1aa2e1269d..5aa7ae6f63 100644 --- a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/permissions/DefaultPermissionsPresenter.kt +++ b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/permissions/DefaultPermissionsPresenter.kt @@ -9,6 +9,10 @@ package io.element.android.features.location.impl.common.permissions import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import com.google.accompanist.permissions.ExperimentalPermissionsApi import com.google.accompanist.permissions.isGranted import com.google.accompanist.permissions.rememberMultiplePermissionsState @@ -32,11 +36,16 @@ class DefaultPermissionsPresenter( @OptIn(ExperimentalPermissionsApi::class) @Composable override fun present(): PermissionsState { - val multiplePermissionsState = rememberMultiplePermissionsState(permissions = permissions) + var permissionsRequested by remember { mutableStateOf(false) } + val multiplePermissionsState = rememberMultiplePermissionsState(permissions = permissions) { + permissionsRequested = true + } fun handleEvent(event: PermissionsEvents) { when (event) { - PermissionsEvents.RequestPermissions -> multiplePermissionsState.launchMultiplePermissionRequest() + PermissionsEvents.RequestPermissions -> { + multiplePermissionsState.launchMultiplePermissionRequest() + } } } @@ -47,6 +56,7 @@ class DefaultPermissionsPresenter( else -> PermissionsState.Permissions.NoneGranted }, shouldShowRationale = multiplePermissionsState.shouldShowRationale, + permissionsAlreadyRequested = permissionsRequested, eventSink = ::handleEvent, ) } diff --git a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/permissions/PermissionsState.kt b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/permissions/PermissionsState.kt index 91191280d0..25ceb3ba29 100644 --- a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/permissions/PermissionsState.kt +++ b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/common/permissions/PermissionsState.kt @@ -11,6 +11,7 @@ package io.element.android.features.location.impl.common.permissions data class PermissionsState( val permissions: Permissions, val shouldShowRationale: Boolean, + val permissionsAlreadyRequested: Boolean, val eventSink: (PermissionsEvents) -> Unit, ) { sealed interface Permissions { diff --git a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/share/ShareLocationPresenter.kt b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/share/ShareLocationPresenter.kt index 5b0d7679f7..a3f3cac432 100644 --- a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/share/ShareLocationPresenter.kt +++ b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/share/ShareLocationPresenter.kt @@ -88,6 +88,8 @@ class ShareLocationPresenter( var dialogState: ShareLocationState.Dialog by remember { mutableStateOf(ShareLocationState.Dialog.None) } + // true when trying to initiate the live location share + var pendingLiveLocationShare by remember { mutableStateOf(false) } val startLiveLocationAction = remember { mutableStateOf>(AsyncAction.Uninitialized) } val currentUser by client.userProfile.collectAsState() val customMapStyleUrl by produceState(AsyncData.Loading()) { @@ -100,34 +102,55 @@ class ShareLocationPresenter( val scope = rememberCoroutineScope() fun checkLocationConstraints() { - // No need to check SendLiveLocationPermissions here - val locationConstraints = checkLocationConstraints(permissionsState, locationActions, SendLiveLocationPermissions.GRANTED) + val locationConstraints = checkLocationConstraints( + permissionsState = permissionsState, + locationActions = locationActions, + // No need to check SendLiveLocationPermissions here + sendLiveLocationPermissions = SendLiveLocationPermissions.GRANTED + ) + if (locationConstraints is LocationConstraintsCheck.PermissionShouldBeRequested) { + permissionsState.eventSink(PermissionsEvents.RequestPermissions) + } trackUserPosition = locationConstraints is LocationConstraintsCheck.Success dialogState = ShareLocationState.Dialog.Constraints(locationConstraints.toDialogState()) } - suspend fun computeLiveLocationDialogState(): ShareLocationState.Dialog { - val hasAcceptedDisclaimer = liveLocationStore.hasAcceptedLiveLocationDisclaimer() - val constraintsResult = checkLocationConstraints(permissionsState, locationActions, sendLiveLocationPermissions) - return when { - !hasAcceptedDisclaimer -> { - ShareLocationState.Dialog.LiveLocationDisclaimer - } - constraintsResult is LocationConstraintsCheck.Success -> { - val durations = LIVE_LOCATION_DURATIONS.map { - LiveLocationDuration(duration = it, formatted = durationFormatter.format(it)) + suspend fun checkLiveLocationConstraints() { + val locationConstraints = checkLocationConstraints( + permissionsState = permissionsState, + locationActions = locationActions, + sendLiveLocationPermissions = sendLiveLocationPermissions, + ) + when (locationConstraints) { + LocationConstraintsCheck.Success -> { + val hasAcceptedDisclaimer = liveLocationStore.hasAcceptedLiveLocationDisclaimer() + dialogState = if (!hasAcceptedDisclaimer) { + ShareLocationState.Dialog.LiveLocationDisclaimer + } else { + val durations = LIVE_LOCATION_DURATIONS.map { + LiveLocationDuration(duration = it, formatted = durationFormatter.format(it)) + } + ShareLocationState.Dialog.LiveLocationDurations(durations.toImmutableList()) } - ShareLocationState.Dialog.LiveLocationDurations(durations.toImmutableList()) } else -> { - ShareLocationState.Dialog.Constraints(constraintsResult.toDialogState()) + if (locationConstraints is LocationConstraintsCheck.PermissionShouldBeRequested) { + permissionsState.eventSink(PermissionsEvents.RequestPermissions) + } + dialogState = ShareLocationState.Dialog.Constraints(locationConstraints.toDialogState()) } } } val userLocationState = userLocationStateFactory.create(permissionsState.isAnyGranted) - LaunchedEffect(permissionsState.permissions) { checkLocationConstraints() } + LaunchedEffect(permissionsState) { + if (pendingLiveLocationShare) { + checkLiveLocationConstraints() + } else { + checkLocationConstraints() + } + } fun handleEvent(event: ShareLocationEvent) { when (event) { @@ -136,7 +159,10 @@ class ShareLocationPresenter( } ShareLocationEvent.StartTrackingUserLocation -> checkLocationConstraints() ShareLocationEvent.StopTrackingUserLocation -> trackUserPosition = false - ShareLocationEvent.DismissDialog -> dialogState = ShareLocationState.Dialog.None + ShareLocationEvent.DismissDialog -> { + pendingLiveLocationShare = false + dialogState = ShareLocationState.Dialog.None + } ShareLocationEvent.OpenAppSettings -> { locationActions.openAppSettings() dialogState = ShareLocationState.Dialog.None @@ -146,15 +172,17 @@ class ShareLocationPresenter( dialogState = ShareLocationState.Dialog.None } ShareLocationEvent.InitiateLiveLocationShare -> scope.launch { - dialogState = computeLiveLocationDialogState() + pendingLiveLocationShare = true + checkLiveLocationConstraints() } ShareLocationEvent.AcceptLiveLocationDisclaimer -> scope.launch { liveLocationStore.setAcceptedLiveLocationDisclaimer() .onSuccess { - dialogState = computeLiveLocationDialogState() + checkLiveLocationConstraints() } } is ShareLocationEvent.StartLiveLocationShare -> scope.launch { + pendingLiveLocationShare = false dialogState = ShareLocationState.Dialog.None startLiveLocationAction.runUpdatingState { liveLocationShareManager.startShare( diff --git a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenter.kt b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenter.kt index e21fbb0605..f187e91c53 100644 --- a/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenter.kt +++ b/features/location/impl/src/main/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenter.kt @@ -90,10 +90,22 @@ import kotlinx.coroutines.launch value = AsyncData.Success(client.getMapStyleUrl().getOrNull()) } - LaunchedEffect(permissionsState.permissions) { - if (permissionsState.isAnyGranted) { - dialogState = LocationConstraintsDialogState.None + fun checkLocationConstraints() { + val locationConstraints = checkLocationConstraints( + permissionsState = permissionsState, + locationActions = locationActions, + // No need to check SendLiveLocationPermissions here + sendLiveLocationPermissions = SendLiveLocationPermissions.GRANTED + ) + if (locationConstraints is LocationConstraintsCheck.PermissionShouldBeRequested) { + permissionsState.eventSink(PermissionsEvents.RequestPermissions) } + isTrackMyLocation = locationConstraints is LocationConstraintsCheck.Success + dialogState = locationConstraints.toDialogState() + } + + LaunchedEffect(permissionsState) { + checkLocationConstraints() } fun handleEvent(event: ShowLocationEvent) { @@ -103,9 +115,7 @@ import kotlinx.coroutines.launch } is ShowLocationEvent.TrackMyLocation -> { if (event.enabled) { - val locationConstraints = checkLocationConstraints(permissionsState, locationActions, SendLiveLocationPermissions.GRANTED) - isTrackMyLocation = locationConstraints is LocationConstraintsCheck.Success - dialogState = locationConstraints.toDialogState() + checkLocationConstraints() } else { isTrackMyLocation = false } diff --git a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/PermissionsStateFactory.kt b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/PermissionsStateFactory.kt index a4352444dc..5c7bacb91a 100644 --- a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/PermissionsStateFactory.kt +++ b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/PermissionsStateFactory.kt @@ -13,10 +13,12 @@ import io.element.android.features.location.impl.common.permissions.PermissionsS fun aPermissionsState( permissions: PermissionsState.Permissions = PermissionsState.Permissions.NoneGranted, shouldShowRationale: Boolean = false, + permissionsRequested: Boolean = false, ): PermissionsState { return PermissionsState( permissions = permissions, shouldShowRationale = shouldShowRationale, + permissionsAlreadyRequested = permissionsRequested, eventSink = {}, ) } diff --git a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/LocationConstraintsCheckTest.kt b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/LocationConstraintsCheckTest.kt index debe95b464..40362bda3c 100644 --- a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/LocationConstraintsCheckTest.kt +++ b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/LocationConstraintsCheckTest.kt @@ -64,10 +64,25 @@ class LocationConstraintsCheckTest { } @Test - fun `checkLocationConstraints returns PermissionDenied when permissions denied without rationale`() { + fun `checkLocationConstraints returns PermissionShouldBeRequested when permissions not yet requested`() { val permissionsState = aPermissionsState( permissions = PermissionsState.Permissions.NoneGranted, shouldShowRationale = false, + permissionsRequested = false, + ) + val locationActions = FakeLocationActions(isLocationEnabled = true) + + val result = checkLocationConstraints(permissionsState, locationActions, SendLiveLocationPermissions.GRANTED) + + assertThat(result).isEqualTo(LocationConstraintsCheck.PermissionShouldBeRequested) + } + + @Test + fun `checkLocationConstraints returns PermissionDenied when permissions already requested and denied without rationale`() { + val permissionsState = aPermissionsState( + permissions = PermissionsState.Permissions.NoneGranted, + shouldShowRationale = false, + permissionsRequested = true, ) val locationActions = FakeLocationActions(isLocationEnabled = true) diff --git a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/permissions/FakePermissionsPresenter.kt b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/permissions/FakePermissionsPresenter.kt index 94d909a7ef..9b8ff7a29a 100644 --- a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/permissions/FakePermissionsPresenter.kt +++ b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/common/permissions/FakePermissionsPresenter.kt @@ -20,6 +20,7 @@ class FakePermissionsPresenter : PermissionsPresenter { private var state = PermissionsState( permissions = PermissionsState.Permissions.NoneGranted, shouldShowRationale = false, + permissionsAlreadyRequested = false, eventSink = ::handleEvent, ) set(value) { diff --git a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/share/ShareLocationPresenterTest.kt b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/share/ShareLocationPresenterTest.kt index f9195cd08d..f4cc135e9e 100644 --- a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/share/ShareLocationPresenterTest.kt +++ b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/share/ShareLocationPresenterTest.kt @@ -160,6 +160,24 @@ class ShareLocationPresenterTest { } } + @Test + fun `initial state with permissions not yet requested triggers permission request`() = runTest { + val shareLocationPresenter = createShareLocationPresenter() + fakePermissionsPresenter.givenState( + aPermissionsState( + permissions = PermissionsState.Permissions.NoneGranted, + shouldShowRationale = false, + permissionsRequested = false, + ) + ) + + shareLocationPresenter.test { + skipItems(2) + cancelAndIgnoreRemainingEvents() + assertThat(fakePermissionsPresenter.events).contains(PermissionsEvents.RequestPermissions) + } + } + @Test fun `initial state with permissions denied`() = runTest { val shareLocationPresenter = createShareLocationPresenter() @@ -167,6 +185,7 @@ class ShareLocationPresenterTest { aPermissionsState( permissions = PermissionsState.Permissions.NoneGranted, shouldShowRationale = false, + permissionsRequested = true, ) ) @@ -291,6 +310,7 @@ class ShareLocationPresenterTest { aPermissionsState( permissions = PermissionsState.Permissions.NoneGranted, shouldShowRationale = false, + permissionsRequested = true, ) ) @@ -359,7 +379,12 @@ class ShareLocationPresenterTest { @Test fun `ShowLiveLocationDurationPicker shows disclaimer when acceptance is missing`() = runTest { - val presenter = createShareLocationPresenter() + val room = FakeJoinedRoom( + baseRoom = FakeBaseRoom( + roomPermissions = grantedSendLiveLocationPermissions() + ) + ) + val presenter = createShareLocationPresenter(joinedRoom = room) fakePermissionsPresenter.givenState( aPermissionsState( permissions = PermissionsState.Permissions.AllGranted, @@ -462,7 +487,12 @@ class ShareLocationPresenterTest { @Test fun `ShowLiveLocationDurationPicker uses the active session disclaimer state`() = runTest { - val joinedRoom = FakeJoinedRoom(baseRoom = FakeBaseRoom(sessionId = SessionId("@alice:server"))) + val joinedRoom = FakeJoinedRoom( + baseRoom = FakeBaseRoom( + sessionId = SessionId("@alice:server"), + roomPermissions = grantedSendLiveLocationPermissions() + ), + ) createLiveLocationStore(sessionId = SessionId("@bob:server")) .setAcceptedLiveLocationDisclaimer() .getOrThrow() @@ -505,12 +535,13 @@ class ShareLocationPresenterTest { aPermissionsState( permissions = PermissionsState.Permissions.NoneGranted, shouldShowRationale = false, + permissionsRequested = true, ) ) shareLocationPresenter.test { val initialState = awaitFirstItem() - // Dismiss initial dialog + // Dismiss initial dialog to allow re-triggering it initialState.eventSink(ShareLocationEvent.DismissDialog) val dismissedState = awaitItem() diff --git a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenterTest.kt b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenterTest.kt index ab06cc5911..b579a79e4f 100644 --- a/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenterTest.kt +++ b/features/location/impl/src/test/kotlin/io/element/android/features/location/impl/show/ShowLocationPresenterTest.kt @@ -37,7 +37,6 @@ import io.element.android.services.toolbox.test.strings.FakeStringProvider import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.test import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest import org.junit.Rule @@ -103,8 +102,8 @@ class ShowLocationPresenterTest { val presenter = createShowLocationPresenter() presenter.test { - val initialState = awaitItem() - assertThat(initialState.isTrackMyLocation).isFalse() + assertThat(awaitItem().isTrackMyLocation).isFalse() + assertThat(awaitItem().isTrackMyLocation).isTrue() } } @@ -129,15 +128,12 @@ class ShowLocationPresenterTest { ) ) presenter.test { - skipItems(1) val initialState = awaitItem() assertThat(initialState.isTrackMyLocation).isFalse() initialState.eventSink(ShowLocationEvent.TrackMyLocation(true)) val trackMyLocationState = awaitItem() - delay(1) - assertThat(trackMyLocationState.isTrackMyLocation).isTrue() // Swipe the map to switch mode @@ -201,12 +197,34 @@ class ShowLocationPresenterTest { } } + @Test + fun `TrackMyLocation with permissions not yet requested triggers permission request`() = runTest { + fakePermissionsPresenter.givenState( + aPermissionsState( + permissions = PermissionsState.Permissions.NoneGranted, + shouldShowRationale = false, + permissionsRequested = false, + ) + ) + + val presenter = createShowLocationPresenter() + presenter.test { + val initialState = awaitItem() + + initialState.eventSink(ShowLocationEvent.TrackMyLocation(true)) + + assertThat(fakePermissionsPresenter.events).contains(PermissionsEvents.RequestPermissions) + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `permission denied dialog dismiss`() = runTest { fakePermissionsPresenter.givenState( aPermissionsState( permissions = PermissionsState.Permissions.NoneGranted, shouldShowRationale = false, + permissionsRequested = true, ) ) @@ -235,6 +253,7 @@ class ShowLocationPresenterTest { aPermissionsState( permissions = PermissionsState.Permissions.NoneGranted, shouldShowRationale = false, + permissionsRequested = true, ) ) diff --git a/features/roommembermoderation/impl/src/test/kotlin/io/element/android/features/roommembermoderation/impl/RoomMemberModerationViewTest.kt b/features/roommembermoderation/impl/src/test/kotlin/io/element/android/features/roommembermoderation/impl/RoomMemberModerationViewTest.kt index 646481715a..3a5926ebc7 100644 --- a/features/roommembermoderation/impl/src/test/kotlin/io/element/android/features/roommembermoderation/impl/RoomMemberModerationViewTest.kt +++ b/features/roommembermoderation/impl/src/test/kotlin/io/element/android/features/roommembermoderation/impl/RoomMemberModerationViewTest.kt @@ -21,6 +21,7 @@ import io.element.android.features.roommembermoderation.api.RoomMemberModeration import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.testtags.TestTags +import io.element.android.tests.testutils.EnsureNeverCalledWithParam import io.element.android.tests.testutils.EnsureNeverCalledWithTwoParams import io.element.android.tests.testutils.EventsRecorder import io.element.android.tests.testutils.clickOn @@ -48,6 +49,8 @@ class RoomMemberModerationViewTest { onSelectAction = callback ) clickOn(R.string.screen_bottom_sheet_manage_room_member_member_user_info) + // Gives time for bottomsheet to hide + mainClock.advanceTimeBy(1_000) } } @@ -217,11 +220,13 @@ class RoomMemberModerationViewTest { private fun AndroidComposeUiTest.setRoomMemberModerationView( state: InternalRoomMemberModerationState, onSelectAction: (ModerationAction, MatrixUser) -> Unit = EnsureNeverCalledWithTwoParams(), + onAvatarClick: ((MatrixUser) -> Unit)? = EnsureNeverCalledWithParam(), ) { setSafeContent { RoomMemberModerationView( state = state, onSelectAction = onSelectAction, + onAvatarClick = onAvatarClick, ) } }