From afb3a2f881a245c28cc7b65888f51ea1a2cbbb3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Wed, 2 Sep 2026 23:29:31 -0400 Subject: [PATCH] feat(ui): add Refresh Metadata action to the album and artist pages (#6078) The Refresh Metadata action was only reachable from the Album and Artist context menus, so it could not be triggered from AlbumShow or ArtistShow. This adds an icon-only button, with a tooltip, to the action toolbar on both detail pages. Like the menu entry, it is only rendered for admins. The button is built on react-admin's Button rather than a plain IconButton: the surrounding toolbars use the former, so the theme colour and the icon-only swap at the xs breakpoint are inherited instead of restated. The dataProvider call and its two notifications move into a new useRefreshMetadata hook, which ContextMenus now shares, keeping a single copy of that logic. --- ui/src/album/AlbumActions.jsx | 7 +- ui/src/album/AlbumActions.test.jsx | 68 ++++++++++++ ui/src/artist/ArtistActions.jsx | 7 ++ ui/src/artist/ArtistActions.test.jsx | 32 +++++- ui/src/common/ContextMenus.jsx | 8 +- ui/src/common/RefreshMetadataButton.jsx | 62 +++++++++++ ui/src/common/RefreshMetadataButton.test.jsx | 104 +++++++++++++++++++ ui/src/common/index.js | 2 + ui/src/common/useRefreshMetadata.js | 16 +++ 9 files changed, 298 insertions(+), 8 deletions(-) create mode 100644 ui/src/album/AlbumActions.test.jsx create mode 100644 ui/src/common/RefreshMetadataButton.jsx create mode 100644 ui/src/common/RefreshMetadataButton.test.jsx create mode 100644 ui/src/common/useRefreshMetadata.js diff --git a/ui/src/album/AlbumActions.jsx b/ui/src/album/AlbumActions.jsx index 96cfab09a..c3b3a4bc2 100644 --- a/ui/src/album/AlbumActions.jsx +++ b/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 = ({ )} +
{isNotSmall && }
diff --git a/ui/src/album/AlbumActions.test.jsx b/ui/src/album/AlbumActions.test.jsx new file mode 100644 index 000000000..246a15558 --- /dev/null +++ b/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( + + + , + ) + + 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() + }) +}) diff --git a/ui/src/artist/ArtistActions.jsx b/ui/src/artist/ArtistActions.jsx index 2f329a034..c2614c05a 100644 --- a/ui/src/artist/ArtistActions.jsx +++ b/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={} /> )} + ) } diff --git a/ui/src/artist/ArtistActions.test.jsx b/ui/src/artist/ArtistActions.test.jsx index ad25177b3..05f53f835 100644 --- a/ui/src/artist/ArtistActions.test.jsx +++ b/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() + }) + }) }) diff --git a/ui/src/common/ContextMenus.jsx b/ui/src/common/ContextMenus.jsx index 606506459..4407392c1 100644 --- a/ui/src/common/ContextMenus.jsx +++ b/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: { diff --git a/ui/src/common/RefreshMetadataButton.jsx b/ui/src/common/RefreshMetadataButton.jsx new file mode 100644 index 000000000..aa0533ade --- /dev/null +++ b/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 ( + + + + + + ) +} + +RefreshMetadataButton.propTypes = { + resource: PropTypes.oneOf(['album', 'artist']).isRequired, + record: PropTypes.object, + className: PropTypes.string, + size: PropTypes.oneOf(['small', 'medium']), +} + +RefreshMetadataButton.defaultProps = { + size: 'small', +} diff --git a/ui/src/common/RefreshMetadataButton.test.jsx b/ui/src/common/RefreshMetadataButton.test.jsx new file mode 100644 index 000000000..7bda0497c --- /dev/null +++ b/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( + + + , + ) + + 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'), + ) + }) +}) diff --git a/ui/src/common/index.js b/ui/src/common/index.js index 047f5b3cf..0177df326 100644 --- a/ui/src/common/index.js +++ b/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' diff --git a/ui/src/common/useRefreshMetadata.js b/ui/src/common/useRefreshMetadata.js new file mode 100644 index 000000000..d34042919 --- /dev/null +++ b/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], + ) +}