diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt index d43c11ac..5b1586d4 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -163,6 +163,40 @@ internal const val ARTISTS_FILTER = "OR (length(:search) < 3 AND (instr(lower(name), lower(:search)) > 0 " + "OR instr(lower(sortName), lower(:search)) > 0))) " +internal const val ARTIST_ALBUM_CREDIT_ART = + "SELECT albums.coverArt FROM (" + + "SELECT albums.id, albums.coverArt, albums.year, albums.sortName FROM albums INDEXED BY index_albums_sourceId_artistId " + + "WHERE albums.sourceId = artists.sourceId AND albums.artistId = artists.id " + + "UNION " + + "SELECT albums.id, albums.coverArt, albums.year, albums.sortName FROM album_artists " + + "JOIN albums ON albums.sourceId = album_artists.sourceId AND albums.id = album_artists.albumId " + + "WHERE album_artists.sourceId = artists.sourceId AND album_artists.artistId = artists.id" + + ") albums ORDER BY albums.year DESC, albums.sortName COLLATE NOCASE, albums.id LIMIT 1" + +internal const val ARTIST_TRACK_CREDIT_ART = + "SELECT albums.coverArt FROM (" + + "SELECT albums.id, albums.coverArt, albums.year, albums.sortName FROM songs " + + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + + "WHERE songs.sourceId = artists.sourceId AND songs.artistId = artists.id " + + "UNION " + + "SELECT albums.id, albums.coverArt, albums.year, albums.sortName FROM song_artists " + + "JOIN songs ON songs.sourceId = song_artists.sourceId AND songs.id = song_artists.songId " + + "JOIN albums ON albums.sourceId = songs.sourceId AND albums.id = songs.albumId " + + "WHERE song_artists.sourceId = artists.sourceId AND song_artists.artistId = artists.id" + + ") albums ORDER BY albums.year DESC, albums.sortName COLLATE NOCASE, albums.id LIMIT 1" + +internal const val ARTIST_COVER_ART = + "COALESCE(artists.coverArt, CASE " + + "WHEN EXISTS (SELECT 1 FROM albums WHERE albums.sourceId = artists.sourceId AND albums.artistId = artists.id) " + + "OR EXISTS (SELECT 1 FROM album_artists " + + "WHERE album_artists.sourceId = artists.sourceId AND album_artists.artistId = artists.id) " + + "THEN ($ARTIST_ALBUM_CREDIT_ART) ELSE ($ARTIST_TRACK_CREDIT_ART) END)" + +internal const val ARTIST_COLUMNS = + "artists.sourceId, artists.id, artists.name, artists.albumCount, artists.starred, " + + "$ARTIST_COVER_ART AS coverArt, artists.playCount, artists.played, artists.songCount, " + + "artists.sortName, artists.credited" + // With the Downloaded filter on, count the albums that actually have a download so the number under // an artist matches the albums its detail screen lists. internal const val ARTISTS_SELECT = @@ -170,8 +204,8 @@ internal const val ARTISTS_SELECT = "CASE WHEN :downloadedFilter = 1 THEN (" + "SELECT COUNT(*) FROM ($DOWNLOADED_ARTIST_ALBUMS)" + ") ELSE artists.albumCount END AS albumCount, " + - "artists.starred, artists.coverArt, artists.playCount, artists.played, artists.songCount, artists.sortName, " + - "artists.credited " + "artists.starred, $ARTIST_COVER_ART AS coverArt, artists.playCount, artists.played, artists.songCount, " + + "artists.sortName, artists.credited " internal const val ARTIST_ORDER_BY_NAME = "sortName COLLATE NOCASE, id" internal const val ARTIST_ORDER_BY_NAME_REVERSED = "sortName COLLATE NOCASE DESC, id DESC" @@ -377,7 +411,7 @@ interface LibraryDao { ): Flow> @Query( - "SELECT * FROM artists WHERE sourceId = :sourceId AND played IS NOT NULL AND $ARTIST_CREDITED " + + "SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND played IS NOT NULL AND $ARTIST_CREDITED " + "ORDER BY played DESC, sortName COLLATE NOCASE, id LIMIT :limit", ) fun recentlyPlayedArtists( @@ -395,7 +429,7 @@ interface LibraryDao { ): Flow> @Query( - "SELECT * FROM artists WHERE sourceId = :sourceId AND playCount > 0 AND $ARTIST_CREDITED " + + "SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND playCount > 0 AND $ARTIST_CREDITED " + "ORDER BY playCount DESC, sortName COLLATE NOCASE, id LIMIT :limit", ) fun mostPlayedArtists( @@ -444,7 +478,7 @@ interface LibraryDao { ): Flow> @Query( - "SELECT * FROM artists WHERE sourceId = :sourceId AND $ARTIST_SEARCH_FILTER AND $ARTIST_CREDITED " + + "SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND $ARTIST_SEARCH_FILTER AND $ARTIST_CREDITED " + "ORDER BY sortName COLLATE NOCASE, id LIMIT :limit", ) fun searchArtists( @@ -856,13 +890,13 @@ interface LibraryDao { fun homeRecentlyAddedAlbums(sourceId: Long): PagingSource @Query( - "SELECT * FROM artists WHERE sourceId = :sourceId AND played IS NOT NULL AND $ARTIST_CREDITED " + + "SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND played IS NOT NULL AND $ARTIST_CREDITED " + "ORDER BY played DESC, sortName COLLATE NOCASE, id", ) fun homeRecentlyPlayedArtists(sourceId: Long): PagingSource @Query( - "SELECT * FROM artists WHERE sourceId = :sourceId AND playCount > 0 AND $ARTIST_CREDITED " + + "SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND playCount > 0 AND $ARTIST_CREDITED " + "ORDER BY playCount DESC, sortName COLLATE NOCASE, id", ) fun homeMostPlayedArtists(sourceId: Long): PagingSource @@ -921,7 +955,7 @@ interface LibraryDao { fun downloadedAlbums(sourceId: Long): PagingSource @Query( - "SELECT artists.* FROM artists " + + "SELECT $ARTIST_COLUMNS FROM artists " + "JOIN (SELECT id, MAX(downloadedAt) AS lastDownloadedAt FROM ($DOWNLOADED_ARTIST_ROWS) " + "WHERE status = 'Completed' GROUP BY id) dl ON dl.id = artists.id " + "WHERE artists.sourceId = :sourceId AND $ARTIST_CREDITED " + @@ -1020,13 +1054,13 @@ interface LibraryDao { albumId: String, ): Album? - @Query("SELECT * FROM artists WHERE sourceId = :sourceId AND id = :artistId") + @Query("SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND id = :artistId") suspend fun artistOnce( sourceId: Long, artistId: String, ): Artist? - @Query("SELECT * FROM artists WHERE sourceId = :sourceId AND id = :artistId") + @Query("SELECT $ARTIST_COLUMNS FROM artists WHERE sourceId = :sourceId AND id = :artistId") fun artist( sourceId: Long, artistId: String, diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryPaging.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryPaging.kt index 935d0b45..6e1c23ec 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryPaging.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryPaging.kt @@ -90,7 +90,7 @@ internal fun artistsDefaultQuery( descending: Boolean, ): RoomRawQuery { val order = artistOrder(sort, descending) - return RoomRawQuery("SELECT * $ARTISTS_FILTER_BASE ORDER BY $order") { stmt -> + return RoomRawQuery("SELECT $ARTIST_COLUMNS $ARTISTS_FILTER_BASE ORDER BY $order") { stmt -> stmt.bindLong(1, sourceId) stmt.bindInt(2, starredFilter) stmt.bindInt(3, starredFilter) diff --git a/app/src/main/kotlin/com/subtracks/ui/components/ArtistChooserSheet.kt b/app/src/main/kotlin/com/subtracks/ui/components/ArtistChooserSheet.kt index 0bbae3cd..314dafe7 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/ArtistChooserSheet.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/ArtistChooserSheet.kt @@ -68,10 +68,8 @@ fun ArtistChooserSheet( colors = ListItemDefaults.colors(containerColor = Color.Transparent), modifier = Modifier.clickable { - scope.launch { - sheetState.hide() - onArtistClick(artist) - } + onArtistClick(artist) + scope.launch { sheetState.hide() } }, ) } diff --git a/app/src/main/kotlin/com/subtracks/ui/components/SearchField.kt b/app/src/main/kotlin/com/subtracks/ui/components/SearchField.kt index 64a68ad8..a64fc5f1 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/SearchField.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/SearchField.kt @@ -20,6 +20,10 @@ import androidx.compose.material3.Surface import androidx.compose.material3.Text 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 @@ -28,6 +32,8 @@ import androidx.compose.ui.graphics.SolidColor import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.contentDescription import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.text.TextRange +import androidx.compose.ui.text.input.TextFieldValue import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import com.subtracks.R @@ -43,6 +49,12 @@ fun SearchField( ) { val hint = stringResource(R.string.search_input_placeholder) val closeLabel = stringResource(R.string.close_search) + var textFieldValue by remember { mutableStateOf(TextFieldValue(value, selection = TextRange(value.length))) } + LaunchedEffect(value) { + if (value != textFieldValue.text) { + textFieldValue = TextFieldValue(value, selection = TextRange(value.length)) + } + } LaunchedEffect(Unit) { focusRequester.requestFocus() } Surface( shape = RoundedCornerShape(8.dp), @@ -62,8 +74,11 @@ fun SearchField( ) Spacer(Modifier.width(8.dp)) BasicTextField( - value = value, - onValueChange = onValueChange, + value = textFieldValue, + onValueChange = { + textFieldValue = it + onValueChange(it.text) + }, singleLine = true, textStyle = MaterialTheme.typography.bodyLarge.copy(color = MaterialTheme.colorScheme.onSurface), cursorBrush = SolidColor(MaterialTheme.colorScheme.primary), @@ -73,7 +88,7 @@ fun SearchField( .focusRequester(focusRequester) .semantics { contentDescription = hint }, decorationBox = { inner -> - if (value.isEmpty()) { + if (textFieldValue.text.isEmpty()) { Text( text = hint, style = MaterialTheme.typography.bodyLarge, diff --git a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt index 533c0542..bc965fda 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt @@ -445,13 +445,6 @@ fun LibraryScreen( } } - fun dismissSearch() { - if (searchActive) { - searchActive = false - onSearchChange("") - } - } - val density = LocalDensity.current val statusBarTop = with(density) { WindowInsets.statusBars.getTop(this).toDp() } val filtersActive = listQuery.starred != StarredFilter.Any || listQuery.downloaded || search.isNotEmpty() @@ -502,10 +495,7 @@ fun LibraryScreen( albums.collectAsLazyPagingItems(), coverArt, listBottomInset, - { - dismissSearch() - onAlbumClick(it) - }, + onAlbumClick, onLongClick = onItemLongClick, filtered = filtersActive, onClearFilters = onClearFilters, @@ -522,10 +512,7 @@ fun LibraryScreen( artists.collectAsLazyPagingItems(), coverArt, listBottomInset, - { - dismissSearch() - onArtistClick(it) - }, + onArtistClick, onLongClick = onItemLongClick, filtered = filtersActive, onClearFilters = onClearFilters, @@ -542,10 +529,7 @@ fun LibraryScreen( playlists.collectAsLazyPagingItems(), coverArt, listBottomInset, - { - dismissSearch() - onPlaylistClick(it) - }, + onPlaylistClick, onLongClick = onItemLongClick, filtered = filtersActive, onClearFilters = onClearFilters, diff --git a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt index 93d87b24..b0dae594 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt @@ -24,6 +24,7 @@ import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -545,6 +546,74 @@ class LibraryDaoTest { ) } + @Test + fun artistArtFallsBackToTheFirstAlbumCreditThenTheFirstTrackCredit() = + runTest { + val sourceId = source() + val dao = db.libraryDao() + dao.upsertArtists( + listOf( + artist(sourceId, "ar-own", "Own", albumCount = 1).copy(coverArt = "own-art"), + artist(sourceId, "ar-album", "Album", albumCount = 2), + artist(sourceId, "ar-track", "Track", albumCount = 1), + artist(sourceId, "ar-null-art", "Null Art", albumCount = 2), + artist(sourceId, "ar-none", "None", albumCount = 0), + ), + ) + dao.upsertAlbums( + listOf( + album(sourceId, "own", "Own", year = 2020, starred = null).copy(coverArt = "own-album"), + album(sourceId, "album-new", "New", year = 2020, starred = null) + .copy(artistId = "ar-album", coverArt = "new-art"), + album(sourceId, "album-old", "Old", year = 2000, starred = null) + .copy(artistId = "ar-album", coverArt = "old-art"), + album(sourceId, "track", "Track", year = 2021, starred = null) + .copy(artistId = "ar-other", coverArt = "track-art"), + album(sourceId, "null-new", "Null New", year = 2022, starred = null) + .copy(artistId = "ar-null-art", coverArt = null), + album(sourceId, "null-old", "Null Old", year = 1990, starred = null) + .copy(artistId = "ar-null-art", coverArt = "later-art"), + ), + ) + dao.upsertSongs( + listOf( + song(sourceId, "s-own", "Own", starred = null).copy(albumId = "own", artistId = "ar-own"), + song(sourceId, "s-album", "Album", starred = null).copy(albumId = "album-new", artistId = "ar-album"), + song(sourceId, "s-track", "Track", starred = null).copy(albumId = "track", artistId = "ar-track"), + song(sourceId, "s-null-new", "Null New", starred = null) + .copy(albumId = "null-new", artistId = "ar-null-art"), + song(sourceId, "s-null-old", "Null Old", starred = null) + .copy(albumId = "null-old", artistId = "ar-null-art"), + ), + ) + + val artists = dao.artistsByName(sourceId, 0, "").page().associateBy { it.id } + + assertEquals("own-art", artists.getValue("ar-own").coverArt) + assertEquals("new-art", artists.getValue("ar-album").coverArt) + assertEquals("track-art", artists.getValue("ar-track").coverArt) + assertNull(artists.getValue("ar-null-art").coverArt) + assertNull(artists.getValue("ar-none").coverArt) + assertEquals("new-art", dao.artistOnce(sourceId, "ar-album")!!.coverArt) + } + + @Test + fun artistArtFallbackFindsCreditsThroughArtistIndexes() = + runTest { + val plan = plan("SELECT $ARTIST_COLUMNS $ARTISTS_FILTER ORDER BY $ARTIST_ORDER_BY_NAME LIMIT 20") + + assertTrue(plan, plan.contains("USING INDEX index_albums_sourceId_artistId (sourceId=? AND artistId=?)")) + assertTrue( + plan, + plan.contains("USING INDEX index_album_artists_sourceId_artistId (sourceId=? AND artistId=?)"), + ) + assertTrue(plan, plan.contains("USING INDEX index_songs_sourceId_artistId (sourceId=? AND artistId=?)")) + assertTrue( + plan, + plan.contains("USING INDEX index_song_artists_sourceId_artistId (sourceId=? AND artistId=?)"), + ) + } + @Test fun albumCreditIdsFindCreditsThroughArtistIndexes() = runTest { diff --git a/app/src/test/kotlin/com/subtracks/ui/LibraryScreenTest.kt b/app/src/test/kotlin/com/subtracks/ui/LibraryScreenTest.kt index 2e833ccb..9d29b19f 100644 --- a/app/src/test/kotlin/com/subtracks/ui/LibraryScreenTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/LibraryScreenTest.kt @@ -232,6 +232,46 @@ class LibraryScreenTest { assertTrue("the tab that was shown should collect its flow", collected.contains("artists")) } + @Test + fun openingAResultKeepsTheListSearchQuery() { + val search = mutableStateOf("xyz") + val show = mutableStateOf(true) + var opened: Album? = null + composeRule.setContent { + if (show.value) { + SubtracksTheme { + LibraryScreen( + selectedTab = LibraryTab.Albums, + onTabSelected = {}, + albums = remember { flowOf(PagingData.from(albums())) }, + artists = remember { flowOf(PagingData.empty()) }, + playlists = remember { flowOf(PagingData.empty()) }, + coverArt = { _, _ -> null }, + onAlbumClick = { opened = it }, + onArtistClick = {}, + onPlaylistClick = {}, + onSync = {}, + onOpenSettings = {}, + search = search.value, + onSearchChange = { search.value = it }, + ) + } + } + } + + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodesWithTag(ALBUM_COVER_TAG).fetchSemanticsNodes().isNotEmpty() + } + composeRule.onAllNodesWithTag(ALBUM_COVER_TAG)[0].performClick() + composeRule.waitForIdle() + + assertEquals("al-1", opened?.id) + assertEquals("xyz", search.value) + + composeRule.runOnIdle { show.value = false } + composeRule.waitForIdle() + } + @Test fun theTabRowReflowsInOneDirectionDuringASwitch() { val pagerState = PagerState(currentPage = 1, pageCount = { LibraryTab.entries.size }) diff --git a/app/src/test/kotlin/com/subtracks/ui/components/ArtistChooserSheetTest.kt b/app/src/test/kotlin/com/subtracks/ui/components/ArtistChooserSheetTest.kt index f85a9e68..21aa69de 100644 --- a/app/src/test/kotlin/com/subtracks/ui/components/ArtistChooserSheetTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/components/ArtistChooserSheetTest.kt @@ -45,6 +45,29 @@ class ArtistChooserSheetTest { assertEquals(ArtistRef("ar2", "B"), opened) } + @Test + fun opensTheChosenArtistBeforeTheSheetFinishesClosing() { + var opened: ArtistRef? = null + composeRule.setContent { + SubtracksTheme { + ArtistChooserSheet( + artists = listOf(ArtistRef("ar1", "A"), ArtistRef("ar2", "B")), + onArtistClick = { opened = it }, + onDismiss = {}, + ) + } + } + + composeRule.mainClock.autoAdvance = false + composeRule.onNodeWithText("B").performClick() + + assertEquals(ArtistRef("ar2", "B"), opened) + + composeRule.mainClock.advanceTimeBy(1_000) + composeRule.mainClock.autoAdvance = true + composeRule.waitForIdle() + } + @Test fun longListsScrollToTheLastArtist() { var opened: ArtistRef? = null diff --git a/app/src/test/kotlin/com/subtracks/ui/search/SearchScreenTest.kt b/app/src/test/kotlin/com/subtracks/ui/search/SearchScreenTest.kt index e732ecf8..ba9be2b9 100644 --- a/app/src/test/kotlin/com/subtracks/ui/search/SearchScreenTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/search/SearchScreenTest.kt @@ -1,10 +1,15 @@ package com.subtracks.ui.search import androidx.activity.ComponentActivity +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.hasSetTextAction import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performTextInput import androidx.test.ext.junit.runners.AndroidJUnit4 import com.github.takahirom.roborazzi.RobolectricDeviceQualifiers import com.subtracks.data.model.Album @@ -84,6 +89,30 @@ class SearchScreenTest { composeRule.onNodeWithText("No results", substring = true).assertIsDisplayed() } + @Test + fun typingResumesAtTheEndOfTheQueryAfterTheScreenIsRecreated() { + var query by mutableStateOf("road") + var show by mutableStateOf(true) + composeRule.setContent { + if (show) { + SubtracksTheme { + SearchScreen(query = query, onQueryChange = { query = it }, results = SearchResults()) + } + } + } + + composeRule.onNode(hasSetTextAction()).performTextInput("s") + assertEquals("roads", query) + + composeRule.runOnIdle { show = false } + composeRule.runOnIdle { show = true } + composeRule.onNode(hasSetTextAction()).performTextInput("!") + assertEquals("roads!", query) + + composeRule.runOnIdle { show = false } + composeRule.waitForIdle() + } + @Test fun idsSharedAcrossTypesDoNotCollide() { composeRule.setContent {