Merge branch 'master' into songs-under-30s-dont-scrobble-on-lastfm/5986

go.mod

@@ -3,7 +3,7 @@ module github.com/navidrome/navidrome
 go 1.27
 
 // Fork to implement raw tags support
-replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3
+replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260905051825-df1d035571df
 
 require (
 	github.com/Masterminds/squirrel v1.5.4

go.sum

@@ -29,8 +29,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1 h1:5RVFMOWjMyRy8cARdy79nAmgYw3hK/4HUq48LQ6Wwqo=
 github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1/go.mod h1:ZXNYxsqcloTdSy/rNShjYzMhyjf0LaoftYK0p+A3h40=
-github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3 h1:j7eSXqgtjhlNfwnMEzRdXnJGZTEw4I7J9TeQAll83bU=
-github.com/deluan/go-taglib v0.0.0-20260720134629-a133b9719ea3/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY=
+github.com/deluan/go-taglib v0.0.0-20260905051825-df1d035571df h1:LdLQVAWVc6hCzqnrfVIEXOhP+r0iSit+EvsXwZDyL70=
+github.com/deluan/go-taglib v0.0.0-20260905051825-df1d035571df/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY=
 github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf h1:tb246l2Zmpt/GpF9EcHCKTtwzrd0HGfEmoODFA/qnk4=
 github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf/go.mod h1:tSgDythFsl0QgS/PFWfIZqcJKnkADWneY80jaVRlqK8=
 github.com/deluan/sanitize v0.0.0-20241120162836-fdfd8fdfaa55 h1:wSCnggTs2f2ji6nFwQmfwgINcmSMj0xF0oHnoyRSPe4=

persistence/playlist_repository.go

@@ -316,11 +316,12 @@ func (r *playlistRepository) refreshCounters(pls *model.Playlist) error {
 	}
 
 	// Update playlist's total duration, size and count
+	now := time.Now()
 	upd := Update("playlist").
 		Set("duration", res.Duration).
 		Set("size", res.Size).
 		Set("song_count", res.Count).
-		Set("updated_at", time.Now()).
+		Set("updated_at", now).
 		Where(Eq{"id": pls.ID})
 	_, err = r.executeSQL(upd)
 	if err != nil {
@@ -329,6 +330,7 @@ func (r *playlistRepository) refreshCounters(pls *model.Playlist) error {
 	pls.SongCount = int(res.Count)
 	pls.Duration = res.Duration
 	pls.Size = int64(res.Size)
+	pls.UpdatedAt = now
 	return nil
 }
 

persistence/smart_playlist_repository.go

@@ -58,7 +58,8 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool {
 		return false
 	}
 
-	now := time.Now()
+	// Reuse the stamp refreshCounters just wrote, so evaluated_at and updated_at agree
+	now := pls.UpdatedAt
 	updSql := Update(r.tableName).Set("evaluated_at", now).Where(Eq{"id": pls.ID})
 	if _, err = r.executeSQL(updSql); err != nil {
 		log.Error(r.ctx, "Error updating smart playlist", "playlist", pls.Name, "id", pls.ID, err)

persistence/smart_playlist_repository_test.go

@@ -45,6 +45,23 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() {
 			})
 		})
 
