Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support pull to refresh artist details, refetching artist and url relationships #969

Merged
merged 5 commits into from
Jun 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions config/detekt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ complexity:
- "Composable"
TooManyFunctions:
active: false
exceptions:
TooGenericExceptionCaught:
active: false
formatting:
ImportOrdering:
active: false
Expand Down
2 changes: 0 additions & 2 deletions data/database/config/baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,5 @@
<ID>ReturnCount:ToRelationDatabaseModel.kt$fun RelationMusicBrainzModel.toRelationDatabaseModel( entityId: String, order: Int, ): RelationWithOrder?</ID>
<ID>SwallowedException:ArtistCreditDao.kt$ArtistCreditDao$ex: Exception</ID>
<ID>SwallowedException:CollectionEntityDao.kt$CollectionEntityDao$ex: Exception</ID>
<ID>TooGenericExceptionCaught:ArtistCreditDao.kt$ArtistCreditDao$ex: Exception</ID>
<ID>TooGenericExceptionCaught:CollectionEntityDao.kt$CollectionEntityDao$ex: Exception</ID>
</CurrentIssues>
</SmellBaseline>
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ class ArtistDao(
).executeAsOneOrNull()
}

fun delete(artistId: String) {
transacter.delete(artistId)
}

private fun toArtistScaffoldModel(
id: String,
name: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ class RelationDao(
},
)

fun deleteRelationshipsExcludingUrlsByEntity(
entityId: String,
) {
transacter.deleteRelationshipsExcludingUrlsByEntity(entityId)
}

fun getEntityUrlRelationships(
entityId: String,
): List<RelationListItemModel> {
Expand All @@ -73,6 +79,12 @@ class RelationDao(
).executeAsList()
}

fun deleteUrlRelationshipsByEntity(
entityId: String,
) {
transacter.deleteUrlRelationshipssByEntity(entityId)
}

private fun toRelationListItemModel(
linkedEntityId: String,
linkedEntity: MusicBrainzEntity,
Expand All @@ -93,12 +105,6 @@ class RelationDao(
additionalInfo = additionalInfo,
)

fun deleteRelationshipsExcludingUrlsByEntity(
entityId: String,
) {
transacter.deleteRelationshipsExcludingUrlsByEntity(entityId)
}

fun getCountOfEachRelationshipType(entityId: String): Flow<List<CountOfEachRelationshipType>> =
transacter.countOfEachRelationshipType(entityId)
.asFlow()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,7 @@ SELECT
FROM artist a
LEFT JOIN mbid_image mi ON mi.mbid = a.id
WHERE id = :artistId;

delete:
DELETE FROM artist
WHERE id = :artistId;
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ attributes LIKE :query OR additional_info LIKE :query)
ORDER BY linked_entity, label, `order`
LIMIT :limit OFFSET :offset;

deleteRelationshipsExcludingUrlsByEntity:
DELETE FROM relation
WHERE entity_id = :entityId AND linked_entity != "url";

getEntityUrlRelationships:
SELECT
`linked_entity_id`,
Expand All @@ -82,9 +86,9 @@ WHERE entity_id = :entityId AND linked_entity = "url" AND
attributes LIKE :query OR additional_info LIKE :query)
ORDER BY linked_entity, label, `order`;

deleteRelationshipsExcludingUrlsByEntity:
deleteUrlRelationshipssByEntity:
DELETE FROM relation
WHERE entity_id = :entityId AND linked_entity != "url";
WHERE entity_id = :entityId AND linked_entity = "url";

countOfEachRelationshipType:
SELECT linked_entity, count(entity_id) AS entity_count
Expand Down
1 change: 0 additions & 1 deletion data/musicbrainz/config/baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,5 @@
<ID>MaxLineLength:RelationshipHeaders.kt$"7fd5fbc0-fbf4-4d04-be23-417d50a4dc30" to ("holds phonographic copyright (℗) for" to "phonographic copyright (℗) by")</ID>
<ID>MaxLineLength:RelationshipHeaders.kt$"fc399d47-23a7-4c28-bfcf-0607a562b644" to ("transliterated/translated track listings" to "transliterated/translated track listing of")</ID>
<ID>MaxLineLength:RelationshipHeaders.kt$"fd841726-ba3c-47f7-af8e-6734ab6243ff" to ("holds phonographic copyright (℗) for" to "phonographic copyright (℗) by")</ID>
<ID>TooGenericExceptionCaught:Logout.kt$Logout$ex: Exception</ID>
</CurrentIssues>
</SmellBaseline>
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ class RelationRepositoryImpl(
entityId = entityId,
)

