From 56223dafd065db4f024842b645b45c6b2e9a2370 Mon Sep 17 00:00:00 2001 From: Alejandro Nijamkin Date: Mon, 10 Oct 2022 10:49:54 -0700 Subject: [PATCH] Fix bug where switching to guest didn't work. The cause was that we were using UserRecord.resolveId() which would return UserHandle.USER_NULL when the record was for a guest. We now use the raw ID in the UserRecord in this case. Also, the logic is being moved from UserSwitcherControllerImpl which is a temporary class meant to help us migrate from the old implementation to the new and is thus not unit tested to UserInteractor, where we can comfortably add unit tests in this CL. Fix: 251740146, 251376342 Test: Issue no longer reproduces after this CL Change-Id: Ib0cbb23d0f6111af7a2eb98278796c34a3e69209 --- .../policy/UserSwitcherControllerImpl.kt | 7 +-- .../user/domain/interactor/UserInteractor.kt | 11 ++++ .../UserInteractorRefactoredTest.kt | 56 +++++++++++++++++++ 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherControllerImpl.kt index 16926566105cf..086b6dd7c7288 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherControllerImpl.kt @@ -30,7 +30,6 @@ import com.android.systemui.qs.user.UserSwitchDialogController import com.android.systemui.user.data.source.UserRecord import com.android.systemui.user.domain.interactor.GuestUserInteractor import com.android.systemui.user.domain.interactor.UserInteractor -import com.android.systemui.user.legacyhelper.data.LegacyUserDataHelper import com.android.systemui.user.legacyhelper.ui.LegacyUserUiHelper import dagger.Lazy import java.io.PrintWriter @@ -203,11 +202,7 @@ constructor( dialogShower: UserSwitchDialogController.DialogShower?, ) { if (useInteractor) { - if (LegacyUserDataHelper.isUser(record)) { - userInteractor.selectUser(record.resolveId()) - } else { - userInteractor.executeAction(LegacyUserDataHelper.toUserActionModel(record)) - } + userInteractor.onRecordSelected(record) } else { _oldImpl.onUserListItemClicked(record, dialogShower) } diff --git a/packages/SystemUI/src/com/android/systemui/user/domain/interactor/UserInteractor.kt b/packages/SystemUI/src/com/android/systemui/user/domain/interactor/UserInteractor.kt index a84238c1559a2..7dc893b7b6c0f 100644 --- a/packages/SystemUI/src/com/android/systemui/user/domain/interactor/UserInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/user/domain/interactor/UserInteractor.kt @@ -390,6 +390,17 @@ constructor( guestUserInteractor.onDeviceBootCompleted() } + /** Switches to the user or executes the action represented by the given record. */ + fun onRecordSelected(record: UserRecord) { + if (LegacyUserDataHelper.isUser(record)) { + // It's safe to use checkNotNull around record.info because isUser only returns true + // if record.info is not null. + selectUser(checkNotNull(record.info).id) + } else { + executeAction(LegacyUserDataHelper.toUserActionModel(record)) + } + } + /** Switches to the user with the given user ID. */ fun selectUser( newlySelectedUserId: Int, diff --git a/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/UserInteractorRefactoredTest.kt b/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/UserInteractorRefactoredTest.kt index 3d5695a09ebca..05d21e1b16a86 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/UserInteractorRefactoredTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/UserInteractorRefactoredTest.kt @@ -47,6 +47,7 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.JUnit4 +import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.ArgumentMatchers.anyInt import org.mockito.Mockito.verify @@ -72,6 +73,61 @@ class UserInteractorRefactoredTest : UserInteractorTest() { whenever(manager.canAddMoreUsers(any())).thenReturn(true) } + @Test + fun `onRecordSelected - user`() = + runBlocking(IMMEDIATE) { + val userInfos = createUserInfos(count = 3, includeGuest = false) + userRepository.setUserInfos(userInfos) + userRepository.setSelectedUserInfo(userInfos[0]) + userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true)) + + underTest.onRecordSelected(UserRecord(info = userInfos[1])) + + verify(activityManager).switchUser(userInfos[1].id) + Unit + } + + @Test + fun `onRecordSelected - switch to guest user`() = + runBlocking(IMMEDIATE) { + val userInfos = createUserInfos(count = 3, includeGuest = true) + userRepository.setUserInfos(userInfos) + userRepository.setSelectedUserInfo(userInfos[0]) + userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true)) + + underTest.onRecordSelected(UserRecord(info = userInfos.last())) + + verify(activityManager).switchUser(userInfos.last().id) + Unit + } + + @Test + fun `onRecordSelected - enter guest mode`() = + runBlocking(IMMEDIATE) { + val userInfos = createUserInfos(count = 3, includeGuest = false) + userRepository.setUserInfos(userInfos) + userRepository.setSelectedUserInfo(userInfos[0]) + userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true)) + + underTest.onRecordSelected(UserRecord(isGuest = true)) + + verify(manager).createGuest(any()) + Unit + } + + @Test + fun `onRecordSelected - action`() = + runBlocking(IMMEDIATE) { + val userInfos = createUserInfos(count = 3, includeGuest = true) + userRepository.setUserInfos(userInfos) + userRepository.setSelectedUserInfo(userInfos[0]) + userRepository.setSettings(UserSwitcherSettingsModel(isUserSwitcherEnabled = true)) + + underTest.onRecordSelected(UserRecord(isAddSupervisedUser = true)) + + verify(activityStarter).startActivity(any(), anyBoolean()) + } + @Test fun `users - switcher enabled`() = runBlocking(IMMEDIATE) {