From 81df219f0cdf44362891e6a79f0cb7e2e4167b2d Mon Sep 17 00:00:00 2001 From: yaturner Date: Wed, 16 Sep 2026 10:33:15 -0700 Subject: [PATCH 1/3] ADFA-5446: Confirm template uninstall/replace instead of failing silently Uninstalling a template refused to run when a same-named file already sat in Downloads, with no way to proceed - the user's only options were delete the Downloads file themselves or give up. Add an "Uninstall template?" confirmation (mirroring the existing Delete confirmation) and, on a Downloads name collision, a "Replace file in Downloads?" prompt that lets the user overwrite it instead of hitting a dead end. TemplateRepository.uninstallTemplate gains an `overwrite` parameter; a collision without it now returns TemplateReplaceConflictException instead of a generic IllegalStateException, so the ViewModel can route it to the new confirmation dialog rather than a plain error toast. Co-Authored-By: Claude Sonnet 5 --- .../repositories/TemplateRepository.kt | 18 +++- .../repositories/TemplateRepositoryImpl.kt | 21 +++- .../templates/TemplateManagerDialogs.kt | 39 ++++++++ .../templates/TemplateManagerScreen.kt | 46 +++++++++ .../ui/models/TemplateManagerUiState.kt | 9 ++ .../viewmodels/TemplateManagerViewModel.kt | 38 ++++++-- .../TemplateRepositoryImplTest.kt | 22 ++++- .../TemplateManagerViewModelTest.kt | 95 +++++++++++++++++++ resources/src/main/res/values/strings.xml | 4 + 9 files changed, 276 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepository.kt b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepository.kt index ee497b0106..9966e238a2 100644 --- a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepository.kt +++ b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepository.kt @@ -1,6 +1,7 @@ package com.itsaky.androidide.repositories import com.itsaky.androidide.templates.manager.models.CgtFileItem +import java.io.IOException /** * Repository interface for template (`.cgt`) file operations. @@ -20,9 +21,22 @@ interface TemplateRepository { /** Moves [item]'s file from Downloads into the templates directory and reloads templates. */ suspend fun installTemplate(item: CgtFileItem): Result - /** Restores a copy of [item]'s file to Downloads, removes it from the templates directory, and reloads templates. */ - suspend fun uninstallTemplate(item: CgtFileItem): Result + /** + * Restores a copy of [item]'s file to Downloads, removes it from the templates directory, and + * reloads templates. Fails with [TemplateReplaceConflictException] if a file of the same name + * already exists in Downloads, unless [overwrite] is true. + */ + suspend fun uninstallTemplate( + item: CgtFileItem, + overwrite: Boolean = false, + ): Result /** Deletes a not-installed [item]'s file from Downloads. */ suspend fun deleteDownloadFile(item: CgtFileItem): Result } + +/** Thrown by [TemplateRepository.uninstallTemplate] when restoring to Downloads would silently + * overwrite an existing file there and the caller didn't opt into replacing it. */ +class TemplateReplaceConflictException( + val fileName: String, +) : IOException("A file named '$fileName' already exists in Downloads") diff --git a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt index 21662008d1..a313b45d23 100644 --- a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt +++ b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt @@ -137,7 +137,10 @@ class TemplateRepositoryImpl( } } - override suspend fun uninstallTemplate(item: CgtFileItem): Result = + override suspend fun uninstallTemplate( + item: CgtFileItem, + overwrite: Boolean, + ): Result = withContext(Dispatchers.IO) { try { check(item.installed) { "'${item.name}' is not installed" } @@ -146,10 +149,20 @@ class TemplateRepositoryImpl( // Restore a copy to Downloads BEFORE removing it from the store: if the restore // throws, the store copy below is never touched, so the user's only copy survives. val restored = File(downloadDir, item.file.name) - check(!restored.exists()) { "A download named '${restored.name}' already exists in $downloadDir" } - item.file.copyTo(restored, overwrite = false) + val hadExistingDownload = restored.exists() + if (hadExistingDownload && !overwrite) { + return@withContext Result.failure(TemplateReplaceConflictException(restored.name)) + } + item.file.copyTo(restored, overwrite = overwrite) if (!item.file.delete()) { - restored.delete() + // Only roll back a copy this call created itself. When `overwrite` replaced a + // pre-existing Downloads file, that original content is already gone - deleting + // `restored` here would destroy the new copy too and leave the user with nothing, + // whereas the still-installed source (its delete just failed) means leaving the + // new copy in place costs nothing and loses no data. + if (!hadExistingDownload) { + restored.delete() + } throw IOException("Failed to delete source file after copying: ${item.file.absolutePath}") } ITemplateProvider.getInstance(reload = true) diff --git a/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt b/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt index dcdb2f54e3..2ca82f190a 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt @@ -44,6 +44,45 @@ fun DeleteTemplateConfirmationDialog( ) } +@Composable +fun UninstallTemplateConfirmationDialog( + item: CgtFileItem, + onConfirm: () -> Unit, + onDismiss: () -> Unit, +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text(stringResource(R.string.title_uninstall_template)) }, + text = { Text(stringResource(R.string.msg_uninstall_template_confirm, item.displayName)) }, + confirmButton = { + TextButton(onClick = onConfirm) { Text(stringResource(R.string.action_uninstall_template)) } + }, + dismissButton = { + TextButton(onClick = onDismiss) { Text(stringResource(android.R.string.cancel)) } + }, + ) +} + +/** Shown when uninstalling [item] would silently overwrite an existing Downloads file of the same name. */ +@Composable +fun ReplaceTemplateInDownloadsDialog( + item: CgtFileItem, + onConfirm: () -> Unit, + onDismiss: () -> Unit, +) { + AlertDialog( + onDismissRequest = onDismiss, + title = { Text(stringResource(R.string.title_replace_template)) }, + text = { Text(stringResource(R.string.msg_replace_template_confirm, item.displayName)) }, + confirmButton = { + TextButton(onClick = onConfirm) { Text(stringResource(R.string.replace)) } + }, + dismissButton = { + TextButton(onClick = onDismiss) { Text(stringResource(android.R.string.cancel)) } + }, + ) +} + /** File-level details for a single-template .cgt (multi-template files use [TemplateListDialog]). */ @Composable fun TemplateFileDetailsDialog( diff --git a/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerScreen.kt b/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerScreen.kt index 4e6da44a51..cd9bd56903 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerScreen.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerScreen.kt @@ -67,6 +67,16 @@ private sealed interface TemplateManagerDialogState : Parcelable { val path: String, ) : TemplateManagerDialogState + @Parcelize + data class UninstallConfirm( + val path: String, + ) : TemplateManagerDialogState + + @Parcelize + data class ReplaceConfirm( + val path: String, + ) : TemplateManagerDialogState + @Parcelize data class FileDetails( val path: String, @@ -153,6 +163,14 @@ fun TemplateManagerScreen( dialogState = TemplateManagerDialogState.DeleteConfirm(effect.item.file.absolutePath) } + is TemplateManagerUiEffect.ShowUninstallConfirmation -> { + dialogState = TemplateManagerDialogState.UninstallConfirm(effect.item.file.absolutePath) + } + + is TemplateManagerUiEffect.ShowReplaceConfirmation -> { + dialogState = TemplateManagerDialogState.ReplaceConfirm(effect.item.file.absolutePath) + } + is TemplateManagerUiEffect.ShowTemplateDetails -> { dialogState = TemplateManagerDialogState.FileDetails(effect.item.file.absolutePath) } @@ -215,6 +233,34 @@ fun TemplateManagerScreen( } } + is TemplateManagerDialogState.UninstallConfirm -> { + val item = uiState.items.firstOrNull { it.file.absolutePath == dialog.path } + if (item != null) { + UninstallTemplateConfirmationDialog( + item = item, + onConfirm = { + viewModel.confirmUninstallTemplate(item) + dialogState = TemplateManagerDialogState.None + }, + onDismiss = { dialogState = TemplateManagerDialogState.None }, + ) + } + } + + is TemplateManagerDialogState.ReplaceConfirm -> { + val item = uiState.items.firstOrNull { it.file.absolutePath == dialog.path } + if (item != null) { + ReplaceTemplateInDownloadsDialog( + item = item, + onConfirm = { + viewModel.confirmUninstallTemplate(item, overwrite = true) + dialogState = TemplateManagerDialogState.None + }, + onDismiss = { dialogState = TemplateManagerDialogState.None }, + ) + } + } + is TemplateManagerDialogState.FileDetails -> { val item = uiState.items.firstOrNull { it.file.absolutePath == dialog.path } if (item != null) { diff --git a/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt b/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt index b47f16115e..10a1fd6165 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt @@ -50,6 +50,15 @@ sealed class TemplateManagerUiEffect { val item: CgtFileItem, ) : TemplateManagerUiEffect() + data class ShowUninstallConfirmation( + val item: CgtFileItem, + ) : TemplateManagerUiEffect() + + /** A Downloads file with the same name already exists; ask before [uninstallTemplate][com.itsaky.androidide.repositories.TemplateRepository.uninstallTemplate] overwrites it. */ + data class ShowReplaceConfirmation( + val item: CgtFileItem, + ) : TemplateManagerUiEffect() + data class ShowTemplateDetails( val item: CgtFileItem, ) : TemplateManagerUiEffect() diff --git a/app/src/main/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModel.kt b/app/src/main/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModel.kt index 13df7bca84..bb60d1e76f 100644 --- a/app/src/main/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModel.kt +++ b/app/src/main/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModel.kt @@ -3,6 +3,7 @@ package com.itsaky.androidide.viewmodels import android.util.Log import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.itsaky.androidide.repositories.TemplateReplaceConflictException import com.itsaky.androidide.repositories.TemplateRepository import com.itsaky.androidide.resources.R import com.itsaky.androidide.templates.manager.models.CgtFileItem @@ -42,7 +43,7 @@ class TemplateManagerViewModel( when (event) { is TemplateManagerUiEvent.LoadTemplates -> loadTemplates() is TemplateManagerUiEvent.InstallTemplate -> installTemplate(event.item) - is TemplateManagerUiEvent.UninstallTemplate -> uninstallTemplate(event.item) + is TemplateManagerUiEvent.UninstallTemplate -> showUninstallConfirmation(event.item) is TemplateManagerUiEvent.DeleteDownloadFile -> showDeleteConfirmation(event.item) is TemplateManagerUiEvent.ShowTemplateDetails -> showTemplateDetails(event.item) is TemplateManagerUiEvent.ShowTemplateList -> showTemplateList(event.item) @@ -101,11 +102,26 @@ class TemplateManagerViewModel( } } - private fun uninstallTemplate(item: CgtFileItem) { + private fun showUninstallConfirmation(item: CgtFileItem) { + viewModelScope.launch { + _uiEffect.send(TemplateManagerUiEffect.ShowUninstallConfirmation(item)) + } + } + + /** + * Uninstalls [item] (called after [TemplateManagerUiEffect.ShowUninstallConfirmation] is + * confirmed). A [TemplateReplaceConflictException] means a same-named file already sits in + * Downloads; [ShowReplaceConfirmation][TemplateManagerUiEffect.ShowReplaceConfirmation] asks + * whether to replace it, and a confirmed replace re-enters this with [overwrite] set. + */ + fun confirmUninstallTemplate( + item: CgtFileItem, + overwrite: Boolean = false, + ) { viewModelScope.launch { _uiState.update { it.copy(isLoading = true) } templateRepository - .uninstallTemplate(item) + .uninstallTemplate(item, overwrite) .onSuccess { Log.d(TAG, "Template uninstalled successfully: ${item.name}") _uiEffect.send(TemplateManagerUiEffect.ShowSuccess(R.string.msg_template_uninstalled)) @@ -113,12 +129,16 @@ class TemplateManagerViewModel( }.onFailure { exception -> Log.e(TAG, "Failed to uninstall template: ${item.name}", exception) _uiState.update { it.copy(isLoading = false) } - _uiEffect.send( - TemplateManagerUiEffect.ShowError( - R.string.msg_template_uninstall_failed, - listOf(exception.message ?: ""), - ), - ) + if (exception is TemplateReplaceConflictException) { + _uiEffect.send(TemplateManagerUiEffect.ShowReplaceConfirmation(item)) + } else { + _uiEffect.send( + TemplateManagerUiEffect.ShowError( + R.string.msg_template_uninstall_failed, + listOf(exception.message ?: ""), + ), + ) + } } } } diff --git a/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt b/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt index c1c0048630..b4b0a1d514 100644 --- a/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt +++ b/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt @@ -98,7 +98,7 @@ class TemplateRepositoryImplTest { } @Test - fun uninstallTemplate_nameCollision_failsWithoutTouchingEitherCopy() = + fun uninstallTemplate_nameCollision_failsWithReplaceConflict_withoutTouchingEitherCopy() = runTest { val source = File(templatesDir, "dup.cgt").apply { writeText("installed") } val existingDownload = File(downloadDir, "dup.cgt").apply { writeText("already in downloads") } @@ -106,6 +106,7 @@ class TemplateRepositoryImplTest { val result = repository.uninstallTemplate(item(source, installed = true)) assertThat(result.isFailure).isTrue() + assertThat(result.exceptionOrNull()).isInstanceOf(TemplateReplaceConflictException::class.java) assertThat(source.exists()).isTrue() assertThat(source.readText()).isEqualTo("installed") assertThat(existingDownload.readText()).isEqualTo("already in downloads") @@ -127,6 +128,25 @@ class TemplateRepositoryImplTest { assertThat(restored.exists()).isFalse() } + @Test + fun uninstallTemplate_overwriteTrue_deleteFails_leavesTheReplacementCopyInDownloads() = + runTest { + val source = File(templatesDir, "uninstall.cgt").apply { writeText("installed") } + val restored = File(downloadDir, "uninstall.cgt").apply { writeText("stale") } + + check(templatesDir.setWritable(false)) { "test setup: could not make templatesDir read-only" } + + val result = repository.uninstallTemplate(item(source, installed = true), overwrite = true) + + assertThat(result.isFailure).isTrue() + assertThat(result.exceptionOrNull()).isInstanceOf(IOException::class.java) + // The source is still installed (its delete failed) - the pre-existing Downloads + // content was already overwritten and can't be recovered either way, so the new copy + // is left in place rather than deleted for nothing. + assertThat(source.exists()).isTrue() + assertThat(restored.readText()).isEqualTo("installed") + } + @Test fun deleteDownloadFile_succeeds_whenNotInstalled() = runTest { diff --git a/app/src/test/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModelTest.kt b/app/src/test/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModelTest.kt index d84bb62bc5..5c66369dfc 100644 --- a/app/src/test/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModelTest.kt +++ b/app/src/test/java/com/itsaky/androidide/viewmodels/TemplateManagerViewModelTest.kt @@ -2,6 +2,7 @@ package com.itsaky.androidide.viewmodels import android.util.Log import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.repositories.TemplateReplaceConflictException import com.itsaky.androidide.repositories.TemplateRepository import com.itsaky.androidide.templates.manager.models.CgtFileItem import com.itsaky.androidide.templates.manager.models.TemplateMetadata @@ -44,6 +45,15 @@ class TemplateManagerViewModelTest { provenance = TemplateProvenance.USER, ) + private val installedItem = + CgtFileItem( + file = File("/tmp/uninstall.cgt"), + name = "uninstall.cgt", + templates = listOf(TemplateMetadata("T", "d", "1.0")), + installed = true, + provenance = TemplateProvenance.USER, + ) + @Before fun stubAndroidLog() { // TemplateManagerViewModel calls android.util.Log.d/Log.e directly; under plain JVM @@ -113,4 +123,89 @@ class TemplateManagerViewModelTest { coVerify(exactly = 1) { repository.listTemplateFiles() } assertThat(viewModel.uiEffect.first() is TemplateManagerUiEffect.ShowError).isTrue() } + + @Test + fun uninstallTemplate_event_asksForConfirmation_withoutCallingRepository() = + runTest { + coEvery { repository.listTemplateFiles() } returns Result.success(emptyList()) + + val viewModel = TemplateManagerViewModel(repository) + advanceUntilIdle() + + viewModel.onEvent(TemplateManagerUiEvent.UninstallTemplate(installedItem)) + advanceUntilIdle() + + coVerify(exactly = 0) { repository.uninstallTemplate(any(), any()) } + val effect = viewModel.uiEffect.first() + assertThat(effect).isInstanceOf(TemplateManagerUiEffect.ShowUninstallConfirmation::class.java) + assertThat((effect as TemplateManagerUiEffect.ShowUninstallConfirmation).item).isEqualTo(installedItem) + } + + @Test + fun confirmUninstallTemplate_onSuccess_reloadsAndSendsShowSuccessEffect() = + runTest { + coEvery { repository.listTemplateFiles() } returns Result.success(emptyList()) + coEvery { repository.uninstallTemplate(installedItem, false) } returns Result.success(Unit) + + val viewModel = TemplateManagerViewModel(repository) + advanceUntilIdle() + + viewModel.confirmUninstallTemplate(installedItem) + advanceUntilIdle() + + coVerify(exactly = 2) { repository.listTemplateFiles() } + assertThat(viewModel.uiEffect.first() is TemplateManagerUiEffect.ShowSuccess).isTrue() + } + + @Test + fun confirmUninstallTemplate_onReplaceConflict_sendsShowReplaceConfirmation_notShowError() = + runTest { + coEvery { repository.listTemplateFiles() } returns Result.success(emptyList()) + coEvery { repository.uninstallTemplate(installedItem, false) } returns + Result.failure(TemplateReplaceConflictException(installedItem.name)) + + val viewModel = TemplateManagerViewModel(repository) + advanceUntilIdle() + + viewModel.confirmUninstallTemplate(installedItem) + advanceUntilIdle() + + // A conflict isn't a reload-worthy outcome and must not be reported as a generic error. + coVerify(exactly = 1) { repository.listTemplateFiles() } + val effect = viewModel.uiEffect.first() + assertThat(effect).isInstanceOf(TemplateManagerUiEffect.ShowReplaceConfirmation::class.java) + assertThat((effect as TemplateManagerUiEffect.ShowReplaceConfirmation).item).isEqualTo(installedItem) + } + + @Test + fun confirmUninstallTemplate_withOverwrite_passesOverwriteThrough_onReplaceConfirm() = + runTest { + coEvery { repository.listTemplateFiles() } returns Result.success(emptyList()) + coEvery { repository.uninstallTemplate(installedItem, true) } returns Result.success(Unit) + + val viewModel = TemplateManagerViewModel(repository) + advanceUntilIdle() + + viewModel.confirmUninstallTemplate(installedItem, overwrite = true) + advanceUntilIdle() + + coVerify(exactly = 1) { repository.uninstallTemplate(installedItem, true) } + assertThat(viewModel.uiEffect.first() is TemplateManagerUiEffect.ShowSuccess).isTrue() + } + + @Test + fun confirmUninstallTemplate_onOtherFailure_sendsShowErrorEffect_withoutReloading() = + runTest { + coEvery { repository.listTemplateFiles() } returns Result.success(emptyList()) + coEvery { repository.uninstallTemplate(installedItem, false) } returns Result.failure(java.io.IOException("boom")) + + val viewModel = TemplateManagerViewModel(repository) + advanceUntilIdle() + + viewModel.confirmUninstallTemplate(installedItem) + advanceUntilIdle() + + coVerify(exactly = 1) { repository.listTemplateFiles() } + assertThat(viewModel.uiEffect.first() is TemplateManagerUiEffect.ShowError).isTrue() + } } diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index 9426a6f6ca..69274af381 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -1430,6 +1430,10 @@ Templates in %1$s Delete template? This permanently deletes \'%1$s\' from Downloads. + Uninstall template? + This moves \'%1$s\' back to Downloads. + Replace file in Downloads? + A file named \'%1$s\' already exists in Downloads. Replace it? File Status Location From e2e08881f169eb51d2a0265dfa0c7710a9d9f413 Mon Sep 17 00:00:00 2001 From: yaturner Date: Wed, 16 Sep 2026 14:34:02 -0700 Subject: [PATCH 2/3] ADFA-5446: Restore collision logging and clarify the overwrite-delete-fails message The replace-conflict path returns early via return@withContext, so it never reached the catch blocks' logger.error call the old IllegalStateException path used to hit - a repeated replace conflict left no log trace. Log it directly where it's decided instead. Also: when overwrite=true and the subsequent source delete fails, the old Downloads content has already been irrecoverably replaced, but the thrown message was identical to the non-overwrite rollback case ("failed to delete source file"), which reads as if nothing happened. The message now says the template exists in both places, since that's what's actually true and rolling back would only destroy the new copy too for nothing. Co-Authored-By: Claude Sonnet 5 --- .../repositories/TemplateRepositoryImpl.kt | 12 +++++++++++- .../repositories/TemplateRepositoryImplTest.kt | 4 ++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt index a313b45d23..75fbe7e69a 100644 --- a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt +++ b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt @@ -151,6 +151,12 @@ class TemplateRepositoryImpl( val restored = File(downloadDir, item.file.name) val hadExistingDownload = restored.exists() if (hadExistingDownload && !overwrite) { + // Not thrown, so it skips the catch blocks below - log it here instead, or a + // repeated replace-conflict leaves no trace for support to find. + logger.warn( + "Uninstall of '{}' would overwrite an existing Downloads file; asking for confirmation", + item.name, + ) return@withContext Result.failure(TemplateReplaceConflictException(restored.name)) } item.file.copyTo(restored, overwrite = overwrite) @@ -162,8 +168,12 @@ class TemplateRepositoryImpl( // new copy in place costs nothing and loses no data. if (!hadExistingDownload) { restored.delete() + throw IOException("Failed to delete source file after copying: ${item.file.absolutePath}") } - throw IOException("Failed to delete source file after copying: ${item.file.absolutePath}") + throw IOException( + "Replaced '${restored.name}' in Downloads, but failed to delete the installed copy at " + + "${item.file.absolutePath} - the template now exists in both places", + ) } ITemplateProvider.getInstance(reload = true) Result.success(Unit) diff --git a/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt b/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt index b4b0a1d514..6d0c810a6c 100644 --- a/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt +++ b/app/src/test/java/com/itsaky/androidide/repositories/TemplateRepositoryImplTest.kt @@ -145,6 +145,10 @@ class TemplateRepositoryImplTest { // is left in place rather than deleted for nothing. assertThat(source.exists()).isTrue() assertThat(restored.readText()).isEqualTo("installed") + // The message must say the file now exists in both places, not the plain "failed to + // delete" wording the non-overwrite rollback case uses - a user reading that after an + // overwrite would wrongly assume their prior Downloads content is still intact. + assertThat(result.exceptionOrNull()?.message).contains("now exists in both places") } @Test From 6588402c5020d2edd2406aed7e05bf3a3832f2d6 Mon Sep 17 00:00:00 2001 From: yaturner Date: Thu, 17 Sep 2026 06:09:46 -0700 Subject: [PATCH 3/3] ADFA-5446: Add KDoc to the uninstall-confirmation public declarations CodeRabbit review nitpicks: TemplateRepositoryImpl.uninstallTemplate's rollback-on-delete-failure contract, ShowUninstallConfirmation, and UninstallTemplateConfirmationDialog were undocumented public declarations. Added brief KDoc to each. Not changed: CodeRabbit's suggestion that the test file's single-arg uninstallTemplate calls need an explicit `overwrite = false` because "the override does not inherit the interface default" is incorrect - Kotlin overrides inherit the base declaration's default parameter value at every call site regardless of the reference's static type, and those exact calls already compile and pass. Co-Authored-By: Claude Sonnet 5 --- .../itsaky/androidide/repositories/TemplateRepositoryImpl.kt | 5 +++++ .../ui/compose/templates/TemplateManagerDialogs.kt | 1 + .../itsaky/androidide/ui/models/TemplateManagerUiState.kt | 1 + 3 files changed, 7 insertions(+) diff --git a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt index 75fbe7e69a..cb2301eae2 100644 --- a/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt +++ b/app/src/main/java/com/itsaky/androidide/repositories/TemplateRepositoryImpl.kt @@ -137,6 +137,11 @@ class TemplateRepositoryImpl( } } + /** + * On a post-copy delete failure: rolls back (deletes the Downloads copy this call created) when + * `overwrite` was false, but leaves the Downloads copy in place when `overwrite` was true, since + * the pre-existing content there was already replaced and can't be recovered either way. + */ override suspend fun uninstallTemplate( item: CgtFileItem, overwrite: Boolean, diff --git a/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt b/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt index 2ca82f190a..07266a1585 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/compose/templates/TemplateManagerDialogs.kt @@ -44,6 +44,7 @@ fun DeleteTemplateConfirmationDialog( ) } +/** Confirm before uninstalling [item], since it moves the file back to Downloads. */ @Composable fun UninstallTemplateConfirmationDialog( item: CgtFileItem, diff --git a/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt b/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt index 10a1fd6165..ae236fb5bb 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/models/TemplateManagerUiState.kt @@ -50,6 +50,7 @@ sealed class TemplateManagerUiEffect { val item: CgtFileItem, ) : TemplateManagerUiEffect() + /** Ask before uninstalling [item], mirroring [ShowDeleteConfirmation]'s pattern for a Downloads file. */ data class ShowUninstallConfirmation( val item: CgtFileItem, ) : TemplateManagerUiEffect()