diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index 83789234..be5cfa43 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -60,9 +60,35 @@ jobs: ANDROID_KEY_ALIAS: ${{ secrets.ANDROID_KEY_ALIAS }} ANDROID_KEY_PASSWORD: ${{ secrets.ANDROID_KEYSTORE_PASSWORD }} + # Job outputs propagate the computed release version to image-release + # so the bundled sidecar file matches what's baked into the APK — + # otherwise the server would report a different version string than + # the installed client and the update banner could thrash. + outputs: + version_name: ${{ steps.ver.outputs.name }} + version_code: ${{ steps.ver.outputs.code }} + steps: - name: Checkout uses: actions/checkout@v4 + with: + # fetch-depth: 0 retrieves full history; default shallow clone + # would return 1 for `git rev-list --count HEAD`, breaking the + # iteration suffix. + fetch-depth: 0 + + - name: Compute release version + id: ver + shell: bash + working-directory: ${{ github.workspace }} + run: | + set -euo pipefail + TAG="${GITHUB_REF#refs/tags/v}" + COMMIT_COUNT=$(git rev-list --count HEAD) + VERSION_NAME="${TAG}.${COMMIT_COUNT}" + echo "name=${VERSION_NAME}" >> "$GITHUB_OUTPUT" + echo "code=${COMMIT_COUNT}" >> "$GITHUB_OUTPUT" + echo "::notice::APK version: ${VERSION_NAME} (code=${COMMIT_COUNT})" - name: Cache Gradle dirs uses: actions/cache@v4 @@ -91,7 +117,10 @@ jobs: echo "ANDROID_KEYSTORE_PATH=${KEYSTORE_PATH}" >> "${GITHUB_ENV}" - name: Build release APK - run: ./gradlew assembleRelease + run: | + ./gradlew assembleRelease \ + -PMINSTREL_VERSION_NAME=${{ steps.ver.outputs.name }} \ + -PMINSTREL_VERSION_CODE=${{ steps.ver.outputs.code }} - name: Upload APK as workflow artifact # @v3 because Gitea Actions emulates GHES and the v2 artifact @@ -204,14 +233,18 @@ jobs: - name: Stage bundled APK + version sidecar if: steps.guard.outputs.ready == 'true' && startsWith(github.ref, 'refs/tags/v') shell: bash + env: + # Pulled from android-release.outputs.version_name so the + # sidecar string the server hands clients matches the + # versionName baked into the APK they're comparing against. + APK_VERSION_NAME: ${{ needs.android-release.outputs.version_name }} run: | set -euxo pipefail - TAG="${GITHUB_REF#refs/tags/}" # The artifact lands as `app-release.apk` (the original Gradle # output name). The Dockerfile COPYs client/* into /app/client/ # and the server reads minstrel.apk + minstrel.apk.version. mv client/app-release.apk client/minstrel.apk - echo "${TAG}" > client/minstrel.apk.version + echo "${APK_VERSION_NAME}" > client/minstrel.apk.version ls -lh client/ - name: Build and push diff --git a/android/app/build.gradle.kts b/android/app/build.gradle.kts index 5b81d850..4e72d441 100644 --- a/android/app/build.gradle.kts +++ b/android/app/build.gradle.kts @@ -21,8 +21,19 @@ android { applicationId = "com.fabledsword.minstrel" minSdk = 26 targetSdk = 36 - versionCode = 1 - versionName = "0.1.0-native" + // versionName / versionCode are released-build values injected by + // CI from the git tag + commit count. Local / debug builds fall + // back to "dev" so the About card reads honestly. Releases ship + // versionName="YYYY.MM.DD." (e.g. "2026.06.02.142") and + // versionCode=, which is monotonic forever and lets the + // shared isVersionNewer comparator distinguish two same-day + // re-cuts (the iteration suffix differs). + val versionNameOverride = + (project.findProperty("MINSTREL_VERSION_NAME") as String?)?.takeIf { it.isNotBlank() } + val versionCodeOverride = + (project.findProperty("MINSTREL_VERSION_CODE") as String?)?.toIntOrNull() + versionCode = versionCodeOverride ?: 1 + versionName = versionNameOverride ?: "dev" testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" vectorDrawables { useSupportLibrary = true } } @@ -139,7 +150,6 @@ dependencies { implementation(libs.compose.ui) implementation(libs.compose.ui.graphics) implementation(libs.compose.material3) - implementation(libs.compose.material.icons.extended) implementation(libs.compose.ui.text.google.fonts) debugImplementation(libs.compose.ui.tooling) implementation(libs.compose.ui.tooling.preview) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/PlaybackErrorsApi.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/PlaybackErrorsApi.kt new file mode 100644 index 00000000..df792717 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/api/endpoints/PlaybackErrorsApi.kt @@ -0,0 +1,33 @@ +package com.fabledsword.minstrel.api.endpoints + +import retrofit2.http.Body +import retrofit2.http.POST + +/** + * Retrofit interface for `POST /api/playback-errors`. Reports a + * client-detected playback failure (zero-duration, decode error, ...) + * so the admin inbox surfaces it. + * + * Failures during dispatch are reported via the MutationQueue path + * for offline replay (see [com.fabledsword.minstrel.cache.mutations.MutationQueue.enqueuePlaybackErrorReport]). + */ +interface PlaybackErrorsApi { + @POST("api/playback-errors") + suspend fun report(@Body body: PlaybackErrorReportRequest) +} + +/** + * POST body for `/api/playback-errors`. `kind` is one of + * "zero_duration" / "load_failed" / "stalled"; server validates against + * the CHECK constraint enum. `detail` is optional free-text (Media3 + * error message, etc.). `clientId` reuses the existing playback + * client identifier the device already sends with `/api/plays/...` + * so support can correlate reports across surfaces. + */ +@kotlinx.serialization.Serializable +data class PlaybackErrorReportRequest( + @kotlinx.serialization.SerialName("track_id") val trackId: String, + val kind: String, + val detail: String? = null, + @kotlinx.serialization.SerialName("client_id") val clientId: String, +) 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 f08f8354..541178cb 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 @@ -21,6 +21,7 @@ object MutationKind { const val QUARANTINE_FLAG: String = "quarantine_flag" const val PLAY_OFFLINE: String = "play_offline" const val REQUEST_CANCEL: String = "request_cancel" + const val PLAYBACK_ERROR_REPORT: String = "playback_error_report" } /** @@ -144,6 +145,13 @@ class MutationQueue @Inject constructor( ), ), ) + + suspend fun enqueuePlaybackErrorReport(payload: PlaybackErrorReportPayload): Long = dao.insert( + CachedMutationEntity( + kind = MutationKind.PLAYBACK_ERROR_REPORT, + payload = json.encodeToString(PlaybackErrorReportPayload.serializer(), payload), + ), + ) } /** @@ -203,3 +211,20 @@ data class PlayOfflinePayload( */ @Serializable data class RequestCancelPayload(val requestId: String) + +/** + * Persisted payload for `MutationKind.PLAYBACK_ERROR_REPORT` — the + * `POST /api/playback-errors` call lost during a connectivity hiccup. + * The replayer re-fires the POST with this body. Server is naturally + * idempotent enough — multiple reports of the same (track, user, + * kind) become multiple rows in the admin inbox, which is acceptable + * (the admin can resolve them all with one action). `clientId` is + * the existing playback client identifier from AuthStore. + */ +@Serializable +data class PlaybackErrorReportPayload( + val trackId: String, + val kind: String, + val detail: String? = null, + val clientId: String, +) 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 06fb336e..4fd4c8fb 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 @@ -5,6 +5,8 @@ import com.fabledsword.minstrel.api.endpoints.DiscoverApi import com.fabledsword.minstrel.api.endpoints.EventsApi import com.fabledsword.minstrel.api.endpoints.FlagRequest import com.fabledsword.minstrel.api.endpoints.LikesApi +import com.fabledsword.minstrel.api.endpoints.PlaybackErrorReportRequest +import com.fabledsword.minstrel.api.endpoints.PlaybackErrorsApi import com.fabledsword.minstrel.api.endpoints.PlaylistsApi import com.fabledsword.minstrel.api.endpoints.QuarantineApi import com.fabledsword.minstrel.api.endpoints.RequestsApi @@ -66,6 +68,7 @@ class MutationReplayer @Inject constructor( private val playlistsApi: PlaylistsApi = retrofit.create() private val eventsApi: EventsApi = retrofit.create() private val requestsApi: RequestsApi = retrofit.create() + private val playbackErrorsApi: PlaybackErrorsApi = retrofit.create() private val mutex = Mutex() @@ -116,6 +119,7 @@ class MutationReplayer @Inject constructor( MutationKind.QUARANTINE_FLAG -> dispatchQuarantineFlag(row.payload) MutationKind.PLAY_OFFLINE -> dispatchPlayOffline(row.payload) MutationKind.REQUEST_CANCEL -> dispatchRequestCancel(row.payload) + MutationKind.PLAYBACK_ERROR_REPORT -> dispatchPlaybackErrorReport(row.payload) else -> { // Unknown kind — drop the row by claiming success so a // stale schema entry can't wedge the queue forever. @@ -258,4 +262,24 @@ class MutationReplayer @Inject constructor( false } } + + private suspend fun dispatchPlaybackErrorReport(payload: String): Boolean { + val decoded = json.decodeFromString(PlaybackErrorReportPayload.serializer(), payload) + return try { + playbackErrorsApi.report( + PlaybackErrorReportRequest( + trackId = decoded.trackId, + kind = decoded.kind, + detail = decoded.detail, + clientId = decoded.clientId, + ), + ) + true + } catch ( + @Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable, + ) { + // Intentional swallow: row stays queued; next drain pass retries. + false + } + } } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/AlbumDetailScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/AlbumDetailScreen.kt index 2d07aa7b..79776ba0 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/AlbumDetailScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/AlbumDetailScreen.kt @@ -109,8 +109,11 @@ private fun AlbumDetailStateContent( playerViewModel: com.fabledsword.minstrel.player.ui.PlayerViewModel, navController: NavHostController, ) { - Crossfade(targetState = state, label = "album-detail") { s -> - when (s) { + // Crossfade keyed on `state::class` so a Success → fresh Success + // refresh (same kind, new detail) doesn't re-fade the whole body; + // only Loading ↔ Success ↔ Error transitions animate. + Crossfade(targetState = state::class, label = "album-detail") { _ -> + when (val s = state) { is AlbumDetailUiState.Loading -> if (s.seed != null) SeededAlbumLoading(s.seed) else SkeletonTrackList() is AlbumDetailUiState.Error -> EmptyState( diff --git a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/ArtistDetailScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/ArtistDetailScreen.kt index 01fa2b7e..335cbf4f 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/ArtistDetailScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/ArtistDetailScreen.kt @@ -87,8 +87,11 @@ fun ArtistDetailScreen( onRefresh = { viewModel.refresh().join() }, modifier = Modifier.fillMaxSize().padding(inner), ) { - Crossfade(targetState = state, label = "artist-detail") { s -> - when (s) { + // Crossfade keyed on `state::class` so a Success → fresh + // Success refresh (same kind, new detail) doesn't re-fade + // the body; only Loading ↔ Success ↔ Error animate. + Crossfade(targetState = state::class, label = "artist-detail") { _ -> + when (val s = state) { is ArtistDetailUiState.Loading -> if (s.seed != null) { SeededArtistLoading(s.seed) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/LibraryScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/LibraryScreen.kt index d01236eb..ee290b44 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/library/ui/LibraryScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/library/ui/LibraryScreen.kt @@ -4,7 +4,6 @@ package com.fabledsword.minstrel.library.ui import androidx.compose.animation.Crossfade import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.fillMaxSize @@ -12,6 +11,8 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.grid.GridCells import androidx.compose.foundation.lazy.grid.LazyVerticalGrid import androidx.compose.foundation.lazy.grid.items +import androidx.compose.foundation.pager.HorizontalPager +import androidx.compose.foundation.pager.rememberPagerState import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -22,11 +23,10 @@ import androidx.compose.material3.Tab import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf -import androidx.compose.runtime.remember -import androidx.compose.runtime.setValue +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp +import kotlinx.coroutines.launch import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavHostController @@ -69,8 +69,12 @@ fun LibraryScreen( navController: NavHostController, viewModel: LibraryViewModel = hiltViewModel(), ) { - var selectedTab by remember { mutableIntStateOf(0) } val tabs = LIBRARY_TABS + // HorizontalPager owns the active-tab state — the TabRow reads + // pagerState.currentPage and animateScrollToPage drives it on tap, + // so swipe and tap stay in sync. + val pagerState = rememberPagerState(pageCount = { tabs.size }) + val scope = rememberCoroutineScope() Scaffold( modifier = Modifier.fillMaxSize(), @@ -91,13 +95,13 @@ fun LibraryScreen( }, ) PrimaryScrollableTabRow( - selectedTabIndex = selectedTab, + selectedTabIndex = pagerState.currentPage, edgePadding = 16.dp, ) { tabs.forEachIndexed { index, label -> Tab( - selected = selectedTab == index, - onClick = { selectedTab = index }, + selected = pagerState.currentPage == index, + onClick = { scope.launch { pagerState.animateScrollToPage(index) } }, text = { Text(label) }, ) } @@ -105,8 +109,11 @@ fun LibraryScreen( } }, ) { inner -> - Box(modifier = Modifier.fillMaxSize().padding(inner)) { - when (selectedTab) { + HorizontalPager( + state = pagerState, + modifier = Modifier.fillMaxSize().padding(inner), + ) { page -> + when (page) { TAB_ARTISTS -> ArtistsTab(viewModel = viewModel, navController = navController) TAB_ALBUMS -> AlbumsTab(viewModel = viewModel, navController = navController) TAB_HISTORY -> HistoryTab( @@ -135,8 +142,13 @@ private fun ArtistsTab( ) { val state by viewModel.uiState.collectAsStateWithLifecycle() PullToRefreshScaffold(onRefresh = { viewModel.refresh().join() }) { - Crossfade(targetState = state, label = "library-artists") { s -> - when (s) { + // Crossfade is keyed on `state::class` (not the whole state) + // so a refresh that emits a fresh UiState.Success — same kind, + // new data — doesn't re-fade the entire grid. Only Loading ↔ + // Success ↔ Error ↔ Empty transitions animate; row-level diff + // is owned by LazyVerticalGrid via its item keys. + Crossfade(targetState = state::class, label = "library-artists") { _ -> + when (val s = state) { UiState.Loading -> SkeletonArtistsGrid() UiState.Empty -> EmptyState( title = "No artists yet", @@ -163,8 +175,8 @@ private fun AlbumsTab( ) { val state by viewModel.uiState.collectAsStateWithLifecycle() PullToRefreshScaffold(onRefresh = { viewModel.refresh().join() }) { - Crossfade(targetState = state, label = "library-albums") { s -> - when (s) { + Crossfade(targetState = state::class, label = "library-albums") { _ -> + when (val s = state) { UiState.Loading -> SkeletonAlbumsGrid() UiState.Empty -> EmptyState( title = "No albums yet", diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt index 12a6e775..f423cc5f 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorReporter.kt @@ -13,19 +13,23 @@ import javax.inject.Singleton private const val DEBOUNCE_MS = 2_000L /** - * Surfaces ExoPlayer track-load failures (404 / decoder failure / - * premature EOS / network drop) as user-visible snackbar messages. + * Surfaces playback failures (load errors + zero-duration tracks) as + * user-visible snackbar messages AND admin-inbox reports. * - * Without this the player just silently skips the dead track — the - * worst kind of bug, because the user can't tell "this file is - * broken" from "the app is flaky". + * Without the snackbar the player just silently skips the dead track — + * the worst kind of bug, because the user can't tell "this file is + * broken" from "the app is flaky". Without the admin report the + * operator never finds out the track is bad and the next user hits + * the same wall. * - * Pattern mirrors Flutter's `playback_error_reporter.dart`: collect - * per-error events from [PlayerController.playbackErrorEvents], - * debounce in a 2s window, and emit "Couldn't play 'X' — skipping" - * for a single error or "Skipped N unplayable tracks" when a burst - * lands inside the window. Stops the user from seeing a stack of N - * toasts on a network blip that fails N tracks in a row. + * The snackbar text mirrors Flutter's `playback_error_reporter.dart`: + * collect [PlayerController.playbackErrorEvents], debounce in a 2s + * window, emit "Couldn't play 'X' — skipping" for a single error or + * "Skipped N unplayable tracks" when a burst lands inside the window. + * + * The server report fires per event (no debounce) via + * [PlaybackErrorRepository], which handles the offline-first + * MutationQueue fallback per the standing rule for server writes. * * ShellScaffold collects [messages] into its existing snackbar host * so the reporter doesn't need its own UI surface. Constructed at @@ -35,6 +39,7 @@ private const val DEBOUNCE_MS = 2_000L @Singleton class PlaybackErrorReporter @Inject constructor( private val playerController: PlayerController, + private val repository: PlaybackErrorRepository, @ApplicationScope private val scope: CoroutineScope, ) { private val outChannel = Channel(Channel.BUFFERED) @@ -46,8 +51,11 @@ class PlaybackErrorReporter @Inject constructor( scope.launch { val buffer = mutableListOf() var debounceJob: kotlinx.coroutines.Job? = null - playerController.playbackErrorEvents.collect { title -> - buffer.add(title) + playerController.playbackErrorEvents.collect { event -> + // Fire-and-forget the server report — repository handles + // success/queue branching so callers don't see throws. + scope.launch { repository.report(event) } + buffer.add(event.title) debounceJob?.cancel() debounceJob = scope.launch { delay(DEBOUNCE_MS) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorRepository.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorRepository.kt new file mode 100644 index 00000000..197d7452 --- /dev/null +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlaybackErrorRepository.kt @@ -0,0 +1,78 @@ +package com.fabledsword.minstrel.player + +import com.fabledsword.minstrel.api.endpoints.PlaybackErrorReportRequest +import com.fabledsword.minstrel.api.endpoints.PlaybackErrorsApi +import com.fabledsword.minstrel.auth.AuthStore +import com.fabledsword.minstrel.cache.mutations.MutationQueue +import com.fabledsword.minstrel.cache.mutations.PlaybackErrorReportPayload +import retrofit2.Retrofit +import retrofit2.create +import java.util.UUID +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Writes a [PlaybackErrorEvent] to the server. Follows the standing + * offline-first rule for server writes: try the POST first, fall back + * to the MutationQueue on transport failure so a tracked report is + * never lost. + * + * Reused client_id comes from [AuthStore.clientId] — the same UUID- + * per-install identifier the play-events reporter already sends, so + * support can correlate playback errors with the surrounding plays. + */ +@Singleton +class PlaybackErrorRepository @Inject constructor( + private val authStore: AuthStore, + private val mutationQueue: MutationQueue, + retrofit: Retrofit, +) { + private val api: PlaybackErrorsApi = retrofit.create() + + /** + * Reports a playback failure. Returns [Outcome.ACCEPTED] on a 2xx, + * [Outcome.QUEUED] when the call was buffered to the MutationQueue + * for later replay. Never throws — callers don't need a try block. + */ + suspend fun report(event: PlaybackErrorEvent): Outcome { + val clientId = resolveClientId() + val payload = PlaybackErrorReportPayload( + trackId = event.trackId, + kind = event.kind, + detail = event.detail, + clientId = clientId, + ) + return try { + api.report( + PlaybackErrorReportRequest( + trackId = payload.trackId, + kind = payload.kind, + detail = payload.detail, + clientId = payload.clientId, + ), + ) + Outcome.ACCEPTED + } catch ( + @Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable, + ) { + // Intentional swallow — see LikesRepository.toggleLike for + // the same offline-first rationale. + mutationQueue.enqueuePlaybackErrorReport(payload) + Outcome.QUEUED + } + } + + /** + * Returns the persisted client id, generating + persisting one on + * first read. Mirrors [PlayEventsReporter.resolveClientId] — the + * value is shared across all client-id-carrying surfaces. + */ + private fun resolveClientId(): String { + authStore.clientId.value?.let { return it } + val fresh = UUID.randomUUID().toString() + authStore.setClientId(fresh) + return fresh + } + + enum class Outcome { ACCEPTED, QUEUED } +} diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt index b7782e64..31dac4b2 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/PlayerController.kt @@ -68,13 +68,23 @@ class PlayerController @Inject constructor( /** * Per-error events for [com.fabledsword.minstrel.player.PlaybackErrorReporter]. - * Emits the failing track's title (or `"Track"` when unknown) each - * time Media3's `onPlayerError` fires — not bound to the lifetime of - * any UI subscriber so a torn-down screen doesn't drop events. - * Conflated channel so backpressure can't stall the player loop. + * Fires twice: once when Media3's `onPlayerError` surfaces a load + * failure (decoder, transport, EOS), and once when the player + * reaches STATE_READY with a duration of zero / TIME_UNSET — the + * "track loaded but has no audio" case the user sees as the + * player sitting frozen on a track. Conflated channel so back- + * pressure can't stall the player loop. */ - private val playbackErrorEventsChannel = Channel(Channel.CONFLATED) - val playbackErrorEvents: Flow = playbackErrorEventsChannel.receiveAsFlow() + private val playbackErrorEventsChannel = Channel(Channel.CONFLATED) + val playbackErrorEvents: Flow = playbackErrorEventsChannel.receiveAsFlow() + + /** + * Tracks which queue index has already been evaluated for the + * zero-duration / load-failed checks so STATE_READY firing + * repeatedly (after every seek, pause/resume) doesn't re-emit + * the same error event. Reset on each onMediaItemTransition. + */ + private var lastEvaluatedItemIndex: Int = -1 /** * Stable queue snapshot kept in sync with the player's MediaItems — @@ -166,17 +176,84 @@ class PlayerController @Inject constructor( } /** - * Seed a fresh radio queue from [trackId] and start playback. - * Replaces the existing queue. The `source` tag is "radio:" - * so the server-side rotation reporter can distinguish radio - * plays from album / playlist plays. No-op on an empty server - * response; throws on transport / non-2xx for the caller to - * surface in a snackbar. + * Seed a fresh radio queue from [trackId]. The `source` tag is + * "radio:" so the server-side rotation reporter can + * distinguish radio plays from album / playlist plays. + * + * Two modes: + * - Nothing in the queue: start the radio cold from track 0 + * (the seed plays first, recommendations follow). + * - Queue already populated: do NOT interrupt — keep the current + * track playing as-is, trim every upcoming track, and append + * the radio results after the current item. If the seed is the + * currently playing track (the common "tap Start Radio on the + * song I'm listening to" case), drop it from the appended list + * so it doesn't immediately repeat. This is an intentional + * divergence from Flutter's `playerActions.startRadio`, which + * always calls `playTracks` and restarts playback from 0. + * + * No-op on an empty server response; throws on transport / non-2xx + * for the caller to surface in a snackbar. */ suspend fun startRadio(trackId: String) { + val controller = mediaController ?: return val tracks = radio.seed(trackId) if (tracks.isEmpty()) return - setQueue(tracks, initialIndex = 0, source = "radio:$trackId") + val source = "radio:$trackId" + if (controller.mediaItemCount == 0) { + setQueue(tracks, initialIndex = 0, source = source) + } else { + appendRadioToQueue(controller, tracks, trackId, source) + } + } + + /** + * Mid-queue radio insert. The current track keeps playing; every + * upcoming item is removed; the radio results are appended after. + * Drops the seed from the appended list when it matches the + * currently-playing track so it doesn't immediately repeat. + */ + private fun appendRadioToQueue( + controller: MediaController, + tracks: List, + seedTrackId: String, + source: String, + ) { + val currentIdx = controller.currentMediaItemIndex + val currentTrack = queueRefs.getOrNull(currentIdx) + val toAppend = if (currentTrack?.id == seedTrackId) tracks.drop(1) else tracks + if (toAppend.isEmpty()) return + val nextIdx = currentIdx + 1 + if (nextIdx < controller.mediaItemCount) { + controller.removeMediaItems(nextIdx, controller.mediaItemCount) + } + queueRefs = queueRefs.take(nextIdx) + toAppend + controller.addMediaItems(toAppend.map { it.toMediaItem(source) }) + } + + /** + * When a STATE_READY transition for a freshly-loaded item reports + * a zero / TIME_UNSET duration, fire a `zero_duration` error event + * and advance past the dead track. Otherwise no-op. + */ + private fun handleZeroDurationIfNeeded(controller: MediaController, idx: Int) { + val current = queueRefs.getOrNull(idx) ?: return + val duration = controller.duration + val isZeroDuration = duration <= 0L || duration == androidx.media3.common.C.TIME_UNSET + if (!isZeroDuration) return + playbackErrorEventsChannel.trySend( + PlaybackErrorEvent( + trackId = current.id, + kind = "zero_duration", + title = current.title.ifEmpty { "Track" }, + detail = "duration=$duration", + ), + ) + if (controller.hasNextMediaItem()) { + controller.seekToNextMediaItem() + } else { + controller.stop() + } } // ── Internal: async connect + Listener-driven UI state sync ────────── @@ -200,12 +277,34 @@ class PlayerController @Inject constructor( controller.addListener( object : Player.Listener { override fun onPlayerError(error: androidx.media3.common.PlaybackException) { - val title = queueRefs - .getOrNull(controller.currentMediaItemIndex) - ?.title - ?.takeIf { it.isNotEmpty() } - ?: "Track" - playbackErrorEventsChannel.trySend(title) + val current = queueRefs.getOrNull(controller.currentMediaItemIndex) + val title = current?.title?.takeIf { it.isNotEmpty() } ?: "Track" + val trackId = current?.id ?: return + playbackErrorEventsChannel.trySend( + PlaybackErrorEvent( + trackId = trackId, + kind = "load_failed", + title = title, + detail = error.message, + ), + ) + } + + override fun onMediaItemTransition( + mediaItem: androidx.media3.common.MediaItem?, + reason: Int, + ) { + // Reset the per-item evaluation guard so the new + // item's STATE_READY transition gets a fresh check. + lastEvaluatedItemIndex = -1 + } + + override fun onPlaybackStateChanged(playbackState: Int) { + if (playbackState != Player.STATE_READY) return + val idx = controller.currentMediaItemIndex + if (idx == lastEvaluatedItemIndex) return + lastEvaluatedItemIndex = idx + handleZeroDurationIfNeeded(controller, idx) } override fun onEvents(player: Player, events: Player.Events) { @@ -334,3 +433,19 @@ class PlayerController @Inject constructor( // surfaces are driven by Media3's MediaSession callbacks separately; // they don't need this poll. private const val POSITION_POLL_INTERVAL_MS = 500L + +/** + * Structured playback failure event for [PlaybackErrorReporter]. Drives + * both the user-facing snackbar ("Couldn't play X — skipping") and the + * /api/playback-errors admin inbox. + * + * `kind` is one of "zero_duration" / "load_failed" / "stalled" matching + * the server's CHECK constraint. `detail` is free-text (the Media3 + * error message, the observed duration value, etc.). + */ +data class PlaybackErrorEvent( + val trackId: String, + val kind: String, + val title: String, + val detail: String? = null, +) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/ui/DominantColor.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/ui/DominantColor.kt index 4298a2dc..b4be6420 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/ui/DominantColor.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/ui/DominantColor.kt @@ -59,9 +59,14 @@ fun rememberDominantColor(coverUrl: String?): Color { val palette = withContext(Dispatchers.Default) { Palette.from(bitmap).generate() } - val swatch = palette.vibrantSwatch - ?: palette.mutedSwatch - ?: palette.dominantSwatch + // Always use the dominant (majority-by-pixel-count) swatch. + // The cover art is the main feature of this view; a vibrant + // accent on the cover should pop against the background, not + // be matched by it. Previously we tried vibrant first, which + // picked the highest-saturation swatch even when it covered + // a tiny fraction of the cover — small bright accents made + // the gradient feel disconnected from the actual image. + val swatch = palette.dominantSwatch if (swatch != null) { extracted = Color(swatch.rgb) } diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/ui/NowPlayingScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/ui/NowPlayingScreen.kt index 468ff0ab..8002842b 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/ui/NowPlayingScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/ui/NowPlayingScreen.kt @@ -12,6 +12,7 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.aspectRatio +import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height @@ -85,6 +86,8 @@ private const val COVER_MAX_WIDTH_DP = 320 private const val TRANSPORT_ICON_DP = 36 private const val PLAY_PAUSE_ICON_DP = 56 private const val POP_GRACE_MS = 500L +private val SCRUB_TRACK_HEIGHT_DP = 4.dp +private val SCRUB_TRACK_CORNER_DP = 2.dp // Vertical drag-down threshold (in pixels) past which the gesture // pops the player. Matches Flutter's 80px threshold in spirit; @@ -488,6 +491,8 @@ private fun ScrubberRow(positionMs: Long, durationMs: Long, onSeek: (Long) -> Un .background(accent), ) }, + // Custom 4dp rounded track — see ScrubTrack for rationale. + track = { _ -> ScrubTrack(fraction = fraction, accent = accent) }, ) Row( modifier = Modifier.fillMaxWidth(), @@ -506,6 +511,34 @@ private fun ScrubberRow(positionMs: Long, durationMs: Long, onSeek: (Long) -> Un } } +/** + * Custom 4dp rounded scrubber track. M3's default Track is 16dp tall + * and reads as a heavy pill rather than a measurement line; making + * the thumb visibly taller than the track (14dp thumb on a 4dp bar) + * restores the "handle on a string" cue your eye reads as "tool, + * draggable." Also drops M3's stop indicator dot, which the web + * scrubber doesn't have. Extracted so [ScrubberRow] stays under + * detekt's LongMethod ceiling. + */ +@Composable +private fun ScrubTrack(fraction: Float, accent: Color) { + Box(modifier = Modifier.fillMaxWidth().height(SCRUB_TRACK_HEIGHT_DP)) { + Box( + modifier = Modifier + .matchParentSize() + .clip(RoundedCornerShape(SCRUB_TRACK_CORNER_DP)) + .background(MaterialTheme.colorScheme.surfaceVariant), + ) + Box( + modifier = Modifier + .fillMaxWidth(fraction) + .fillMaxHeight() + .clip(RoundedCornerShape(SCRUB_TRACK_CORNER_DP)) + .background(accent), + ) + } +} + @Composable private fun TransportRow( isPlaying: Boolean, diff --git a/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt index 1504c0a4..99250266 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/playlists/ui/PlaylistDetailScreen.kt @@ -288,7 +288,10 @@ private fun PlaylistDetailContent( playerViewModel: com.fabledsword.minstrel.player.ui.PlayerViewModel, navController: NavHostController, ) { - Crossfade(targetState = state, label = "playlist-detail") { s -> when (s) { + // Crossfade keyed on `state::class` so refreshes that emit a + // fresh Success (same kind, new detail) don't re-fade the body; + // only Loading ↔ Success ↔ Error transitions animate. + Crossfade(targetState = state::class, label = "playlist-detail") { _ -> when (val s = state) { is PlaylistDetailUiState.Loading -> s.seed?.let { SeededPlaylistLoading(it) } ?: SkeletonPlaylistTrackList() is PlaylistDetailUiState.Error -> ErrorBlock(s.message, viewModel::refresh) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt index d6b32c7e..a1707c59 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/shared/widgets/MainAppBarActions.kt @@ -11,12 +11,11 @@ import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.outlined.LibraryMusic import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavHostController import com.composables.icons.lucide.House +import com.composables.icons.lucide.LibraryBig import com.composables.icons.lucide.Lucide import com.composables.icons.lucide.EllipsisVertical import com.composables.icons.lucide.Search as SearchIcon @@ -59,14 +58,7 @@ fun MainAppBarActions( } if (currentRouteName != Library::class.qualifiedName) { IconButton(onClick = { navController.navigate(Library) }) { - // Intentional cross-family icon — Lucide has no - // equivalent "library + music" glyph and the literal - // library icons (Library / LibraryBig / SquareLibrary) - // don't read as "music collection" without a label. - // Material's Outlined LibraryMusic is the canonical - // music-library symbol; the outlined variant matches - // Lucide's stroke style closely enough to blend. - Icon(Icons.Outlined.LibraryMusic, contentDescription = "Library") + Icon(Lucide.LibraryBig, contentDescription = "Library") } } if (currentRouteName != SearchRoute::class.qualifiedName) { diff --git a/android/gradle/libs.versions.toml b/android/gradle/libs.versions.toml index 5fc19f10..8a2be34f 100644 --- a/android/gradle/libs.versions.toml +++ b/android/gradle/libs.versions.toml @@ -51,11 +51,6 @@ compose-ui-graphics = { module = "androidx.compose.ui:ui-graphics" } compose-ui-tooling = { module = "androidx.compose.ui:ui-tooling" } compose-ui-tooling-preview = { module = "androidx.compose.ui:ui-tooling-preview" } compose-material3 = { module = "androidx.compose.material3:material3" } -# material-icons-extended supplies the full Material Icons catalog -# (Outlined / Filled / Rounded / Sharp / TwoTone). We use the Outlined -# variant for LibraryMusic — its stroke style blends with Lucide. -# Version pinned by compose-bom. -compose-material-icons-extended = { module = "androidx.compose.material:material-icons-extended" } compose-ui-text-google-fonts = { module = "androidx.compose.ui:ui-text-google-fonts" } hilt-android = { module = "com.google.dagger:hilt-android", version.ref = "hilt" } hilt-compiler = { module = "com.google.dagger:hilt-compiler", version.ref = "hilt" } diff --git a/internal/api/api.go b/internal/api/api.go index 9935cfc0..1912eece 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -107,6 +107,10 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev authed.Delete("/quarantine/{track_id}", h.handleUnflag) authed.Get("/quarantine/mine", h.handleListMyQuarantine) + // Client-reported playback errors (zero-duration tracks, + // load failures). Admin-only inbox; any user can report. + authed.Post("/playback-errors", h.handleReportPlaybackError) + // Self-hosted in-app update channel (#397). Auth-gated to // prevent anonymous bandwidth abuse on the APK stream; // /apk additionally per-user rate-limited. @@ -127,6 +131,9 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev admin.Get("/quarantine", h.handleListAdminQuarantine) admin.Post("/quarantine/{track_id}/resolve", h.handleResolveQuarantine) + + admin.Get("/playback-errors", h.handleListAdminPlaybackErrors) + admin.Post("/playback-errors/{id}/resolve", h.handleResolvePlaybackError) admin.Post("/quarantine/{track_id}/delete-file", h.handleDeleteQuarantineFile) admin.Post("/quarantine/{track_id}/delete-via-lidarr", h.handleDeleteQuarantineViaLidarr) admin.Get("/quarantine/actions", h.handleListQuarantineActions) diff --git a/internal/api/playback_errors.go b/internal/api/playback_errors.go new file mode 100644 index 00000000..fb7da802 --- /dev/null +++ b/internal/api/playback_errors.go @@ -0,0 +1,242 @@ +package api + +import ( + "encoding/json" + "errors" + "net/http" + "strconv" + + "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5" + + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Valid `kind` values. Mirrors the CHECK constraint in migration 0032 +// — keep both lists in sync if you add a kind here, otherwise inserts +// pass the handler whitelist and trip the DB constraint instead. +var validPlaybackErrorKinds = map[string]struct{}{ + "zero_duration": {}, + "load_failed": {}, + "stalled": {}, +} + +// Valid `resolution` values. Hidden / deleted / requested are stamped +// automatically when the admin clicks the matching action from the +// inbox; fixed / ignored are operator-driven (no action taken on the +// track itself). Mirrors the CHECK constraint in migration 0032. +var validPlaybackErrorResolutions = map[string]struct{}{ + "hidden": {}, + "deleted": {}, + "requested": {}, + "fixed": {}, + "ignored": {}, +} + +// reportPlaybackErrorRequest is the body of POST /api/playback-errors. +// TrackID is a UUID string; Kind is one of [validPlaybackErrorKinds]; +// Detail is optional free-text from the client (e.g. ExoPlayer error +// message); ClientID identifies the device/browser so support can +// correlate reports across surfaces. +type reportPlaybackErrorRequest struct { + TrackID string `json:"track_id"` + Kind string `json:"kind"` + Detail *string `json:"detail,omitempty"` + ClientID string `json:"client_id"` +} + +// adminPlaybackErrorView is one row in the admin inbox response. The +// track / album / artist fields are joined so the SPA can render +// without a second round-trip per row. +type adminPlaybackErrorView struct { + ID string `json:"id"` + TrackID string `json:"track_id"` + UserID string `json:"user_id"` + Username string `json:"username"` + ClientID string `json:"client_id"` + Kind string `json:"kind"` + Detail *string `json:"detail,omitempty"` + OccurredAt string `json:"occurred_at"` + ResolvedAt *string `json:"resolved_at,omitempty"` + Resolution *string `json:"resolution,omitempty"` + TrackTitle string `json:"track_title"` + TrackFilePath string `json:"track_file_path"` + ArtistName string `json:"artist_name"` + AlbumTitle string `json:"album_title"` + AlbumID string `json:"album_id"` +} + +// resolvePlaybackErrorRequest is the body of POST +// /api/admin/playback-errors/{id}/resolve. The resolution string is +// validated against [validPlaybackErrorResolutions]. +type resolvePlaybackErrorRequest struct { + Resolution string `json:"resolution"` +} + +// Pagination defaults for the admin list endpoint. 50 fits the +// admin inbox table on a laptop without scrolling; max 200 prevents +// a misbehaving client from asking for the whole table at once. +const ( + defaultPlaybackErrorPageSize = 50 + maxPlaybackErrorPageSize = 200 +) + +// handleReportPlaybackError implements POST /api/playback-errors. +// Any signed-in user can report; the handler validates kind + track +// existence and inserts a row for admin review. +func (h *handlers) handleReportPlaybackError(w http.ResponseWriter, r *http.Request) { + user, ok := requireUser(w, r) + if !ok { + return + } + var req reportPlaybackErrorRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeErr(w, apierror.BadRequest("bad_request", "invalid JSON body")) + return + } + trackID, ok := parseUUID(req.TrackID) + if !ok { + writeErr(w, apierror.BadRequest("bad_request", "invalid track_id")) + return + } + if _, ok := validPlaybackErrorKinds[req.Kind]; !ok { + writeErr(w, apierror.BadRequest("bad_request", "invalid kind")) + return + } + if req.ClientID == "" { + writeErr(w, apierror.BadRequest("bad_request", "client_id required")) + return + } + if _, err := dbq.New(h.pool).GetTrackByID(r.Context(), trackID); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeErr(w, &apierror.Error{Status: 404, Code: "not_found", Message: "track not found"}) + return + } + h.logger.Error("api: playback_error: lookup track", "err", err) + writeErr(w, apierror.Internal(err)) + return + } + row, err := dbq.New(h.pool).InsertPlaybackError(r.Context(), dbq.InsertPlaybackErrorParams{ + TrackID: trackID, + UserID: user.ID, + ClientID: req.ClientID, + Kind: req.Kind, + Detail: req.Detail, + }) + if err != nil { + h.logger.Error("api: playback_error: insert", "err", err) + writeErr(w, apierror.Internal(err)) + return + } + writeJSON(w, http.StatusCreated, map[string]string{"id": uuidToString(row.ID)}) +} + +// handleListAdminPlaybackErrors implements GET +// /api/admin/playback-errors?resolved=false&offset=0&limit=50. +func (h *handlers) handleListAdminPlaybackErrors(w http.ResponseWriter, r *http.Request) { + resolved := r.URL.Query().Get("resolved") == "true" + limit := parsePlaybackErrorLimit(r.URL.Query().Get("limit")) + offset := parsePlaybackErrorOffset(r.URL.Query().Get("offset")) + rows, err := dbq.New(h.pool).ListAdminPlaybackErrors(r.Context(), dbq.ListAdminPlaybackErrorsParams{ + ResolvedFilter: resolved, + Off: int32(offset), + Lim: int32(limit), + }) + if err != nil { + h.logger.Error("admin: list playback_errors", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + out := make([]adminPlaybackErrorView, 0, len(rows)) + for _, row := range rows { + view := adminPlaybackErrorView{ + ID: uuidToString(row.ID), + TrackID: uuidToString(row.TrackID), + UserID: uuidToString(row.UserID), + Username: row.Username, + ClientID: row.ClientID, + Kind: row.Kind, + Detail: row.Detail, + OccurredAt: formatTimestamp(row.OccurredAt), + Resolution: row.Resolution, + TrackTitle: row.TrackTitle, + TrackFilePath: row.TrackFilePath, + ArtistName: row.ArtistName, + AlbumTitle: row.AlbumTitle, + AlbumID: uuidToString(row.AlbumID), + } + if row.ResolvedAt.Valid { + ts := formatTimestamp(row.ResolvedAt) + view.ResolvedAt = &ts + } + out = append(out, view) + } + writeJSON(w, http.StatusOK, out) +} + +// handleResolvePlaybackError implements POST +// /api/admin/playback-errors/{id}/resolve. +func (h *handlers) handleResolvePlaybackError(w http.ResponseWriter, r *http.Request) { + admin, ok := auth.UserFromContext(r.Context()) + if !ok { + writeAdminJSONErr(w, http.StatusUnauthorized, "unauthorized") + return + } + id, ok := parseUUID(chi.URLParam(r, "id")) + if !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + var req resolvePlaybackErrorRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeAdminJSONErr(w, http.StatusBadRequest, "bad_request") + return + } + if _, ok := validPlaybackErrorResolutions[req.Resolution]; !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_resolution") + return + } + resolution := req.Resolution + row, err := dbq.New(h.pool).ResolvePlaybackError(r.Context(), dbq.ResolvePlaybackErrorParams{ + ID: id, + ResolvedBy: admin.ID, + Resolution: &resolution, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + writeAdminJSONErr(w, http.StatusNotFound, "not_found") + return + } + h.logger.Error("admin: resolve playback_error", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + writeJSON(w, http.StatusOK, map[string]string{"id": uuidToString(row.ID)}) +} + +func parsePlaybackErrorLimit(raw string) int { + if raw == "" { + return defaultPlaybackErrorPageSize + } + n, err := strconv.Atoi(raw) + if err != nil || n <= 0 { + return defaultPlaybackErrorPageSize + } + if n > maxPlaybackErrorPageSize { + return maxPlaybackErrorPageSize + } + return n +} + +func parsePlaybackErrorOffset(raw string) int { + if raw == "" { + return 0 + } + n, err := strconv.Atoi(raw) + if err != nil || n < 0 { + return 0 + } + return n +} diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 230daadc..1856b21a 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -383,6 +383,19 @@ type PlaySession struct { ClientID *string } +type PlaybackError struct { + ID pgtype.UUID + TrackID pgtype.UUID + UserID pgtype.UUID + ClientID string + Kind string + Detail *string + OccurredAt pgtype.Timestamptz + ResolvedAt pgtype.Timestamptz + ResolvedBy pgtype.UUID + Resolution *string +} + type Playlist struct { ID pgtype.UUID UserID pgtype.UUID diff --git a/internal/db/dbq/playback_errors.sql.go b/internal/db/dbq/playback_errors.sql.go new file mode 100644 index 00000000..607441b9 --- /dev/null +++ b/internal/db/dbq/playback_errors.sql.go @@ -0,0 +1,210 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: playback_errors.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const insertPlaybackError = `-- name: InsertPlaybackError :one +INSERT INTO playback_errors (track_id, user_id, client_id, kind, detail) +VALUES ($1, $2, $3, $4, $5) +RETURNING id, track_id, user_id, client_id, kind, detail, occurred_at, + resolved_at, resolved_by, resolution +` + +type InsertPlaybackErrorParams struct { + TrackID pgtype.UUID + UserID pgtype.UUID + ClientID string + Kind string + Detail *string +} + +// Records a client-reported playback failure. The handler validates +// the kind enum before this runs; the CHECK constraint is a +// belt-and-braces guard. +func (q *Queries) InsertPlaybackError(ctx context.Context, arg InsertPlaybackErrorParams) (PlaybackError, error) { + row := q.db.QueryRow(ctx, insertPlaybackError, + arg.TrackID, + arg.UserID, + arg.ClientID, + arg.Kind, + arg.Detail, + ) + var i PlaybackError + err := row.Scan( + &i.ID, + &i.TrackID, + &i.UserID, + &i.ClientID, + &i.Kind, + &i.Detail, + &i.OccurredAt, + &i.ResolvedAt, + &i.ResolvedBy, + &i.Resolution, + ) + return i, err +} + +const listAdminPlaybackErrors = `-- name: ListAdminPlaybackErrors :many +SELECT + pe.id AS id, + pe.track_id AS track_id, + pe.user_id AS user_id, + u.username AS username, + pe.client_id AS client_id, + pe.kind AS kind, + pe.detail AS detail, + pe.occurred_at AS occurred_at, + pe.resolved_at AS resolved_at, + pe.resolution AS resolution, + t.title AS track_title, + t.file_path AS track_file_path, + ar.name AS artist_name, + al.title AS album_title, + al.id AS album_id +FROM playback_errors pe +JOIN users u ON u.id = pe.user_id +JOIN tracks t ON t.id = pe.track_id +JOIN albums al ON al.id = t.album_id +JOIN artists ar ON ar.id = t.artist_id +WHERE ($1::bool AND pe.resolved_at IS NOT NULL) + OR (NOT $1::bool AND pe.resolved_at IS NULL) +ORDER BY pe.occurred_at DESC +LIMIT $3::int OFFSET $2::int +` + +type ListAdminPlaybackErrorsParams struct { + ResolvedFilter bool + Off int32 + Lim int32 +} + +type ListAdminPlaybackErrorsRow struct { + ID pgtype.UUID + TrackID pgtype.UUID + UserID pgtype.UUID + Username string + ClientID string + Kind string + Detail *string + OccurredAt pgtype.Timestamptz + ResolvedAt pgtype.Timestamptz + Resolution *string + TrackTitle string + TrackFilePath string + ArtistName string + AlbumTitle string + AlbumID pgtype.UUID +} + +// Admin inbox query. Returns one row per playback_errors row joined +// with track / album / artist metadata so the SPA can render without +// a second round-trip. Filter by resolved status via the sqlc.arg — +// pass true for the Resolved tab, false for Unresolved (the default). +func (q *Queries) ListAdminPlaybackErrors(ctx context.Context, arg ListAdminPlaybackErrorsParams) ([]ListAdminPlaybackErrorsRow, error) { + rows, err := q.db.Query(ctx, listAdminPlaybackErrors, arg.ResolvedFilter, arg.Off, arg.Lim) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListAdminPlaybackErrorsRow + for rows.Next() { + var i ListAdminPlaybackErrorsRow + if err := rows.Scan( + &i.ID, + &i.TrackID, + &i.UserID, + &i.Username, + &i.ClientID, + &i.Kind, + &i.Detail, + &i.OccurredAt, + &i.ResolvedAt, + &i.Resolution, + &i.TrackTitle, + &i.TrackFilePath, + &i.ArtistName, + &i.AlbumTitle, + &i.AlbumID, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const resolveAllPlaybackErrorsForTrack = `-- name: ResolveAllPlaybackErrorsForTrack :execrows +UPDATE playback_errors + SET resolved_at = now(), + resolved_by = $2, + resolution = $3 + WHERE track_id = $1 AND resolved_at IS NULL +` + +type ResolveAllPlaybackErrorsForTrackParams struct { + TrackID pgtype.UUID + ResolvedBy pgtype.UUID + Resolution *string +} + +// Bulk-resolve every unresolved error for a track when an admin acts +// on it from a non-inbox surface (existing quarantine flow, etc.). +// Returns the affected row count so the caller can surface "N reports +// auto-resolved" in the response if useful. +func (q *Queries) ResolveAllPlaybackErrorsForTrack(ctx context.Context, arg ResolveAllPlaybackErrorsForTrackParams) (int64, error) { + result, err := q.db.Exec(ctx, resolveAllPlaybackErrorsForTrack, arg.TrackID, arg.ResolvedBy, arg.Resolution) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const resolvePlaybackError = `-- name: ResolvePlaybackError :one +UPDATE playback_errors + SET resolved_at = now(), + resolved_by = $2, + resolution = $3 + WHERE id = $1 +RETURNING id, track_id, user_id, client_id, kind, detail, occurred_at, + resolved_at, resolved_by, resolution +` + +type ResolvePlaybackErrorParams struct { + ID pgtype.UUID + ResolvedBy pgtype.UUID + Resolution *string +} + +// Marks a single error resolved. Idempotent over (id, resolution) — +// a second resolve with the same resolution is a no-op rewrite. Returns +// the row so the handler can echo it. The CHECK constraint enforces +// the resolution enum. +func (q *Queries) ResolvePlaybackError(ctx context.Context, arg ResolvePlaybackErrorParams) (PlaybackError, error) { + row := q.db.QueryRow(ctx, resolvePlaybackError, arg.ID, arg.ResolvedBy, arg.Resolution) + var i PlaybackError + err := row.Scan( + &i.ID, + &i.TrackID, + &i.UserID, + &i.ClientID, + &i.Kind, + &i.Detail, + &i.OccurredAt, + &i.ResolvedAt, + &i.ResolvedBy, + &i.Resolution, + ) + return i, err +} diff --git a/internal/db/migrations/0032_playback_errors.down.sql b/internal/db/migrations/0032_playback_errors.down.sql new file mode 100644 index 00000000..43ae6b48 --- /dev/null +++ b/internal/db/migrations/0032_playback_errors.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS playback_errors; diff --git a/internal/db/migrations/0032_playback_errors.up.sql b/internal/db/migrations/0032_playback_errors.up.sql new file mode 100644 index 00000000..5f4b8d7e --- /dev/null +++ b/internal/db/migrations/0032_playback_errors.up.sql @@ -0,0 +1,41 @@ +-- Client-reported playback errors. Populated by clients when a track +-- fails to play (zero duration, decode error, etc.); surfaced in the +-- admin /admin/playback-errors inbox so the operator can hide / delete +-- / re-request the offending track. +-- +-- resolved_at + resolved_by + resolution are NULL until an admin acts +-- on the row. Auto-resolved by the client when the admin clicks Hide / +-- Delete / Re-request from the inbox row, with the resolution string +-- recording which path was taken. The partial index keeps the +-- unresolved-queue lookup fast even as resolved history accumulates. +-- +-- CHECK constraints on kind/resolution gate the enum values so client +-- typos can't pollute the data (per the standing rule about +-- enum-CHECK whitelists needing migrations). + +CREATE TABLE playback_errors ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + track_id uuid NOT NULL REFERENCES tracks(id) ON DELETE CASCADE, + user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE, + client_id text NOT NULL, + kind text NOT NULL, + detail text, + occurred_at timestamptz NOT NULL DEFAULT now(), + resolved_at timestamptz, + resolved_by uuid REFERENCES users(id), + resolution text, + CONSTRAINT playback_errors_kind_check + CHECK (kind IN ('zero_duration', 'load_failed', 'stalled')), + CONSTRAINT playback_errors_resolution_check + CHECK (resolution IS NULL OR resolution IN + ('hidden', 'deleted', 'requested', 'fixed', 'ignored')), + CONSTRAINT playback_errors_resolved_consistency + CHECK ((resolved_at IS NULL) = (resolution IS NULL)) +); + +CREATE INDEX idx_playback_errors_unresolved + ON playback_errors (occurred_at DESC) + WHERE resolved_at IS NULL; + +CREATE INDEX idx_playback_errors_track + ON playback_errors (track_id); diff --git a/internal/db/queries/playback_errors.sql b/internal/db/queries/playback_errors.sql new file mode 100644 index 00000000..c89da41f --- /dev/null +++ b/internal/db/queries/playback_errors.sql @@ -0,0 +1,63 @@ +-- name: InsertPlaybackError :one +-- Records a client-reported playback failure. The handler validates +-- the kind enum before this runs; the CHECK constraint is a +-- belt-and-braces guard. +INSERT INTO playback_errors (track_id, user_id, client_id, kind, detail) +VALUES ($1, $2, $3, $4, $5) +RETURNING id, track_id, user_id, client_id, kind, detail, occurred_at, + resolved_at, resolved_by, resolution; + +-- name: ListAdminPlaybackErrors :many +-- Admin inbox query. Returns one row per playback_errors row joined +-- with track / album / artist metadata so the SPA can render without +-- a second round-trip. Filter by resolved status via the sqlc.arg — +-- pass true for the Resolved tab, false for Unresolved (the default). +SELECT + pe.id AS id, + pe.track_id AS track_id, + pe.user_id AS user_id, + u.username AS username, + pe.client_id AS client_id, + pe.kind AS kind, + pe.detail AS detail, + pe.occurred_at AS occurred_at, + pe.resolved_at AS resolved_at, + pe.resolution AS resolution, + t.title AS track_title, + t.file_path AS track_file_path, + ar.name AS artist_name, + al.title AS album_title, + al.id AS album_id +FROM playback_errors pe +JOIN users u ON u.id = pe.user_id +JOIN tracks t ON t.id = pe.track_id +JOIN albums al ON al.id = t.album_id +JOIN artists ar ON ar.id = t.artist_id +WHERE (sqlc.arg(resolved_filter)::bool AND pe.resolved_at IS NOT NULL) + OR (NOT sqlc.arg(resolved_filter)::bool AND pe.resolved_at IS NULL) +ORDER BY pe.occurred_at DESC +LIMIT sqlc.arg(lim)::int OFFSET sqlc.arg(off)::int; + +-- name: ResolvePlaybackError :one +-- Marks a single error resolved. Idempotent over (id, resolution) — +-- a second resolve with the same resolution is a no-op rewrite. Returns +-- the row so the handler can echo it. The CHECK constraint enforces +-- the resolution enum. +UPDATE playback_errors + SET resolved_at = now(), + resolved_by = $2, + resolution = $3 + WHERE id = $1 +RETURNING id, track_id, user_id, client_id, kind, detail, occurred_at, + resolved_at, resolved_by, resolution; + +-- name: ResolveAllPlaybackErrorsForTrack :execrows +-- Bulk-resolve every unresolved error for a track when an admin acts +-- on it from a non-inbox surface (existing quarantine flow, etc.). +-- Returns the affected row count so the caller can surface "N reports +-- auto-resolved" in the response if useful. +UPDATE playback_errors + SET resolved_at = now(), + resolved_by = $2, + resolution = $3 + WHERE track_id = $1 AND resolved_at IS NULL; diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index 20ca2da3..96e621dc 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -3,6 +3,7 @@ import { api } from './client'; import { qk } from './queries'; import type { ActionResult, + AdminPlaybackError, AdminQuarantineRow, LidarrConfig, LidarrMetadataProfile, @@ -11,7 +12,8 @@ import type { LidarrRequest, LidarrRequestStatus, LidarrRootFolder, - LidarrTestResult + LidarrTestResult, + PlaybackErrorResolution } from './types'; // Admin Lidarr config ----------------------------------------------------- @@ -174,6 +176,34 @@ export function createQuarantineActionsQuery(limit: number = 50) { }); } +// Admin playback errors --------------------------------------------------- + +export async function listAdminPlaybackErrors( + resolved: boolean = false +): Promise { + return api.get( + `/api/admin/playback-errors?resolved=${resolved}` + ); +} + +export async function resolvePlaybackError( + id: string, + resolution: PlaybackErrorResolution +): Promise<{ id: string }> { + return api.post<{ id: string }>( + `/api/admin/playback-errors/${id}/resolve`, + { resolution } + ); +} + +export function createAdminPlaybackErrorsQuery(resolved: boolean = false) { + return createQuery({ + queryKey: qk.adminPlaybackErrors(resolved), + queryFn: () => listAdminPlaybackErrors(resolved), + staleTime: 30_000 + }); +} + // Admin cover art --------------------------------------------------------- export type RefetchAlbumCoverResponse = { diff --git a/web/src/lib/api/queries.ts b/web/src/lib/api/queries.ts index 418ffaa6..37dafb5c 100644 --- a/web/src/lib/api/queries.ts +++ b/web/src/lib/api/queries.ts @@ -43,6 +43,8 @@ export const qk = { adminQuarantine: () => ['adminQuarantine'] as const, adminQuarantineActions: (limit?: number) => ['adminQuarantineActions', { limit: limit ?? 50 }] as const, + adminPlaybackErrors: (resolved?: boolean) => + ['adminPlaybackErrors', { resolved: resolved ?? false }] as const, scanStatus: () => ['scanStatus'] as const, scanSchedule: () => ['scanSchedule'] as const, coverage: () => ['coverage'] as const, diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index 667d4b49..26796f06 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -264,6 +264,33 @@ export type AdminQuarantineReport = { created_at: string; }; +// What GET /api/admin/playback-errors returns per row +export type PlaybackErrorKind = 'zero_duration' | 'load_failed' | 'stalled'; +export type PlaybackErrorResolution = + | 'hidden' + | 'deleted' + | 'requested' + | 'fixed' + | 'ignored'; + +export type AdminPlaybackError = { + id: string; + track_id: string; + user_id: string; + username: string; + client_id: string; + kind: PlaybackErrorKind; + detail?: string | null; + occurred_at: string; + resolved_at?: string | null; + resolution?: PlaybackErrorResolution | null; + track_title: string; + track_file_path: string; + artist_name: string; + album_title: string; + album_id: string; +}; + export type LidarrQuarantineAction = 'resolved' | 'deleted_file' | 'deleted_via_lidarr'; export type LidarrQuarantineActionRow = { diff --git a/web/src/lib/components/AdminTabs.svelte b/web/src/lib/components/AdminTabs.svelte index 07429f1c..78ad4cc6 100644 --- a/web/src/lib/components/AdminTabs.svelte +++ b/web/src/lib/components/AdminTabs.svelte @@ -4,11 +4,12 @@ type Item = { href: string; label: string }; const items: Item[] = [ - { href: '/admin', label: 'Overview' }, - { href: '/admin/integrations', label: 'Integrations' }, - { href: '/admin/requests', label: 'Requests' }, - { href: '/admin/quarantine', label: 'Quarantine' }, - { href: '/admin/users', label: 'Users' } + { href: '/admin', label: 'Overview' }, + { href: '/admin/integrations', label: 'Integrations' }, + { href: '/admin/requests', label: 'Requests' }, + { href: '/admin/quarantine', label: 'Quarantine' }, + { href: '/admin/playback-errors', label: 'Playback errors' }, + { href: '/admin/users', label: 'Users' } ]; function isActive(href: string): boolean { diff --git a/web/src/lib/components/AdminTabs.test.ts b/web/src/lib/components/AdminTabs.test.ts index d76ddbdf..73bd812f 100644 --- a/web/src/lib/components/AdminTabs.test.ts +++ b/web/src/lib/components/AdminTabs.test.ts @@ -52,7 +52,7 @@ describe('AdminTabs', () => { ); }); - test('renders exactly five tabs', () => { + test('renders all six tabs in order', () => { state.pageUrl = new URL('http://localhost/admin'); render(AdminTabs); const links = screen.getAllByRole('link'); @@ -61,6 +61,7 @@ describe('AdminTabs', () => { 'Integrations', 'Requests', 'Quarantine', + 'Playback errors', 'Users' ]); }); diff --git a/web/src/routes/admin/playback-errors/+page.svelte b/web/src/routes/admin/playback-errors/+page.svelte new file mode 100644 index 00000000..86fdf9a0 --- /dev/null +++ b/web/src/routes/admin/playback-errors/+page.svelte @@ -0,0 +1,316 @@ + + +{pageTitle('Admin · Playback errors')} + +
+
+
+

