From 98fd9f3f93f6b3f8b4025aeb50f715f01a3a8bf5 Mon Sep 17 00:00:00 2001
From: Alejandro Nijamkin <nijamkin@google.com>
Date: Wed, 19 Oct 2022 14:21:24 -0700
Subject: [PATCH] Guest user is last.

There is a bug where the guest user might get placed between two
non-guest users in cases where a non-guest user was created _after_ the
guest user was created. This is because in ag/20155790, we added sorting
by creation time but missed this requirement.

This CL fixes that.

Fix: 254258116
Test: This was done using TDD (test-driven development). The test was
first updated to fail, then the production code was changed to make the
test pass. Finally, I manually verified that a guest user, which I
created before a normal user, is displayed at the end of the user list
in both the full-screen user switcher and the popup version. Also
verified in the bouncer drop-down.

Change-Id: I0f4eaaddc4b264eb7457e0f731412f2cf93e2e8b
---
 .../user/data/repository/UserRepository.kt    |  7 +++++-
 .../UserRepositoryImplRefactoredTest.kt       | 24 +++++++++++--------
 2 files changed, 20 insertions(+), 11 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..4f3c62a538381 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
@@ -220,7 +220,12 @@ constructor(
             val result = withContext(backgroundDispatcher) { manager.aliveUsers }
 
             if (result != null) {
-                _userInfos.value = result.sortedBy { it.creationTime }
+                _userInfos.value =
+                    result
+                        // Users should be sorted by ascending creation time.
+                        .sortedBy { it.creationTime }
+                        // The guest user is always last, regardless of creation time.
+                        .sortedBy { it.isGuest }
             }
         }
     }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/user/data/repository/UserRepositoryImplRefactoredTest.kt b/packages/SystemUI/tests/src/com/android/systemui/user/data/repository/UserRepositoryImplRefactoredTest.kt
index d951f366c5952..525d8371c9ff5 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/user/data/repository/UserRepositoryImplRefactoredTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/user/data/repository/UserRepositoryImplRefactoredTest.kt
@@ -110,7 +110,7 @@ class UserRepositoryImplRefactoredTest : UserRepositoryImplTest() {
         val thirdExpectedValue =
             setUpUsers(
                 count = 2,
-                hasGuest = true,
+                isLastGuestUser = true,
                 selectedIndex = 1,
             )
         underTest.refreshUsers()
@@ -121,21 +121,25 @@ class UserRepositoryImplRefactoredTest : UserRepositoryImplTest() {
     }
 
     @Test
-    fun `refreshUsers - sorts by creation time`() = runSelfCancelingTest {
+    fun `refreshUsers - sorts by creation time - guest user last`() = runSelfCancelingTest {
         underTest = create(this)
         val unsortedUsers =
             setUpUsers(
                 count = 3,
                 selectedIndex = 0,
+                isLastGuestUser = true,
+            )
+        unsortedUsers[0].creationTime = 999
+        unsortedUsers[1].creationTime = 900
+        unsortedUsers[2].creationTime = 950
+        val expectedUsers =
+            listOf(
+                unsortedUsers[1],
+                unsortedUsers[0],
+                unsortedUsers[2], // last because this is the guest
             )
-        unsortedUsers[0].creationTime = 900
-        unsortedUsers[1].creationTime = 700
-        unsortedUsers[2].creationTime = 999
-        val expectedUsers = listOf(unsortedUsers[1], unsortedUsers[0], unsortedUsers[2])
         var userInfos: List<UserInfo>? = null
-        var selectedUserInfo: UserInfo? = null
         underTest.userInfos.onEach { userInfos = it }.launchIn(this)
-        underTest.selectedUserInfo.onEach { selectedUserInfo = it }.launchIn(this)
 
         underTest.refreshUsers()
         assertThat(userInfos).isEqualTo(expectedUsers)
@@ -143,14 +147,14 @@ class UserRepositoryImplRefactoredTest : UserRepositoryImplTest() {
 
     private fun setUpUsers(
         count: Int,
-        hasGuest: Boolean = false,
+        isLastGuestUser: Boolean = false,
         selectedIndex: Int = 0,
     ): List<UserInfo> {
         val userInfos =
             (0 until count).map { index ->
                 createUserInfo(
                     index,
-                    isGuest = hasGuest && index == count - 1,
+                    isGuest = isLastGuestUser && index == count - 1,
                 )
             }
         whenever(manager.aliveUsers).thenReturn(userInfos)