+		Context("after an evaluation", func() {
+			It("stamps updated_at and evaluated_at with the same instant", func() {
+				newPls := model.Playlist{Name: "Evaluated", OwnerID: "userid", Rules: rules}
+				Expect(repo.Put(&newPls)).To(Succeed())
+				DeferCleanup(func() { _ = repo.Delete(newPls.ID) })
+
+				refreshed, err := repo.GetWithTracks(newPls.ID, true, false)
+				Expect(err).ToNot(HaveOccurred())
+
+				stored, err := repo.Get(newPls.ID)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(stored.EvaluatedAt).ToNot(BeNil())
+				Expect(stored.UpdatedAt).To(BeTemporally("==", *stored.EvaluatedAt))
+				Expect(refreshed.UpdatedAt).To(BeTemporally("==", stored.UpdatedAt))
+			})
+		})
+
 		Context("invalid rules", func() {
 			It("fails to Put it in the DB", func() {
 				rules = &criteria.Criteria{

server/subsonic/playlists.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"time"
 
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/log"
@@ -133,15 +132,7 @@ func (api *Router) buildPlaylist(ctx context.Context, p model.Playlist) response
 	pls.SongCount = int32(p.SongCount)
 	pls.Duration = int32(p.Duration)
 	pls.Created = p.CreatedAt
-	if p.IsSmartPlaylist() {
-		if p.EvaluatedAt != nil {
-			pls.Changed = *p.EvaluatedAt
-		} else {
-			pls.Changed = time.Now()
-		}
-	} else {
-		pls.Changed = p.UpdatedAt
-	}
+	pls.Changed = p.UpdatedAt
 
 	player, ok := request.PlayerFrom(ctx)
 	if ok && isClientInList(conf.Server.Subsonic.MinimalClients, player.Client) {

server/subsonic/playlists_test.go

@@ -220,7 +220,7 @@ var _ = Describe("buildPlaylist", func() {
 				Expect(result.SongCount).To(Equal(int32(10)))
 				Expect(result.Duration).To(Equal(int32(600)))
 				Expect(result.Created).To(Equal(playlist.CreatedAt))
-				Expect(result.Changed).To(Equal(evaluatedAt))
+				Expect(result.Changed).To(Equal(playlist.UpdatedAt))
 
 				// These should not be set
 				Expect(result.Comment).To(BeEmpty())
@@ -245,7 +245,7 @@ var _ = Describe("buildPlaylist", func() {
 				Expect(result.SongCount).To(Equal(int32(10)))
 				Expect(result.Duration).To(Equal(int32(600)))
 				Expect(result.Created).To(Equal(playlist.CreatedAt))
-				Expect(result.Changed).To(Equal(*playlist.EvaluatedAt))
+				Expect(result.Changed).To(Equal(playlist.UpdatedAt))
 				Expect(result.Comment).To(Equal("Test comment"))
 				Expect(result.Owner).To(Equal("admin"))
 				Expect(result.Public).To(BeTrue())
@@ -271,6 +271,21 @@ var _ = Describe("buildPlaylist", func() {
 			})
 		})
 
+		Context("when it was never evaluated", func() {
+			BeforeEach(func() {
+				playlist.EvaluatedAt = nil
+				player := model.Player{Client: "regular-client"}
+				ctx = request.WithPlayer(ctx, player)
+			})
+
+			It("omits validUntil but still reports changed", func() {
+				result := router.buildPlaylist(ctx, playlist)
+
+				Expect(result.ValidUntil).To(BeNil())
+				Expect(result.Changed).To(Equal(playlist.UpdatedAt))
+			})
+		})
+
 		Context("with a per-playlist refreshDelay", func() {
 			BeforeEach(func() {
 				playlist.Rules.RefreshDelay = 24 * time.Hour

ui/src/album/AlbumActions.jsx

@@ -27,7 +27,7 @@ import {
 } from '../actions'
 import { formatBytes } from '../utils'
 import config from '../config'
-import { ToggleFieldsMenu } from '../common'
+import { RefreshMetadataButton, ToggleFieldsMenu } from '../common'
 
 const useStyles = makeStyles({
   toolbar: { display: 'flex', justifyContent: 'space-between', width: '100%' },
@@ -138,6 +138,11 @@ const AlbumActions = ({
               <CloudDownloadOutlinedIcon />
             </AlbumButton>
           )}
+          <RefreshMetadataButton
+            resource="album"
+            record={record}
+            size="small"
+          />
         </div>
         <div>{isNotSmall && <ToggleFieldsMenu resource="albumSong" />}</div>
       </div>

ui/src/album/AlbumActions.test.jsx

@@ -0,0 +1,68 @@
+import React from 'react'
+import { render, screen, fireEvent, waitFor } from '@testing-library/react'
+import { ThemeProvider, createTheme } from '@material-ui/core/styles'
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import AlbumActions from './AlbumActions'
+
+const { mockConfig, mockPermissions } = vi.hoisted(() => ({
+  mockConfig: {
+    enableSharing: false,
+    enableDownloads: false,
+    losslessFormats: 'FLAC,WAV',
+  },
+  mockPermissions: { value: 'admin' },
+}))
+vi.mock('../config', () => ({ default: mockConfig }))
+
+vi.mock('react-redux', () => ({
+  useDispatch: () => vi.fn(),
+  useSelector: () => ({}),
+}))
+
+const mockRefreshMetadata = vi.fn()
+
+vi.mock('react-admin', async (importOriginal) => {
+  const actual = await importOriginal()
+  return {
+    ...actual,
+    useNotify: () => vi.fn(),
+    useDataProvider: () => ({ refreshMetadata: mockRefreshMetadata }),
+    usePermissions: () => ({ permissions: mockPermissions.value }),
+    useTranslate: () => (x) => x,
+  }
+})
+
+describe('AlbumActions', () => {
+  const record = { id: 'al1', name: 'Album', size: 1024 }
+  const refreshLabel = 'resources.album.actions.refresh'
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockPermissions.value = 'admin'
+    mockRefreshMetadata.mockResolvedValue({ data: { id: 'al1' } })
+  })
+
+  const renderAlbumActions = () =>
+    render(
+      <ThemeProvider theme={createTheme()}>
+        <AlbumActions record={record} ids={[]} data={{}} />
+      </ThemeProvider>,
+    )
+
+  it('refreshes the album metadata for admins', async () => {
+    renderAlbumActions()
+    fireEvent.click(screen.getByRole('button', { name: refreshLabel }))
+
+    await waitFor(() =>
+      expect(mockRefreshMetadata).toHaveBeenCalledWith('album', 'al1'),
+    )
+  })
+
+  it('hides the action for non-admin users', () => {
+    mockPermissions.value = 'regular'
+    renderAlbumActions()
+    expect(
+      screen.queryByRole('button', { name: refreshLabel }),
+    ).not.toBeInTheDocument()
+  })
+})

ui/src/artist/ArtistActions.jsx

@@ -26,6 +26,7 @@ import {
 import config from '../config'
 import { formatBytes } from '../utils'
 import { artistDownloadSize } from '../common/artist'
+import { RefreshMetadataButton } from '../common/RefreshMetadataButton'
 
 const useStyles = makeStyles((theme) => ({
   toolbar: {
@@ -33,6 +34,7 @@ const useStyles = makeStyles((theme) => ({
     padding: '0 !important',
     background: 'transparent',
     boxShadow: 'none',
+    alignItems: 'center',
     '& .MuiToolbar-root': {
       minHeight: 'auto',
       padding: '0 !important',
@@ -174,6 +176,11 @@ const ArtistActions = ({ className, record, ...rest }) => {
           icon={<CloudDownloadOutlinedIcon />}
         />
       )}
+      <RefreshMetadataButton
+        resource="artist"
+        record={record}
+        size={isMobile ? 'small' : 'medium'}
+      />
     </TopToolbar>
   )
 }

ui/src/artist/ArtistActions.test.jsx

@@ -18,20 +18,26 @@ vi.mock('../subsonic', () => ({
   default: { getSimilarSongs2: vi.fn(), getTopSongs: vi.fn() },
 }))
 
-const { mockConfig } = vi.hoisted(() => ({
+const { mockConfig, mockPermissions } = vi.hoisted(() => ({
   mockConfig: { enableSharing: true, enableDownloads: true },
+  mockPermissions: { value: 'admin' },
 }))
 vi.mock('../config', () => ({ default: mockConfig }))
 
 const mockNotify = vi.fn()
 const mockGetList = vi.fn().mockResolvedValue({ data: [{ id: 's1' }] })
+const mockRefreshMetadata = vi.fn().mockResolvedValue({ data: { id: 'ar1' } })
 
 vi.mock('react-admin', async (importOriginal) => {
   const actual = await importOriginal()
   return {
     ...actual,
     useNotify: () => mockNotify,
-    useDataProvider: () => ({ getList: mockGetList }),
+    useDataProvider: () => ({
+      getList: mockGetList,
+      refreshMetadata: mockRefreshMetadata,
+    }),
+    usePermissions: () => ({ permissions: mockPermissions.value }),
     useTranslate: () => (x) => x,
   }
 })
@@ -64,6 +70,7 @@ describe('ArtistActions', () => {
     vi.spyOn(console, 'error').mockImplementation(() => {})
     mockConfig.enableSharing = true
     mockConfig.enableDownloads = true
+    mockPermissions.value = 'admin'
 
     const songWithReplayGain = {
       id: 'rec1',
@@ -290,4 +297,25 @@ describe('ArtistActions', () => {
       expect(screen.queryByText(/ra\.action\.download/)).not.toBeInTheDocument()
     })
   })
+
+  describe('Refresh metadata action', () => {
+    const refreshLabel = 'resources.album.actions.refresh'
+
+    it('refreshes the artist metadata for admins', async () => {
+      renderArtistActions()
+      fireEvent.click(screen.getByRole('button', { name: refreshLabel }))
+
+      await waitFor(() =>
+        expect(mockRefreshMetadata).toHaveBeenCalledWith('artist', 'ar1'),
+      )
+    })
+
+    it('hides the action for non-admin users', () => {
+      mockPermissions.value = 'regular'
+      renderArtistActions()
+      expect(
+        screen.queryByRole('button', { name: refreshLabel }),
+      ).not.toBeInTheDocument()
+    })
+  })
 })

ui/src/common/ContextMenus.jsx

@@ -30,6 +30,7 @@ import { LoveButton } from './LoveButton'
 import config from '../config'
 import { formatBytes } from '../utils'
 import { artistDownloadSize } from './artist'
+import { useRefreshMetadata } from './useRefreshMetadata'
 
 const useStyles = makeStyles({
   noWrap: {
@@ -75,6 +76,7 @@ const ContextMenu = ({
   const translate = useTranslate()
   const notify = useNotify()
   const { permissions } = usePermissions()
+  const refreshMetadata = useRefreshMetadata()
   const [anchorEl, setAnchorEl] = useState(null)
 
   const isArtist = resource === 'artist'
@@ -139,11 +141,7 @@ const ContextMenu = ({
       enabled: permissions === 'admin',
       needData: false,
       label: translate('resources.album.actions.refresh'),
-      action: (record) =>
-        dataProvider
-          .refreshMetadata(resource, record.id)
-          .then(() => notify('message.metadataRefreshStarted'))
-          .catch(() => notify('ra.page.error', 'warning')),
+      action: (record) => refreshMetadata(resource, record.id),
     },
     ...(!hideInfo && {
       info: {

ui/src/common/RefreshMetadataButton.jsx

@@ -0,0 +1,62 @@
+import React, { useCallback } from 'react'
+import PropTypes from 'prop-types'
+import Tooltip from '@material-ui/core/Tooltip'
+import { makeStyles } from '@material-ui/core/styles'
+import { Button, usePermissions, useTranslate } from 'react-admin'
+import { MdRefresh } from 'react-icons/md'
+import { useRefreshMetadata } from './useRefreshMetadata'
+
+const useStyles = makeStyles({
+  // Tooltip needs a ref-holding child, and react-admin's Button does not forward one.
+  wrapper: { display: 'inline-flex', verticalAlign: 'middle' },
+  button: { minWidth: 'auto' },
+})
+
+// react-admin's Button, not an IconButton: the toolbars use it, so colour and the icon-only swap
+// at xs match without restating either rule.
+export const RefreshMetadataButton = ({
+  resource,
+  record,
+  className,
+  size,
+}) => {
+  const translate = useTranslate()
+  const { permissions } = usePermissions()
+  const refreshMetadata = useRefreshMetadata()
+  const classes = useStyles()
+
+  const handleClick = useCallback(
+    () => refreshMetadata(resource, record?.id),
+    [refreshMetadata, resource, record],
+  )
+
+  if (permissions !== 'admin' || !record?.id) return null
+
+  const label = translate('resources.album.actions.refresh')
+  return (
+    <Tooltip title={label}>
+      <span className={classes.wrapper}>
+        <Button
+          aria-label={label}
+          className={className}
+          classes={{ button: classes.button }}
+          size={size}
+          onClick={handleClick}
+        >
+          <MdRefresh />
+        </Button>
+      </span>
+    </Tooltip>
+  )
+}
+
+RefreshMetadataButton.propTypes = {
+  resource: PropTypes.oneOf(['album', 'artist']).isRequired,
+  record: PropTypes.object,
+  className: PropTypes.string,
+  size: PropTypes.oneOf(['small', 'medium']),
+}
+
+RefreshMetadataButton.defaultProps = {
+  size: 'small',
+}

ui/src/common/RefreshMetadataButton.test.jsx

@@ -0,0 +1,104 @@
+import React from 'react'
+import { render, screen, fireEvent, waitFor } from '@testing-library/react'
+import { ThemeProvider, createTheme } from '@material-ui/core/styles'
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { RefreshMetadataButton } from './RefreshMetadataButton'
+
+const mockNotify = vi.fn()
+const mockRefreshMetadata = vi.fn()
+const { mockPermissions } = vi.hoisted(() => ({
+  mockPermissions: { value: 'admin' },
+}))
+
+vi.mock('react-admin', async (importOriginal) => {
+  const actual = await importOriginal()
+  return {
+    ...actual,
+    useNotify: () => mockNotify,
+    useDataProvider: () => ({ refreshMetadata: mockRefreshMetadata }),
+    usePermissions: () => ({ permissions: mockPermissions.value }),
+    useTranslate: () => (x) => x,
+  }
+})
+
+describe('RefreshMetadataButton', () => {
+  const record = { id: 'al-1', name: 'Album' }
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockPermissions.value = 'admin'
+    mockRefreshMetadata.mockResolvedValue({ data: { id: 'al-1' } })
+  })
+
+  const renderButton = (props = {}) =>
+    render(
+      <ThemeProvider theme={createTheme()}>
+        <RefreshMetadataButton resource="album" record={record} {...props} />
+      </ThemeProvider>,
+    )
+
+  it('renders an icon-only button labelled by the refresh action', () => {
+    renderButton()
+    const button = screen.getByRole('button', {
+      name: 'resources.album.actions.refresh',
+    })
+    expect(button).toBeInTheDocument()
+    expect(button).toHaveTextContent('')
+  })
+
+  it('shows the label as a tooltip on hover', async () => {
+    renderButton()
+    fireEvent.mouseOver(
+      screen.getByRole('button', {
+        name: 'resources.album.actions.refresh',
+      }),
+    )
+    await waitFor(() =>
+      expect(screen.getByRole('tooltip')).toHaveTextContent(
+        'resources.album.actions.refresh',
+      ),
+    )
+  })
+
+  it('renders nothing for non-admin users', () => {
+    mockPermissions.value = 'regular'
+    const { container } = renderButton()
+    expect(container).toBeEmptyDOMElement()
+  })
+
+  it('renders nothing without a record', () => {
+    const { container } = renderButton({ record: undefined })
+    expect(container).toBeEmptyDOMElement()
+  })
+
+  it('requests a refresh for the record and notifies success', async () => {
+    renderButton()
+    fireEvent.click(screen.getByRole('button'))
+
+    await waitFor(() =>
+      expect(mockRefreshMetadata).toHaveBeenCalledWith('album', 'al-1'),
+    )
+    await waitFor(() =>
+      expect(mockNotify).toHaveBeenCalledWith('message.metadataRefreshStarted'),
+    )
+  })
+
+  it('passes the artist resource through', async () => {
+    renderButton({ resource: 'artist', record: { id: 'ar-1' } })
+    fireEvent.click(screen.getByRole('button'))
+
+    await waitFor(() =>
+      expect(mockRefreshMetadata).toHaveBeenCalledWith('artist', 'ar-1'),
+    )
+  })
+
+  it('notifies a warning when the request fails', async () => {
+    mockRefreshMetadata.mockRejectedValue(new Error('boom'))
+    renderButton()
+    fireEvent.click(screen.getByRole('button'))
+
+    await waitFor(() =>
+      expect(mockNotify).toHaveBeenCalledWith('ra.page.error', 'warning'),
+    )
+  })
+})

ui/src/common/index.js

@@ -36,6 +36,8 @@ export * from './useTraceUpdate'
 export * from './Writable'
 export * from './SongSimpleList'
 export * from './RatingField'
+export * from './RefreshMetadataButton'
+export * from './useRefreshMetadata'
 export * from './useRating'
 export * from './useSelectedFields'
 export * from './ToggleFieldsMenu'

ui/src/common/useRefreshMetadata.js

@@ -0,0 +1,16 @@
+import { useCallback } from 'react'
+import { useDataProvider, useNotify } from 'react-admin'
+
+export const useRefreshMetadata = () => {
+  const dataProvider = useDataProvider()
+  const notify = useNotify()
+
+  return useCallback(
+    (resource, id) =>
+      dataProvider
+        .refreshMetadata(resource, id)
+        .then(() => notify('message.metadataRefreshStarted'))
+        .catch(() => notify('ra.page.error', 'warning')),
+    [dataProvider, notify],
+  )
+}