Playback errors

+

+ Tracks that failed to play on a client. Reported automatically by + the Android player when a track loads with zero duration or + decode-fails — the player skips fast and logs here for triage. +

+
+ {#if !query.isPending && !query.isError && !resolved} + + {rows.length} unresolved + + {/if} +
+ + +
+ + +
+ + {#if query.isError} +

Couldn't load: {errMessage(query.error)}

+ {:else if query.isPending} +

Loading…

+ {:else if rows.length === 0} +

+ {resolved ? 'No resolved errors yet.' : 'No unresolved errors.'} +

+ {:else} +
    + {#each rows as r (r.id)} +
  • +
    +
    + + {r.track_title} + + + {r.artist_name} · {r.album_title} + + + {kindLabel(r.kind)} + + {#if resolved && r.resolution} + + {resolutionLabel(r.resolution)} + + {/if} +
    +
    + by {r.username} · {relativeTime(r.occurred_at)} + {#if r.detail} · {r.detail}{/if} +
    +
    + {r.track_file_path} +
    +
    + {#if !resolved} + {@const a = actionsFor(r)} + + {/if} +
    • + {/each} +
+ {/if} +
+ + { deleteFileOpen = null; }} +> + {#if deleteFileOpen} +

+ This removes {deleteFileOpen.track_title} + from the library and deletes the underlying file. The error row + will be marked resolved. +

+

{deleteFileOpen.track_file_path}

+
+ + +
+ {/if} + + + { resolveOpen = null; }} +> + {#if resolveOpen} +

+ Marking {resolveOpen.track_title} resolved. + Pick how you handled it: +

+ +
+ + +
+ {/if} +