From 6ca9a400ab54ca9255a3dd5c68bd89cdac4bc1b2 Mon Sep 17 00:00:00 2001 From: Aaron Liu Date: Tue, 18 Oct 2022 10:46:52 -0700 Subject: [PATCH] [Bouncer] Remove duplicate manage users. Remove a duplicate manage users menu item in the user switcher activity. A manage users action was added further upstream for the data sources with the new impl and old impl, therefore manually adding the manage users action is not necessary for newimpl or oldimpl. This was removed before, but there seemed to be a faulty merge conflict resolution on my part. Bug: 241811308, 254257455 Test: Observed 2 manage users in the User switchers before my change. Observed 1 manage users action after my change. Change-Id: I23efa9fedb6987e6427954662c07229767802bb8 --- .../user/data/repository/UserRepository.kt | 2 ++ .../user/domain/interactor/UserInteractor.kt | 13 +-------- .../ui/viewmodel/UserSwitcherViewModel.kt | 12 +------- .../UserRepositoryImplUnrefactoredTest.kt | 6 +++- .../UserInteractorUnrefactoredTest.kt | 28 +++++-------------- .../ui/viewmodel/UserSwitcherViewModelTest.kt | 20 +++++++++++++ 6 files changed, 36 insertions(+), 45 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/user/data/repository/UserRepository.kt b/packages/SystemUI/src/com/android/systemui/user/data/repository/UserRepository.kt index 919e699652bcf..d768b6dc195a4 100644 --- a/packages/SystemUI/src/com/android/systemui/user/data/repository/UserRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/user/data/repository/UserRepository.kt @@ -321,6 +321,7 @@ constructor( return when { isAddUser -> false isAddSupervisedUser -> false + isManageUsers -> false isGuest -> info != null else -> true } @@ -346,6 +347,7 @@ constructor( isAddUser -> UserActionModel.ADD_USER isAddSupervisedUser -> UserActionModel.ADD_SUPERVISED_USER isGuest -> UserActionModel.ENTER_GUEST_MODE + isManageUsers -> UserActionModel.NAVIGATE_TO_USER_MANAGEMENT else -> error("Don't know how to convert to UserActionModel: $this") } } 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 ba5a82a42d943..0d5c64b83e6e7 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 @@ -236,18 +236,7 @@ constructor( } .flatMapLatest { isActionable -> if (isActionable) { - repository.actions.map { actions -> - actions + - if (actions.isNotEmpty()) { - // If we have actions, we add NAVIGATE_TO_USER_MANAGEMENT - // because that's a user switcher specific action that is - // not known to the our data source or other features. - listOf(UserActionModel.NAVIGATE_TO_USER_MANAGEMENT) - } else { - // If no actions, don't add the navigate action. - emptyList() - } - } + repository.actions } else { // If not actionable it means that we're not allowed to show actions // when diff --git a/packages/SystemUI/src/com/android/systemui/user/ui/viewmodel/UserSwitcherViewModel.kt b/packages/SystemUI/src/com/android/systemui/user/ui/viewmodel/UserSwitcherViewModel.kt index 219dae29117fe..d857e85bac53a 100644 --- a/packages/SystemUI/src/com/android/systemui/user/ui/viewmodel/UserSwitcherViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/user/ui/viewmodel/UserSwitcherViewModel.kt @@ -62,17 +62,7 @@ private constructor( val isMenuVisible: Flow = _isMenuVisible /** The user action menu. */ val menu: Flow> = - userInteractor.actions.map { actions -> - if (isNewImpl && actions.isNotEmpty()) { - // If we have actions, we add NAVIGATE_TO_USER_MANAGEMENT because that's a user - // switcher specific action that is not known to the our data source or other - // features. - actions + listOf(UserActionModel.NAVIGATE_TO_USER_MANAGEMENT) - } else { - actions - } - .map { action -> toViewModel(action) } - } + userInteractor.actions.map { actions -> actions.map { action -> toViewModel(action) } } /** Whether the button to open the user action menu is visible. */ val isOpenMenuButtonVisible: Flow = menu.map { it.isNotEmpty() } diff --git a/packages/SystemUI/tests/src/com/android/systemui/user/data/repository/UserRepositoryImplUnrefactoredTest.kt b/packages/SystemUI/tests/src/com/android/systemui/user/data/repository/UserRepositoryImplUnrefactoredTest.kt index d4b41c18e123e..a363a037c4992 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/user/data/repository/UserRepositoryImplUnrefactoredTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/user/data/repository/UserRepositoryImplUnrefactoredTest.kt @@ -97,6 +97,7 @@ class UserRepositoryImplUnrefactoredTest : UserRepositoryImplTest() { createUserRecord(2), createActionRecord(UserActionModel.ADD_SUPERVISED_USER), createActionRecord(UserActionModel.ENTER_GUEST_MODE), + createActionRecord(UserActionModel.NAVIGATE_TO_USER_MANAGEMENT), ) ) var models: List? = null @@ -176,15 +177,17 @@ class UserRepositoryImplUnrefactoredTest : UserRepositoryImplTest() { createUserRecord(2), createActionRecord(UserActionModel.ADD_SUPERVISED_USER), createActionRecord(UserActionModel.ENTER_GUEST_MODE), + createActionRecord(UserActionModel.NAVIGATE_TO_USER_MANAGEMENT), ) ) var models: List? = null val job = underTest.actions.onEach { models = it }.launchIn(this) - assertThat(models).hasSize(3) + assertThat(models).hasSize(4) assertThat(models?.get(0)).isEqualTo(UserActionModel.ADD_USER) assertThat(models?.get(1)).isEqualTo(UserActionModel.ADD_SUPERVISED_USER) assertThat(models?.get(2)).isEqualTo(UserActionModel.ENTER_GUEST_MODE) + assertThat(models?.get(3)).isEqualTo(UserActionModel.NAVIGATE_TO_USER_MANAGEMENT) job.cancel() } @@ -200,6 +203,7 @@ class UserRepositoryImplUnrefactoredTest : UserRepositoryImplTest() { isAddUser = action == UserActionModel.ADD_USER, isAddSupervisedUser = action == UserActionModel.ADD_SUPERVISED_USER, isGuest = action == UserActionModel.ENTER_GUEST_MODE, + isManageUsers = action == UserActionModel.NAVIGATE_TO_USER_MANAGEMENT, ) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/UserInteractorUnrefactoredTest.kt b/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/UserInteractorUnrefactoredTest.kt index c3a9705bf6baa..6a17c8ddc63d7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/UserInteractorUnrefactoredTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/user/domain/interactor/UserInteractorUnrefactoredTest.kt @@ -64,13 +64,7 @@ open class UserInteractorUnrefactoredTest : UserInteractorTest() { @Test fun `actions - not actionable when locked and not locked`() = runBlocking(IMMEDIATE) { - userRepository.setActions( - listOf( - UserActionModel.ENTER_GUEST_MODE, - UserActionModel.ADD_USER, - UserActionModel.ADD_SUPERVISED_USER, - ) - ) + setActions() userRepository.setActionableWhenLocked(false) keyguardRepository.setKeyguardShowing(false) @@ -92,13 +86,7 @@ open class UserInteractorUnrefactoredTest : UserInteractorTest() { @Test fun `actions - actionable when locked and not locked`() = runBlocking(IMMEDIATE) { - userRepository.setActions( - listOf( - UserActionModel.ENTER_GUEST_MODE, - UserActionModel.ADD_USER, - UserActionModel.ADD_SUPERVISED_USER, - ) - ) + setActions() userRepository.setActionableWhenLocked(true) keyguardRepository.setKeyguardShowing(false) @@ -120,13 +108,7 @@ open class UserInteractorUnrefactoredTest : UserInteractorTest() { @Test fun `actions - actionable when locked and locked`() = runBlocking(IMMEDIATE) { - userRepository.setActions( - listOf( - UserActionModel.ENTER_GUEST_MODE, - UserActionModel.ADD_USER, - UserActionModel.ADD_SUPERVISED_USER, - ) - ) + setActions() userRepository.setActionableWhenLocked(true) keyguardRepository.setKeyguardShowing(true) @@ -182,6 +164,10 @@ open class UserInteractorUnrefactoredTest : UserInteractorTest() { verify(activityStarter).startActivity(any(), anyBoolean()) } + private fun setActions() { + userRepository.setActions(UserActionModel.values().toList()) + } + companion object { private val IMMEDIATE = Dispatchers.Main.immediate } diff --git a/packages/SystemUI/tests/src/com/android/systemui/user/ui/viewmodel/UserSwitcherViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/user/ui/viewmodel/UserSwitcherViewModelTest.kt index 0344e3f991e2d..c12a868dbaed9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/user/ui/viewmodel/UserSwitcherViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/user/ui/viewmodel/UserSwitcherViewModelTest.kt @@ -267,6 +267,26 @@ class UserSwitcherViewModelTest : SysuiTestCase() { job.cancel() } + @Test + fun `menu actions`() = + runBlocking(IMMEDIATE) { + userRepository.setActions(UserActionModel.values().toList()) + var actions: List? = null + val job = underTest.menu.onEach { actions = it }.launchIn(this) + + assertThat(actions?.map { it.viewKey }) + .isEqualTo( + listOf( + UserActionModel.ENTER_GUEST_MODE.ordinal.toLong(), + UserActionModel.ADD_USER.ordinal.toLong(), + UserActionModel.ADD_SUPERVISED_USER.ordinal.toLong(), + UserActionModel.NAVIGATE_TO_USER_MANAGEMENT.ordinal.toLong(), + ) + ) + + job.cancel() + } + @Test fun `isFinishRequested - finishes when user is switched`() = runBlocking(IMMEDIATE) {