From 6e39471a70ad3a3133d4999a1fc7402db7861024 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 2 Aug 2026 19:03:10 -0400 Subject: [PATCH] =?UTF-8?q?feat(discover):=20snooze=20affordance=20on=20An?= =?UTF-8?q?droid=20+=20web=20suggestion=20cards=20=E2=80=94=20#2375?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the snooze from slice 3 (#2374), so it's now touchable on both clients (rule #27 — the server side alone was never shippable). Copy is "Not right now" everywhere, never a dislike (rule #101). The parked list even says so out loud: "Nothing here counts against your taste profile." Both clients flip the card in place to a "Not right now" state with an Undo, rather than yanking it out of the grid under the cursor. The row leaves on the next refetch; the persistent way back is a parked-list section below the deck. That list isn't optional garnish — a snoozed candidate is by definition absent from the deck, so without it the DELETE endpoint is unreachable. Android routes the write through the offline MutationQueue per rule #100, as ONE toggle kind (SUGGESTION_SNOOZE_TOGGLE) carrying the desired state rather than two action kinds. That reuses the LIKE_TOGGLE collapse: a queued snooze the user has since undone is dropped unsent instead of replaying after the undo and re-hiding an artist they asked to see. The collapse helper is now a pure top-level function so that rule is unit tested rather than inferred. The repository does NOT enqueue on a 4xx — a permanent rejection would replay to the same failure and would raise a misleading "will sync when online" hint. The common case is a 404 from un-snoozing a row that already lapsed, which is the user's intended end state anyway. Also: an empty deck used to have one meaning (no listening signal yet). It can now also mean "you parked them all", so the empty copy branches — telling that user to go listen to something would be wrong advice. Co-Authored-By: Claude Opus 5 (1M context) --- .../minstrel/api/endpoints/DiscoverApi.kt | 30 ++++ .../minstrel/cache/mutations/MutationQueue.kt | 40 +++++ .../cache/mutations/MutationReplayer.kt | 93 +++++++--- .../discover/data/DiscoverRepository.kt | 80 +++++++++ .../minstrel/discover/ui/DiscoverScreen.kt | 80 ++++++++- .../minstrel/discover/ui/DiscoverTiles.kt | 67 +++++++- .../minstrel/discover/ui/DiscoverViewModel.kt | 53 ++++++ .../fabledsword/minstrel/models/Discover.kt | 56 ++++++ .../minstrel/models/wire/DiscoverWire.kt | 30 ++++ .../mutations/SupersededToggleIdsTest.kt | 132 ++++++++++++++ .../models/SuggestionSnoozeRefTest.kt | 67 ++++++++ web/src/lib/api/queries.ts | 1 + web/src/lib/api/suggestions.test.ts | 64 ++++++- web/src/lib/api/suggestions.ts | 35 +++- web/src/lib/api/types.ts | 10 ++ .../lib/components/DiscoverResultCard.svelte | 70 +++++++- .../lib/components/DiscoverResultCard.test.ts | 43 +++++ web/src/lib/components/SuggestionFeed.svelte | 131 +++++++++++++- web/src/lib/components/SuggestionFeed.test.ts | 162 +++++++++++++++--- 19 files changed, 1176 insertions(+), 68 deletions(-) create mode 100644 android/app/src/test/java/com/fabledsword/minstrel/cache/mutations/SupersededToggleIdsTest.kt create mode 100644 android/app/src/test/java/com/fabledsword/minstrel/models/SuggestionSnoozeRefTest.kt diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/DiscoverApi.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/DiscoverApi.kt index 9614eacf..cd0215ea 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/DiscoverApi.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/DiscoverApi.kt @@ -3,9 +3,13 @@ package com.fabledsword.minstrel.api.endpoints import com.fabledsword.minstrel.models.wire.ArtistSuggestionWire import com.fabledsword.minstrel.models.wire.CreateRequestBody import com.fabledsword.minstrel.models.wire.LidarrSearchResultWire +import com.fabledsword.minstrel.models.wire.SnoozeSuggestionBody +import com.fabledsword.minstrel.models.wire.SuggestionSnoozeWire import retrofit2.http.Body +import retrofit2.http.DELETE import retrofit2.http.GET import retrofit2.http.POST +import retrofit2.http.Path import retrofit2.http.Query /** @@ -30,4 +34,30 @@ interface DiscoverApi { @POST("api/requests") suspend fun createRequest(@Body body: CreateRequestBody) + + /** + * Parks a suggestion — "not right now", NOT a dislike. Time-boxed + * server-side (90 days) and never fed into the taste profile. + * + * [body] must carry the artist's name: candidates are out-of-library, so + * the server has no local row to resolve a display name from and returns + * 400 without it. + */ + @POST("api/discover/suggestions/{mbid}/snooze") + suspend fun snoozeSuggestion( + @Path("mbid") mbid: String, + @Body body: SnoozeSuggestionBody, + ) + + /** Brings a parked suggestion back. 404 when it wasn't snoozed. */ + @DELETE("api/discover/suggestions/{mbid}/snooze") + suspend fun unsnoozeSuggestion(@Path("mbid") mbid: String) + + /** + * Currently-parked suggestions. Server filters expired rows, so every + * row returned is still snoozed. This is the only route back to an + * un-snooze once the card has left the deck. + */ + @GET("api/discover/snoozes") + suspend fun listSnoozes(): List } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt index 79dda6ce..84a0bdf0 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationQueue.kt @@ -35,6 +35,12 @@ object MutationKind { // background avoids the duplicate + orphan row the old offline-on-stop // path produced (see 2026-06-11 contract audit). const val PLAY_ENDED: String = "play_ended" + + // #2374 suggestion snooze. ONE toggle kind rather than separate + // snooze/unsnooze kinds, mirroring LIKE_TOGGLE, so a snooze followed by + // an undo collapses to the latest intent instead of replaying as two + // opposed calls whose order decides the outcome. + const val SUGGESTION_SNOOZE_TOGGLE: String = "suggestion_snooze_toggle" } /** @@ -152,6 +158,25 @@ class MutationQueue @Inject constructor( ), ) + /** + * Queues a suggestion snooze (or its undo) for replay. [desiredSnoozed] + * is the TARGET state, so repeated taps collapse to one replay. + * + * [name] is carried even for an un-snooze, where the server ignores it, + * so a single payload shape serves both directions. + */ + suspend fun enqueueSuggestionSnoozeToggle( + mbid: String, + name: String, + desiredSnoozed: Boolean, + ): Long = insertUserDriven( + MutationKind.SUGGESTION_SNOOZE_TOGGLE, + json.encodeToString( + SuggestionSnoozeTogglePayload.serializer(), + SuggestionSnoozeTogglePayload(mbid, name, desiredSnoozed), + ), + ) + suspend fun enqueueRequestCancel(requestId: String): Long = insertUserDriven( MutationKind.REQUEST_CANCEL, json.encodeToString( @@ -192,6 +217,21 @@ class MutationQueue @Inject constructor( } } +/** + * Persisted payload for `MutationKind.SUGGESTION_SNOOZE_TOGGLE` (#2374). + * `desiredSnoozed` is the *target* state, matching [LikeTogglePayload], so + * the replayer can collapse repeated toggles for one candidate down to the + * last intent. Both directions are idempotent server-side: re-snoozing + * extends the window, and un-snoozing something already back is a 404 the + * replayer treats as permanent (nothing left to do). + */ +@Serializable +data class SuggestionSnoozeTogglePayload( + val mbid: String, + val name: String, + val desiredSnoozed: Boolean, +) + /** * Persisted payload for `MutationKind.QUARANTINE_UNFLAG` — the * `DELETE /api/quarantine/{trackId}` call lost during a connectivity diff --git a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt index c3ddfb2d..ef54a76a 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/cache/mutations/MutationReplayer.kt @@ -16,6 +16,7 @@ import com.fabledsword.minstrel.connectivity.NetworkStatusController import com.fabledsword.minstrel.connectivity.ServerHealth import com.fabledsword.minstrel.models.wire.PlayEndedRequest import com.fabledsword.minstrel.models.wire.PlayOfflineRequest +import com.fabledsword.minstrel.models.wire.SnoozeSuggestionBody import com.fabledsword.minstrel.auth.AuthStore import com.fabledsword.minstrel.cache.db.dao.CachedMutationDao import com.fabledsword.minstrel.cache.db.entities.CachedMutationEntity @@ -114,11 +115,12 @@ class MutationReplayer @Inject constructor( private suspend fun drain() { val rows = dao.getAll() - // Collapse superseded like-toggles: only the latest desired state per - // (entity) is replayed; older toggles for the same entity are dropped - // unsent. Without this, partial-failure + differential retry could - // replay an older toggle last and invert the final like state. - val superseded = supersededLikeToggleIds(rows) + // Collapse superseded toggles (likes, suggestion snoozes): only the + // latest desired state per entity is replayed; older toggles for the + // same entity are dropped unsent. Without this, partial-failure + + // differential retry could replay an older toggle last and invert the + // final state — a snooze the user already undid would come back. + val superseded = supersededToggleIds(rows, json) for (row in rows) { if (row.id in superseded) { dao.delete(row.id) @@ -131,25 +133,6 @@ class MutationReplayer @Inject constructor( } } - /** Row ids of like-toggles superseded by a later toggle for the same entity. */ - private fun supersededLikeToggleIds(rows: List): Set { - val latestByEntity = HashMap() - val superseded = HashSet() - rows.asSequence() - .filter { it.kind == MutationKind.LIKE_TOGGLE } - .forEach { row -> - val decoded = runCatching { - json.decodeFromString(LikeTogglePayload.serializer(), row.payload) - }.getOrNull() - if (decoded != null) { - val key = "${decoded.entityType}:${decoded.entityId}" - // `rows` is ascending by id, so a prior entry is always older. - latestByEntity.put(key, row.id)?.let(superseded::add) - } - } - return superseded - } - private suspend fun outcomeFor(row: CachedMutationEntity): Outcome = try { dispatch(row) } catch (e: HttpException) { @@ -182,6 +165,7 @@ class MutationReplayer @Inject constructor( MutationKind.PLAY_ENDED -> dispatchPlayEnded(row.payload) MutationKind.REQUEST_CANCEL -> dispatchRequestCancel(row.payload) MutationKind.PLAYBACK_ERROR_REPORT -> dispatchPlaybackErrorReport(row.payload) + MutationKind.SUGGESTION_SNOOZE_TOGGLE -> dispatchSuggestionSnoozeToggle(row.payload) // Unknown kind — drop so a stale schema entry can't wedge the queue. else -> Outcome.DROP } @@ -277,6 +261,24 @@ class MutationReplayer @Inject constructor( return Outcome.SENT } + /** + * Replays a suggestion snooze in whichever direction the payload asks for. + * + * The un-snooze branch can legitimately 404 (the row already lapsed, or a + * previous attempt landed and the response was lost). [outcomeFor] classes + * 404 as permanent → DROP, which is right: the user's intended end state + * already holds, so there is nothing left to send. + */ + private suspend fun dispatchSuggestionSnoozeToggle(payload: String): Outcome { + val decoded = json.decodeFromString(SuggestionSnoozeTogglePayload.serializer(), payload) + if (decoded.desiredSnoozed) { + discoverApi.snoozeSuggestion(decoded.mbid, SnoozeSuggestionBody(name = decoded.name)) + } else { + discoverApi.unsnoozeSuggestion(decoded.mbid) + } + return Outcome.SENT + } + private suspend fun dispatchPlaybackErrorReport(payload: String): Outcome { val decoded = json.decodeFromString(PlaybackErrorReportPayload.serializer(), payload) playbackErrorsApi.report( @@ -297,3 +299,46 @@ class MutationReplayer @Inject constructor( const val HTTP_TOO_MANY = 429 } } + +/** + * Row ids of desired-state toggles superseded by a later toggle for the same + * entity. Applies to every kind whose payload encodes a TARGET state rather + * than an action — like-toggles and suggestion snoozes (#2374) — because + * replaying a stale one last would invert the final state. + * + * Top-level and pure so it can be unit-tested without standing up a Retrofit + * instance. [rows] must be ascending by id (FIFO), which is what + * `CachedMutationDao.getAll()` returns. + */ +internal fun supersededToggleIds(rows: List, json: Json): Set { + val latestByEntity = HashMap() + val superseded = HashSet() + rows.asSequence() + .mapNotNull { row -> toggleKeyOf(row, json)?.let { key -> key to row.id } } + .forEach { (key, id) -> + // Ascending ids mean a prior entry for this key is always older. + latestByEntity.put(key, id)?.let(superseded::add) + } + return superseded +} + +/** + * Collapse key for a toggle row, or null when the row isn't a toggle — or its + * payload won't decode. Undecodable rows are deliberately left alone rather + * than grouped under a shared "corrupt" key, so one bad row can't suppress a + * good one behind it; the dispatcher DROPs it on its own. + * + * The kind is part of the key so two toggle kinds can never collide on the + * same entity id. + */ +private fun toggleKeyOf(row: CachedMutationEntity, json: Json): String? = when (row.kind) { + MutationKind.LIKE_TOGGLE -> runCatching { + json.decodeFromString(LikeTogglePayload.serializer(), row.payload) + }.getOrNull()?.let { "${row.kind}:${it.entityType}:${it.entityId}" } + + MutationKind.SUGGESTION_SNOOZE_TOGGLE -> runCatching { + json.decodeFromString(SuggestionSnoozeTogglePayload.serializer(), row.payload) + }.getOrNull()?.let { "${row.kind}:${it.mbid}" } + + else -> null +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/discover/data/DiscoverRepository.kt b/android/app/src/main/java/com/fabledsword/minstrel/discover/data/DiscoverRepository.kt index 2d561fd4..c89178e4 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/discover/data/DiscoverRepository.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/discover/data/DiscoverRepository.kt @@ -7,10 +7,14 @@ import com.fabledsword.minstrel.models.ArtistSuggestionRef import com.fabledsword.minstrel.models.LidarrRequestKind import com.fabledsword.minstrel.models.LidarrSearchResultRef import com.fabledsword.minstrel.models.SeedContributionRef +import com.fabledsword.minstrel.models.SuggestionSnoozeRef import com.fabledsword.minstrel.models.wire.ArtistSuggestionWire import com.fabledsword.minstrel.models.wire.CreateRequestBody import com.fabledsword.minstrel.models.wire.LidarrSearchResultWire import com.fabledsword.minstrel.models.wire.SeedContributionWire +import com.fabledsword.minstrel.models.wire.SnoozeSuggestionBody +import com.fabledsword.minstrel.models.wire.SuggestionSnoozeWire +import retrofit2.HttpException import retrofit2.Retrofit import retrofit2.create import javax.inject.Inject @@ -46,6 +50,69 @@ class DiscoverRepository @Inject constructor( suspend fun listSuggestions(): List = api.listSuggestions().map { it.toDomain() } + suspend fun listSnoozes(): List = + api.listSnoozes().map { it.toDomain() } + + /** + * Parks a suggestion ("not right now"). Offline-first per rule #100: on + * transport failure the target state is queued for the replayer rather + * than dropped. + * + * Always reports success to the caller. Unlike a request, a snooze has no + * meaningful failed state to show — the user asked for a card to go away, + * and it will, either now or when the queue drains. + */ + suspend fun snoozeSuggestion(mbid: String, name: String): Unit = toggleSnooze( + mbid = mbid, + name = name, + desiredSnoozed = true, + ) { api.snoozeSuggestion(mbid, SnoozeSuggestionBody(name = name)) } + + /** Brings a parked suggestion back. Same offline-first contract. */ + suspend fun unsnoozeSuggestion(mbid: String, name: String): Unit = toggleSnooze( + mbid = mbid, + name = name, + desiredSnoozed = false, + ) { api.unsnoozeSuggestion(mbid) } + + private suspend fun toggleSnooze( + mbid: String, + name: String, + desiredSnoozed: Boolean, + call: suspend () -> Unit, + ) { + try { + call() + } catch (e: HttpException) { + // A 4xx is the server's considered answer, not a lost call, so + // queueing it would be wrong twice over: the replay is guaranteed + // to fail again, and the enqueue would raise a "will sync when + // online" snackbar for something already settled. The common case + // is a 404 from un-snoozing a row that already lapsed — which is + // the end state the user wanted anyway. + if (!isPermanent(e.code())) { + mutationQueue.enqueueSuggestionSnoozeToggle(mbid, name, desiredSnoozed) + } + } catch ( + @Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable, + ) { + // Transport failure — intentional swallow, same offline-first + // rationale as createRequest above. The queue carries the desired + // STATE, so a later undo supersedes this rather than fighting it + // on replay. + mutationQueue.enqueueSuggestionSnoozeToggle(mbid, name, desiredSnoozed) + } + } + + /** + * Mirrors MutationReplayer's classification so the enqueue decision here + * and the drop decision there can't disagree: 4xx is permanent except the + * two "retry me" statuses. + */ + private fun isPermanent(code: Int): Boolean = + code in HTTP_CLIENT_ERR_MIN..HTTP_CLIENT_ERR_MAX && + code != HTTP_TIMEOUT && code != HTTP_TOO_MANY + suspend fun search(query: String, kind: LidarrRequestKind): List = api.search(query = query, kind = kind.wire).map { it.toDomain() } @@ -85,6 +152,13 @@ class DiscoverRepository @Inject constructor( RequestOutcome.QUEUED } } + + private companion object { + const val HTTP_CLIENT_ERR_MIN = 400 + const val HTTP_CLIENT_ERR_MAX = 499 + const val HTTP_TIMEOUT = 408 + const val HTTP_TOO_MANY = 429 + } } // ── Mappers (internal — wire types stay out of UI) ── @@ -112,6 +186,12 @@ private fun SeedContributionWire.toDomain(): SeedContributionRef = SeedContribut isLiked = isLiked, ) +private fun SuggestionSnoozeWire.toDomain(): SuggestionSnoozeRef = SuggestionSnoozeRef( + mbid = mbid, + name = name, + snoozedUntil = snoozedUntil, +) + private fun RequestCreatePayload.toBody(): CreateRequestBody = CreateRequestBody( kind = kind, artistMbid = artistMbid, diff --git a/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverScreen.kt index 56630ac7..02a21224 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverScreen.kt @@ -40,6 +40,7 @@ import com.fabledsword.minstrel.discover.data.RequestOutcome import com.fabledsword.minstrel.models.ArtistSuggestionRef import com.fabledsword.minstrel.models.LidarrRequestKind import com.fabledsword.minstrel.models.LidarrSearchResultRef +import com.fabledsword.minstrel.models.SuggestionSnoozeRef import com.fabledsword.minstrel.nav.Discover import com.fabledsword.minstrel.shared.widgets.ErrorRetry import com.fabledsword.minstrel.shared.widgets.LoadingCentered @@ -104,6 +105,18 @@ private fun DiscoverBody( ResultsState.Idle -> SuggestionsPane( state = state.suggestions, locallyRequestedMbids = state.locallyRequestedMbids, + snoozeUi = SnoozeUi( + locallySnoozedMbids = state.locallySnoozedMbids, + snoozes = state.snoozes, + // No snackbar on snooze: the row itself flips to "Not + // right now" with an Undo, so a snackbar would only + // repeat what the user can already see — and cover the + // next row while doing it. + onSnooze = { s -> scope.launch { viewModel.snoozeSuggestion(s) } }, + onUnsnooze = { mbid, name -> + scope.launch { viewModel.unsnoozeSuggestion(mbid, name) } + }, + ), onRequest = { s -> scope.launch { val outcome = viewModel.requestSuggestion(s) @@ -178,10 +191,23 @@ private fun KindChips(kind: LidarrRequestKind, onChange: (LidarrRequestKind) -> } } +/** + * The snooze surface's data and callbacks, bundled rather than threaded + * through as four more parameters — the pane grew from one action to three + * with slice 4 and the signatures stopped being readable. + */ +private data class SnoozeUi( + val locallySnoozedMbids: Set, + val snoozes: List, + val onSnooze: (ArtistSuggestionRef) -> Unit, + val onUnsnooze: (String, String) -> Unit, +) + @Composable private fun SuggestionsPane( state: SuggestionState, locallyRequestedMbids: Set, + snoozeUi: SnoozeUi, onRequest: (ArtistSuggestionRef) -> Unit, onRetry: () -> Unit, ) { @@ -194,6 +220,7 @@ private fun SuggestionsPane( ) is SuggestionState.Loaded -> SuggestionsList( items = state.items.filter { it.mbid !in locallyRequestedMbids }, + snoozeUi = snoozeUi, onRequest = onRequest, ) } @@ -202,6 +229,7 @@ private fun SuggestionsPane( @Composable private fun SuggestionsList( items: List, + snoozeUi: SnoozeUi, onRequest: (ArtistSuggestionRef) -> Unit, ) { LazyColumn( @@ -210,13 +238,61 @@ private fun SuggestionsList( ) { item { SuggestionsHeader() } if (items.isEmpty()) { - item { CenteredMessage("Listen to or like an artist to fill this in.") } + // An empty deck used to mean one thing — no listening signal yet. + // With snoozing it can also mean "you parked them all", and telling + // that user to go listen to something would be wrong advice. + item { + CenteredMessage( + if (snoozeUi.snoozes.isEmpty()) { + "Listen to or like an artist to fill this in." + } else { + "Nothing new right now — the artists you've parked are below." + }, + ) + } } else { items(items = items, key = { it.mbid }) { s -> - SuggestionTile(s = s, onRequest = { onRequest(s) }) + SuggestionTile( + s = s, + snoozed = s.mbid in snoozeUi.locallySnoozedMbids, + onRequest = { onRequest(s) }, + onSnooze = { snoozeUi.onSnooze(s) }, + onUnsnooze = { snoozeUi.onUnsnooze(s.mbid, s.name) }, + ) HorizontalDivider() } } + // Parked candidates live at the bottom of the same scroll, not behind a + // separate screen: it's a short list the user rarely needs, but it must + // be reachable — a snoozed candidate is gone from the deck above, so + // this is the only way back to it. + if (snoozeUi.snoozes.isNotEmpty()) { + item { SnoozedHeader() } + items(items = snoozeUi.snoozes, key = { "snoozed-${it.mbid}" }) { row -> + SnoozedTile( + row = row, + onUnsnooze = { snoozeUi.onUnsnooze(row.mbid, row.name) }, + ) + HorizontalDivider() + } + } + } +} + +@Composable +private fun SnoozedHeader() { + Column(modifier = Modifier.padding(horizontal = 16.dp, vertical = 12.dp)) { + HorizontalDivider(modifier = Modifier.padding(bottom = 12.dp)) + Text( + text = "Not right now", + style = MaterialTheme.typography.titleMedium, + color = MaterialTheme.colorScheme.onBackground, + ) + Text( + text = "These come back on their own. Nothing here counts against your taste profile.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) } } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverTiles.kt b/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverTiles.kt index 613c7bae..1d18d392 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverTiles.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverTiles.kt @@ -14,8 +14,10 @@ import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.AssistChip import androidx.compose.material3.Button import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -24,14 +26,22 @@ import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import coil3.compose.AsyncImage +import com.composables.icons.lucide.Clock import com.composables.icons.lucide.Disc3 import com.composables.icons.lucide.Lucide import com.composables.icons.lucide.User import com.fabledsword.minstrel.models.ArtistSuggestionRef import com.fabledsword.minstrel.models.LidarrSearchResultRef +import com.fabledsword.minstrel.models.SuggestionSnoozeRef @Composable -internal fun SuggestionTile(s: ArtistSuggestionRef, onRequest: () -> Unit) { +internal fun SuggestionTile( + s: ArtistSuggestionRef, + snoozed: Boolean, + onRequest: () -> Unit, + onSnooze: () -> Unit, + onUnsnooze: () -> Unit, +) { Row( modifier = Modifier .fillMaxWidth() @@ -48,9 +58,13 @@ internal fun SuggestionTile(s: ArtistSuggestionRef, onRequest: () -> Unit) { maxLines = 1, overflow = TextOverflow.Ellipsis, ) - if (s.attributionText.isNotEmpty()) { + // Once parked, the "because you liked X" line is no longer the + // useful thing to say — confirming what just happened is. + val secondary = + if (snoozed) "Not right now — hidden for a while" else s.attributionText + if (secondary.isNotEmpty()) { Text( - text = s.attributionText, + text = secondary, style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, maxLines = 2, @@ -58,7 +72,52 @@ internal fun SuggestionTile(s: ArtistSuggestionRef, onRequest: () -> Unit) { ) } } - Button(onClick = onRequest) { Text("Request") } + if (snoozed) { + TextButton(onClick = onUnsnooze) { Text("Undo") } + } else { + Button(onClick = onRequest) { Text("Request") } + IconButton(onClick = onSnooze) { + Icon( + imageVector = Lucide.Clock, + // Rule #101: the label states what happens, and passes no + // judgement on the music. Never "not for me". + contentDescription = "Not right now — hide ${s.name} for a while", + tint = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + } +} + +/** + * One row of the parked list. This exists because a snoozed candidate is by + * definition absent from the deck above, so without it there is no route back + * to an un-snooze once the card has gone. + */ +@Composable +internal fun SnoozedTile(row: SuggestionSnoozeRef, onUnsnooze: () -> Unit) { + Row( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp, vertical = 8.dp), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(12.dp), + ) { + Column(modifier = Modifier.weight(1f)) { + Text( + text = row.name, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurface, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + Text( + text = "Back ${row.returnsIn()}", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + TextButton(onClick = onUnsnooze) { Text("Bring back") } } } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverViewModel.kt b/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverViewModel.kt index ab0c7997..f26011c3 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverViewModel.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/discover/ui/DiscoverViewModel.kt @@ -10,6 +10,7 @@ import com.fabledsword.minstrel.discover.data.RequestOutcome import com.fabledsword.minstrel.models.ArtistSuggestionRef import com.fabledsword.minstrel.models.LidarrRequestKind import com.fabledsword.minstrel.models.LidarrSearchResultRef +import com.fabledsword.minstrel.models.SuggestionSnoozeRef import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Job import kotlinx.coroutines.flow.MutableStateFlow @@ -27,6 +28,17 @@ data class DiscoverState( val suggestions: SuggestionState = SuggestionState.Loading, val results: ResultsState = ResultsState.Idle, val locallyRequestedMbids: Set = emptySet(), + /** + * Parked candidates, for the manage list under the feed. Empty is the + * normal case and hides the section entirely. + */ + val snoozes: List = emptyList(), + /** + * Just-snoozed MBIDs. These keep their row visible showing an Undo rather + * than yanking it out from under the user's finger; the row is gone on the + * next load, and [snoozes] is the way back after that. + */ + val locallySnoozedMbids: Set = emptySet(), ) sealed interface SuggestionState { @@ -96,6 +108,47 @@ class DiscoverViewModel @Inject constructor( ) } } + // Refresh the parked list alongside the deck: a snooze made on another + // client should show up here, and one whose window lapsed should drop + // off. Sequenced after the deck load rather than raced with it so the + // two panes can't disagree about a candidate mid-refresh. + loadSnoozes() + } + + /** + * Loads the parked list. Failure is deliberately silent: this is a + * secondary pane, and an error banner for it would sit above the suggestion + * feed the user actually came for. The list stays as-is and the next + * refresh retries. + */ + private suspend fun loadSnoozes() { + try { + val rows = repository.listSnoozes() + internal.update { it.copy(snoozes = rows) } + } catch ( + @Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable, + ) { + // Keep whatever we last showed rather than blanking the section. + } + } + + /** + * Parks a suggestion. Flips the row locally first so the tap registers + * immediately; the repository handles the offline case, so there is no + * failure branch to revert here — unlike the web client, where the fetch + * either lands or doesn't. + */ + suspend fun snoozeSuggestion(s: ArtistSuggestionRef) { + internal.update { it.copy(locallySnoozedMbids = it.locallySnoozedMbids + s.mbid) } + repository.snoozeSuggestion(s.mbid, s.name) + loadSnoozes() + } + + /** Brings a parked suggestion back, from either the card or the list. */ + suspend fun unsnoozeSuggestion(mbid: String, name: String) { + internal.update { it.copy(locallySnoozedMbids = it.locallySnoozedMbids - mbid) } + repository.unsnoozeSuggestion(mbid, name) + loadSnoozes() } fun runSearch() { diff --git a/android/app/src/main/java/com/fabledsword/minstrel/models/Discover.kt b/android/app/src/main/java/com/fabledsword/minstrel/models/Discover.kt index b04e9fe8..4c1b39e4 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/models/Discover.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/models/Discover.kt @@ -1,5 +1,8 @@ package com.fabledsword.minstrel.models +import kotlinx.datetime.Instant +import kotlin.math.roundToInt + /** * Kind of Lidarr request being created. Wire form is the lowercase * enum name; the helper [wire] keeps that mapping in one place. @@ -67,3 +70,56 @@ data class ArtistSuggestionRef( private const val MAX_ATTRIBUTION_PHRASES = 3 } } + +/** + * A suggestion the user parked with "not right now" (#2374). + * + * Deliberately NOT a dislike: it carries no verdict on the artist, expires on + * its own, and never reaches the taste profile. Anything that treats this as + * negative preference signal is a bug. + * + * [snoozedUntil] is the raw RFC3339 string from the wire. Only the server + * decides whether a snooze is still in effect — every row the client receives + * already is — so this is read purely to phrase "back in about 3 months". + */ +data class SuggestionSnoozeRef( + val mbid: String, + val name: String, + val snoozedUntil: String, +) { + /** + * Relative return phrase for the manage list. Relative rather than a + * calendar date because the exact day a 90-day snooze lapses is noise the + * user never asked for. + * + * [nowMs] is injectable so this is testable without freezing the clock. + * Returns "shortly" for an unparseable or already-past timestamp: the row + * is on screen, so the server still considers it snoozed, and guessing is + * better than rendering an empty line. + */ + fun returnsIn(nowMs: Long = System.currentTimeMillis()): String { + val untilMs = runCatching { Instant.parse(snoozedUntil).toEpochMilliseconds() } + .getOrNull() ?: return "shortly" + // Already lapsed by our clock, yet the server still returned it — the + // two disagree. Say something plausible rather than "today", which + // would read as a real prediction. + if (untilMs <= nowMs) return "shortly" + val days = ((untilMs - nowMs).toDouble() / MILLIS_PER_DAY).roundToInt() + return when { + days < 1 -> "today" + days == 1 -> "tomorrow" + days < DAYS_BEFORE_MONTHS -> "in $days days" + else -> { + val months = (days.toDouble() / DAYS_PER_MONTH).roundToInt() + if (months == 1) "in about a month" else "in about $months months" + } + } + } + + private companion object { + const val MILLIS_PER_DAY = 86_400_000.0 + // Below this, days read more naturally than a rounded month count. + const val DAYS_BEFORE_MONTHS = 45 + const val DAYS_PER_MONTH = 30.0 + } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/DiscoverWire.kt b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/DiscoverWire.kt index bf09d37d..5ad6ac60 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/models/wire/DiscoverWire.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/models/wire/DiscoverWire.kt @@ -48,6 +48,36 @@ data class SeedContributionWire( @SerialName("is_liked") val isLiked: Boolean = false, ) +/** + * One row of `GET /api/discover/snoozes` — a suggestion the user parked + * with "not right now". The server only returns rows that are still in + * effect, so the client never compares [snoozedUntil] against the clock to + * decide whether to show it; it reads it only to say when the artist comes + * back. + */ +@Serializable +data class SuggestionSnoozeWire( + val mbid: String = "", + val name: String = "", + @SerialName("snoozed_until") val snoozedUntil: String = "", + @SerialName("created_at") val createdAt: String = "", +) + +/** + * Body for `POST /api/discover/suggestions/{mbid}/snooze`. + * + * [name] is required by the server, not decorative: suggestions are + * out-of-library, so there is no artists row to resolve a display name from + * and the snooze list would have nothing to render. Omitting it is a 400. + * + * No `days` field. The duration is the server's to own (90 days); pinning it + * client-side would freeze the default at whatever this build shipped. + */ +@Serializable +data class SnoozeSuggestionBody( + val name: String, +) + /** * Body posted to `POST /api/requests`. Mirrors the Flutter `createRequest` * payload shape. Optional fields are emitted only when non-null diff --git a/android/app/src/test/java/com/fabledsword/minstrel/cache/mutations/SupersededToggleIdsTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/cache/mutations/SupersededToggleIdsTest.kt new file mode 100644 index 00000000..b280ab80 --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/minstrel/cache/mutations/SupersededToggleIdsTest.kt @@ -0,0 +1,132 @@ +package com.fabledsword.minstrel.cache.mutations + +import com.fabledsword.minstrel.cache.db.entities.CachedMutationEntity +import kotlinx.serialization.json.Json +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Collapse rules for desired-state toggles in the offline queue. + * + * The hazard this guards against is real and silent: without collapsing, a + * queued snooze that replays AFTER the user's undo re-hides an artist they + * asked to see again, and nothing surfaces the contradiction. + */ +class SupersededToggleIdsTest { + + private val json = Json { ignoreUnknownKeys = true } + + private fun snoozeRow(id: Long, mbid: String, desiredSnoozed: Boolean) = CachedMutationEntity( + id = id, + kind = MutationKind.SUGGESTION_SNOOZE_TOGGLE, + payload = json.encodeToString( + SuggestionSnoozeTogglePayload.serializer(), + SuggestionSnoozeTogglePayload(mbid, "Name", desiredSnoozed), + ), + ) + + private fun likeRow(id: Long, entityId: String, desired: Boolean) = CachedMutationEntity( + id = id, + kind = MutationKind.LIKE_TOGGLE, + payload = json.encodeToString( + LikeTogglePayload.serializer(), + LikeTogglePayload("artist", entityId, desired), + ), + ) + + @Test + fun `a snooze followed by its undo drops the snooze`() { + val rows = listOf( + snoozeRow(1, "mb-a", desiredSnoozed = true), + snoozeRow(2, "mb-a", desiredSnoozed = false), + ) + // Only the later intent (the undo) survives to be replayed. + assertEquals(setOf(1L), supersededToggleIds(rows, json)) + } + + @Test + fun `toggles for different candidates never collapse into each other`() { + val rows = listOf( + snoozeRow(1, "mb-a", desiredSnoozed = true), + snoozeRow(2, "mb-b", desiredSnoozed = true), + ) + assertTrue(supersededToggleIds(rows, json).isEmpty()) + } + + @Test + fun `only the newest of several toggles for one candidate survives`() { + val rows = listOf( + snoozeRow(1, "mb-a", desiredSnoozed = true), + snoozeRow(2, "mb-a", desiredSnoozed = false), + snoozeRow(3, "mb-a", desiredSnoozed = true), + ) + assertEquals(setOf(1L, 2L), supersededToggleIds(rows, json)) + } + + // The kind is part of the collapse key, so a snooze and a like that happen + // to share an id string must not shadow one another. + @Test + fun `a like and a snooze on the same id string do not collide`() { + val rows = listOf( + likeRow(1, "same-id", desired = true), + snoozeRow(2, "same-id", desiredSnoozed = true), + ) + assertTrue(supersededToggleIds(rows, json).isEmpty()) + } + + @Test + fun `like toggles still collapse — the pre-existing behaviour is intact`() { + val rows = listOf( + likeRow(1, "artist-1", desired = true), + likeRow(2, "artist-1", desired = false), + ) + assertEquals(setOf(1L), supersededToggleIds(rows, json)) + } + + @Test + fun `non-toggle kinds are never collapsed, even repeated for one entity`() { + // Two appends to the same playlist are two real actions, not one + // desired state — collapsing them would lose a write. + val rows = listOf( + CachedMutationEntity( + id = 1, + kind = MutationKind.PLAYLIST_APPEND, + payload = json.encodeToString( + PlaylistAppendPayload.serializer(), + PlaylistAppendPayload("pl-1", listOf("t1")), + ), + ), + CachedMutationEntity( + id = 2, + kind = MutationKind.PLAYLIST_APPEND, + payload = json.encodeToString( + PlaylistAppendPayload.serializer(), + PlaylistAppendPayload("pl-1", listOf("t2")), + ), + ), + ) + assertTrue(supersededToggleIds(rows, json).isEmpty()) + } + + // A row whose payload won't decode gets no key at all, rather than sharing + // a "corrupt" bucket — otherwise one bad row could suppress a good one + // behind it. The dispatcher DROPs the bad row on its own. + @Test + fun `an undecodable payload does not suppress a valid later row`() { + val rows = listOf( + CachedMutationEntity( + id = 1, + kind = MutationKind.SUGGESTION_SNOOZE_TOGGLE, + payload = "{ not json", + ), + snoozeRow(2, "mb-a", desiredSnoozed = true), + ) + assertTrue(supersededToggleIds(rows, json).isEmpty()) + } + + @Test + fun `an empty queue collapses nothing`() { + assertTrue(supersededToggleIds(emptyList(), json).isEmpty()) + } +} diff --git a/android/app/src/test/java/com/fabledsword/minstrel/models/SuggestionSnoozeRefTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/models/SuggestionSnoozeRefTest.kt new file mode 100644 index 00000000..f1123e46 --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/minstrel/models/SuggestionSnoozeRefTest.kt @@ -0,0 +1,67 @@ +package com.fabledsword.minstrel.models + +import kotlinx.datetime.Instant +import org.junit.jupiter.api.Test +import kotlin.test.assertEquals + +/** + * `returnsIn` phrasing for the parked-suggestions list (#2375). The clock is + * injected rather than frozen, so these assertions are stable. + */ +class SuggestionSnoozeRefTest { + + private val now = 1_800_000_000_000L // fixed epoch ms; any value works + + private fun snoozeIn(days: Double) = SuggestionSnoozeRef( + mbid = "mb", + name = "Parked", + snoozedUntil = Instant + .fromEpochMilliseconds(now + (days * 86_400_000L).toLong()) + .toString(), + ) + + @Test + fun `the default 90-day snooze reads as about 3 months`() { + assertEquals("in about 3 months", snoozeIn(90.0).returnsIn(now)) + } + + @Test + fun `a month reads in the singular`() { + assertEquals("in about a month", snoozeIn(30.0).returnsIn(now)) + } + + @Test + fun `under the month threshold it counts days`() { + assertEquals("in 14 days", snoozeIn(14.0).returnsIn(now)) + } + + @Test + fun `tomorrow is named, not rendered as 1 days`() { + assertEquals("tomorrow", snoozeIn(1.0).returnsIn(now)) + } + + @Test + fun `later today rounds down to today rather than going negative`() { + assertEquals("today", snoozeIn(0.1).returnsIn(now)) + } + + // The server only ever returns unexpired rows, so a past timestamp means + // our clock and the server's disagree. The row is on screen either way, so + // say something plausible rather than leaving the line blank. + @Test + fun `an already-past expiry degrades to shortly`() { + assertEquals("shortly", snoozeIn(-5.0).returnsIn(now)) + } + + @Test + fun `an unparseable timestamp degrades to shortly`() { + val row = SuggestionSnoozeRef(mbid = "mb", name = "Parked", snoozedUntil = "not-a-date") + assertEquals("shortly", row.returnsIn(now)) + } + + @Test + fun `an empty timestamp degrades to shortly`() { + val row = SuggestionSnoozeRef(mbid = "mb", name = "Parked", snoozedUntil = "") + assertEquals("shortly", row.returnsIn(now)) + } +} diff --git a/web/src/lib/api/queries.ts b/web/src/lib/api/queries.ts index f59d7260..73db1e60 100644 --- a/web/src/lib/api/queries.ts +++ b/web/src/lib/api/queries.ts @@ -58,6 +58,7 @@ export const qk = { smtpConfig: () => ['smtpConfig'] as const, suggestions: (limit?: number) => ['suggestions', { limit: limit ?? 12 }] as const, + suggestionSnoozes: () => ['suggestionSnoozes'] as const, home: () => ['home'] as const, albumsAlpha: () => ['albumsAlpha'] as const, artistTracks: (artistId: string) => ['artistTracks', artistId] as const, diff --git a/web/src/lib/api/suggestions.test.ts b/web/src/lib/api/suggestions.test.ts index 094bd4ba..86b2c861 100644 --- a/web/src/lib/api/suggestions.test.ts +++ b/web/src/lib/api/suggestions.test.ts @@ -1,13 +1,18 @@ import { afterEach, describe, expect, test, vi } from 'vitest'; vi.mock('./client', () => ({ - api: { get: vi.fn() } + api: { get: vi.fn(), post: vi.fn(), del: vi.fn() } })); -import { listSuggestions } from './suggestions'; +import { + listSuggestions, + listSnoozes, + snoozeSuggestion, + unsnoozeSuggestion +} from './suggestions'; import { qk } from './queries'; import { api } from './client'; -import type { ArtistSuggestion } from './types'; +import type { ArtistSuggestion, SuggestionSnooze } from './types'; afterEach(() => vi.clearAllMocks()); @@ -40,3 +45,56 @@ describe('suggestions client', () => { expect(qk.suggestions(20)).toEqual(['suggestions', { limit: 20 }]); }); }); + +describe('suggestion snoozes (#2375)', () => { + test('snoozeSuggestion sends the name — the server 400s without it', async () => { + (api.post as ReturnType).mockResolvedValueOnce(null); + await snoozeSuggestion('mb-1', 'Parked Artist'); + expect(api.post).toHaveBeenCalledWith('/api/discover/suggestions/mb-1/snooze', { + name: 'Parked Artist' + }); + }); + + test('snoozeSuggestion sends no days, leaving the default to the server', async () => { + (api.post as ReturnType).mockResolvedValueOnce(null); + await snoozeSuggestion('mb-1', 'Parked Artist'); + const body = (api.post as ReturnType).mock.calls[0][1] as Record; + expect(body).not.toHaveProperty('days'); + }); + + // MBIDs are UUIDs today, but the column is free-text and the value comes + // from an external similarity feed, so it goes through encodeURIComponent. + test('the mbid is URL-encoded into the path', async () => { + (api.post as ReturnType).mockResolvedValueOnce(null); + await snoozeSuggestion('weird/id?x', 'Odd'); + expect(api.post).toHaveBeenCalledWith( + '/api/discover/suggestions/weird%2Fid%3Fx/snooze', + { name: 'Odd' } + ); + }); + + test('unsnoozeSuggestion DELETEs the same path', async () => { + (api.del as ReturnType).mockResolvedValueOnce(null); + await unsnoozeSuggestion('mb-1'); + expect(api.del).toHaveBeenCalledWith('/api/discover/suggestions/mb-1/snooze'); + }); + + test('listSnoozes hits the snoozes collection', async () => { + const fixture: SuggestionSnooze[] = [ + { + mbid: 'mb-1', + name: 'Parked Artist', + snoozed_until: '2026-11-01T00:00:00Z', + created_at: '2026-08-03T00:00:00Z' + } + ]; + (api.get as ReturnType).mockResolvedValueOnce(fixture); + const got = await listSnoozes(); + expect(api.get).toHaveBeenCalledWith('/api/discover/snoozes'); + expect(got).toEqual(fixture); + }); + + test('qk.suggestionSnoozes key shape', () => { + expect(qk.suggestionSnoozes()).toEqual(['suggestionSnoozes']); + }); +}); diff --git a/web/src/lib/api/suggestions.ts b/web/src/lib/api/suggestions.ts index 2d998898..2abcd30a 100644 --- a/web/src/lib/api/suggestions.ts +++ b/web/src/lib/api/suggestions.ts @@ -1,7 +1,7 @@ import { createQuery } from '@tanstack/svelte-query'; import { api } from './client'; import { qk } from './queries'; -import type { ArtistSuggestion } from './types'; +import type { ArtistSuggestion, SuggestionSnooze } from './types'; export async function listSuggestions(limit = 12): Promise { return api.get(`/api/discover/suggestions?limit=${limit}`); @@ -14,3 +14,36 @@ export function createSuggestionsQuery(limit = 12) { staleTime: 5 * 60_000 // 5 minutes — see M5c spec §5 }); } + +// Parks a suggestion for the server's default period (90 days). `name` is +// REQUIRED by the server and is not optional bookkeeping: candidates are +// out-of-library, so there is no artists row to resolve a display name from +// and the snooze list would have nothing to render. Omitting it is a 400. +// +// No `days` is sent. There is deliberately no duration UI yet — that knob is +// slice 6 (#2377) — and hardcoding a value here would pin the default to the +// client instead of the server that owns it. +export async function snoozeSuggestion(mbid: string, name: string): Promise { + await api.post(`/api/discover/suggestions/${encodeURIComponent(mbid)}/snooze`, { name }); +} + +// Brings a parked suggestion back immediately. The server 404s an MBID that +// was never snoozed; callers treat that as already-unsnoozed rather than as a +// failure, since the end state the user asked for is the one they get. +export async function unsnoozeSuggestion(mbid: string): Promise { + await api.del(`/api/discover/suggestions/${encodeURIComponent(mbid)}/snooze`); +} + +export async function listSnoozes(): Promise { + return api.get('/api/discover/snoozes'); +} + +export function createSnoozesQuery() { + return createQuery({ + queryKey: qk.suggestionSnoozes(), + queryFn: listSnoozes + // No staleTime, unlike the suggestions query: this list is the only route + // back to an un-snooze, so it must reflect a snooze made seconds ago + // rather than a cached view of the world. + }); +} diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index c8cdf030..5772c4d1 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -334,6 +334,16 @@ export type ArtistSuggestion = { image_url?: string; // resolved on-demand from Lidarr; absent → card placeholder }; +// One parked suggestion — "not right now", not a dislike. The server only +// ever returns rows whose snoozed_until is still in the future, so the client +// never has to compare against the clock to decide what to show. +export type SuggestionSnooze = { + mbid: string; + name: string; + snoozed_until: string; // RFC3339 + created_at: string; +}; + // Mirrors internal/api/types.go HomePayload. All slices are non-null // per the server contract — empty sections render as []. export type HomePayload = { diff --git a/web/src/lib/components/DiscoverResultCard.svelte b/web/src/lib/components/DiscoverResultCard.svelte index 5738092b..ea3ae967 100644 --- a/web/src/lib/components/DiscoverResultCard.svelte +++ b/web/src/lib/components/DiscoverResultCard.svelte @@ -1,10 +1,14 @@
@@ -58,7 +128,16 @@ {#if !query.isPending && suggestions.length === 0} -

Listen to something or like an artist to start getting suggestions.

+ +

+ {snoozes.length === 0 + ? 'Listen to something or like an artist to start getting suggestions.' + : "Nothing new right now — the artists you've parked are below."} +

{:else if suggestions.length > 0}
{#each suggestions.filter(visible) as s (s.mbid)} @@ -66,11 +145,49 @@ kind="artist" title={s.name} imageUrl={s.image_url} - state="requestable" + state={cardState(s)} attribution={attributionText(s.attribution)} onRequest={() => onRequest(s)} + onSnooze={() => onSnooze(s)} + onUnsnooze={() => onUnsnooze(s.mbid, s.name)} /> {/each}
{/if} + + + {#if snoozes.length > 0} +
+

+ Not right now +

+

+ These come back on their own. Nothing here counts against your taste profile. +

+
    + {#each snoozes as snoozed (snoozed.mbid)} +
  • +
    +
    {snoozed.name}
    +
    + Back {returnsIn(snoozed.snoozed_until)} +
    +
    + +
    • + {/each} +
+
+ {/if}
diff --git a/web/src/lib/components/SuggestionFeed.test.ts b/web/src/lib/components/SuggestionFeed.test.ts index 0e6fdb43..984dedbb 100644 --- a/web/src/lib/components/SuggestionFeed.test.ts +++ b/web/src/lib/components/SuggestionFeed.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import { render, screen, fireEvent } from '@testing-library/svelte'; import { mockQuery } from '../../test-utils/query'; @@ -9,17 +9,30 @@ vi.mock('@tanstack/svelte-query', async (orig) => { }); vi.mock('$lib/api/suggestions', () => ({ - createSuggestionsQuery: vi.fn() + createSuggestionsQuery: vi.fn(), + createSnoozesQuery: vi.fn(), + snoozeSuggestion: vi.fn().mockResolvedValue(undefined), + unsnoozeSuggestion: vi.fn().mockResolvedValue(undefined) })); vi.mock('$lib/api/requests', () => ({ createRequest: vi.fn().mockResolvedValue({}) })); +const pushToastMock = vi.fn(); +vi.mock('$lib/stores/toast.svelte', () => ({ + pushToast: (...args: unknown[]) => pushToastMock(...args) +})); + import SuggestionFeed from './SuggestionFeed.svelte'; -import { createSuggestionsQuery } from '$lib/api/suggestions'; +import { + createSuggestionsQuery, + createSnoozesQuery, + snoozeSuggestion, + unsnoozeSuggestion +} from '$lib/api/suggestions'; import { createRequest } from '$lib/api/requests'; -import type { ArtistSuggestion } from '$lib/api/types'; +import type { ArtistSuggestion, SuggestionSnooze } from '$lib/api/types'; const oneSeed: ArtistSuggestion = { mbid: 'mb1', @@ -51,46 +64,50 @@ const threeSeeds: ArtistSuggestion = { ] }; +/** Days from now as an RFC3339 string, for snooze fixtures. */ +function inDays(n: number): string { + return new Date(Date.now() + n * 86_400_000).toISOString(); +} + +function setSuggestions(data: ArtistSuggestion[]) { + (createSuggestionsQuery as ReturnType).mockReturnValue(mockQuery({ data })); +} + +function setSnoozes(data: SuggestionSnooze[]) { + (createSnoozesQuery as ReturnType).mockReturnValue(mockQuery({ data })); +} + +beforeEach(() => setSnoozes([])); afterEach(() => vi.clearAllMocks()); describe('SuggestionFeed', () => { test('renders one card per suggestion', () => { - (createSuggestionsQuery as ReturnType).mockReturnValue( - mockQuery({ data: [oneSeed, twoSeeds] }) - ); + setSuggestions([oneSeed, twoSeeds]); render(SuggestionFeed); expect(screen.getByText('Outsider')).toBeInTheDocument(); expect(screen.getByText('Outsider Two')).toBeInTheDocument(); }); test('attribution copy: 1 seed → "Because you liked X."', () => { - (createSuggestionsQuery as ReturnType).mockReturnValue( - mockQuery({ data: [oneSeed] }) - ); + setSuggestions([oneSeed]); render(SuggestionFeed); expect(screen.getByText(/because you liked seed\./i)).toBeInTheDocument(); }); test('attribution copy: 2 seeds → "Because you liked A and played B."', () => { - (createSuggestionsQuery as ReturnType).mockReturnValue( - mockQuery({ data: [twoSeeds] }) - ); + setSuggestions([twoSeeds]); render(SuggestionFeed); expect(screen.getByText(/because you liked a and played b\./i)).toBeInTheDocument(); }); test('attribution copy: 3 seeds → Oxford comma', () => { - (createSuggestionsQuery as ReturnType).mockReturnValue( - mockQuery({ data: [threeSeeds] }) - ); + setSuggestions([threeSeeds]); render(SuggestionFeed); expect(screen.getByText(/because you liked x, played y, and played z\./i)).toBeInTheDocument(); }); test('Request button calls createRequest with artist-kind body', async () => { - (createSuggestionsQuery as ReturnType).mockReturnValue( - mockQuery({ data: [oneSeed] }) - ); + setSuggestions([oneSeed]); render(SuggestionFeed); await fireEvent.click(screen.getByRole('button', { name: /request outsider/i })); expect(createRequest).toHaveBeenCalledWith({ @@ -102,8 +119,113 @@ describe('SuggestionFeed', () => { }); test('empty state when data is []', () => { - (createSuggestionsQuery as ReturnType).mockReturnValue(mockQuery({ data: [] })); + setSuggestions([]); render(SuggestionFeed); expect(screen.getByText(/listen to something or like an artist/i)).toBeInTheDocument(); }); }); + +describe('SuggestionFeed snooze (#2375)', () => { + test('snooze sends BOTH mbid and name — the server 400s without the name', async () => { + setSuggestions([oneSeed]); + render(SuggestionFeed); + await fireEvent.click(screen.getByRole('button', { name: /not right now/i })); + expect(snoozeSuggestion).toHaveBeenCalledWith('mb1', 'Outsider'); + }); + + test('the card stays in place showing Undo, rather than vanishing', async () => { + setSuggestions([oneSeed]); + render(SuggestionFeed); + await fireEvent.click(screen.getByRole('button', { name: /not right now/i })); + // Still on screen — the disappearance happens on refetch, not under the + // cursor (rule #24). + expect(screen.getByText('Outsider')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /bring outsider back/i })).toBeInTheDocument(); + expect(screen.getByRole('status')).toHaveTextContent('Not right now'); + }); + + test('a failed snooze reverts the card and says so', async () => { + (snoozeSuggestion as ReturnType).mockRejectedValueOnce(new Error('offline')); + setSuggestions([oneSeed]); + render(SuggestionFeed); + await fireEvent.click(screen.getByRole('button', { name: /not right now/i })); + // Back to requestable — a snooze that silently did nothing would leave + // the user tapping it again. + expect(screen.getByRole('button', { name: /request outsider/i })).toBeInTheDocument(); + expect(pushToastMock).toHaveBeenCalledWith("Couldn't hide Outsider", 'error'); + }); + + test('undo on the card calls unsnoozeSuggestion', async () => { + setSuggestions([oneSeed]); + render(SuggestionFeed); + await fireEvent.click(screen.getByRole('button', { name: /not right now/i })); + await fireEvent.click(screen.getByRole('button', { name: /bring outsider back/i })); + expect(unsnoozeSuggestion).toHaveBeenCalledWith('mb1'); + }); + + test('the snoozed list is the way back once the card is gone', async () => { + // Deck empty, one parked artist: exactly the state after a refetch. + setSuggestions([]); + setSnoozes([ + { mbid: 'mbX', name: 'Parked', snoozed_until: inDays(90), created_at: inDays(0) } + ]); + render(SuggestionFeed); + expect(screen.getByRole('heading', { name: /not right now/i })).toBeInTheDocument(); + expect(screen.getByText('Parked')).toBeInTheDocument(); + await fireEvent.click(screen.getByRole('button', { name: /bring parked back now/i })); + expect(unsnoozeSuggestion).toHaveBeenCalledWith('mbX'); + }); + + test('a 404 from unsnooze is not surfaced as an error', async () => { + (unsnoozeSuggestion as ReturnType).mockRejectedValueOnce({ status: 404 }); + setSuggestions([]); + setSnoozes([ + { mbid: 'mbX', name: 'Parked', snoozed_until: inDays(90), created_at: inDays(0) } + ]); + render(SuggestionFeed); + await fireEvent.click(screen.getByRole('button', { name: /bring parked back now/i })); + // Already-unsnoozed IS the end state the user asked for. + expect(pushToastMock).not.toHaveBeenCalled(); + }); + + test('a non-404 unsnooze failure does surface', async () => { + (unsnoozeSuggestion as ReturnType).mockRejectedValueOnce({ status: 500 }); + setSuggestions([]); + setSnoozes([ + { mbid: 'mbX', name: 'Parked', snoozed_until: inDays(90), created_at: inDays(0) } + ]); + render(SuggestionFeed); + await fireEvent.click(screen.getByRole('button', { name: /bring parked back now/i })); + expect(pushToastMock).toHaveBeenCalledWith("Couldn't bring Parked back", 'error'); + }); + + test('return time reads as a relative phrase, not a calendar date', () => { + setSuggestions([]); + setSnoozes([ + { mbid: 'a', name: 'Quarter', snoozed_until: inDays(90), created_at: inDays(0) }, + { mbid: 'b', name: 'Fortnight', snoozed_until: inDays(14), created_at: inDays(0) } + ]); + render(SuggestionFeed); + expect(screen.getByText(/back in about 3 months/i)).toBeInTheDocument(); + expect(screen.getByText(/back in 14 days/i)).toBeInTheDocument(); + }); + + test('no snoozed section when nothing is parked', () => { + setSuggestions([oneSeed]); + setSnoozes([]); + render(SuggestionFeed); + expect(screen.queryByRole('heading', { name: /not right now/i })).not.toBeInTheDocument(); + }); + + // An empty deck has two causes now, and the advice differs. Telling someone + // who parked everything to go listen to music would be wrong. + test('empty-deck copy distinguishes "no signal" from "you parked them all"', () => { + setSuggestions([]); + setSnoozes([ + { mbid: 'a', name: 'Parked', snoozed_until: inDays(90), created_at: inDays(0) } + ]); + render(SuggestionFeed); + expect(screen.getByText(/nothing new right now/i)).toBeInTheDocument(); + expect(screen.queryByText(/listen to something or like an artist/i)).not.toBeInTheDocument(); + }); +});