override fun deleteUrlRelationshipsByEntity(entityId: String) {
relationDao.deleteUrlRelationshipsByEntity(entityId)
}

override fun getCountOfEachRelationshipType(entityId: String): Flow<List<RelationTypeCount>> =
relationDao.getCountOfEachRelationshipType(entityId).map {
it.map { countOfEachRelationshipType: CountOfEachRelationshipType ->
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package ly.david.musicsearch.data.repository.artist

import ly.david.musicsearch.data.musicbrainz.models.core.ArtistMusicBrainzModel
import ly.david.musicsearch.data.musicbrainz.api.MusicBrainzApi
import ly.david.musicsearch.core.models.artist.ArtistScaffoldModel
import ly.david.musicsearch.data.database.dao.ArtistDao
import ly.david.musicsearch.data.musicbrainz.api.MusicBrainzApi
import ly.david.musicsearch.data.musicbrainz.models.core.ArtistMusicBrainzModel
import ly.david.musicsearch.data.repository.internal.toRelationWithOrderList
import ly.david.musicsearch.domain.artist.ArtistRepository
import ly.david.musicsearch.domain.relation.RelationRepository
Expand All @@ -14,19 +14,34 @@ class ArtistRepositoryImpl(
private val relationRepository: RelationRepository,
) : ArtistRepository {

override suspend fun lookupArtist(artistId: String): ArtistScaffoldModel {
override suspend fun lookupArtist(
artistId: String,
forceRefresh: Boolean,
): ArtistScaffoldModel {
if (forceRefresh) {
relationRepository.deleteUrlRelationshipsByEntity(artistId)
artistDao.delete(artistId)
}

val artistScaffoldModel = artistDao.getArtistForDetails(artistId)
val urlRelations = relationRepository.getEntityUrlRelationships(artistId)
val hasUrlsBeenSavedForEntity = relationRepository.hasUrlsBeenSavedFor(artistId)
if (artistScaffoldModel != null && hasUrlsBeenSavedForEntity) {
if (
artistScaffoldModel != null &&
hasUrlsBeenSavedForEntity &&
!forceRefresh
) {
return artistScaffoldModel.copy(
urls = urlRelations,
)
}

val artistMusicBrainzModel = musicBrainzApi.lookupArtist(artistId)
cache(artistMusicBrainzModel)
return lookupArtist(artistId)
return lookupArtist(
artistId = artistId,
forceRefresh = false,
)
}

private fun cache(artist: ArtistMusicBrainzModel) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,8 @@ package ly.david.musicsearch.domain.artist
import ly.david.musicsearch.core.models.artist.ArtistScaffoldModel

interface ArtistRepository {
suspend fun lookupArtist(artistId: String): ArtistScaffoldModel
suspend fun lookupArtist(
artistId: String,
forceRefresh: Boolean,
): ArtistScaffoldModel
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,9 @@ interface RelationRepository {
entityId: String,
): List<RelationListItemModel>

fun deleteUrlRelationshipsByEntity(
entityId: String,
)

fun getCountOfEachRelationshipType(entityId: String): Flow<List<RelationTypeCount>>
}
Original file line number Diff line number Diff line change
Expand Up @@ -170,18 +170,18 @@ internal fun AreaUi(
when (state.tabs[page]) {
AreaTab.DETAILS -> {
DetailsWithErrorHandling(
modifier = Modifier
.padding(innerPadding)
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
showError = state.isError,
onRetryClick = {
onRefresh = {
eventSink(AreaUiEvent.ForceRefresh)
},
scaffoldModel = state.area,
) {
AreaDetailsUi(
area = it,
modifier = Modifier
.padding(innerPadding)
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
filterText = state.query,
lazyListState = detailsLazyListState,
onItemClick = { entity, id, title ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import ly.david.musicsearch.core.models.artist.ArtistScaffoldModel
import ly.david.musicsearch.core.models.getNameWithDisambiguation
import ly.david.musicsearch.core.models.history.LookupHistory
import ly.david.musicsearch.core.models.network.MusicBrainzEntity
import ly.david.musicsearch.data.common.network.RecoverableNetworkException
import ly.david.musicsearch.data.spotify.ArtistImageRepository
import ly.david.musicsearch.domain.artist.ArtistRepository
import ly.david.musicsearch.domain.history.usecase.IncrementLookupHistory
Expand Down Expand Up @@ -61,6 +60,7 @@ internal class ArtistPresenter(
@Composable
override fun present(): ArtistUiState {
var title by rememberSaveable { mutableStateOf(screen.title.orEmpty()) }
var isLoading by rememberSaveable { mutableStateOf(true) }
var isError by rememberSaveable { mutableStateOf(false) }
var recordedHistory by rememberSaveable { mutableStateOf(false) }
var query by rememberSaveable { mutableStateOf("") }
Expand All @@ -87,14 +87,18 @@ internal class ArtistPresenter(

LaunchedEffect(forceRefreshDetails) {
try {
val artistScaffoldModel = repository.lookupArtist(screen.id)
isLoading = true
val artistScaffoldModel = repository.lookupArtist(
artistId = screen.id,
forceRefresh = forceRefreshDetails,
)
if (title.isEmpty()) {
title = artistScaffoldModel.getNameWithDisambiguation()
}
artist = artistScaffoldModel
imageUrl = fetchArtistImage(artistScaffoldModel)
isError = false
} catch (ex: RecoverableNetworkException) {
} catch (ex: Exception) {
logger.e(ex)
isError = true
}
Expand All @@ -109,6 +113,8 @@ internal class ArtistPresenter(
)
recordedHistory = true
}
isLoading = false
forceRefreshDetails = false
}

LaunchedEffect(
Expand Down Expand Up @@ -221,6 +227,7 @@ internal class ArtistPresenter(

return ArtistUiState(
title = title,
isLoading = isLoading,
isError = isError,
artist = artist,
imageUrl = imageUrl,
Expand Down Expand Up @@ -257,6 +264,7 @@ internal class ArtistPresenter(
@Stable
internal data class ArtistUiState(
val title: String,
val isLoading: Boolean,
val isError: Boolean,
val artist: ArtistScaffoldModel?,
val imageUrl: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,19 +152,19 @@ internal fun ArtistUi(
when (state.tabs[page]) {
ArtistTab.DETAILS -> {
DetailsWithErrorHandling(
modifier = Modifier.padding(innerPadding),
modifier = Modifier
.padding(innerPadding)
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
showLoading = state.isLoading,
showError = state.isError,
onRetryClick = {
onRefresh = {
eventSink(ArtistUiEvent.ForceRefresh)
},
scaffoldModel = state.artist,
) { artist ->
ArtistDetailsUi(
artist = artist,
modifier = Modifier
.padding(innerPadding)
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
filterText = state.query,
imageUrl = state.imageUrl,
lazyListState = detailsLazyListState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,18 @@ internal fun EventUi(
when (state.tabs[page]) {
EventTab.DETAILS -> {
DetailsWithErrorHandling(
modifier = Modifier.padding(innerPadding),
modifier = Modifier
.padding(innerPadding)
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
showError = state.isError,
onRetryClick = {
onRefresh = {
eventSink(EventUiEvent.ForceRefresh)
},
scaffoldModel = state.event,
) { event ->
EventDetailsUi(
event = event,
modifier = Modifier
.padding(innerPadding)
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
filterText = state.query,
lazyListState = detailsLazyListState,
onItemClick = { entity, id, title ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,12 @@ internal fun GenreUi(
},
) { innerPadding ->
DetailsWithErrorHandling(
modifier = Modifier.padding(innerPadding),
showError = isError,
onRetryClick = onRetryClick,
onRefresh = onRetryClick,
scaffoldModel = genre,
) {
FullScreenContent(modifier = Modifier.padding(innerPadding)) {
FullScreenContent {
Text(
modifier = Modifier.padding(16.dp),
textAlign = TextAlign.Center,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,18 @@ internal fun InstrumentUi(
when (state.tabs[page]) {
InstrumentTab.DETAILS -> {
DetailsWithErrorHandling(
modifier = Modifier.padding(innerPadding),
modifier = Modifier
.padding(innerPadding)
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
showError = state.isError,
onRetryClick = {
onRefresh = {
eventSink(InstrumentUiEvent.ForceRefresh)
},
scaffoldModel = state.instrument,
) { instrument ->
InstrumentDetailsUi(
instrument = instrument,
modifier = Modifier
.padding(innerPadding)
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
filterText = state.query,
lazyListState = detailsLazyListState,
onItemClick = { entity, id, title ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,19 +131,18 @@ internal fun LabelUi(
when (state.tabs[page]) {
LabelTab.DETAILS -> {
DetailsWithErrorHandling(
modifier = Modifier.padding(innerPadding),
modifier = Modifier
.padding(innerPadding)
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
showError = state.isError,
onRetryClick = {
onRefresh = {
eventSink(LabelUiEvent.ForceRefresh)
},
scaffoldModel = state.label,
) { label ->
LabelDetailsUi(
label = label,
modifier = Modifier
.padding(innerPadding)
.fillMaxSize()
.nestedScroll(scrollBehavior.nestedScrollConnection),
filterText = state.query,
lazyListState = detailsLazyListState,
onItemClick = { entity, id, title ->
Expand Down
Loading
Loading