From a77ce6984ef9b9457fdd5301df7f5c09f3f0a003 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Mon, 28 Sep 2026 07:40:04 +0000 Subject: [PATCH] let a server be edited and move its delete into the edit screen --- .../com/subtracks/data/db/SourcesDao.kt | 14 ++++ .../subtracks/data/repo/SourceRepository.kt | 27 ++++++- .../main/kotlin/com/subtracks/di/AppModule.kt | 4 +- .../main/kotlin/com/subtracks/ui/AppRoot.kt | 14 ++++ .../subtracks/ui/settings/AddSourceScreen.kt | 58 ++++++++++++++- .../ui/settings/AddSourceViewModel.kt | 71 ++++++++++++++++--- .../subtracks/ui/settings/SettingsScreen.kt | 12 ++-- .../ui/settings/SettingsViewModel.kt | 9 --- .../data/repo/SourceRepositoryTest.kt | 37 ++++++++++ .../com/subtracks/ui/AppScreenshotTest.kt | 34 ++++++++- 10 files changed, 250 insertions(+), 30 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/db/SourcesDao.kt b/app/src/main/kotlin/com/subtracks/data/db/SourcesDao.kt index 6deca027..845e852e 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/SourcesDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/SourcesDao.kt @@ -17,6 +17,12 @@ interface SourcesDao { @Query("SELECT * FROM sources WHERE id = :id") fun source(id: Long): Flow + @Query("SELECT * FROM sources WHERE id = :id") + suspend fun sourceOnce(id: Long): Source? + + @Query("SELECT COUNT(*) FROM sources") + suspend fun sourceCount(): Int + @Query("SELECT id FROM sources WHERE isActive = 1 LIMIT 1") fun activeSourceId(): Flow @@ -64,4 +70,12 @@ interface SourcesDao { "WHERE s.isActive = 1 LIMIT 1", ) suspend fun activeSubsonicConfigOnce(): SubsonicConfig? + + @Query( + "SELECT s.id AS id, s.name AS name, s.address AS address, c.username AS username, " + + "c.password AS password, c.useTokenAuth AS useTokenAuth " + + "FROM sources s JOIN subsonic_sources c ON c.sourceId = s.id " + + "WHERE s.id = :sourceId LIMIT 1", + ) + suspend fun subsonicConfigOnce(sourceId: Long): SubsonicConfig? } diff --git a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt index 9b298356..1009d6a2 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt @@ -83,6 +83,8 @@ class SourceRepository( fun activeConfig(): Flow = db.sourcesDao().activeSubsonicConfig() + suspend fun sourceConfigOnce(id: Long): SubsonicConfig? = db.sourcesDao().subsonicConfigOnce(id) + fun coverArt( coverArt: String?, thumbnail: Boolean = false, @@ -151,13 +153,36 @@ class SourceRepository( suspend fun selectSource(id: Long) = db.sourcesDao().setActiveSource(id) - suspend fun deleteSource(id: Long) { + suspend fun updateSource( + id: Long, + name: String, + address: String, + username: String, + password: String, + useTokenAuth: Boolean, + ) { val dao = db.sourcesDao() + val source = dao.sourceOnce(id) ?: return + dao.upsertSource(source.copy(name = name.trim(), address = normalizeAddress(address))) + dao.upsertSubsonicSource( + SubsonicSource( + sourceId = id, + username = username.trim(), + password = password, + useTokenAuth = useTokenAuth, + ), + ) + } + + suspend fun deleteSource(id: Long): Boolean { + val dao = db.sourcesDao() + if (dao.sourceCount() <= 1) return false val wasActive = dao.activeSourceIdOnce() == id dao.deleteSource(id) if (wasActive) { dao.firstSourceId()?.let { dao.setActiveSource(it) } } + return true } suspend fun ping( diff --git a/app/src/main/kotlin/com/subtracks/di/AppModule.kt b/app/src/main/kotlin/com/subtracks/di/AppModule.kt index 338331f4..c93e30b1 100644 --- a/app/src/main/kotlin/com/subtracks/di/AppModule.kt +++ b/app/src/main/kotlin/com/subtracks/di/AppModule.kt @@ -73,9 +73,9 @@ fun appModule( single { PlaybackController(get(), get(), get(), get(), showMessage = toast) } viewModel { RootViewModel(get()) } viewModel { LibraryViewModel(get(), get(), get(), get(), get(), get()) } - viewModel { SettingsViewModel(get(), get(), get()) } + viewModel { SettingsViewModel(get(), get()) } viewModel { DownloadsViewModel(get(), get(), get()) } - viewModel { AddSourceViewModel(get(), get()) } + viewModel { params -> AddSourceViewModel(get(), get(), get(), params.getOrNull()) } viewModel { params -> AlbumDetailViewModel(get(), get(), get(), get(), params.get()) } viewModel { params -> ArtistDetailViewModel(get(), get(), get(), get(), params.get()) } viewModel { params -> PlaylistDetailViewModel(get(), get(), get(), get(), params.get()) } diff --git a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt index 4daa16d6..8532130f 100644 --- a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt +++ b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt @@ -108,6 +108,7 @@ private object Routes { const val SETTINGS = "settings" const val DOWNLOADS = "downloads" const val ADD_SERVER = "add-server" + const val EDIT_SERVER = "edit-server/{sourceId}" const val ALBUM_DETAIL = "album/{albumId}?coverArt={coverArt}" const val ARTIST_DETAIL = "artist/{artistId}?coverArt={coverArt}" const val PLAYLIST_DETAIL = "playlist/{playlistId}" @@ -123,6 +124,8 @@ private object Routes { ) = "artist/${Uri.encode(id)}?coverArt=${Uri.encode(coverArt.orEmpty())}" fun playlist(id: String) = "playlist/${Uri.encode(id)}" + + fun editServer(id: Long) = "edit-server/$id" } internal data class BackStackKey( @@ -308,6 +311,7 @@ private fun MainNavigation() { SettingsRoute( onAddServer = { navController.navigate(Routes.ADD_SERVER) }, onOpenDownloads = { navController.navigate(Routes.DOWNLOADS) }, + onEditServer = { id -> navController.navigate(Routes.editServer(id)) }, onBack = { navController.popBackStack() }, ) } @@ -388,6 +392,16 @@ private fun MainNavigation() { onBack = { navController.popBackStack() }, ) } + composable( + route = Routes.EDIT_SERVER, + arguments = listOf(navArgument("sourceId") { type = NavType.LongType }), + ) { entry -> + AddSourceRoute( + onSaved = { navController.popBackStack() }, + onBack = { navController.popBackStack() }, + sourceId = entry.arguments?.getLong("sourceId"), + ) + } } AnimatedVisibility( diff --git a/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceScreen.kt b/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceScreen.kt index 9e1a7fc8..4d862261 100644 --- a/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceScreen.kt @@ -14,11 +14,14 @@ import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.rounded.ArrowBack +import androidx.compose.material.icons.rounded.Delete +import androidx.compose.material3.AlertDialog import androidx.compose.material3.Button import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton +import androidx.compose.material3.IconButtonDefaults import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedButton import androidx.compose.material3.OutlinedTextField @@ -26,11 +29,14 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.Switch import androidx.compose.material3.SwitchDefaults import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusRequester @@ -40,12 +46,14 @@ import androidx.compose.ui.text.input.PasswordVisualTransformation import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle import org.koin.compose.viewmodel.koinViewModel +import org.koin.core.parameter.parametersOf @Composable fun AddSourceRoute( onSaved: () -> Unit, onBack: (() -> Unit)?, - viewModel: AddSourceViewModel = koinViewModel(), + sourceId: Long? = null, + viewModel: AddSourceViewModel = koinViewModel(key = sourceId?.toString() ?: "new") { parametersOf(sourceId) }, ) { val state by viewModel.state.collectAsStateWithLifecycle() AddSourceScreen( @@ -57,6 +65,7 @@ fun AddSourceRoute( onTokenAuthChange = viewModel::setTokenAuth, onTest = viewModel::testConnection, onSave = { viewModel.save(onSaved) }, + onDelete = { viewModel.delete(onSaved) }, onBack = onBack, ) } @@ -72,11 +81,13 @@ fun AddSourceScreen( onTokenAuthChange: (Boolean) -> Unit, onTest: () -> Unit, onSave: () -> Unit, + onDelete: () -> Unit, onBack: (() -> Unit)?, modifier: Modifier = Modifier, ) { val nameFocus = remember { FocusRequester() } val addressFocus = remember { FocusRequester() } + var confirmingDelete by remember { mutableStateOf(false) } LaunchedEffect(state.nameError, state.addressError) { when { state.nameError -> nameFocus.requestFocus() @@ -87,7 +98,7 @@ fun AddSourceScreen( modifier = modifier, topBar = { TopAppBar( - title = { Text("Add server") }, + title = { Text(if (state.isEditing) "Edit server" else "Add server") }, navigationIcon = { if (onBack != null) { IconButton(onClick = onBack) { @@ -95,6 +106,17 @@ fun AddSourceScreen( } } }, + actions = { + if (state.isEditing) { + IconButton( + onClick = { confirmingDelete = true }, + enabled = state.canDelete && !state.busy, + colors = IconButtonDefaults.iconButtonColors(contentColor = MaterialTheme.colorScheme.error), + ) { + Icon(Icons.Rounded.Delete, contentDescription = "Delete server") + } + } + }, ) }, bottomBar = { @@ -141,10 +163,21 @@ fun AddSourceScreen( verticalArrangement = Arrangement.spacedBy(12.dp), ) { Text( - "Connect a Subsonic-compatible server such as Navidrome, gonic or Airsonic.", + if (state.isEditing) { + "Update this server's connection details." + } else { + "Connect a Subsonic-compatible server such as Navidrome, gonic or Airsonic." + }, style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, ) + if (state.isEditing && !state.canDelete) { + Text( + "This is the only server. Add another before you can remove this one.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } OutlinedTextField( value = state.name, onValueChange = onNameChange, @@ -206,4 +239,23 @@ fun AddSourceScreen( } } } + + if (confirmingDelete) { + AlertDialog( + onDismissRequest = { confirmingDelete = false }, + title = { Text("Delete server?") }, + text = { Text("${state.name} and its downloaded songs will be removed from this device.") }, + confirmButton = { + TextButton( + onClick = { + confirmingDelete = false + onDelete() + }, + ) { Text("Delete") } + }, + dismissButton = { + TextButton(onClick = { confirmingDelete = false }) { Text("Cancel") } + }, + ) + } } diff --git a/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceViewModel.kt index 89d66dc7..9ef35d68 100644 --- a/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceViewModel.kt @@ -2,6 +2,7 @@ package com.subtracks.ui.settings import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.subtracks.data.repo.DownloadRepository import com.subtracks.data.repo.SourceRepository import com.subtracks.data.sync.SyncManager import kotlinx.coroutines.flow.MutableStateFlow @@ -20,6 +21,8 @@ data class AddSourceState( val isError: Boolean = false, val nameError: Boolean = false, val addressError: Boolean = false, + val isEditing: Boolean = false, + val canDelete: Boolean = false, ) internal fun AddSourceState.validated(): AddSourceState = @@ -33,10 +36,34 @@ internal fun AddSourceState.validated(): AddSourceState = class AddSourceViewModel( private val sourceRepository: SourceRepository, private val syncManager: SyncManager, + private val downloadRepository: DownloadRepository, + private val sourceId: Long? = null, ) : ViewModel() { - private val _state = MutableStateFlow(AddSourceState()) + private val _state = MutableStateFlow(AddSourceState(isEditing = sourceId != null)) val state: StateFlow = _state + init { + sourceId?.let { id -> + viewModelScope.launch { + val config = sourceRepository.sourceConfigOnce(id) ?: return@launch + _state.update { + it.copy( + name = config.name, + address = config.address, + username = config.username, + password = config.password, + useTokenAuth = config.useTokenAuth, + ) + } + } + } + viewModelScope.launch { + sourceRepository.sources().collect { sources -> + _state.update { it.copy(canDelete = sources.size > 1) } + } + } + } + fun setName(value: String) = _state.update { it.copy(name = value, nameError = false) } fun setAddress(value: String) = _state.update { it.copy(address = value, addressError = false) } @@ -78,13 +105,25 @@ class AddSourceViewModel( val current = _state.value viewModelScope.launch { runCatching { - sourceRepository.addSource( - name = current.name, - address = current.address, - username = current.username, - password = current.password, - useTokenAuth = current.useTokenAuth, - ) + val id = sourceId + if (id == null) { + sourceRepository.addSource( + name = current.name, + address = current.address, + username = current.username, + password = current.password, + useTokenAuth = current.useTokenAuth, + ) + } else { + sourceRepository.updateSource( + id = id, + name = current.name, + address = current.address, + username = current.username, + password = current.password, + useTokenAuth = current.useTokenAuth, + ) + } }.fold( onSuccess = { syncManager.requestSync() @@ -99,4 +138,20 @@ class AddSourceViewModel( ) } } + + fun delete(onDeleted: () -> Unit) { + val id = sourceId ?: return + _state.update { it.copy(busy = true, message = null) } + viewModelScope.launch { + if (!sourceRepository.deleteSource(id)) { + _state.update { + it.copy(busy = false, message = "At least one server is required", isError = true) + } + return@launch + } + downloadRepository.removeSource(id) + _state.update { it.copy(busy = false) } + onDeleted() + } + } } diff --git a/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt b/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt index eea0f2bf..2ffe7349 100644 --- a/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt @@ -19,8 +19,8 @@ import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.rounded.ArrowBack +import androidx.compose.material.icons.automirrored.rounded.KeyboardArrowRight import androidx.compose.material.icons.rounded.Add -import androidx.compose.material.icons.rounded.Delete import androidx.compose.material3.AlertDialog import androidx.compose.material3.Button import androidx.compose.material3.ExperimentalMaterial3Api @@ -60,6 +60,7 @@ private enum class SettingsDialog { WifiQuality, MobileQuality, SyncConcurrency fun SettingsRoute( onAddServer: () -> Unit, onOpenDownloads: () -> Unit, + onEditServer: (Long) -> Unit, onBack: () -> Unit, viewModel: SettingsViewModel = koinViewModel(), ) { @@ -75,7 +76,7 @@ fun SettingsRoute( mobileQuality = mobileQuality, syncConcurrency = syncConcurrency, onSelectSource = viewModel::selectSource, - onDeleteSource = viewModel::deleteSource, + onEditServer = onEditServer, onWifiQualityChange = viewModel::setWifiQuality, onMobileQualityChange = viewModel::setMobileQuality, onSyncConcurrencyChange = viewModel::setSyncConcurrency, @@ -94,7 +95,7 @@ fun SettingsScreen( mobileQuality: StreamQuality, syncConcurrency: Int, onSelectSource: (Long) -> Unit, - onDeleteSource: (Long) -> Unit, + onEditServer: (Long) -> Unit, onWifiQualityChange: (StreamQuality) -> Unit, onMobileQualityChange: (StreamQuality) -> Unit, onSyncConcurrencyChange: (Int) -> Unit, @@ -139,10 +140,9 @@ fun SettingsScreen( ) }, trailingContent = { - IconButton(onClick = { onDeleteSource(source.id) }) { - Icon(Icons.Rounded.Delete, contentDescription = "Remove ${source.name}") - } + Icon(Icons.AutoMirrored.Rounded.KeyboardArrowRight, contentDescription = null) }, + modifier = Modifier.clickable(onClickLabel = "Edit ${source.name}") { onEditServer(source.id) }, colors = ListItemDefaults.colors(containerColor = Color.Transparent), ) } diff --git a/app/src/main/kotlin/com/subtracks/ui/settings/SettingsViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/settings/SettingsViewModel.kt index 6eced07d..af40eb62 100644 --- a/app/src/main/kotlin/com/subtracks/ui/settings/SettingsViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/settings/SettingsViewModel.kt @@ -6,7 +6,6 @@ import com.subtracks.data.model.Source import com.subtracks.data.net.NetworkMode import com.subtracks.data.prefs.StreamQuality import com.subtracks.data.prefs.UserPreferences -import com.subtracks.data.repo.DownloadRepository import com.subtracks.data.repo.SourceRepository import com.subtracks.data.source.DEFAULT_FETCH_CONCURRENCY import kotlinx.coroutines.flow.SharingStarted @@ -16,7 +15,6 @@ import kotlinx.coroutines.launch class SettingsViewModel( private val sourceRepository: SourceRepository, - private val downloadRepository: DownloadRepository, private val userPreferences: UserPreferences, ) : ViewModel() { val sources: StateFlow> = @@ -44,13 +42,6 @@ class SettingsViewModel( viewModelScope.launch { sourceRepository.selectSource(id) } } - fun deleteSource(id: Long) { - viewModelScope.launch { - downloadRepository.removeSource(id) - sourceRepository.deleteSource(id) - } - } - fun setWifiQuality(quality: StreamQuality) { viewModelScope.launch { userPreferences.setStreamQuality(NetworkMode.Wifi, quality) } } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt index 33f3099e..b7165d4e 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt @@ -83,6 +83,43 @@ class SourceRepositoryTest { ) } + @Test + fun editingASourceRewritesItsDetailsWithoutAddingOne() = + runTest { + repository.addSource("first", "http://a.example", "u1", "p1", true) + val id = repository.activeSourceId().first()!! + repository.addSource("second", "http://b.example", "u2", "p2", true) + + repository.updateSource(id, "renamed", "c.example", "u3", "p3", false) + + val sources = repository.sources().first() + assertEquals(2, sources.size) + val edited = sources.first { it.id == id } + assertEquals("renamed", edited.name) + assertEquals("http://c.example/", edited.address) + assertFalse("editing must not change which source is active", edited.isActive) + val config = repository.sourceConfigOnce(id)!! + assertEquals("u3", config.username) + assertEquals("p3", config.password) + assertFalse(config.useTokenAuth) + } + + @Test + fun theLastSourceCannotBeDeleted() = + runTest { + repository.addSource("only", "http://a.example", "u", "p", true) + val id = repository.activeSourceId().first()!! + + assertFalse("the last source must survive deletion", repository.deleteSource(id)) + assertEquals(1, repository.sources().first().size) + + repository.addSource("second", "http://b.example", "u", "p", true) + + assertTrue(repository.deleteSource(id)) + assertEquals(listOf("second"), repository.sources().first().map { it.name }) + assertEquals("second", repository.activeConfig().first()?.name) + } + @Test fun aTokenAuthSourceIsProbedAndSwitchedToPassword() = runBlocking { diff --git a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt index 41db43ae..f2aaa96e 100644 --- a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt @@ -344,7 +344,7 @@ class AppScreenshotTest { mobileQuality = StreamQuality(192, "mp3"), syncConcurrency = 4, onSelectSource = {}, - onDeleteSource = {}, + onEditServer = {}, onWifiQualityChange = {}, onMobileQualityChange = {}, onSyncConcurrencyChange = {}, @@ -390,6 +390,7 @@ class AppScreenshotTest { onTokenAuthChange = {}, onTest = {}, onSave = {}, + onDelete = {}, onBack = null, ) } @@ -398,6 +399,37 @@ class AppScreenshotTest { composeRule.onRoot().captureRoboImage(file) } + @Test + fun editSource() { + composeRule.setContent { + SubtracksTheme { + AddSourceScreen( + state = + AddSourceState( + name = "Home", + address = "https://music.example.com", + username = "austin", + password = "hunter2", + useTokenAuth = true, + isEditing = true, + canDelete = true, + ), + onNameChange = {}, + onAddressChange = {}, + onUsernameChange = {}, + onPasswordChange = {}, + onTokenAuthChange = {}, + onTest = {}, + onSave = {}, + onDelete = {}, + onBack = {}, + ) + } + } + awaitText("Edit server") + composeRule.onRoot().captureRoboImage("src/test/screenshots/edit_source.png") + } + @Test fun nowPlaying() { composeRule.setContent {