Edits to activity in send intent action are not saved

CHANGELOG.md

@@ -9,6 +9,8 @@
 
 ## Fixed
 
+- #2160 edits to the activity in a send intent action are no longer discarded when the screen is
+  recreated (for example on a configuration change) before saving.
 - #2099 do not spam notifications that Expert mode failed to start on WiFi disconnection or the ADB
   pairing is broken.
 - #2220 make invisible floating buttons more visible when editing.

base/src/main/java/io/github/sds100/keymapper/base/system/intents/ConfigIntentViewModel.kt

@@ -233,6 +233,8 @@ class ConfigIntentViewModel @Inject constructor(
     private val _returnResult = MutableSharedFlow<ConfigIntentResult>()
     val returnResult = _returnResult.asSharedFlow()
 
+    private var isResultLoaded = false
+
     fun setActivityTargetChecked(isChecked: Boolean) {
         if (isChecked) {
             target.value = IntentTarget.ACTIVITY
@@ -402,6 +404,13 @@ class ConfigIntentViewModel @Inject constructor(
     }
 
     fun loadResult(result: ConfigIntentResult) {
+        // Only apply the initial argument once so that recreating the screen does not overwrite
+        // the user's edits with the original value. See issue #2160.
+        if (isResultLoaded) {
+            return
+        }
+        isResultLoaded = true
+
         val intent = Intent.parseUri(result.uri, 0)
 
         description.value = result.description
@@ -439,21 +448,37 @@ class ConfigIntentViewModel @Inject constructor(
 
             val extraType = when (value) {
                 is Boolean -> BoolExtraType
+
                 is BooleanArray -> BoolArrayExtraType
+
                 is Int -> IntExtraType
+
                 is IntArray -> IntArrayExtraType
+
                 is Long -> LongExtraType
+
                 is LongArrayExtraType -> LongArrayExtraType
+
                 is Byte -> ByteExtraType
+
                 is ByteArrayExtraType -> ByteArrayExtraType
+
                 is Double -> DoubleExtraType
+
                 is DoubleArray -> DoubleArrayExtraType
+
                 is Float -> FloatExtraType
+
                 is FloatArray -> FloatArrayExtraType
+
                 is Short -> ShortExtraType
+
                 is ShortArray -> ShortArrayExtraType
+
                 is String -> StringExtraType
+
                 is Array<*> -> StringArrayExtraType
+
                 else -> throw IllegalArgumentException(
                     "Don't know how to convert this extra (${value.javaClass.name}) to an IntentExtraType",
                 )

base/src/test/java/io/github/sds100/keymapper/base/system/intents/ConfigIntentViewModelRecreationTest.kt

@@ -0,0 +1,83 @@
+package io.github.sds100.keymapper.base.system.intents
+
+import android.content.Intent
+import androidx.arch.core.executor.testing.InstantTaskExecutorRule
+import io.github.sds100.keymapper.base.utils.ui.DialogProviderImpl
+import io.github.sds100.keymapper.base.utils.ui.FakeResourceProvider
+import io.github.sds100.keymapper.system.apps.ActivityInfo
+import io.github.sds100.keymapper.system.intents.IntentTarget
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.test.UnconfinedTestDispatcher
+import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.test.setMain
+import org.hamcrest.MatcherAssert.assertThat
+import org.hamcrest.Matchers.`is`
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * Regression test for issue #2160. The fragment applies the initial argument by calling
+ * [ConfigIntentViewModel.loadResult] from onCreate, which runs again when the screen is recreated
+ * (for example on a configuration change) while the ViewModel survives. Loading must be applied
+ * only once so that a recreation does not discard the user's edits with the original value.
+ */
+@ExperimentalCoroutinesApi
+@RunWith(RobolectricTestRunner::class)
+class ConfigIntentViewModelRecreationTest {
+
+    @get:Rule
+    var instantExecutorRule = InstantTaskExecutorRule()
+
+    private val testDispatcher = UnconfinedTestDispatcher()
+    private lateinit var viewModel: ConfigIntentViewModel
+
+    @Before
+    fun setUp() {
+        Dispatchers.setMain(testDispatcher)
+        viewModel = ConfigIntentViewModel(FakeResourceProvider(), DialogProviderImpl())
+    }
+
+    @Test
+    fun loadResult_recreationAfterChangingActivity_keepsEditedActivity() = runTest(testDispatcher) {
+        val original = ConfigIntentResult(
+            uri = "#Intent;package=com.example.a;component=com.example.a/.MainActivity;end",
+            target = IntentTarget.ACTIVITY,
+            description = "Open App",
+            extras = emptyList(),
+        )
+
+        // Edit an existing intent action.
+        viewModel.loadResult(original)
+
+        // The user picks a different activity.
+        viewModel.setActivity(ActivityInfo("com.example.b.SecondActivity", "com.example.b"))
+
+        // The screen is recreated, so onCreate applies the original argument again.
+        viewModel.loadResult(original)
+
+        assertThat(viewModel.targetPackage.value, `is`("com.example.b"))
+        assertThat(viewModel.targetClass.value, `is`("com.example.b.SecondActivity"))
+
+        // The saved intent uri must contain the edited activity, not the original one.
+        val result = collectDoneResult()
+        val component = Intent.parseUri(result.uri, 0).component
+        assertThat(component?.packageName, `is`("com.example.b"))
+        assertThat(component?.className, `is`("com.example.b.SecondActivity"))
+    }
+
+    private suspend fun collectDoneResult(): ConfigIntentResult {
+        var result: ConfigIntentResult? = null
+        val job = kotlinx.coroutines.CoroutineScope(testDispatcher).launch {
+            result = viewModel.returnResult.first()
+        }
+        viewModel.onDoneClick()
+        job.join()
+        return result!!
+    }
+}