mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
feat(artist): add Share and Download actions to the Artist detail page (#5944)
* feat(ui): add Share button to artist detail page * feat(ui): add Download button to artist detail page * fix(ui): scope artist share/download to album-artist content Gate the artist Share/Download actions on album-artist stats and show the album-artist size, since ZipArtist and the share query only cover album_artist_id songs. Previously the total (role-inclusive) size was shown and guest-only artists could produce an empty archive. Applies to the artist toolbar, the shared context menu, and the download dialog title. * fix: match artist download/share to album-artist participation ZipArtist and the artist share query filtered the deprecated album_artist_id column, which only stores the first album artist of a track. Secondary album-artists (co-credited but not first) got an empty download/share even though the UI offered it. Filter by the album-artist role participation instead, matching the artist's album-artist stats used to gate the actions. Also cover the artist-specific size branch of the download dialog. * fix(share): scope artist shares to the owner's libraries The artist share query broadened to album-artist participation, which could pull a secondary album artist's tracks from libraries the (non-admin) share owner cannot access into the public share. Load the artist share as the owner so their library access is applied, mirroring how playlist shares already work. Adds a repository test covering co-album-artist inclusion and library scoping. * test(share): assert album participation branch of artist shares Link the co-album-artist fixtures to albums and assert share.Albums (used by Subsonic getShares) includes the accessible album and excludes the one in a library the owner cannot access, so the album participation + scoping branch is covered too. * fix: exclude missing files from artist download/share actions An artist's stats still count files that went missing, so the toolbar/context menu could offer Download/Share for an artist whose files are all gone, while the share query (missing=false) returns nothing and downloads open dead paths. Hide the actions when the artist is missing and exclude missing files from ZipArtist, matching the share semantics. * refactor: dedupe artist download-size and share-owner lookups Extract the 'album-artist download size (or none when missing)' rule into a single artistDownloadSize() helper shared by the toolbar, context menu, and download dialog, and factor the duplicated share-owner context lookup into a shareRepository.ownerContext() method used by both the artist and playlist share cases. * refactor(ui): move artistDownloadSize helper to common utils is for domain-agnostic, potentially portable code; this helper is Navidrome-specific (artist stats shape), so it belongs in common. Consumers import it directly from common/artist to avoid pulling in the common barrel.
This commit is contained in:
parent
8978c7b9fa
commit
752b38609c
@ -14,6 +14,7 @@ import (
|
||||
"github.com/navidrome/navidrome/core/stream"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/persistence"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
"github.com/navidrome/navidrome/utils/str"
|
||||
)
|
||||
@ -40,7 +41,13 @@ func (a *archiver) ZipAlbum(ctx context.Context, id string, format string, bitra
|
||||
}
|
||||
|
||||
func (a *archiver) ZipArtist(ctx context.Context, id string, format string, bitrate int, out io.Writer) error {
|
||||
return a.zipAlbums(ctx, id, format, bitrate, out, squirrel.Eq{"album_artist_id": id})
|
||||
// Match by album-artist participation, not the deprecated album_artist_id
|
||||
// column (first album artist only), so co-album-artists are included too.
|
||||
filter := squirrel.And{
|
||||
persistence.ParticipantIDFilter("media_file", id, model.RoleAlbumArtist),
|
||||
squirrel.Eq{"missing": false},
|
||||
}
|
||||
return a.zipAlbums(ctx, id, format, bitrate, out, filter)
|
||||
}
|
||||
|
||||
func (a *archiver) zipAlbums(ctx context.Context, id string, format string, bitrate int, out io.Writer, filters squirrel.Sqlizer) error {
|
||||
|
||||
@ -11,6 +11,7 @@ import (
|
||||
"github.com/navidrome/navidrome/core"
|
||||
"github.com/navidrome/navidrome/core/stream"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/persistence"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
"github.com/stretchr/testify/mock"
|
||||
@ -69,8 +70,11 @@ var _ = Describe("Archiver", func() {
|
||||
|
||||
mfRepo := &mockMediaFileRepository{}
|
||||
mfRepo.On("GetAll", []model.QueryOptions{{
|
||||
Filters: squirrel.Eq{"album_artist_id": "1"},
|
||||
Sort: "album",
|
||||
Filters: squirrel.And{
|
||||
persistence.ParticipantIDFilter("media_file", "1", model.RoleAlbumArtist),
|
||||
squirrel.Eq{"missing": false},
|
||||
},
|
||||
Sort: "album",
|
||||
}}).Return(mfs, nil)
|
||||
|
||||
ds.On("MediaFile", mock.Anything).Return(mfRepo)
|
||||
|
||||
@ -82,13 +82,20 @@ func (r *shareRepository) loadMedia(share *model.Share) error {
|
||||
}
|
||||
switch share.ResourceType {
|
||||
case "artist":
|
||||
albumRepo := NewAlbumRepository(r.ctx, r.db)
|
||||
share.Albums, err = albumRepo.GetAll(model.QueryOptions{Filters: noMissing(Eq{"album_artist_id": ids}), Sort: "artist"})
|
||||
// Match by album-artist participation, not the deprecated album_artist_id
|
||||
// column (first album artist only), so co-album-artists are included too.
|
||||
// Load as the share owner so their library access is applied.
|
||||
ctx, err := r.ownerContext(share)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
mfRepo := NewMediaFileRepository(r.ctx, r.db)
|
||||
share.Tracks, err = mfRepo.GetAll(model.QueryOptions{Filters: noMissing(Eq{"album_artist_id": ids}), Sort: "artist"})
|
||||
albumRepo := NewAlbumRepository(ctx, r.db)
|
||||
share.Albums, err = albumRepo.GetAll(model.QueryOptions{Filters: noMissing(ParticipantIDFilter("album", ids, model.RoleAlbumArtist)), Sort: "artist"})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
mfRepo := NewMediaFileRepository(ctx, r.db)
|
||||
share.Tracks, err = mfRepo.GetAll(model.QueryOptions{Filters: noMissing(ParticipantIDFilter("media_file", ids, model.RoleAlbumArtist)), Sort: "artist"})
|
||||
return err
|
||||
case "album":
|
||||
albumRepo := NewAlbumRepository(r.ctx, r.db)
|
||||
@ -101,14 +108,10 @@ func (r *shareRepository) loadMedia(share *model.Share) error {
|
||||
return err
|
||||
case "playlist":
|
||||
// Load tracks as the share owner so their library access is applied.
|
||||
owner, err := NewUserRepository(r.ctx, r.db).Get(share.UserID)
|
||||
ctx, err := r.ownerContext(share)
|
||||
if err != nil {
|
||||
return fmt.Errorf("loading share owner %q: %w", share.UserID, err)
|
||||
return err
|
||||
}
|
||||
if owner == nil {
|
||||
return fmt.Errorf("share owner %q not found", share.UserID)
|
||||
}
|
||||
ctx := request.WithUser(r.ctx, *owner)
|
||||
plsRepo := NewPlaylistRepository(ctx, r.db)
|
||||
// Tracks returns nil when the playlist is no longer visible to the owner
|
||||
// (e.g. it was made private after the share was created); leave the share
|
||||
@ -133,6 +136,19 @@ func (r *shareRepository) loadMedia(share *model.Share) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// ownerContext returns a context scoped to the share owner, so repository
|
||||
// queries apply the owner's library access when a public share is rendered.
|
||||
func (r *shareRepository) ownerContext(share *model.Share) (context.Context, error) {
|
||||
owner, err := NewUserRepository(r.ctx, r.db).Get(share.UserID)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("loading share owner %q: %w", share.UserID, err)
|
||||
}
|
||||
if owner == nil {
|
||||
return nil, fmt.Errorf("share owner %q not found", share.UserID)
|
||||
}
|
||||
return request.WithUser(r.ctx, *owner), nil
|
||||
}
|
||||
|
||||
func sortByIdPosition(mfs model.MediaFiles, ids []string) model.MediaFiles {
|
||||
m := map[string]int{}
|
||||
for i, mf := range mfs {
|
||||
|
||||
@ -228,6 +228,89 @@ var _ = Describe("ShareRepository", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Artist share library scoping", func() {
|
||||
var otherLib model.Library
|
||||
var owner model.User
|
||||
const primaryID = "share-aa-primary"
|
||||
const secondaryID = "share-aa-secondary"
|
||||
|
||||
BeforeEach(func() {
|
||||
adminCtx := request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
|
||||
b := GetDBXBuilder()
|
||||
|
||||
// A second library the owner has no access to
|
||||
lr := NewLibraryRepository(adminCtx, b)
|
||||
otherLib = model.Library{ID: 0, Name: "Artist Share Other Library", Path: "/share/artist/other"}
|
||||
Expect(lr.Put(&otherLib)).To(Succeed())
|
||||
|
||||
ar := NewArtistRepository(adminCtx, b)
|
||||
Expect(createArtistWithLibrary(ar, &model.Artist{ID: primaryID, Name: "AA Primary", OrderArtistName: "aa primary"}, 1)).To(Succeed())
|
||||
Expect(createArtistWithLibrary(ar, &model.Artist{ID: secondaryID, Name: "AA Secondary", OrderArtistName: "aa secondary"}, 1)).To(Succeed())
|
||||
|
||||
// Secondary is a co-album-artist (not the first): album_artist_id points at
|
||||
// primary, so the legacy-column filter would miss both tracks.
|
||||
aaParticipants := model.Participants{model.RoleAlbumArtist: {
|
||||
{Artist: model.Artist{ID: primaryID, Name: "AA Primary"}},
|
||||
{Artist: model.Artist{ID: secondaryID, Name: "AA Secondary"}},
|
||||
}}
|
||||
alr := NewAlbumRepository(adminCtx, b)
|
||||
Expect(alr.Put(&model.Album{ID: "art-album-ok", LibraryID: 1, Name: "Art Album OK", AlbumArtistID: primaryID, AlbumArtist: "AA Primary", Participants: aaParticipants})).To(Succeed())
|
||||
Expect(alr.Put(&model.Album{ID: "art-album-other", LibraryID: otherLib.ID, Name: "Art Album Other", AlbumArtistID: primaryID, AlbumArtist: "AA Primary", Participants: aaParticipants})).To(Succeed())
|
||||
|
||||
mr := NewMediaFileRepository(adminCtx, b)
|
||||
Expect(mr.Put(&model.MediaFile{ID: "art-ok", LibraryID: 1, AlbumID: "art-album-ok", Path: "a/ok.mp3", Title: "ArtOK", AlbumArtistID: primaryID, Participants: aaParticipants})).To(Succeed())
|
||||
Expect(mr.Put(&model.MediaFile{ID: "art-other", LibraryID: otherLib.ID, AlbumID: "art-album-other", Path: "a/other.mp3", Title: "ArtOther", AlbumArtistID: primaryID, Participants: aaParticipants})).To(Succeed())
|
||||
|
||||
// Non-admin owner with access to library 1 only
|
||||
owner = createUserWithLibraries("artist-share-owner", []int{1})
|
||||
ur := NewUserRepository(adminCtx, b)
|
||||
Expect(ur.Put(&owner)).To(Succeed())
|
||||
Expect(ur.SetUserLibraries(owner.ID, []int{1})).To(Succeed())
|
||||
|
||||
_, err := b.NewQuery(`
|
||||
INSERT INTO share (id, user_id, description, resource_type, resource_ids, created_at, updated_at)
|
||||
VALUES ({:id}, {:user}, {:desc}, {:type}, {:ids}, {:created}, {:updated})
|
||||
`).Bind(map[string]any{
|
||||
"id": "art-share", "user": owner.ID, "desc": "Artist scope share",
|
||||
"type": "artist", "ids": secondaryID, "created": time.Now(), "updated": time.Now(),
|
||||
}).Execute()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
AfterEach(func() {
|
||||
adminCtx := request.WithUser(log.NewContext(GinkgoT().Context()), adminUser)
|
||||
b := GetDBXBuilder()
|
||||
_, _ = b.NewQuery(`DELETE FROM share WHERE id = 'art-share'`).Execute()
|
||||
mr := NewMediaFileRepository(adminCtx, b).(*mediaFileRepository)
|
||||
_, _ = mr.executeSQL(squirrel.Delete("media_file").Where(squirrel.Eq{"id": []string{"art-ok", "art-other"}}))
|
||||
alr := NewAlbumRepository(adminCtx, b).(*albumRepository)
|
||||
_, _ = alr.executeSQL(squirrel.Delete("album").Where(squirrel.Eq{"id": []string{"art-album-ok", "art-album-other"}}))
|
||||
ar := NewArtistRepository(adminCtx, b).(*artistRepository)
|
||||
_, _ = ar.executeSQL(squirrel.Delete("artist").Where(squirrel.Eq{"id": []string{primaryID, secondaryID}}))
|
||||
lr := NewLibraryRepository(adminCtx, b).(*libraryRepository)
|
||||
_ = lr.delete(squirrel.Eq{"id": otherLib.ID})
|
||||
_ = NewUserRepository(adminCtx, b).Delete(owner.ID)
|
||||
})
|
||||
|
||||
It("includes co-album-artist tracks the owner can access and excludes those they cannot", func() {
|
||||
// Read as admin (mimics the public-share render path); loadMedia must still
|
||||
// scope to the owner's libraries.
|
||||
adminRepo := NewShareRepository(request.WithUser(log.NewContext(GinkgoT().Context()), adminUser), GetDBXBuilder())
|
||||
share, err := adminRepo.Get("art-share")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
Expect(share.Tracks).To(ContainElement(HaveField("ID", "art-ok")),
|
||||
"a co-album-artist track (not matched by album_artist_id) must be included")
|
||||
Expect(share.Tracks).ToNot(ContainElement(HaveField("ID", "art-other")),
|
||||
"a track outside the owner's libraries must not appear in the share")
|
||||
|
||||
Expect(share.Albums).To(ContainElement(HaveField("ID", "art-album-ok")),
|
||||
"a co-album-artist album must be included")
|
||||
Expect(share.Albums).ToNot(ContainElement(HaveField("ID", "art-album-other")),
|
||||
"an album outside the owner's libraries must not appear in the share")
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Ownership Checks", func() {
|
||||
var ownerUser = model.User{ID: "2222", UserName: "regular-user"}
|
||||
var otherUser = model.User{ID: "3333", UserName: "third-user"}
|
||||
|
||||
@ -13,9 +13,19 @@ import {
|
||||
} from 'react-admin'
|
||||
import ShuffleIcon from '@material-ui/icons/Shuffle'
|
||||
import PlayArrowIcon from '@material-ui/icons/PlayArrow'
|
||||
import ShareIcon from '@material-ui/icons/Share'
|
||||
import CloudDownloadOutlinedIcon from '@material-ui/icons/CloudDownloadOutlined'
|
||||
import { IoIosRadio } from 'react-icons/io'
|
||||
import { playShuffle, playTopSongs } from './actions.js'
|
||||
import { playSimilar } from '../common/playbackActions.js'
|
||||
import {
|
||||
openShareMenu,
|
||||
openDownloadMenu,
|
||||
DOWNLOAD_MENU_ARTIST,
|
||||
} from '../actions'
|
||||
import config from '../config'
|
||||
import { formatBytes } from '../utils'
|
||||
import { artistDownloadSize } from '../common/artist'
|
||||
|
||||
const useStyles = makeStyles((theme) => ({
|
||||
toolbar: {
|
||||
@ -62,6 +72,9 @@ const ArtistActions = ({ className, record, ...rest }) => {
|
||||
const [loadingAction, setLoadingAction] = React.useState(null)
|
||||
const isLoading = !!loadingAction
|
||||
|
||||
const albumArtistSize = artistDownloadSize(record)
|
||||
const hasAlbumArtistContent = Boolean(albumArtistSize)
|
||||
|
||||
const handlePlay = React.useCallback(async () => {
|
||||
setLoadingAction('play')
|
||||
try {
|
||||
@ -101,6 +114,14 @@ const ArtistActions = ({ className, record, ...rest }) => {
|
||||
}
|
||||
}, [dispatch, notify, record])
|
||||
|
||||
const handleShare = React.useCallback(() => {
|
||||
dispatch(openShareMenu([record.id], 'artist', record.name))
|
||||
}, [dispatch, record])
|
||||
|
||||
const handleDownload = React.useCallback(() => {
|
||||
dispatch(openDownloadMenu(record, DOWNLOAD_MENU_ARTIST))
|
||||
}, [dispatch, record])
|
||||
|
||||
return (
|
||||
<TopToolbar
|
||||
className={`${className} ${classes.toolbar}`}
|
||||
@ -133,6 +154,26 @@ const ArtistActions = ({ className, record, ...rest }) => {
|
||||
loading={loadingAction === 'radio'}
|
||||
icon={<IoIosRadio className={classes.radioIcon} />}
|
||||
/>
|
||||
{config.enableSharing && hasAlbumArtistContent && (
|
||||
<LoadingButton
|
||||
onClick={handleShare}
|
||||
label={translate('ra.action.share')}
|
||||
className={classes.button}
|
||||
size={isMobile ? 'small' : 'medium'}
|
||||
icon={<ShareIcon />}
|
||||
/>
|
||||
)}
|
||||
{config.enableDownloads && hasAlbumArtistContent && (
|
||||
<LoadingButton
|
||||
onClick={handleDownload}
|
||||
label={`${translate('ra.action.download')} (${formatBytes(
|
||||
albumArtistSize,
|
||||
)})`}
|
||||
className={classes.button}
|
||||
size={isMobile ? 'small' : 'medium'}
|
||||
icon={<CloudDownloadOutlinedIcon />}
|
||||
/>
|
||||
)}
|
||||
</TopToolbar>
|
||||
)
|
||||
}
|
||||
|
||||
@ -4,6 +4,11 @@ import { TestContext } from 'ra-test'
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import ArtistActions from './ArtistActions'
|
||||
import subsonic from '../subsonic'
|
||||
import {
|
||||
openShareMenu,
|
||||
openDownloadMenu,
|
||||
DOWNLOAD_MENU_ARTIST,
|
||||
} from '../actions'
|
||||
import { ThemeProvider, createTheme } from '@material-ui/core/styles'
|
||||
|
||||
const mockDispatch = vi.fn()
|
||||
@ -13,6 +18,11 @@ vi.mock('../subsonic', () => ({
|
||||
default: { getSimilarSongs2: vi.fn(), getTopSongs: vi.fn() },
|
||||
}))
|
||||
|
||||
const { mockConfig } = vi.hoisted(() => ({
|
||||
mockConfig: { enableSharing: true, enableDownloads: true },
|
||||
}))
|
||||
vi.mock('../config', () => ({ default: mockConfig }))
|
||||
|
||||
const mockNotify = vi.fn()
|
||||
const mockGetList = vi.fn().mockResolvedValue({ data: [{ id: 's1' }] })
|
||||
|
||||
@ -27,7 +37,11 @@ vi.mock('react-admin', async (importOriginal) => {
|
||||
})
|
||||
|
||||
describe('ArtistActions', () => {
|
||||
const defaultRecord = { id: 'ar1', name: 'Artist' }
|
||||
const defaultRecord = {
|
||||
id: 'ar1',
|
||||
name: 'Artist',
|
||||
stats: { albumartist: { songCount: 3, albumCount: 1, size: 1024 } },
|
||||
}
|
||||
|
||||
const renderArtistActions = (record = defaultRecord) => {
|
||||
const theme = createTheme()
|
||||
@ -48,6 +62,8 @@ describe('ArtistActions', () => {
|
||||
vi.clearAllMocks()
|
||||
// Mock console.error to suppress error logging in tests
|
||||
vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
mockConfig.enableSharing = true
|
||||
mockConfig.enableDownloads = true
|
||||
|
||||
const songWithReplayGain = {
|
||||
id: 'rec1',
|
||||
@ -227,4 +243,51 @@ describe('ArtistActions', () => {
|
||||
expect(mockDispatch).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('Share action', () => {
|
||||
it('shows the share button and dispatches openShareMenu when clicked', () => {
|
||||
renderArtistActions()
|
||||
fireEvent.click(screen.getByText('ra.action.share'))
|
||||
expect(mockDispatch).toHaveBeenCalledWith(
|
||||
openShareMenu(['ar1'], 'artist', 'Artist'),
|
||||
)
|
||||
})
|
||||
|
||||
it('hides the share button when sharing is disabled', () => {
|
||||
mockConfig.enableSharing = false
|
||||
renderArtistActions()
|
||||
expect(screen.queryByText('ra.action.share')).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
describe('Download action', () => {
|
||||
it('shows the download button with album-artist size and dispatches openDownloadMenu when clicked', () => {
|
||||
renderArtistActions()
|
||||
expect(screen.getByText('ra.action.download (1 KB)')).toBeInTheDocument()
|
||||
fireEvent.click(screen.getByText(/ra\.action\.download/))
|
||||
expect(mockDispatch).toHaveBeenCalledWith(
|
||||
openDownloadMenu(defaultRecord, DOWNLOAD_MENU_ARTIST),
|
||||
)
|
||||
})
|
||||
|
||||
it('hides the download button when downloads are disabled', () => {
|
||||
mockConfig.enableDownloads = false
|
||||
renderArtistActions()
|
||||
expect(screen.queryByText(/ra\.action\.download/)).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
describe('Album-artist gating', () => {
|
||||
it('hides Share and Download for artists with no album-artist content', () => {
|
||||
renderArtistActions({ id: 'ar1', name: 'Artist', stats: {} })
|
||||
expect(screen.queryByText('ra.action.share')).not.toBeInTheDocument()
|
||||
expect(screen.queryByText(/ra\.action\.download/)).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('hides Share and Download for a missing artist', () => {
|
||||
renderArtistActions({ ...defaultRecord, missing: true })
|
||||
expect(screen.queryByText('ra.action.share')).not.toBeInTheDocument()
|
||||
expect(screen.queryByText(/ra\.action\.download/)).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -24,6 +24,7 @@ import {
|
||||
import { LoveButton } from './LoveButton'
|
||||
import config from '../config'
|
||||
import { formatBytes } from '../utils'
|
||||
import { artistDownloadSize } from './artist'
|
||||
|
||||
const useStyles = makeStyles({
|
||||
noWrap: {
|
||||
@ -70,6 +71,9 @@ const ContextMenu = ({
|
||||
const notify = useNotify()
|
||||
const [anchorEl, setAnchorEl] = useState(null)
|
||||
|
||||
const isArtist = resource === 'artist'
|
||||
const downloadSize = isArtist ? artistDownloadSize(record) : record?.size
|
||||
|
||||
const options = {
|
||||
play: {
|
||||
enabled: true,
|
||||
@ -103,7 +107,7 @@ const ContextMenu = ({
|
||||
},
|
||||
...(!hideShare && {
|
||||
share: {
|
||||
enabled: config.enableSharing,
|
||||
enabled: config.enableSharing && (!isArtist || downloadSize),
|
||||
needData: false,
|
||||
label: translate('ra.action.share'),
|
||||
action: (record) =>
|
||||
@ -111,9 +115,9 @@ const ContextMenu = ({
|
||||
},
|
||||
}),
|
||||
download: {
|
||||
enabled: config.enableDownloads && record.size,
|
||||
enabled: config.enableDownloads && downloadSize,
|
||||
needData: false,
|
||||
label: `${translate('ra.action.download')} (${formatBytes(record.size)})`,
|
||||
label: `${translate('ra.action.download')} (${formatBytes(downloadSize)})`,
|
||||
action: () => {
|
||||
dispatch(
|
||||
openDownloadMenu(
|
||||
|
||||
78
ui/src/common/ContextMenus.test.jsx
Normal file
78
ui/src/common/ContextMenus.test.jsx
Normal file
@ -0,0 +1,78 @@
|
||||
import React from 'react'
|
||||
import { render, fireEvent, screen } from '@testing-library/react'
|
||||
import { TestContext } from 'ra-test'
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { ThemeProvider, createTheme } from '@material-ui/core/styles'
|
||||
import { AlbumContextMenu, ArtistContextMenu } from './ContextMenus'
|
||||
|
||||
const mockDispatch = vi.fn()
|
||||
vi.mock('react-redux', () => ({ useDispatch: () => mockDispatch }))
|
||||
|
||||
const { mockConfig } = vi.hoisted(() => ({
|
||||
mockConfig: {
|
||||
enableSharing: true,
|
||||
enableDownloads: true,
|
||||
enableFavourites: false,
|
||||
},
|
||||
}))
|
||||
vi.mock('../config', () => ({ default: mockConfig }))
|
||||
|
||||
vi.mock('react-admin', async (importOriginal) => {
|
||||
const actual = await importOriginal()
|
||||
return {
|
||||
...actual,
|
||||
useNotify: () => vi.fn(),
|
||||
useDataProvider: () => ({ getList: vi.fn() }),
|
||||
useTranslate: () => (x) => x,
|
||||
}
|
||||
})
|
||||
|
||||
describe('ContextMenus', () => {
|
||||
const renderMenu = (Menu, record) => {
|
||||
render(
|
||||
<TestContext>
|
||||
<ThemeProvider theme={createTheme()}>
|
||||
<Menu record={record} />
|
||||
</ThemeProvider>
|
||||
</TestContext>,
|
||||
)
|
||||
fireEvent.click(screen.getByLabelText('more'))
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockConfig.enableSharing = true
|
||||
mockConfig.enableDownloads = true
|
||||
})
|
||||
|
||||
describe('ArtistContextMenu', () => {
|
||||
const withAlbumArtist = {
|
||||
id: 'ar1',
|
||||
name: 'Artist',
|
||||
stats: { albumartist: { songCount: 3, albumCount: 1, size: 1024 } },
|
||||
}
|
||||
|
||||
it('shows the album-artist size on the download item', () => {
|
||||
renderMenu(ArtistContextMenu, withAlbumArtist)
|
||||
expect(screen.getByText('ra.action.download (1 KB)')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('hides download and share for artists with no album-artist content', () => {
|
||||
renderMenu(ArtistContextMenu, { id: 'ar1', name: 'Artist', stats: {} })
|
||||
expect(screen.queryByText(/ra\.action\.download/)).not.toBeInTheDocument()
|
||||
expect(screen.queryByText('ra.action.share')).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
describe('AlbumContextMenu', () => {
|
||||
it('uses the total size on the album download item', () => {
|
||||
renderMenu(AlbumContextMenu, {
|
||||
id: 'al1',
|
||||
name: 'Album',
|
||||
duration: 100,
|
||||
size: 1024 * 1024,
|
||||
})
|
||||
expect(screen.getByText('ra.action.download (1 MB)')).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
})
|
||||
6
ui/src/common/artist.js
Normal file
6
ui/src/common/artist.js
Normal file
@ -0,0 +1,6 @@
|
||||
// Size of an artist's downloadable album-artist content, or undefined when there
|
||||
// is nothing to download (a missing artist, or no album-artist songs). Download
|
||||
// and Share only cover album-artist songs, so callers gate on this, not the
|
||||
// role-inclusive total.
|
||||
export const artistDownloadSize = (record) =>
|
||||
record?.missing ? undefined : record?.stats?.albumartist?.size
|
||||
24
ui/src/common/artist.test.js
Normal file
24
ui/src/common/artist.test.js
Normal file
@ -0,0 +1,24 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { artistDownloadSize } from './artist'
|
||||
|
||||
describe('artistDownloadSize', () => {
|
||||
it('returns the album-artist size', () => {
|
||||
expect(
|
||||
artistDownloadSize({ stats: { albumartist: { size: 1024 } } }),
|
||||
).toEqual(1024)
|
||||
})
|
||||
|
||||
it('returns undefined for a missing artist', () => {
|
||||
expect(
|
||||
artistDownloadSize({
|
||||
missing: true,
|
||||
stats: { albumartist: { size: 1024 } },
|
||||
}),
|
||||
).toBeUndefined()
|
||||
})
|
||||
|
||||
it('returns undefined when there is no album-artist content', () => {
|
||||
expect(artistDownloadSize({ stats: {} })).toBeUndefined()
|
||||
expect(artistDownloadSize(undefined)).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@ -1,4 +1,5 @@
|
||||
export * from './AddToPlaylistButton'
|
||||
export * from './artist'
|
||||
export * from './ArtistLinkField'
|
||||
export * from './BatchPlayButton'
|
||||
export * from './BitrateField'
|
||||
|
||||
@ -8,8 +8,9 @@ import {
|
||||
DialogTitle,
|
||||
} from '@material-ui/core'
|
||||
import subsonic from '../subsonic'
|
||||
import { closeDownloadMenu } from '../actions'
|
||||
import { closeDownloadMenu, DOWNLOAD_MENU_ARTIST } from '../actions'
|
||||
import { formatBytes } from '../utils'
|
||||
import { artistDownloadSize } from '../common/artist'
|
||||
import { useTranscodingOptions } from './useTranscodingOptions'
|
||||
|
||||
const DownloadMenuDialog = () => {
|
||||
@ -22,6 +23,12 @@ const DownloadMenuDialog = () => {
|
||||
const { TranscodingOptionsInput, format, maxBitRate, originalFormat } =
|
||||
useTranscodingOptions()
|
||||
|
||||
// Artist downloads only include album-artist songs, so show that size
|
||||
const downloadSize =
|
||||
recordType === DOWNLOAD_MENU_ARTIST
|
||||
? artistDownloadSize(record)
|
||||
: record?.size
|
||||
|
||||
const handleClose = (e) => {
|
||||
dispatch(closeDownloadMenu())
|
||||
e.stopPropagation()
|
||||
@ -55,7 +62,7 @@ const DownloadMenuDialog = () => {
|
||||
smart_count: 1,
|
||||
}).toLocaleLowerCase(),
|
||||
name: record?.name || record?.title,
|
||||
size: formatBytes(record?.size),
|
||||
size: formatBytes(downloadSize),
|
||||
})}
|
||||
</DialogTitle>
|
||||
<DialogContent>
|
||||
|
||||
58
ui/src/dialogs/DownloadMenuDialog.test.jsx
Normal file
58
ui/src/dialogs/DownloadMenuDialog.test.jsx
Normal file
@ -0,0 +1,58 @@
|
||||
import * as React from 'react'
|
||||
import { TestContext } from 'ra-test'
|
||||
import { render, screen, cleanup } from '@testing-library/react'
|
||||
import { describe, afterEach, it, expect, vi } from 'vitest'
|
||||
import DownloadMenuDialog from './DownloadMenuDialog'
|
||||
import { DOWNLOAD_MENU_ALBUM, DOWNLOAD_MENU_ARTIST } from '../actions'
|
||||
|
||||
vi.mock('./useTranscodingOptions', () => ({
|
||||
useTranscodingOptions: () => ({
|
||||
TranscodingOptionsInput: () => null,
|
||||
format: '',
|
||||
maxBitRate: 0,
|
||||
originalFormat: true,
|
||||
}),
|
||||
}))
|
||||
|
||||
vi.mock('react-admin', async (importOriginal) => {
|
||||
const actual = await importOriginal()
|
||||
return {
|
||||
...actual,
|
||||
useTranslate: () => (key, opts) =>
|
||||
opts?.size ? `${key}:${opts.name}:${opts.size}` : key,
|
||||
}
|
||||
})
|
||||
|
||||
const renderDialog = (record, recordType) =>
|
||||
render(
|
||||
<TestContext
|
||||
initialState={{ downloadMenuDialog: { open: true, record, recordType } }}
|
||||
>
|
||||
<DownloadMenuDialog />
|
||||
</TestContext>,
|
||||
)
|
||||
|
||||
describe('DownloadMenuDialog', () => {
|
||||
afterEach(cleanup)
|
||||
|
||||
it('shows the album-artist size (not the total) for an artist download', () => {
|
||||
renderDialog(
|
||||
{
|
||||
id: 'ar1',
|
||||
name: 'Artist',
|
||||
size: 999999999,
|
||||
stats: { albumartist: { size: 1024 } },
|
||||
},
|
||||
DOWNLOAD_MENU_ARTIST,
|
||||
)
|
||||
expect(screen.getByText(/:Artist:1 KB$/)).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('shows the total size for an album download', () => {
|
||||
renderDialog(
|
||||
{ id: 'al1', name: 'Album', size: 1024 * 1024 },
|
||||
DOWNLOAD_MENU_ALBUM,
|
||||
)
|
||||
expect(screen.getByText(/:Album:1 MB$/)).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
Loading…
x
Reference in New Issue
Block a user