From 10353cd8b63e74e8139d96a3588f8eedc4895e4a Mon Sep 17 00:00:00 2001 From: Antony Sargent Date: Thu, 14 Mar 2019 12:58:56 -0700 Subject: [PATCH] Fix for affordances to add a mobile subscription This changes the behavior for when we show an affordance to add a mobile subscription via the eSIM manager (if eSIM is supported). On the Network & internet page, the behavior we now want for the "Mobile network" pref has several possibilities as detailed below. No existing subscriptions: - Summary is "Add a network" - Tapping on pref leads to the add subscription flow One existing subscription: - Summary is the name of the subscription (usually the carrier name) - Tapping left-hand side of the pref leads to mobile network details - A "+" button is shown on right of pref, and tapping "+" leads to the add subscription flow 2 or more existing subscriptions: - Summary is " SIMs" - Tapping left-hand side of the pref leads to a new page with the list of subscriptions - A "+" button is shown on right of pref, and tapping "+" leads to the add subscription flow The existing code controlling the "Mobile network" pref (in MobileNetworkSummaryController.java) already had behavior similar to this, but needed to be updated to no longer rely on whether we're in DSDS mode. Also, the page with the list of subscriptions that you can go to in the "2 or more existing subscriptions" case mentioned above has an "Add more" link at the bottom which leads to the same eSIM manager flow, but it was always showing this link even if the device doesn't support eSIM. This CL also fixes that problem. Bug: 127870567 Test: make RunSettingsRoboTests Change-Id: Ie8cca47ac81f8992fa6ecf1940bdfed411fd333b --- res/xml/mobile_network_list.xml | 2 + .../network/MobileNetworkListController.java | 21 ++++-- .../MobileNetworkSummaryController.java | 54 ++++++-------- .../MobileNetworkListControllerTest.java | 26 +++++++ .../MobileNetworkSummaryControllerTest.java | 74 ++++++------------- 5 files changed, 88 insertions(+), 89 deletions(-) diff --git a/res/xml/mobile_network_list.xml b/res/xml/mobile_network_list.xml index b72540f72d4..c2baf460401 100644 --- a/res/xml/mobile_network_list.xml +++ b/res/xml/mobile_network_list.xml @@ -16,11 +16,13 @@ diff --git a/src/com/android/settings/network/MobileNetworkListController.java b/src/com/android/settings/network/MobileNetworkListController.java index 7de6cdde953..79715e323ee 100644 --- a/src/com/android/settings/network/MobileNetworkListController.java +++ b/src/com/android/settings/network/MobileNetworkListController.java @@ -24,14 +24,10 @@ import android.content.Intent; import android.provider.Settings; import android.telephony.SubscriptionInfo; import android.telephony.SubscriptionManager; +import android.telephony.euicc.EuiccManager; import android.util.ArrayMap; -import androidx.lifecycle.Lifecycle; -import androidx.lifecycle.LifecycleObserver; -import androidx.lifecycle.OnLifecycleEvent; -import androidx.preference.Preference; -import androidx.preference.PreferenceScreen; - +import com.android.internal.annotations.VisibleForTesting; import com.android.settings.R; import com.android.settings.network.telephony.MobileNetworkActivity; import com.android.settingslib.core.AbstractPreferenceController; @@ -39,6 +35,12 @@ import com.android.settingslib.core.AbstractPreferenceController; import java.util.List; import java.util.Map; +import androidx.lifecycle.Lifecycle; +import androidx.lifecycle.LifecycleObserver; +import androidx.lifecycle.OnLifecycleEvent; +import androidx.preference.Preference; +import androidx.preference.PreferenceScreen; + /** * This populates the entries on a page which lists all available mobile subscriptions. Each entry * has the name of the subscription with some subtext giving additional detail, and clicking on the @@ -48,6 +50,9 @@ public class MobileNetworkListController extends AbstractPreferenceController im LifecycleObserver, SubscriptionsChangeListener.SubscriptionsChangeListenerClient { private static final String TAG = "MobileNetworkListCtlr"; + @VisibleForTesting + static final String KEY_ADD_MORE = "add_more"; + private SubscriptionManager mSubscriptionManager; private SubscriptionsChangeListener mChangeListener; private PreferenceScreen mPreferenceScreen; @@ -76,6 +81,8 @@ public class MobileNetworkListController extends AbstractPreferenceController im public void displayPreference(PreferenceScreen screen) { super.displayPreference(screen); mPreferenceScreen = screen; + final EuiccManager euiccManager = mContext.getSystemService(EuiccManager.class); + mPreferenceScreen.findPreference(KEY_ADD_MORE).setVisible(euiccManager.isEnabled()); update(); } @@ -93,7 +100,7 @@ public class MobileNetworkListController extends AbstractPreferenceController im final List subscriptions = SubscriptionUtil.getAvailableSubscriptions( mSubscriptionManager); for (SubscriptionInfo info : subscriptions) { - int subId = info.getSubscriptionId(); + final int subId = info.getSubscriptionId(); Preference pref = existingPreferences.remove(subId); if (pref == null) { pref = new Preference(mPreferenceScreen.getContext()); diff --git a/src/com/android/settings/network/MobileNetworkSummaryController.java b/src/com/android/settings/network/MobileNetworkSummaryController.java index a1fef4c93a9..56735abbf40 100644 --- a/src/com/android/settings/network/MobileNetworkSummaryController.java +++ b/src/com/android/settings/network/MobileNetworkSummaryController.java @@ -16,8 +16,6 @@ package com.android.settings.network; -import static android.telephony.TelephonyManager.MultiSimVariants.UNKNOWN; - import static androidx.lifecycle.Lifecycle.Event.ON_PAUSE; import static androidx.lifecycle.Lifecycle.Event.ON_RESUME; @@ -25,7 +23,6 @@ import android.content.Context; import android.content.Intent; import android.telephony.SubscriptionInfo; import android.telephony.SubscriptionManager; -import android.telephony.TelephonyManager; import android.telephony.euicc.EuiccManager; import com.android.settings.R; @@ -52,7 +49,6 @@ public class MobileNetworkSummaryController extends AbstractPreferenceController private SubscriptionManager mSubscriptionManager; private SubscriptionsChangeListener mChangeListener; - private TelephonyManager mTelephonyMgr; private EuiccManager mEuiccManager; private AddPreference mPreference; @@ -74,7 +70,6 @@ public class MobileNetworkSummaryController extends AbstractPreferenceController public MobileNetworkSummaryController(Context context, Lifecycle lifecycle) { super(context); mSubscriptionManager = context.getSystemService(SubscriptionManager.class); - mTelephonyMgr = mContext.getSystemService(TelephonyManager.class); mEuiccManager = mContext.getSystemService(EuiccManager.class); if (lifecycle != null) { mChangeListener = new SubscriptionsChangeListener(context, this); @@ -124,48 +119,43 @@ public class MobileNetworkSummaryController extends AbstractPreferenceController mContext.startActivity(intent); } - private boolean shouldShowAddButton() { - // The add button should only show up if the device is in multi-sim mode and the eSIM - // manager is enabled. - return mTelephonyMgr.getMultiSimConfiguration() != UNKNOWN && mEuiccManager.isEnabled(); - } - private void update() { if (mPreference == null) { return; } - final boolean showAddButton = shouldShowAddButton(); refreshSummary(mPreference); - if (!showAddButton) { - mPreference.setOnAddClickListener(null); - } else { - mPreference.setAddWidgetEnabled(!mChangeListener.isAirplaneModeOn()); - mPreference.setOnAddClickListener(p -> { - startAddSimFlow(); - }); - } - final List subs = SubscriptionUtil.getAvailableSubscriptions( - mSubscriptionManager); mPreference.setOnPreferenceClickListener(null); + mPreference.setOnAddClickListener(null); mPreference.setFragment(null); mPreference.setEnabled(!mChangeListener.isAirplaneModeOn()); + + final List subs = SubscriptionUtil.getAvailableSubscriptions( + mSubscriptionManager); + if (subs.isEmpty()) { - if (showAddButton) { - mPreference.setEnabled(false); - } else if (mEuiccManager.isEnabled()) { + if (mEuiccManager.isEnabled()) { mPreference.setOnPreferenceClickListener((Preference pref) -> { startAddSimFlow(); return true; }); } - } else if (subs.size() == 1) { - mPreference.setOnPreferenceClickListener((Preference pref) -> { - final Intent intent = new Intent(mContext, MobileNetworkActivity.class); - mContext.startActivity(intent); - return true; - }); } else { - mPreference.setFragment(MobileNetworkListFragment.class.getCanonicalName()); + // We have one or more existing subscriptions, so we want the plus button if eSIM is + // supported. + if (mEuiccManager.isEnabled()) { + mPreference.setAddWidgetEnabled(!mChangeListener.isAirplaneModeOn()); + mPreference.setOnAddClickListener(p -> startAddSimFlow()); + } + + if (subs.size() == 1) { + mPreference.setOnPreferenceClickListener((Preference pref) -> { + final Intent intent = new Intent(mContext, MobileNetworkActivity.class); + mContext.startActivity(intent); + return true; + }); + } else { + mPreference.setFragment(MobileNetworkListFragment.class.getCanonicalName()); + } } } diff --git a/tests/robotests/src/com/android/settings/network/MobileNetworkListControllerTest.java b/tests/robotests/src/com/android/settings/network/MobileNetworkListControllerTest.java index 1325650b463..10264ab4448 100644 --- a/tests/robotests/src/com/android/settings/network/MobileNetworkListControllerTest.java +++ b/tests/robotests/src/com/android/settings/network/MobileNetworkListControllerTest.java @@ -21,6 +21,7 @@ import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -31,6 +32,7 @@ import android.app.Activity; import android.content.Context; import android.content.Intent; import android.telephony.SubscriptionInfo; +import android.telephony.euicc.EuiccManager; import org.junit.After; import org.junit.Before; @@ -50,6 +52,9 @@ import androidx.preference.PreferenceScreen; @RunWith(RobolectricTestRunner.class) public class MobileNetworkListControllerTest { + @Mock + EuiccManager mEuiccManager; + @Mock private Lifecycle mLifecycle; @@ -58,12 +63,17 @@ public class MobileNetworkListControllerTest { private Context mContext; private MobileNetworkListController mController; + private Preference mAddMorePreference; @Before public void setUp() { MockitoAnnotations.initMocks(this); mContext = spy(Robolectric.setupActivity(Activity.class)); + when(mContext.getSystemService(EuiccManager.class)).thenReturn(mEuiccManager); when(mPreferenceScreen.getContext()).thenReturn(mContext); + mAddMorePreference = new Preference(mContext); + when(mPreferenceScreen.findPreference(MobileNetworkListController.KEY_ADD_MORE)).thenReturn( + mAddMorePreference); mController = new MobileNetworkListController(mContext, mLifecycle); } @@ -78,6 +88,22 @@ public class MobileNetworkListControllerTest { mController.onResume(); } + @Test + public void displayPreference_eSimNotSupported_addMoreLinkNotVisible() { + when(mEuiccManager.isEnabled()).thenReturn(false); + mController.displayPreference(mPreferenceScreen); + mController.onResume(); + assertThat(mAddMorePreference.isVisible()).isFalse(); + } + + @Test + public void displayPreference_eSimSupported_addMoreLinkIsVisible() { + when(mEuiccManager.isEnabled()).thenReturn(true); + mController.displayPreference(mPreferenceScreen); + mController.onResume(); + assertThat(mAddMorePreference.isVisible()).isTrue(); + } + @Test public void displayPreference_twoSubscriptions_correctlySetup() { final SubscriptionInfo sub1 = createMockSubscription(1, "sub1"); diff --git a/tests/robotests/src/com/android/settings/network/MobileNetworkSummaryControllerTest.java b/tests/robotests/src/com/android/settings/network/MobileNetworkSummaryControllerTest.java index ba152b98995..3404ca2d1f5 100644 --- a/tests/robotests/src/com/android/settings/network/MobileNetworkSummaryControllerTest.java +++ b/tests/robotests/src/com/android/settings/network/MobileNetworkSummaryControllerTest.java @@ -16,13 +16,9 @@ package com.android.settings.network; -import static android.telephony.TelephonyManager.MultiSimVariants.DSDS; -import static android.telephony.TelephonyManager.MultiSimVariants.UNKNOWN; - import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.ArgumentMatchers.notNull; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; @@ -37,7 +33,6 @@ import android.content.Intent; import android.net.ConnectivityManager; import android.provider.Settings; import android.telephony.SubscriptionInfo; -import android.telephony.TelephonyManager; import android.telephony.euicc.EuiccManager; import android.text.TextUtils; @@ -64,8 +59,6 @@ public class MobileNetworkSummaryControllerTest { @Mock private Lifecycle mLifecycle; @Mock - private TelephonyManager mTelephonyManager; - @Mock private EuiccManager mEuiccManager; @Mock private PreferenceScreen mPreferenceScreen; @@ -78,9 +71,7 @@ public class MobileNetworkSummaryControllerTest { public void setUp() { MockitoAnnotations.initMocks(this); mContext = spy(Robolectric.setupActivity(Activity.class)); - when(mContext.getSystemService(eq(TelephonyManager.class))).thenReturn(mTelephonyManager); when(mContext.getSystemService(EuiccManager.class)).thenReturn(mEuiccManager); - when(mTelephonyManager.getMultiSimConfiguration()).thenReturn(UNKNOWN); when(mEuiccManager.isEnabled()).thenReturn(true); mController = new MobileNetworkSummaryController(mContext, mLifecycle); @@ -97,7 +88,7 @@ public class MobileNetworkSummaryControllerTest { @Test public void isAvailable_wifiOnlyMode_notAvailable() { - ConnectivityManager cm = mock(ConnectivityManager.class); + final ConnectivityManager cm = mock(ConnectivityManager.class); when(cm.isNetworkSupported(ConnectivityManager.TYPE_MOBILE)).thenReturn(false); when(mContext.getSystemService(ConnectivityManager.class)).thenReturn(cm); assertThat(mController.isAvailable()).isFalse(); @@ -212,24 +203,7 @@ public class MobileNetworkSummaryControllerTest { } @Test - public void addButton_noSubscriptionsSingleSimMode_noAddClickListener() { - mController.displayPreference(mPreferenceScreen); - mController.onResume(); - verify(mPreference, never()).setOnAddClickListener(notNull()); - } - - @Test - public void addButton_oneSubscriptionSingleSimMode_noAddClickListener() { - final SubscriptionInfo sub1 = mock(SubscriptionInfo.class); - SubscriptionUtil.setAvailableSubscriptionsForTesting(Arrays.asList(sub1)); - mController.displayPreference(mPreferenceScreen); - mController.onResume(); - verify(mPreference, never()).setOnAddClickListener(notNull()); - } - - @Test - public void addButton_noSubscriptionsMultiSimModeNoEuiccMgr_noAddClickListener() { - when(mTelephonyManager.getMultiSimConfiguration()).thenReturn(DSDS); + public void addButton_noSubscriptionsNoEuiccMgr_noAddClickListener() { when(mEuiccManager.isEnabled()).thenReturn(false); mController.displayPreference(mPreferenceScreen); mController.onResume(); @@ -237,41 +211,43 @@ public class MobileNetworkSummaryControllerTest { } @Test - public void addButton_noSubscriptionsMultiSimMode_hasAddClickListenerAndPrefDisabled() { - when(mTelephonyManager.getMultiSimConfiguration()).thenReturn(DSDS); - mController.displayPreference(mPreferenceScreen); - mController.onResume(); - assertThat(mPreference.isEnabled()).isFalse(); - verify(mPreference, never()).setOnAddClickListener(isNull()); - verify(mPreference).setOnAddClickListener(notNull()); - } - - @Test - public void addButton_oneSubscriptionMultiSimMode_hasAddClickListener() { - when(mTelephonyManager.getMultiSimConfiguration()).thenReturn(DSDS); + public void addButton_oneSubscriptionNoEuiccMgr_noAddClickListener() { + when(mEuiccManager.isEnabled()).thenReturn(false); + final SubscriptionInfo sub1 = mock(SubscriptionInfo.class); + SubscriptionUtil.setAvailableSubscriptionsForTesting(Arrays.asList(sub1)); + mController.displayPreference(mPreferenceScreen); + mController.onResume(); + verify(mPreference, never()).setOnAddClickListener(notNull()); + } + + @Test + public void addButton_noSubscriptions_noAddClickListener() { + mController.displayPreference(mPreferenceScreen); + mController.onResume(); + verify(mPreference, never()).setOnAddClickListener(notNull()); + } + + @Test + public void addButton_oneSubscription_hasAddClickListener() { final SubscriptionInfo sub1 = mock(SubscriptionInfo.class); SubscriptionUtil.setAvailableSubscriptionsForTesting(Arrays.asList(sub1)); mController.displayPreference(mPreferenceScreen); mController.onResume(); - verify(mPreference, never()).setOnAddClickListener(isNull()); verify(mPreference).setOnAddClickListener(notNull()); } @Test - public void addButton_twoSubscriptionsMultiSimMode_hasAddClickListener() { - when(mTelephonyManager.getMultiSimConfiguration()).thenReturn(DSDS); + public void addButton_twoSubscriptions_hasAddClickListener() { final SubscriptionInfo sub1 = mock(SubscriptionInfo.class); final SubscriptionInfo sub2 = mock(SubscriptionInfo.class); SubscriptionUtil.setAvailableSubscriptionsForTesting(Arrays.asList(sub1, sub2)); mController.displayPreference(mPreferenceScreen); mController.onResume(); - verify(mPreference, never()).setOnAddClickListener(isNull()); verify(mPreference).setOnAddClickListener(notNull()); } @Test public void addButton_oneSubscriptionAirplaneModeTurnedOn_addButtonGetsDisabled() { - when(mTelephonyManager.getMultiSimConfiguration()).thenReturn(DSDS); final SubscriptionInfo sub1 = mock(SubscriptionInfo.class); SubscriptionUtil.setAvailableSubscriptionsForTesting(Arrays.asList(sub1)); mController.displayPreference(mPreferenceScreen); @@ -280,14 +256,13 @@ public class MobileNetworkSummaryControllerTest { Settings.Global.putInt(mContext.getContentResolver(), Settings.Global.AIRPLANE_MODE_ON, 1); mController.onAirplaneModeChanged(true); - ArgumentCaptor captor = ArgumentCaptor.forClass(Boolean.class); + final ArgumentCaptor captor = ArgumentCaptor.forClass(Boolean.class); verify(mPreference, atLeastOnce()).setAddWidgetEnabled(captor.capture()); assertThat(captor.getValue()).isFalse(); } @Test public void onResume_oneSubscriptionAirplaneMode_isDisabled() { - when(mTelephonyManager.getMultiSimConfiguration()).thenReturn(DSDS); Settings.Global.putInt(mContext.getContentResolver(), Settings.Global.AIRPLANE_MODE_ON, 1); final SubscriptionInfo sub1 = mock(SubscriptionInfo.class); SubscriptionUtil.setAvailableSubscriptionsForTesting(Arrays.asList(sub1)); @@ -296,7 +271,7 @@ public class MobileNetworkSummaryControllerTest { assertThat(mPreference.isEnabled()).isFalse(); - ArgumentCaptor captor = ArgumentCaptor.forClass(Boolean.class); + final ArgumentCaptor captor = ArgumentCaptor.forClass(Boolean.class); verify(mPreference, atLeastOnce()).setAddWidgetEnabled(captor.capture()); assertThat(captor.getValue()).isFalse(); } @@ -318,7 +293,6 @@ public class MobileNetworkSummaryControllerTest { @Test public void onAirplaneModeChanged_oneSubscriptionAirplaneModeGetsTurnedOff_isEnabled() { - when(mTelephonyManager.getMultiSimConfiguration()).thenReturn(DSDS); Settings.Global.putInt(mContext.getContentResolver(), Settings.Global.AIRPLANE_MODE_ON, 1); final SubscriptionInfo sub1 = mock(SubscriptionInfo.class); SubscriptionUtil.setAvailableSubscriptionsForTesting(Arrays.asList(sub1)); @@ -332,7 +306,7 @@ public class MobileNetworkSummaryControllerTest { assertThat(mPreference.isEnabled()).isTrue(); - ArgumentCaptor captor = ArgumentCaptor.forClass(Boolean.class); + final ArgumentCaptor captor = ArgumentCaptor.forClass(Boolean.class); verify(mPreference, atLeastOnce()).setAddWidgetEnabled(eq(false)); verify(mPreference, atLeastOnce()).setAddWidgetEnabled(captor.capture()); assertThat(captor.getValue()).isTrue();