mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-15 22:07:08 +02:00
fix(artwork): surface priority names in the explain UI and tidy up loose ends
Moves the CLI's priority-name lookup into core/artwork so the native API and UI can render "scan" instead of a bare priority number, replaces a brittle string comparison with a presence check on the stored artwork, renders the source path in the details section, drops the unused hash field, and aligns date rendering with the rest of the dialog.
This commit is contained in:
parent
fd46e594e1
commit
f283da0630
9 changed files with 73 additions and 49 deletions
|
|
@ -8,7 +8,6 @@ import (
|
|||
"io"
|
||||
"os"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
|
|
@ -264,7 +263,7 @@ func configState(rep statusReport) string {
|
|||
func printQueueStats(w io.Writer, stats []model.ArtworkQueueStat, total int64, countHeader, indent string) {
|
||||
fmt.Fprintf(w, "%sKIND\tPRIORITY\t%s\n", indent, countHeader)
|
||||
for _, s := range stats {
|
||||
fmt.Fprintf(w, "%s%s\t%s\t%d\n", indent, kindName(s.ItemKind), priorityName(s.Priority), s.Count)
|
||||
fmt.Fprintf(w, "%s%s\t%s\t%d\n", indent, kindName(s.ItemKind), artwork.PriorityName(s.Priority), s.Count)
|
||||
}
|
||||
fmt.Fprintf(w, "%sTOTAL\t\t%d\n", indent, total)
|
||||
}
|
||||
|
|
@ -276,37 +275,14 @@ func kindName(prefix string) string {
|
|||
return prefix
|
||||
}
|
||||
|
||||
type artworkPriority struct {
|
||||
name string
|
||||
value int
|
||||
}
|
||||
|
||||
// knownPriorities is the one listing behind both the name and the parse, so they cannot drift.
|
||||
var knownPriorities = []artworkPriority{
|
||||
{"bump", model.ArtworkPriorityBump},
|
||||
{"scan", model.ArtworkPriorityScan},
|
||||
{"recheck", model.ArtworkPriorityRecheck},
|
||||
{"backfill", model.ArtworkPriorityBackfill},
|
||||
}
|
||||
|
||||
// priorityName falls back to the number: a row written by a newer version still has to print.
|
||||
func priorityName(p int) string {
|
||||
for _, ap := range knownPriorities {
|
||||
if ap.value == p {
|
||||
return ap.name
|
||||
}
|
||||
}
|
||||
return strconv.Itoa(p)
|
||||
}
|
||||
|
||||
func priorityNames() string {
|
||||
return strings.Join(slice.Map(knownPriorities, func(ap artworkPriority) string { return ap.name }), ", ")
|
||||
return strings.Join(slice.Map(artwork.KnownPriorities, func(ap artwork.Priority) string { return ap.Name }), ", ")
|
||||
}
|
||||
|
||||
func parseArtworkPriority(s string) (int, error) {
|
||||
for _, ap := range knownPriorities {
|
||||
if ap.name == s {
|
||||
return ap.value, nil
|
||||
for _, ap := range artwork.KnownPriorities {
|
||||
if ap.Name == s {
|
||||
return ap.Value, nil
|
||||
}
|
||||
}
|
||||
return 0, fmt.Errorf("invalid priority %q, expected one of: %s", s, priorityNames())
|
||||
|
|
@ -801,7 +777,7 @@ func formatExplain(rep artwork.ExplainReport) string {
|
|||
case rep.Queued == nil:
|
||||
fmt.Fprintln(w, " (not queued)")
|
||||
default:
|
||||
fmt.Fprintf(w, " Priority:\t%s (%d)\n", priorityName(rep.Queued.Priority), rep.Queued.Priority)
|
||||
fmt.Fprintf(w, " Priority:\t%s (%d)\n", artwork.PriorityName(rep.Queued.Priority), rep.Queued.Priority)
|
||||
fmt.Fprintf(w, " Attempts:\t%d\n", rep.Queued.Attempts)
|
||||
fmt.Fprintf(w, " Retry at:\t%s\n", formatTime(rep.Queued.RetryAt))
|
||||
}
|
||||
|
|
|
|||
|
|
@ -940,7 +940,7 @@ var _ = Describe("parseArtworkPriority", func() {
|
|||
It("accepts every name status prints", func() {
|
||||
for _, p := range []int{model.ArtworkPriorityRecheck, model.ArtworkPriorityBackfill,
|
||||
model.ArtworkPriorityScan, model.ArtworkPriorityBump} {
|
||||
Expect(parseArtworkPriority(priorityName(p))).To(Equal(p))
|
||||
Expect(parseArtworkPriority(artwork.PriorityName(p))).To(Equal(p))
|
||||
}
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -132,7 +132,7 @@ func Explain(ctx context.Context, ds model.DataStore, ag *agents.Agents, kind mo
|
|||
}
|
||||
}
|
||||
// Set even for a kind the chain never walks, so a caller that asked to walk (e.g. --live on a
|
||||
// playlist) is reported as having tried, matching the pre-refactor CLI's unconditional flag.
|
||||
// playlist) is reported as having tried.
|
||||
rep.Walked = opts.Walk != nil
|
||||
if !Explainable(kind) {
|
||||
return rep, nil
|
||||
|
|
@ -192,3 +192,27 @@ func (r ExplainReport) GaveUpAfter() []TraceStep {
|
|||
}
|
||||
return DecodeTrace(r.Stored.LastFailure, "")
|
||||
}
|
||||
|
||||
// Priority names one of the queue's fixed priority levels.
|
||||
type Priority struct {
|
||||
Name string
|
||||
Value int
|
||||
}
|
||||
|
||||
// KnownPriorities is the one listing behind both the name and the parse, so they cannot drift.
|
||||
var KnownPriorities = []Priority{
|
||||
{"bump", model.ArtworkPriorityBump},
|
||||
{"scan", model.ArtworkPriorityScan},
|
||||
{"recheck", model.ArtworkPriorityRecheck},
|
||||
{"backfill", model.ArtworkPriorityBackfill},
|
||||
}
|
||||
|
||||
// PriorityName falls back to the number: a row written by a newer version still has to print.
|
||||
func PriorityName(p int) string {
|
||||
for _, ap := range KnownPriorities {
|
||||
if ap.Value == p {
|
||||
return ap.Name
|
||||
}
|
||||
}
|
||||
return strconv.Itoa(p)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -92,6 +92,16 @@ var _ = Describe("FormatAgents", func() {
|
|||
})
|
||||
})
|
||||
|
||||
var _ = Describe("PriorityName", func() {
|
||||
It("names a known priority", func() {
|
||||
Expect(artwork.PriorityName(model.ArtworkPriorityScan)).To(Equal("scan"))
|
||||
})
|
||||
|
||||
It("falls back to the number for an unknown priority", func() {
|
||||
Expect(artwork.PriorityName(999)).To(Equal("999"))
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("Explain", func() {
|
||||
var ds *tests.MockDataStore
|
||||
var artRepo *tests.MockArtworkRepo
|
||||
|
|
|
|||
|
|
@ -28,15 +28,15 @@ type traceStepDTO struct {
|
|||
|
||||
type storedDTO struct {
|
||||
Source string `json:"source"`
|
||||
Hash string `json:"hash,omitempty"`
|
||||
SourcePath string `json:"sourcePath,omitempty"`
|
||||
AttemptedAt string `json:"attemptedAt,omitempty"`
|
||||
}
|
||||
|
||||
type queuedDTO struct {
|
||||
Priority int `json:"priority"`
|
||||
Attempts int `json:"attempts"`
|
||||
RetryAt string `json:"retryAt,omitempty"`
|
||||
Priority int `json:"priority"`
|
||||
PriorityName string `json:"priorityName"`
|
||||
Attempts int `json:"attempts"`
|
||||
RetryAt string `json:"retryAt,omitempty"`
|
||||
}
|
||||
|
||||
type configDTO struct {
|
||||
|
|
@ -121,16 +121,16 @@ func toExplainDTO(rep artwork.ExplainReport) explainDTO {
|
|||
if rep.Stored != nil {
|
||||
dto.Stored = &storedDTO{
|
||||
Source: rep.Stored.Source,
|
||||
Hash: rep.Stored.Hash,
|
||||
SourcePath: rep.Stored.SourcePath,
|
||||
AttemptedAt: rfc3339(rep.Stored.AttemptedAt),
|
||||
}
|
||||
}
|
||||
if rep.Queued != nil {
|
||||
dto.Queued = &queuedDTO{
|
||||
Priority: rep.Queued.Priority,
|
||||
Attempts: rep.Queued.Attempts,
|
||||
RetryAt: rfc3339(rep.Queued.RetryAt),
|
||||
Priority: rep.Queued.Priority,
|
||||
PriorityName: artwork.PriorityName(rep.Queued.Priority),
|
||||
Attempts: rep.Queued.Attempts,
|
||||
RetryAt: rfc3339(rep.Queued.RetryAt),
|
||||
}
|
||||
}
|
||||
if steps := rep.LastAttemptFailed(); len(steps) > 0 {
|
||||
|
|
|
|||
|
|
@ -95,7 +95,6 @@ var _ = Describe("GET /artwork/explain", func() {
|
|||
|
||||
Expect(got["stored"]).To(Equal(map[string]any{
|
||||
"source": "external:deezer",
|
||||
"hash": "abc",
|
||||
"sourcePath": "/music/Radiohead/folder.jpg",
|
||||
"attemptedAt": attemptedAt.Format(time.RFC3339),
|
||||
}))
|
||||
|
|
@ -128,9 +127,10 @@ var _ = Describe("GET /artwork/explain", func() {
|
|||
var got map[string]any
|
||||
Expect(json.Unmarshal(w.Body.Bytes(), &got)).To(Succeed())
|
||||
Expect(got["queued"]).To(Equal(map[string]any{
|
||||
"priority": float64(model.ArtworkPriorityScan),
|
||||
"attempts": float64(1),
|
||||
"retryAt": retryAt.Format(time.RFC3339),
|
||||
"priority": float64(model.ArtworkPriorityScan),
|
||||
"priorityName": "scan",
|
||||
"attempts": float64(1),
|
||||
"retryAt": retryAt.Format(time.RFC3339),
|
||||
}))
|
||||
Expect(got["lastAttemptFailed"]).To(Equal([]any{
|
||||
map[string]any{"candidate": "external:deezer", "outcome": "error", "detail": "timeout"},
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import PropTypes from 'prop-types'
|
|||
import { Chip, Link, TableCell, TableRow } from '@material-ui/core'
|
||||
import { makeStyles } from '@material-ui/core/styles'
|
||||
import { useDataProvider, usePermissions, useTranslate } from 'react-admin'
|
||||
import { DateField } from './DateField'
|
||||
|
||||
const OUTCOME_COLORS = {
|
||||
hit: '#4caf50',
|
||||
|
|
@ -96,7 +97,7 @@ export const ArtworkInfo = ({ resource, id }) => {
|
|||
|
||||
if (!isAdmin || !report) return null
|
||||
|
||||
const recorded = report.chainOrigin !== 'not recorded'
|
||||
const recorded = !!report.stored
|
||||
return (
|
||||
<>
|
||||
<TableRow>
|
||||
|
|
@ -109,7 +110,7 @@ export const ArtworkInfo = ({ resource, id }) => {
|
|||
<>
|
||||
<Row label={translate('artwork.source')}>{report.stored?.source}</Row>
|
||||
<Row label={translate('artwork.attemptedAt')}>
|
||||
{report.stored?.attemptedAt}
|
||||
<DateField record={report.stored} source="attemptedAt" showTime />
|
||||
</Row>
|
||||
</>
|
||||
) : (
|
||||
|
|
@ -141,16 +142,21 @@ export const ArtworkInfo = ({ resource, id }) => {
|
|||
title={translate('artwork.gaveUpAfter')}
|
||||
steps={report.gaveUpAfter}
|
||||
/>
|
||||
{report.stored?.sourcePath && (
|
||||
<Row label={translate('artwork.sourcePath')}>
|
||||
{report.stored.sourcePath}
|
||||
</Row>
|
||||
)}
|
||||
{report.queued && (
|
||||
<>
|
||||
<Row label={translate('artwork.priority')}>
|
||||
{report.queued.priority}
|
||||
{report.queued.priorityName}
|
||||
</Row>
|
||||
<Row label={translate('artwork.attempts')}>
|
||||
{report.queued.attempts}
|
||||
</Row>
|
||||
<Row label={translate('artwork.retryAt')}>
|
||||
{report.queued.retryAt}
|
||||
<DateField record={report.queued} source="retryAt" showTime />
|
||||
</Row>
|
||||
</>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -22,7 +22,11 @@ vi.mock('react-admin', async (importOriginal) => {
|
|||
const report = {
|
||||
result: 'resolved from external:deezer',
|
||||
chainOrigin: 'recorded 2026-09-01T10:00:00Z',
|
||||
stored: { source: 'external:deezer', attemptedAt: '2026-09-01T10:00:00Z' },
|
||||
stored: {
|
||||
source: 'external:deezer',
|
||||
sourcePath: '/music/Radiohead/folder.jpg',
|
||||
attemptedAt: '2026-09-01T10:00:00Z',
|
||||
},
|
||||
steps: [
|
||||
{ candidate: 'cover.*', outcome: 'miss' },
|
||||
{
|
||||
|
|
@ -31,6 +35,7 @@ const report = {
|
|||
detail: 'https://cdn/x.jpg',
|
||||
},
|
||||
],
|
||||
queued: { priority: 20, priorityName: 'scan', attempts: 1, retryAt: '' },
|
||||
config: { setting: 'ArtistArtPriority', value: 'external' },
|
||||
}
|
||||
|
||||
|
|
@ -74,6 +79,8 @@ describe('<ArtworkInfo />', () => {
|
|||
await userEvent.click(screen.getByText('artwork.showDetails'))
|
||||
expect(screen.getByText('cover.*')).toBeInTheDocument()
|
||||
expect(screen.getByText('ArtistArtPriority:')).toBeInTheDocument()
|
||||
expect(screen.getByText('/music/Radiohead/folder.jpg')).toBeInTheDocument()
|
||||
expect(screen.getByText('scan')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('shows the not-recorded state', async () => {
|
||||
|
|
|
|||
|
|
@ -656,6 +656,7 @@
|
|||
"result": "Result",
|
||||
"source": "Source",
|
||||
"attemptedAt": "Attempted at",
|
||||
"sourcePath": "Source path",
|
||||
"showDetails": "Show details",
|
||||
"hideDetails": "Hide details",
|
||||
"priority": "Priority",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue