Merge "Provide confirmation if creating new APN is not filled in enough and add a save button." into main

src/com/android/settings/network/apn/ApnEditPageProvider.kt

@@ -39,7 +39,6 @@ import com.android.settings.network.apn.ApnNetworkTypes.getNetworkTypeDisplayNam
 import com.android.settings.network.apn.ApnNetworkTypes.getNetworkTypeSelectedOptionsState
 import com.android.settingslib.spa.framework.common.SettingsPageProvider
 import com.android.settingslib.spa.framework.compose.LocalNavController
-import com.android.settingslib.spa.framework.compose.OnBackEffect
 import com.android.settingslib.spa.widget.editor.SettingsExposedDropdownMenuBox
 import com.android.settingslib.spa.widget.editor.SettingsExposedDropdownMenuCheckBox
 import com.android.settingslib.spa.widget.editor.SettingsOutlinedTextField
@@ -98,29 +97,35 @@ fun ApnPage(apnDataInit: ApnData, apnDataCur: MutableState<ApnData>, uriInit: Ur
     val networkTypeSelectedOptionsState = remember {
         getNetworkTypeSelectedOptionsState(apnData.networkType)
     }
-    OnBackEffect{
-        validateAndSaveApnData(
-            apnDataInit,
-            apnData,
-            context,
-            uriInit,
-            networkTypeSelectedOptionsState
-        )
-    }
     val navController = LocalNavController.current
     RegularScaffold(
-        title = if(apnDataInit.newApn) stringResource(id = R.string.apn_add) else stringResource(id = R.string.apn_edit),
+        title = if (apnDataInit.newApn) stringResource(id = R.string.apn_add) else stringResource(id = R.string.apn_edit),
+        actions = {
+            IconButton(onClick = {
+                if (!apnData.validEnabled) apnData = apnData.copy(validEnabled = true)
+                val valid = validateAndSaveApnData(
+                    apnDataInit,
+                    apnData,
+                    context,
+                    uriInit,
+                    networkTypeSelectedOptionsState
+                )
+                if (valid) navController.navigateBack()
+            }) { Icon(imageVector = Icons.Outlined.Done, contentDescription = null) }
+        },
     ) {
         Column {
             SettingsOutlinedTextField(
                 value = apnData.name,
                 label = stringResource(R.string.apn_name),
-                enabled = apnData.nameEnabled
+                enabled = apnData.nameEnabled,
+                errorMessage = validateName(apnData.validEnabled, apnData.name, context)
             ) { apnData = apnData.copy(name = it) }
             SettingsOutlinedTextField(
                 value = apnData.apn,
                 label = stringResource(R.string.apn_apn),
-                enabled = apnData.apnEnabled
+                enabled = apnData.apnEnabled,
+                errorMessage = validateAPN(apnData.validEnabled, apnData.apn, context)
             ) { apnData = apnData.copy(apn = it) }
             SettingsOutlinedTextField(
                 value = apnData.proxy,
@@ -150,7 +155,7 @@ fun ApnPage(apnDataInit: ApnData, apnDataCur: MutableState<ApnData>, uriInit: Ur
             SettingsOutlinedTextField(
                 value = apnData.mmsc,
                 label = stringResource(R.string.apn_mmsc),
-                errorMessage = validateMMSC(apnData.mmsc, context),
+                errorMessage = validateMMSC(apnData.validEnabled, apnData.mmsc, context),
                 enabled = apnData.mmscEnabled
             ) { apnData = apnData.copy(mmsc = it) }
             SettingsOutlinedTextField(
@@ -172,7 +177,11 @@ fun ApnPage(apnDataInit: ApnData, apnDataCur: MutableState<ApnData>, uriInit: Ur
             SettingsOutlinedTextField(
                 value = apnData.apnType,
                 label = stringResource(R.string.apn_type),
-                enabled = apnData.apnTypeEnabled
+                enabled = apnData.apnTypeEnabled,
+                errorMessage = validateAPNType(
+                    apnData.validEnabled, apnData.apnType,
+                    apnData.customizedConfig.readOnlyApnTypes, context
+                )
             ) { apnData = apnData.copy(apnType = updateApnType(apnData.copy(apnType = it))) }
             SettingsExposedDropdownMenuBox(
                 label = stringResource(R.string.apn_protocol),
@@ -209,7 +218,6 @@ fun ApnPage(apnDataInit: ApnData, apnDataCur: MutableState<ApnData>, uriInit: Ur
                         override val title = stringResource(R.string.menu_delete)
                         override val onClick = {
                             deleteApn(uriInit, context)
-                            apnData = apnData.copy(saveEnabled = false)
                             navController.navigateBack()
                         }
                     }

src/com/android/settings/network/apn/ApnStatus.kt

@@ -69,7 +69,7 @@ data class ApnData(
     val networkTypeEnabled: Boolean = true,
     val newApn: Boolean = false,
     val subId: Int = -1,
-    val saveEnabled: Boolean = true,
+    val validEnabled: Boolean = false,
     val customizedConfig: CustomizedConfig = CustomizedConfig()
 ) {
     fun getContentValues(context: Context): ContentValues {
@@ -96,7 +96,8 @@ data class ApnData(
         values.put(Telephony.Carriers.EDITED_STATUS, Telephony.Carriers.USER_EDITED)
         if (newApn) {
             val simCarrierId =
-                context.getSystemService(TelephonyManager::class.java)!!.createForSubscriptionId(subId)
+                context.getSystemService(TelephonyManager::class.java)!!
+                    .createForSubscriptionId(subId)
                     .getSimCarrierId()
             values.put(Telephony.Carriers.CARRIER_ID, simCarrierId)
         }
@@ -231,22 +232,12 @@ fun validateAndSaveApnData(
     uriInit: Uri,
     networkTypeSelectedOptionsState: SnapshotStateList<Int>
 ): Boolean {
-    // Can not be saved
-    if (!apnData.saveEnabled) {
-        return false
-    }
     // Nothing to do if it's a read only APN
     if (apnData.customizedConfig.readOnlyApn) {
         return true
     }
-    var errorMsg = validateApnData(apnData, context)
+    val errorMsg = validateApnData(apnData, context)
     if (errorMsg != null) {
-        //TODO: showError(this)
-        return false
-    }
-    errorMsg = validateMMSC(apnData.mmsc, context)
-    if (errorMsg != null) {
-        //TODO: showError(this)
         return false
     }
     val newApnData = apnData.copy(networkType = getNetworkType(networkTypeSelectedOptionsState))
@@ -268,37 +259,23 @@ fun validateAndSaveApnData(
  * @return An error message if the apn data is invalid, otherwise return null.
  */
 fun validateApnData(apnData: ApnData, context: Context): String? {
-    var errorMsg: String? = null
+    var errorMsg: String?
     val name = apnData.name
     val apn = apnData.apn
-    if (name == "") {
+    errorMsg = if (name == "") {
-        errorMsg = context.resources.getString(R.string.error_name_empty)
+        context.resources.getString(R.string.error_name_empty)
     } else if (apn == "") {
-        errorMsg = context.resources.getString(R.string.error_apn_empty)
+        context.resources.getString(R.string.error_apn_empty)
+    } else {
+        validateMMSC(apnData.validEnabled, apnData.mmsc, context)
     }
     if (errorMsg == null) {
-        // if carrier does not allow editing certain apn types, make sure type does not include
+        errorMsg = validateAPNType(
-        // those
+            apnData.validEnabled,
-        if (!ArrayUtils.isEmpty(apnData.customizedConfig.readOnlyApnTypes)
+            apnData.apnType,
-            && apnTypesMatch(
+            apnData.customizedConfig.readOnlyApnTypes,
-                apnData.customizedConfig.readOnlyApnTypes,
+            context
-                getUserEnteredApnType(apnData.apnType, apnData.customizedConfig.readOnlyApnTypes)
+        )
-            )
-        ) {
-            val stringBuilder = StringBuilder()
-            for (type in apnData.customizedConfig.readOnlyApnTypes) {
-                stringBuilder.append(type).append(", ")
-                Log.d(TAG, "validateApnData: appending type: $type")
-            }
-            // remove last ", "
-            if (stringBuilder.length >= 2) {
-                stringBuilder.delete(stringBuilder.length - 2, stringBuilder.length)
-            }
-            errorMsg = String.format(
-                context.resources.getString(R.string.error_adding_apn_type),
-                stringBuilder
-            )
-        }
     }
     return errorMsg
 }
@@ -536,7 +513,39 @@ fun deleteApn(uri: Uri, context: Context) {
     contentResolver.delete(uri, null, null)
 }
 
-fun validateMMSC(mmsc: String, context: Context): String? {
+fun validateMMSC(validEnabled: Boolean, mmsc: String, context: Context): String? {
-    return if (mmsc.matches(Regex("^https?:\\/\\/.+"))) null
+    return if (validEnabled && !mmsc.matches(Regex("^https?:\\/\\/.+")))
-            else context.resources.getString(R.string.error_mmsc_valid)
+        context.resources.getString(R.string.error_mmsc_valid)
+    else null
+}
+
+fun validateName(validEnabled: Boolean, name: String, context: Context): String? {
+    return if (validEnabled && (name == "")) context.resources.getString(R.string.error_name_empty)
+    else null
+}
+
+fun validateAPN(validEnabled: Boolean, apn: String, context: Context): String? {
+    return if (validEnabled && (apn == "")) context.resources.getString(R.string.error_apn_empty)
+    else null
+}
+
+fun validateAPNType(
+    validEnabled: Boolean,
+    apnType: String,
+    readOnlyApnTypes: List<String>,
+    context: Context
+): String? {
+    // if carrier does not allow editing certain apn types, make sure type does not include those
+    if (validEnabled && !ArrayUtils.isEmpty(readOnlyApnTypes)
+        && apnTypesMatch(
+            readOnlyApnTypes,
+            getUserEnteredApnType(apnType, readOnlyApnTypes)
+        )
+    ) {
+        return String.format(
+            context.resources.getString(R.string.error_adding_apn_type),
+            readOnlyApnTypes.joinToString(", ")
+        )
+    }
+    return null
 }