diff --git a/CHANGELOG.md b/CHANGELOG.md index b3e75de0ea..fca90c0ea8 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/base/src/main/java/io/github/sds100/keymapper/base/system/intents/ConfigIntentViewModel.kt b/base/src/main/java/io/github/sds100/keymapper/base/system/intents/ConfigIntentViewModel.kt index 1473281ba6..c8ea64c986 100644 --- a/base/src/main/java/io/github/sds100/keymapper/base/system/intents/ConfigIntentViewModel.kt +++ b/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() 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", ) diff --git a/base/src/test/java/io/github/sds100/keymapper/base/system/intents/ConfigIntentViewModelRecreationTest.kt b/base/src/test/java/io/github/sds100/keymapper/base/system/intents/ConfigIntentViewModelRecreationTest.kt new file mode 100644 index 0000000000..ac29369c54 --- /dev/null +++ b/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!! + } +}