From caef61b5eb7c11077c2de042667b9bc89f7a3213 Mon Sep 17 00:00:00 2001 From: Tetiana Meronyk Date: Wed, 1 May 2024 13:15:34 +0000 Subject: [PATCH] Refactor multiuser toggle to control user switch feature The name "Allow multiple users" is too ambiguous. It sounds like by toggling it off, the feature is completely disabled. In fact, it only hides user switcher. In conjunction with hiding other users from the list, it makes it appear as all the users get deleted when the toggle is off. On the contrary, users might be running in background when the toggle is off. After this change, the new name better represents the intention behind this toggle, as well as makes the UI more intuitive. The users are not being hidden anymore. But switching preference gets disabled. Since the toggle can only be enabled or disabled by owner (after this refactoring), it means that Owner has full control over multiuser settings and is able to perform actions on users without having to enable the toggle. Bug: 336762423 Test: atest UserSettingsTest && atest UserDetailsSettingsTest Flag: android.multiuser.new_multiuser_settings_ux Change-Id: Id9d507039b58d3df66fe78710409716fd4816890 --- res/values/strings.xml | 2 +- ...AddUserWhenLockedPreferenceController.java | 12 ++++- .../GuestTelephonyPreferenceController.java | 4 +- ...RemoveGuestOnExitPreferenceController.java | 12 ++++- .../settings/users/UserDetailsSettings.java | 13 ++++- .../android/settings/users/UserSettings.java | 39 +++++++++++--- .../users/UserDetailsSettingsTest.java | 20 ++++++++ .../settings/users/UserSettingsTest.java | 51 ++++++++++++++++--- 8 files changed, 132 insertions(+), 21 deletions(-) diff --git a/res/values/strings.xml b/res/values/strings.xml index 075056db64e..3180f67480b 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -12643,7 +12643,7 @@ Use print service - Allow multiple users + Allow user switch allow, multiple, user, permit, many diff --git a/src/com/android/settings/users/AddUserWhenLockedPreferenceController.java b/src/com/android/settings/users/AddUserWhenLockedPreferenceController.java index ce5533e5c64..4830fd700f7 100644 --- a/src/com/android/settings/users/AddUserWhenLockedPreferenceController.java +++ b/src/com/android/settings/users/AddUserWhenLockedPreferenceController.java @@ -44,7 +44,11 @@ public class AddUserWhenLockedPreferenceController extends TogglePreferenceContr } else { restrictedSwitchPreference.setDisabledByAdmin( mUserCaps.disallowAddUser() ? mUserCaps.getEnforcedAdmin() : null); - restrictedSwitchPreference.setVisible(mUserCaps.mUserSwitcherEnabled); + if (android.multiuser.Flags.newMultiuserSettingsUx()) { + restrictedSwitchPreference.setVisible(true); + } else { + restrictedSwitchPreference.setVisible(mUserCaps.mUserSwitcherEnabled); + } } } @@ -55,7 +59,11 @@ public class AddUserWhenLockedPreferenceController extends TogglePreferenceContr } else if (mUserCaps.disallowAddUser() || mUserCaps.disallowAddUserSetByAdmin()) { return DISABLED_FOR_USER; } else { - return mUserCaps.mUserSwitcherEnabled ? AVAILABLE : CONDITIONALLY_UNAVAILABLE; + if (android.multiuser.Flags.newMultiuserSettingsUx()) { + return AVAILABLE; + } else { + return mUserCaps.mUserSwitcherEnabled ? AVAILABLE : CONDITIONALLY_UNAVAILABLE; + } } } diff --git a/src/com/android/settings/users/GuestTelephonyPreferenceController.java b/src/com/android/settings/users/GuestTelephonyPreferenceController.java index 4fbd4493bb7..64563df548d 100644 --- a/src/com/android/settings/users/GuestTelephonyPreferenceController.java +++ b/src/com/android/settings/users/GuestTelephonyPreferenceController.java @@ -45,6 +45,8 @@ public class GuestTelephonyPreferenceController extends TogglePreferenceControll public int getAvailabilityStatus() { if (!mUserCaps.isAdmin() || !mUserCaps.mCanAddGuest) { return DISABLED_FOR_USER; + } else if (android.multiuser.Flags.newMultiuserSettingsUx()) { + return AVAILABLE; } else { return mUserCaps.mUserSwitcherEnabled ? AVAILABLE : CONDITIONALLY_UNAVAILABLE; } @@ -74,7 +76,7 @@ public class GuestTelephonyPreferenceController extends TogglePreferenceControll public void updateState(Preference preference) { super.updateState(preference); mUserCaps.updateAddUserCapabilities(mContext); - preference.setVisible(isAvailable() && mUserCaps.mUserSwitcherEnabled + preference.setVisible(isAvailable() && mContext.getPackageManager().hasSystemFeature(PackageManager.FEATURE_TELEPHONY) && !UserManager.isHeadlessSystemUserMode()); } diff --git a/src/com/android/settings/users/RemoveGuestOnExitPreferenceController.java b/src/com/android/settings/users/RemoveGuestOnExitPreferenceController.java index 01df5fddc2e..94fad9782b3 100644 --- a/src/com/android/settings/users/RemoveGuestOnExitPreferenceController.java +++ b/src/com/android/settings/users/RemoveGuestOnExitPreferenceController.java @@ -72,7 +72,11 @@ public class RemoveGuestOnExitPreferenceController extends BasePreferenceControl } else { restrictedSwitchPreference.setDisabledByAdmin( mUserCaps.disallowAddUser() ? mUserCaps.getEnforcedAdmin() : null); - restrictedSwitchPreference.setVisible(mUserCaps.mUserSwitcherEnabled); + if (android.multiuser.Flags.newMultiuserSettingsUx()) { + restrictedSwitchPreference.setVisible(true); + } else { + restrictedSwitchPreference.setVisible(mUserCaps.mUserSwitcherEnabled); + } } } @@ -89,7 +93,11 @@ public class RemoveGuestOnExitPreferenceController extends BasePreferenceControl || mUserCaps.disallowAddUserSetByAdmin()) { return DISABLED_FOR_USER; } else { - return mUserCaps.mUserSwitcherEnabled ? AVAILABLE : CONDITIONALLY_UNAVAILABLE; + if (android.multiuser.Flags.newMultiuserSettingsUx()) { + return AVAILABLE; + } else { + return mUserCaps.mUserSwitcherEnabled ? AVAILABLE : CONDITIONALLY_UNAVAILABLE; + } } } diff --git a/src/com/android/settings/users/UserDetailsSettings.java b/src/com/android/settings/users/UserDetailsSettings.java index 71dd43f92a0..588f01aaa79 100644 --- a/src/com/android/settings/users/UserDetailsSettings.java +++ b/src/com/android/settings/users/UserDetailsSettings.java @@ -126,7 +126,11 @@ public class UserDetailsSettings extends SettingsPreferenceFragment @Override public void onResume() { super.onResume(); - mSwitchUserPref.setEnabled(canSwitchUserNow()); + if (android.multiuser.Flags.newMultiuserSettingsUx()) { + mSwitchUserPref.setEnabled(canSwitchUserNow() && mUserCaps.mUserSwitcherEnabled); + } else { + mSwitchUserPref.setEnabled(canSwitchUserNow()); + } if (mUserInfo.isGuest() && mGuestUserAutoCreated) { mRemoveUserPref.setEnabled((mUserInfo.flags & UserInfo.FLAG_INITIALIZED) != 0); } @@ -358,7 +362,12 @@ public class UserDetailsSettings extends SettingsPreferenceFragment mSwitchUserPref.setDisabledByAdmin(RestrictedLockUtilsInternal.getDeviceOwner(context)); } else { mSwitchUserPref.setDisabledByAdmin(null); - mSwitchUserPref.setSelectable(true); + if (android.multiuser.Flags.newMultiuserSettingsUx()) { + mSwitchUserPref.setEnabled(mUserCaps.mUserSwitcherEnabled); + mSwitchUserPref.setSelectable(mUserCaps.mUserSwitcherEnabled); + } else { + mSwitchUserPref.setSelectable(true); + } mSwitchUserPref.setOnPreferenceClickListener(this); } if (mUserInfo.isMain() || mUserInfo.isGuest() || !UserManager.isMultipleAdminEnabled() diff --git a/src/com/android/settings/users/UserSettings.java b/src/com/android/settings/users/UserSettings.java index bf21c9b913a..6fe79da8f08 100644 --- a/src/com/android/settings/users/UserSettings.java +++ b/src/com/android/settings/users/UserSettings.java @@ -1198,15 +1198,23 @@ public class UserSettings extends SettingsPreferenceFragment } List users; - if (mUserCaps.mUserSwitcherEnabled) { + if (android.multiuser.Flags.newMultiuserSettingsUx()) { // Only users that can be switched to should show up here. // e.g. Managed profiles appear under Accounts Settings instead users = mUserManager.getAliveUsers().stream() .filter(UserInfo::supportsSwitchToByUser) .collect(Collectors.toList()); } else { - // Only current user will be displayed in case of multi-user switch is disabled - users = List.of(mUserManager.getUserInfo(context.getUserId())); + if (mUserCaps.mUserSwitcherEnabled) { + // Only users that can be switched to should show up here. + // e.g. Managed profiles appear under Accounts Settings instead + users = mUserManager.getAliveUsers().stream() + .filter(UserInfo::supportsSwitchToByUser) + .collect(Collectors.toList()); + } else { + // Only current user will be displayed in case of multi-user switch is disabled + users = List.of(mUserManager.getUserInfo(context.getUserId())); + } } final ArrayList missingIcons = new ArrayList<>(); @@ -1423,10 +1431,16 @@ public class UserSettings extends SettingsPreferenceFragment } else { pref.setDisabledByAdmin(null); } - if (mUserCaps.mUserSwitcherEnabled) { + if (android.multiuser.Flags.newMultiuserSettingsUx()) { mGuestUserCategory.addPreference(pref); // guest user preference is shown hence also make guest category visible mGuestUserCategory.setVisible(true); + } else { + if (mUserCaps.mUserSwitcherEnabled) { + mGuestUserCategory.addPreference(pref); + // guest user preference is shown hence also make guest category visible + mGuestUserCategory.setVisible(true); + } } isGuestAlreadyCreated = true; } @@ -1453,7 +1467,8 @@ public class UserSettings extends SettingsPreferenceFragment if (!isGuestAlreadyCreated && mUserCaps.mCanAddGuest && mUserManager.canAddMoreUsers(UserManager.USER_TYPE_FULL_GUEST) && WizardManagerHelper.isDeviceProvisioned(context) - && mUserCaps.mUserSwitcherEnabled) { + && (mUserCaps.mUserSwitcherEnabled + || android.multiuser.Flags.newMultiuserSettingsUx())) { Drawable icon = context.getDrawable( com.android.settingslib.R.drawable.ic_account_circle); mAddGuest.setIcon(centerAndTint(icon)); @@ -1466,7 +1481,12 @@ public class UserSettings extends SettingsPreferenceFragment mAddGuest.setEnabled(false); } else { mAddGuest.setTitle(com.android.settingslib.R.string.guest_new_guest); - mAddGuest.setEnabled(canSwitchUserNow()); + if (android.multiuser.Flags.newMultiuserSettingsUx() + && mUserCaps.mDisallowAddUserSetByAdmin) { + mAddGuest.setDisabledByAdmin(mUserCaps.mEnforcedAdmin); + } else { + mAddGuest.setEnabled(canSwitchUserNow()); + } } } else { mAddGuest.setVisible(false); @@ -1496,7 +1516,8 @@ public class UserSettings extends SettingsPreferenceFragment boolean canAddRestrictedProfile) { if ((mUserCaps.mCanAddUser && !mUserCaps.mDisallowAddUserSetByAdmin) && WizardManagerHelper.isDeviceProvisioned(context) - && mUserCaps.mUserSwitcherEnabled) { + && (mUserCaps.mUserSwitcherEnabled + || android.multiuser.Flags.newMultiuserSettingsUx())) { addUser.setVisible(true); addUser.setSelectable(true); final boolean canAddMoreUsers = @@ -1514,6 +1535,10 @@ public class UserSettings extends SettingsPreferenceFragment addUser.setDisabledByAdmin( mUserCaps.mDisallowAddUser ? mUserCaps.mEnforcedAdmin : null); } + } else if (android.multiuser.Flags.newMultiuserSettingsUx() + && mUserCaps.mDisallowAddUserSetByAdmin) { + addUser.setVisible(true); + addUser.setDisabledByAdmin(mUserCaps.mEnforcedAdmin); } else { addUser.setVisible(false); } diff --git a/tests/robotests/src/com/android/settings/users/UserDetailsSettingsTest.java b/tests/robotests/src/com/android/settings/users/UserDetailsSettingsTest.java index 44e1cc6986c..e035274a5de 100644 --- a/tests/robotests/src/com/android/settings/users/UserDetailsSettingsTest.java +++ b/tests/robotests/src/com/android/settings/users/UserDetailsSettingsTest.java @@ -42,9 +42,13 @@ import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.content.pm.UserInfo; +import android.multiuser.Flags; import android.os.Bundle; import android.os.UserHandle; import android.os.UserManager; +import android.platform.test.annotations.RequiresFlagsEnabled; +import android.platform.test.flag.junit.CheckFlagsRule; +import android.platform.test.flag.junit.DeviceFlagsValueProvider; import android.telephony.TelephonyManager; import androidx.fragment.app.FragmentActivity; @@ -63,6 +67,7 @@ import com.android.settingslib.core.instrumentation.MetricsFeatureProvider; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -123,6 +128,8 @@ public class UserDetailsSettingsTest { private Bundle mArguments; private UserInfo mUserInfo; + @Rule + public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule(); @Before public void setUp() { MockitoAnnotations.initMocks(this); @@ -244,6 +251,19 @@ public class UserDetailsSettingsTest { verify(mSwitchUserPref).setEnabled(false); } + @Test + @RequiresFlagsEnabled({Flags.FLAG_NEW_MULTIUSER_SETTINGS_UX}) + public void onResume_UserSwitcherDisabled_shouldDisableSwitchPref() { + setupSelectedUser(); + mUserCapabilities.mUserSwitcherEnabled = false; + mFragment.mSwitchUserPref = mSwitchUserPref; + mFragment.onAttach(mContext); + + mFragment.onResume(); + + verify(mSwitchUserPref).setEnabled(false); + } + @Test public void onResume_switchDisallowed_shouldDisableSwitchPref() { setupSelectedUser(); diff --git a/tests/robotests/src/com/android/settings/users/UserSettingsTest.java b/tests/robotests/src/com/android/settings/users/UserSettingsTest.java index 5826ca25cc9..f35b64ecf04 100644 --- a/tests/robotests/src/com/android/settings/users/UserSettingsTest.java +++ b/tests/robotests/src/com/android/settings/users/UserSettingsTest.java @@ -46,10 +46,15 @@ import android.content.pm.ResolveInfo; import android.content.pm.UserInfo; import android.graphics.Bitmap; import android.graphics.drawable.Drawable; +import android.multiuser.Flags; import android.os.Bundle; import android.os.Looper; import android.os.UserHandle; import android.os.UserManager; +import android.platform.test.annotations.RequiresFlagsDisabled; +import android.platform.test.annotations.RequiresFlagsEnabled; +import android.platform.test.flag.junit.CheckFlagsRule; +import android.platform.test.flag.junit.DeviceFlagsValueProvider; import android.provider.Settings; import android.text.SpannableStringBuilder; import android.view.Menu; @@ -75,6 +80,7 @@ import com.android.settingslib.search.SearchIndexableRaw; import org.junit.After; import org.junit.Before; import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.AdditionalMatchers; @@ -142,6 +148,9 @@ public class UserSettingsTest { private UserSettings mFragment; private UserCapabilities mUserCapabilities; + @Rule + public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule(); + @Before public void setUp() { MockitoAnnotations.initMocks(this); @@ -406,6 +415,7 @@ public class UserSettingsTest { } @Test + @RequiresFlagsDisabled({Flags.FLAG_NEW_MULTIUSER_SETTINGS_UX}) public void updateUserList_addUserDisallowedByAdmin_shouldNotShowAddUser() { RestrictedLockUtils.EnforcedAdmin enforcedAdmin = mock( RestrictedLockUtils.EnforcedAdmin.class); @@ -420,6 +430,22 @@ public class UserSettingsTest { verify(mAddUserPreference).setVisible(false); } + @Test + @RequiresFlagsEnabled({Flags.FLAG_NEW_MULTIUSER_SETTINGS_UX}) + public void updateUserList_addUserDisallowedByAdmin_shouldShowPrefDisabledByAdmin() { + RestrictedLockUtils.EnforcedAdmin enforcedAdmin = mock( + RestrictedLockUtils.EnforcedAdmin.class); + + mUserCapabilities.mEnforcedAdmin = enforcedAdmin; + mUserCapabilities.mCanAddUser = false; + mUserCapabilities.mDisallowAddUser = true; + mUserCapabilities.mDisallowAddUserSetByAdmin = true; + doReturn(true).when(mAddUserPreference).isEnabled(); + + mFragment.updateUserList(); + + verify(mAddUserPreference).setDisabledByAdmin(enforcedAdmin); + } @Test public void updateUserList_cannotAddUserButCanSwitchUser_shouldNotShowAddUser() { mUserCapabilities.mCanAddUser = false; @@ -461,18 +487,31 @@ public class UserSettingsTest { } @Test - public void updateUserList_userSwitcherDisabled_shouldNotShowAddUser() { + public void updateUserList_userSwitcherDisabled_shouldShowAddUser() { givenUsers(getAdminUser(true)); mUserCapabilities.mCanAddUser = true; mUserCapabilities.mUserSwitcherEnabled = false; mFragment.updateUserList(); - verify(mAddUserPreference).setVisible(false); + verify(mAddUserPreference).setVisible(true); } @Test - public void updateUserList_userSwitcherDisabled_shouldNotShowAddGuest() { + public void updateUserList_userSwitcherDisabled_shouldShowAddGuest() { + givenUsers(getAdminUser(true)); + mUserCapabilities.mCanAddGuest = true; + mUserCapabilities.mUserSwitcherEnabled = false; + doReturn(true) + .when(mUserManager).canAddMoreUsers(eq(UserManager.USER_TYPE_FULL_GUEST)); + + mFragment.updateUserList(); + + verify(mAddGuestPreference).setVisible(true); + } + + @Test + public void updateUserList_userSwitcherDisabledCannotAddMoreGuests_shouldNotShowAddGuest() { givenUsers(getAdminUser(true)); mUserCapabilities.mCanAddGuest = true; mUserCapabilities.mUserSwitcherEnabled = false; @@ -533,18 +572,18 @@ public class UserSettingsTest { } @Test - public void updateUserList_existingSecondaryUser_shouldAddOnlyCurrUser_MultiUserIsDisabled() { + public void updateUserList_existingSecondaryUser_shouldAddAllUsers_MultiUserIsDisabled() { givenUsers(getAdminUser(true), getSecondaryUser(false)); mUserCapabilities.mUserSwitcherEnabled = false; mFragment.updateUserList(); ArgumentCaptor captor = ArgumentCaptor.forClass(UserPreference.class); - verify(mFragment.mUserListCategory, times(1)) + verify(mFragment.mUserListCategory, times(2)) .addPreference(captor.capture()); List userPrefs = captor.getAllValues(); - assertThat(userPrefs.size()).isEqualTo(1); + assertThat(userPrefs.size()).isEqualTo(2); assertThat(userPrefs.get(0)).isSameInstanceAs(mMePreference); }