remote/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-11 11:57:12 +02:00
Code Issues Projects Releases Packages Wiki Activity

refactor: share the explain kind list and time format, trim the DTO

Browse source 
The explainable-kind list and the report's time format each existed twice,
kept in sync by a comment pointing at the other copy. Both now live in
core/artwork beside their siblings.

FormatAgents returned a CLI-worded sentence, so the web UI rendered starred
agents with no legend at all. It now reports a bool and each caller words its
own note.

Drop kind, id, chainOrigin and the numeric priority from the explain
response: all four were serialized and never read, and chainOrigin is
derivable from stored.attemptedAt on an endpoint that never walks.

ExpandInfoDialog takes an optional resource instead of a node-or-map content
prop. A page mounts one dialog per resource, which removes the silent
blank-dialog failure mode when a map was missing a key.

Outcome chips read the MUI palette rather than hardcoded hex, so they follow
the dark theme.
This commit is contained in:
Deluan 2026-09-04 07:29:54 -04:00
commit 8ba8e7f2d7
13 changed files with 142 additions and 136 deletions
Download patch file Download diff file Expand all files Collapse all files

35
cmd/artwork.go
View file

@ -9,7 +9,6 @@ import (
"os"
"slices"
"strings"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
@ -75,7 +74,7 @@ var artworkExplainCmd = &cobra.Command{
Short: "Explain why an item's artwork resolved the way it did",
Long: "Explain why an item's artwork resolved the way it did.\n\n" +
"The item can be given as a bare id, a full artwork id (e.g. al-<id>), or a <kind> <id> pair.\n" +
"<kind> is one of: " + kindPrefixes(explainKinds) + ".\n" +
"<kind> is one of: " + kindPrefixes(artwork.ExplainKinds) + ".\n" +
"A disc artwork id is the album id and the disc number, joined by a colon: <albumID>:2",
Args: cobra.RangeArgs(1, 2),
Run: func(cmd *cobra.Command, args []string) {
@ -654,13 +653,6 @@ func refreshItems(ctx context.Context, ds model.DataStore, targets []model.Artwo
return failed
}
// explainKinds is every kind explain accepts: it reports stored state and config too, so a kind
// with no chain to walk still has something to answer with.
var explainKinds = []model.Kind{
model.KindArtistArtwork, model.KindAlbumArtwork, model.KindDiscArtwork,
model.KindMediaFileArtwork, model.KindPlaylistArtwork, model.KindRadioArtwork,
}
func kindPrefixes(kinds []model.Kind) string {
return strings.Join(model.KindPrefixes(kinds), ", ")
}
@ -725,6 +717,14 @@ func artworkKindAndID(ctx context.Context, ds model.DataStore, arg string) (mode
// cliUnavailableNote marks agents the CLI cannot construct; a running server loads them all.
const cliUnavailableNote = " (* not available to the CLI)"
// cliAgents words the CLI's own legend for the starred agents FormatAgents reports.
func cliAgents(rep artwork.ExplainReport) string {
if rep.AgentsIncomplete {
return rep.Agents + cliUnavailableNote
}
return rep.Agents
}
// writeSteps prints the trace rows. An empty last cell would end tabwriter's column block and
// break the alignment, so a missing detail is rendered as a dash.
func writeSteps(w io.Writer, indent string, steps []artwork.TraceStep) {
@ -767,7 +767,7 @@ func formatExplain(rep artwork.ExplainReport) string {
if rep.Stored.SourcePath != "" {
fmt.Fprintf(w, " Source path:\t%s\n", rep.Stored.SourcePath)
}
fmt.Fprintf(w, " Attempted at:\t%s\n", formatTime(rep.Stored.AttemptedAt))
fmt.Fprintf(w, " Attempted at:\t%s\n", artwork.FormatTime(rep.Stored.AttemptedAt))
}
fmt.Fprintln(w, "\nQueue")
@ -779,7 +779,7 @@ func formatExplain(rep artwork.ExplainReport) string {
default:
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))
fmt.Fprintf(w, " Retry at:\t%s\n", artwork.FormatTime(rep.Queued.RetryAt))
}
if rep.Queued != nil {
writeStepTable(w, "Last attempt failed", rep.LastAttemptFailed())
@ -794,7 +794,7 @@ func formatExplain(rep artwork.ExplainReport) string {
} else {
fmt.Fprintf(w, " %s:\t%s\n", setting, value)
if rep.Agents != "" {
fmt.Fprintf(w, " Agents:\t%s\n", rep.Agents)
fmt.Fprintf(w, " Agents:\t%s\n", cliAgents(rep))
}
}
@ -832,18 +832,11 @@ func formatExplain(rep artwork.ExplainReport) string {
return sb.String()
}
func formatTime(t time.Time) string {
if t.IsZero() {
return "-"
}
return t.Format(time.RFC3339)
}
func runExplain(ctx context.Context, args []string) {
defer db.Init(ctx)()
ds, ctx := getAdminContext(ctx)
targets, failures, err := resolveArtworkTargets(ctx, ds, args, explainKinds)
targets, failures, err := resolveArtworkTargets(ctx, ds, args, artwork.ExplainKinds)
if err != nil {
log.Fatal(ctx, err)
}
@ -855,7 +848,7 @@ func runExplain(ctx context.Context, args []string) {
}
kind, id := targets[0].Kind, targets[0].ID
opts := artwork.ExplainOptions{UnavailableNote: cliUnavailableNote}
var opts artwork.ExplainOptions
// Only artist and album reach an agent, and the load must precede the resolver, which reads the
// same manager. Leaving ag nil elsewhere avoids handing agents.GetAgents a not-yet-loaded manager.
var ag *agents.Agents

26
cmd/artwork_test.go
View file

@ -41,8 +41,8 @@ var _ = Describe("parseArtworkKind", func() {
_, err := parseArtworkKind(prefix, valid)
Expect(err).ToNot(HaveOccurred())
},
Entry("explain reads disc artwork", "dc", explainKinds),
Entry("explain reads media file artwork", "mf", explainKinds),
Entry("explain reads disc artwork", "dc", artwork.ExplainKinds),
Entry("explain reads media file artwork", "mf", artwork.ExplainKinds),
// Disc artwork has no state to clear and the worker cannot resolve it, so refresh must not
// accept it: the queue row would be rejected on every drain.
Entry("refresh re-queues media files", "mf", artwork.RefreshableKinds),
@ -65,7 +65,7 @@ var _ = Describe("resolveArtworkTargets", func() {
})
It("accepts the explicit <kind> <id> leader shared by every id", func() {
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"al", "x", "y"}, explainKinds)
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"al", "x", "y"}, artwork.ExplainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(failures).To(BeEmpty())
Expect(targets).To(Equal([]model.ArtworkID{
@ -78,20 +78,20 @@ var _ = Describe("resolveArtworkTargets", func() {
})
It("resolves a bare id by looking it up across tables", func() {
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"artist1"}, explainKinds)
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"artist1"}, artwork.ExplainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(failures).To(BeEmpty())
Expect(targets).To(Equal([]model.ArtworkID{{Kind: model.KindArtistArtwork, ID: "artist1"}}))
})
It("reads the kind from a full artwork id prefix without a database lookup", func() {
targets, _, err := resolveArtworkTargets(ctx, ds, []string{"al-realalbum"}, explainKinds)
targets, _, err := resolveArtworkTargets(ctx, ds, []string{"al-realalbum"}, artwork.ExplainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(targets).To(Equal([]model.ArtworkID{{Kind: model.KindAlbumArtwork, ID: "realalbum"}}))
})
It("strips the hash suffix from a full artwork id", func() {
targets, _, err := resolveArtworkTargets(ctx, ds, []string{"al-realalbum_0123456789abcdef"}, explainKinds)
targets, _, err := resolveArtworkTargets(ctx, ds, []string{"al-realalbum_0123456789abcdef"}, artwork.ExplainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(targets).To(Equal([]model.ArtworkID{{Kind: model.KindAlbumArtwork, ID: "realalbum"}}))
})
@ -105,7 +105,7 @@ var _ = Describe("resolveArtworkTargets", func() {
})
It("collects an id that matches nothing and has no kind prefix", func() {
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"nope"}, explainKinds)
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"nope"}, artwork.ExplainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(targets).To(BeEmpty())
Expect(failures).To(HaveLen(1))
@ -113,7 +113,7 @@ var _ = Describe("resolveArtworkTargets", func() {
})
It("resolves the valid ids and collects the unresolvable ones", func() {
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"artist1", "nope", "al-realalbum"}, explainKinds)
targets, failures, err := resolveArtworkTargets(ctx, ds, []string{"artist1", "nope", "al-realalbum"}, artwork.ExplainKinds)
Expect(err).ToNot(HaveOccurred())
Expect(targets).To(Equal([]model.ArtworkID{
{Kind: model.KindArtistArtwork, ID: "artist1"}, {Kind: model.KindAlbumArtwork, ID: "realalbum"}}))
@ -142,6 +142,16 @@ var _ = Describe("formatExplain", func() {
}
})
It("explains the star on an agent the CLI could not construct", func() {
rep.AgentsIncomplete = true
Expect(formatExplain(rep)).To(ContainSubstring("(* not available to the CLI)"))
})
It("leaves the agent line unadorned when every agent is available", func() {
Expect(formatExplain(rep)).ToNot(ContainSubstring("not available to the CLI"))
})
It("reports the item, its config and the chain it walked", func() {
out := formatExplain(rep)
Expect(out).To(ContainSubstring("Radiohead"))

45
core/artwork/explain.go
View file

@ -67,10 +67,11 @@ func ImageAgentNames(ag *agents.Agents, kind model.Kind) []string {
}
// FormatAgents accounts for every configured agent: one that cannot be constructed never reaches
// the chain, so the raw list alone overstates it. unavailableNote is appended only if some are.
func FormatAgents(configured string, available []string, unavailableNote string) string {
// the chain, so the raw list alone overstates it. Those are starred, and the bool lets each
// caller word its own legend.
func FormatAgents(configured string, available []string) (string, bool) {
if strings.TrimSpace(configured) == "" {
return "(none)"
return "(none)", false
}
var unavailable bool
names := slice.Map(strings.Split(configured, ","), func(name string) string {
@ -81,11 +82,7 @@ func FormatAgents(configured string, available []string, unavailableNote string)
unavailable = true
return name + "*"
})
line := strings.Join(names, ", ")
if unavailable {
line += unavailableNote
}
return line
return strings.Join(names, ", "), unavailable
}
// ExplainOptions configures a single explain. The zero value reads history and never
@ -93,22 +90,22 @@ func FormatAgents(configured string, available []string, unavailableNote string)
type ExplainOptions struct {
// Walk builds a resolver that records into the trace; nil reads the recorded trace instead.
Walk func(*ChainTrace) *TracingResolver
// UnavailableNote is appended to the agent line when a configured agent is missing.
UnavailableNote string
}
// ExplainReport is everything known about how one item's artwork resolved.
type ExplainReport struct {
Kind model.Kind
ID string
Name string
Stored *model.ItemArtwork
Queued *model.ArtworkQueueItem
Steps []TraceStep
Source string
Agents string
Walked bool
ResolveErr error
Kind model.Kind
ID string
Name string
Stored *model.ItemArtwork
Queued *model.ArtworkQueueItem
Steps []TraceStep
Source string
Agents string
// AgentsIncomplete reports that some configured agent is starred in Agents.
AgentsIncomplete bool
Walked bool
ResolveErr error
}
// Explain gathers everything known about how kind/id's artwork resolved: stored state, the
@ -138,7 +135,7 @@ func Explain(ctx context.Context, ds model.DataStore, ag *agents.Agents, kind mo
return rep, nil
}
if ag != nil && (kind == model.KindArtistArtwork || kind == model.KindAlbumArtwork) {
rep.Agents = FormatAgents(conf.Server.Agents, ImageAgentNames(ag, kind), opts.UnavailableNote)
rep.Agents, rep.AgentsIncomplete = FormatAgents(conf.Server.Agents, ImageAgentNames(ag, kind))
}
switch {
@ -163,13 +160,13 @@ func (r ExplainReport) ChainOrigin() string {
return "walked now"
}
if r.Stored != nil {
return "recorded " + formatAttemptedAt(r.Stored.AttemptedAt)
return "recorded " + FormatTime(r.Stored.AttemptedAt)
}
return "not recorded"
}
// formatAttemptedAt matches the CLI's formatTime: a zero time reads as unset, not as year 1.
func formatAttemptedAt(t time.Time) string {
// FormatTime renders a timestamp for the report: a zero time reads as unset, not as year 1.
func FormatTime(t time.Time) string {
if t.IsZero() {
return "-"
}

17
core/artwork/explain_test.go
View file

@ -79,16 +79,21 @@ var _ = Describe("ConfigFor", func() {
var _ = Describe("FormatAgents", func() {
It("reports none when nothing is configured", func() {
Expect(artwork.FormatAgents(" ", nil, "")).To(Equal("(none)"))
line, incomplete := artwork.FormatAgents(" ", nil)
Expect(line).To(Equal("(none)"))
Expect(incomplete).To(BeFalse())
})
It("keeps the configured order and marks what is unavailable", func() {
got := artwork.FormatAgents("spotify, lastfm", []string{"lastfm"}, " (* missing)")
Expect(got).To(Equal("spotify*, lastfm (* missing)"))
got, incomplete := artwork.FormatAgents("spotify, lastfm", []string{"lastfm"})
Expect(incomplete).To(BeTrue())
Expect(got).To(Equal("spotify*, lastfm"))
})
It("omits the note when every configured agent is available", func() {
Expect(artwork.FormatAgents("lastfm", []string{"lastfm"}, " (* missing)")).To(Equal("lastfm"))
It("reports complete when every configured agent is available", func() {
line, incomplete := artwork.FormatAgents("lastfm", []string{"lastfm"})
Expect(line).To(Equal("lastfm"))
Expect(incomplete).To(BeFalse())
})
})
@ -143,7 +148,7 @@ var _ = Describe("Explain", func() {
Expect(rep.ChainOrigin()).To(ContainSubstring("recorded"))
})
It("renders a zero attempted-at the same way the CLI's formatTime does", func() {
It("renders a zero attempted-at as unset, not as year 1", func() {
rep := artwork.ExplainReport{Stored: &model.ItemArtwork{Source: "folder"}}
Expect(rep.ChainOrigin()).To(Equal("recorded -"))
})

7
core/artwork/housekeeping.go
View file

@ -25,6 +25,13 @@ var ReprocessKinds = []model.Kind{
// artwork is read through on every request and cached by content key, so it has neither.
func KeepsState(kind model.Kind) bool { return kind != model.KindDiscArtwork }
// ExplainKinds is every kind Explain accepts: it reports stored state and config too, so a kind
// with no chain to walk still has something to answer with.
var ExplainKinds = []model.Kind{
model.KindArtistArtwork, model.KindAlbumArtwork, model.KindDiscArtwork,
model.KindMediaFileArtwork, model.KindPlaylistArtwork, model.KindRadioArtwork,
}
// RefreshableKinds is every kind Refresh can clear and re-queue, so it holds exactly the kinds
// KeepsState admits. Media files are absent from ReprocessKinds but belong here: the worker
// resolves them, it just never enumerates them in bulk.

27
server/nativeapi/artwork_explain.go
View file

@ -13,13 +13,6 @@ import (
"github.com/navidrome/navidrome/model"
)
// explainableKinds is every kind the endpoint accepts, matching the CLI's explainKinds: a kind
// with no chain to walk still has stored state and config to report.
var explainableKinds = []model.Kind{
model.KindArtistArtwork, model.KindAlbumArtwork, model.KindDiscArtwork,
model.KindMediaFileArtwork, model.KindPlaylistArtwork, model.KindRadioArtwork,
}
type traceStepDTO struct {
Candidate string `json:"candidate"`
Outcome string `json:"outcome"`
@ -33,7 +26,6 @@ type storedDTO struct {
}
type queuedDTO struct {
Priority int `json:"priority"`
PriorityName string `json:"priorityName"`
Attempts int `json:"attempts"`
RetryAt string `json:"retryAt,omitempty"`
@ -45,11 +37,8 @@ type configDTO struct {
}
type explainDTO struct {
Kind string `json:"kind"`
ID string `json:"id"`
Name string `json:"name"`
Result string `json:"result"`
ChainOrigin string `json:"chainOrigin"`
Steps []traceStepDTO `json:"steps"`
Stored *storedDTO `json:"stored,omitempty"`
Queued *queuedDTO `json:"queued,omitempty"`
@ -57,6 +46,7 @@ type explainDTO struct {
GaveUpAfter []traceStepDTO `json:"gaveUpAfter,omitempty"`
Config *configDTO `json:"config,omitempty"`
Agents string `json:"agents,omitempty"`
AgentsIncomplete bool `json:"agentsIncomplete,omitempty"`
}
func (api *Router) addArtworkExplainRoute(r chi.Router) {
@ -67,7 +57,7 @@ func (api *Router) explainArtwork() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
kind, ok := model.ParseKind(r.URL.Query().Get("kind"))
if !ok || !slices.Contains(explainableKinds, kind) {
if !ok || !slices.Contains(artwork.ExplainKinds, kind) {
http.Error(w, "invalid artwork kind", http.StatusBadRequest)
return
}
@ -110,13 +100,11 @@ func toStepDTOs(steps []artwork.TraceStep) []traceStepDTO {
func toExplainDTO(rep artwork.ExplainReport) explainDTO {
dto := explainDTO{
Kind: rep.Kind.Prefix(),
ID: rep.ID,
Name: rep.Name,
Result: rep.Result(),
ChainOrigin: rep.ChainOrigin(),
Steps: toStepDTOs(rep.Steps),
Agents: rep.Agents,
Name: rep.Name,
Result: rep.Result(),
Steps: toStepDTOs(rep.Steps),
Agents: rep.Agents,
AgentsIncomplete: rep.AgentsIncomplete,
}
if rep.Stored != nil {
dto.Stored = &storedDTO{
@ -127,7 +115,6 @@ func toExplainDTO(rep artwork.ExplainReport) explainDTO {
}
if rep.Queued != nil {
dto.Queued = &queuedDTO{
Priority: rep.Queued.Priority,
PriorityName: artwork.PriorityName(rep.Queued.Priority),
Attempts: rep.Queued.Attempts,
RetryAt: rfc3339(rep.Queued.RetryAt),

4
server/nativeapi/artwork_explain_test.go
View file

@ -90,7 +90,6 @@ var _ = Describe("GET /artwork/explain", func() {
Expect(json.Unmarshal(w.Body.Bytes(), &got)).To(Succeed())
Expect(got["name"]).To(Equal("Radiohead"))
Expect(got["result"]).To(Equal("resolved from external:deezer"))
Expect(got["chainOrigin"]).To(ContainSubstring("recorded"))
Expect(got["config"]).To(HaveKeyWithValue("setting", "ArtistArtPriority"))
Expect(got["stored"]).To(Equal(map[string]any{
@ -127,7 +126,6 @@ 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),
"priorityName": "scan",
"attempts": float64(1),
"retryAt": retryAt.Format(time.RFC3339),
@ -149,6 +147,6 @@ var _ = Describe("GET /artwork/explain", func() {
Expect(got).ToNot(HaveKey("queued"))
Expect(got).ToNot(HaveKey("lastAttemptFailed"))
Expect(got).ToNot(HaveKey("gaveUpAfter"))
Expect(got["chainOrigin"]).To(Equal("not recorded"))
Expect(got).ToNot(HaveKey("chainOrigin"))
})
})

5
ui/src/artist/DesktopArtistDetails.jsx
View file

@ -161,9 +161,8 @@ const DesktopArtistDetails = ({ artistInfo, record, biography }) => {
/>
)}
</Card>
<ExpandInfoDialog
content={{ album: <AlbumInfo />, artist: <ArtistInfo /> }}
/>
<ExpandInfoDialog resource="album" content={<AlbumInfo />} />
<ExpandInfoDialog resource="artist" content={<ArtistInfo />} />
</div>
)
}

33
ui/src/common/ArtworkInfo.jsx
View file

@ -2,21 +2,19 @@ import React, { useEffect, useState } from 'react'
import PropTypes from 'prop-types'
import { Chip, Link, TableCell, TableRow } from '@material-ui/core'
import { makeStyles } from '@material-ui/core/styles'
import clsx from 'clsx'
import { useDataProvider, usePermissions, useTranslate } from 'react-admin'
import { DateField } from './DateField'
const OUTCOME_COLORS = {
hit: '#4caf50',
miss: '#9e9e9e',
skipped: '#9e9e9e',
error: '#f44336',
unreadable: '#f44336',
}
const useStyles = makeStyles({
chip: { color: '#fff', height: 20 },
const useStyles = makeStyles((theme) => ({
chip: { color: theme.palette.common.white, height: 20 },
hit: { backgroundColor: theme.palette.success.main },
error: { backgroundColor: theme.palette.error.main },
neutral: { backgroundColor: theme.palette.grey[500] },
toggle: { cursor: 'pointer' },
})
}))
const OUTCOME_CLASS = { hit: 'hit', error: 'error', unreadable: 'error' }
const OutcomeChip = ({ outcome }) => {
const classes = useStyles()
@ -24,8 +22,10 @@ const OutcomeChip = ({ outcome }) => {
<Chip
size="small"
label={outcome}
className={classes.chip}
style={{ backgroundColor: OUTCOME_COLORS[outcome] || '#9e9e9e' }}
className={clsx(
classes.chip,
classes[OUTCOME_CLASS[outcome] || 'neutral'],
)}
/>
)
}
@ -164,7 +164,12 @@ export const ArtworkInfo = ({ resource, id }) => {
<Row label={report.config.setting}>{report.config.value}</Row>
)}
{report.agents && (
<Row label={translate('artwork.agents')}>{report.agents}</Row>
<Row label={translate('artwork.agents')}>
{report.agents}
{report.agentsIncomplete && (
<div>{translate('artwork.agentsIncomplete')}</div>
)}
</Row>
)}
</>
)}

6
ui/src/dataProvider/wrapperDataProvider.js
View file

@ -4,7 +4,7 @@ import { REST_URL } from '../consts'
const dataProvider = jsonServerProvider(REST_URL, httpClient)
const REFRESH_KIND = { album: 'al', artist: 'ar' }
const ARTWORK_KIND = { album: 'al', artist: 'ar' }
const isAdmin = () => {
const role = localStorage.getItem('role')
@ -226,12 +226,12 @@ const wrapperDataProvider = {
// The endpoint answers 204 with no body, but react-admin rejects any response without a
// `data` key, so the id stands in for one.
refreshMetadata: (resource, id) =>
httpClient(`${REST_URL}/metadata/${REFRESH_KIND[resource]}/${id}/refresh`, {
httpClient(`${REST_URL}/metadata/${ARTWORK_KIND[resource]}/${id}/refresh`, {
method: 'POST',
}).then(() => ({ data: { id } })),
explainArtwork: (resource, id) =>
httpClient(
`${REST_URL}/artwork/explain?kind=${REFRESH_KIND[resource]}&id=${encodeURIComponent(id)}`,
`${REST_URL}/artwork/explain?kind=${ARTWORK_KIND[resource]}&id=${encodeURIComponent(id)}`,
).then(({ json }) => ({ data: json })),
}

26
ui/src/dialogs/ExpandInfoDialog.jsx
View file

@ -11,14 +11,16 @@ import {
} from '@material-ui/core'
import { closeExtendedInfoDialog } from '../actions'
const ExpandInfoDialog = ({ title, content }) => {
const { open, record, resource } = useSelector(
(state) => state.expandInfoDialog,
)
const ExpandInfoDialog = ({ title, content, resource }) => {
const {
open,
record,
resource: openFor,
} = useSelector((state) => state.expandInfoDialog)
const dispatch = useDispatch()
const translate = useTranslate()
// A node renders for any record; a map lets one page serve more than one resource.
const body = React.isValidElement(content) ? content : content?.[resource]
// One page may mount several of these; each claims the resource it was given.
const mine = !resource || resource === openFor
const handleClose = (e) => {
dispatch(closeExtendedInfoDialog())
@ -27,7 +29,7 @@ const ExpandInfoDialog = ({ title, content }) => {
return (
<Dialog
open={open}
open={open && mine}
onClose={handleClose}
aria-labelledby="info-dialog-album"
fullWidth={true}
@ -37,8 +39,10 @@ const ExpandInfoDialog = ({ title, content }) => {
{translate(title || 'resources.song.actions.info')}
</DialogTitle>
<DialogContent>
{record && body && (
<RecordContextProvider value={record}>{body}</RecordContextProvider>
{record && mine && (
<RecordContextProvider value={record}>
{content}
</RecordContextProvider>
)}
</DialogContent>
<DialogActions>
@ -52,8 +56,8 @@ const ExpandInfoDialog = ({ title, content }) => {
ExpandInfoDialog.propTypes = {
title: PropTypes.string,
content: PropTypes.oneOfType([PropTypes.element, PropTypes.object])
.isRequired,
content: PropTypes.element.isRequired,
resource: PropTypes.string,
}
export default ExpandInfoDialog

50
ui/src/dialogs/ExpandInfoDialog.test.jsx
View file

@ -4,51 +4,51 @@ import { render, screen, cleanup } from '@testing-library/react'
import { describe, afterEach, it, expect } from 'vitest'
import ExpandInfoDialog from './ExpandInfoDialog'
const renderDialog = (content, resource) =>
const renderDialogs = (openFor, dialogs) =>
render(
<TestContext
initialState={{
expandInfoDialog: {
open: true,
record: { id: 'r1', name: 'Record' },
resource,
resource: openFor,
},
}}
>
<ExpandInfoDialog content={content} />
{dialogs}
</TestContext>,
)
describe('ExpandInfoDialog', () => {
afterEach(cleanup)
it('renders a node content as-is, regardless of resource', () => {
renderDialog(<div>Song Info</div>, 'song')
it('renders an unclaimed dialog for any resource', () => {
renderDialogs('song', <ExpandInfoDialog content={<div>Song Info</div>} />)
expect(screen.getByText('Song Info')).toBeInTheDocument()
})
it('resolves the content by resource when given a map', () => {
renderDialog(
{ album: <div>Album Info</div>, artist: <div>Artist Info</div> },
// Guards the artist detail page, which mounts both dialogs: an album card's Get Info
// must open AlbumInfo, and the page's own ArtistInfo must stay shut.
it.each([
['artist', 'Artist Info', 'Album Info'],
['album', 'Album Info', 'Artist Info'],
])('opens only the %s dialog', (openFor, shown, hidden) => {
renderDialogs(
openFor,
<>
<ExpandInfoDialog resource="album" content={<div>Album Info</div>} />
<ExpandInfoDialog resource="artist" content={<div>Artist Info</div>} />
</>,
)
expect(screen.getByText(shown)).toBeInTheDocument()
expect(screen.queryByText(hidden)).not.toBeInTheDocument()
})
it('stays shut when no mounted dialog claims the resource', () => {
renderDialogs(
'artist',
<ExpandInfoDialog resource="album" content={<div>Album Info</div>} />,
)
expect(screen.getByText('Artist Info')).toBeInTheDocument()
expect(screen.queryByText('Album Info')).not.toBeInTheDocument()
})
// Guards the artist detail page, which maps both resources to one dialog: an album
// card's Get Info must resolve to AlbumInfo, not the page's own ArtistInfo.
it('resolves the album entry, not artist, when the resource is album', () => {
renderDialog(
{ album: <div>Album Info</div>, artist: <div>Artist Info</div> },
'album',
)
expect(screen.getByText('Album Info')).toBeInTheDocument()
expect(screen.queryByText('Artist Info')).not.toBeInTheDocument()
})
it('renders nothing when the map has no entry for the resource', () => {
renderDialog({ album: <div>Album Info</div> }, 'artist')
expect(screen.queryByText('Album Info')).not.toBeInTheDocument()
})
})

1
ui/src/i18n/en.json
View file

@ -665,6 +665,7 @@
"lastAttemptFailed": "Last attempt failed",
"gaveUpAfter": "Gave up after",
"agents": "Agents",
"agentsIncomplete": "* This agent is configured but cannot supply images",
"notRecorded": "No resolution recorded yet"
},
"player": {