From 9aa81eb4e024645f2164d3a1bbc7210bb86c47dd Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Mon, 17 Mar 2025 09:57:57 +0000 Subject: [PATCH 01/21] Adding environmental variable so that navidrome can detect if its running as an MSI install for insights --- release/wix/navidrome.wxs | 1 + 1 file changed, 1 insertion(+) diff --git a/release/wix/navidrome.wxs b/release/wix/navidrome.wxs index ec8b164e8..1392b7dab 100644 --- a/release/wix/navidrome.wxs +++ b/release/wix/navidrome.wxs @@ -61,6 +61,7 @@ Arguments='service execute --configfile "[INSTALLDIR]navidrome.ini" --logfile "[ND_DATAFOLDER]\navidrome.log"' /> + From 93e35904657667ee7731ef3ab5f072f6042278c3 Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Mon, 17 Mar 2025 10:08:33 +0000 Subject: [PATCH 02/21] Renaming to be ND_PACKAGE_TYPE so we can reuse this for the .deb/.rpm stats as well --- core/metrics/insights.go | 4 ++++ core/metrics/insights/data.go | 1 + release/wix/navidrome.wxs | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/core/metrics/insights.go b/core/metrics/insights.go index 6076be0a5..d2ba6a2ab 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -6,6 +6,7 @@ import ( "encoding/json" "math" "net/http" + "os" "path/filepath" "runtime" "runtime/debug" @@ -153,6 +154,9 @@ var staticData = sync.OnceValue(func() insights.Data { data.Build.Settings, data.Build.GoVersion = buildInfo() data.OS.Containerized = consts.InContainer + // Install info + data.OS.Packaged = os.Getenv("ND_PACKAGE_TYPE") + // OS info data.OS.Type = runtime.GOOS data.OS.Arch = runtime.GOARCH diff --git a/core/metrics/insights/data.go b/core/metrics/insights/data.go index 9df547b4a..5c3c2672f 100644 --- a/core/metrics/insights/data.go +++ b/core/metrics/insights/data.go @@ -16,6 +16,7 @@ type Data struct { Containerized bool `json:"containerized"` Arch string `json:"arch"` NumCPU int `json:"numCPU"` + Packaged string `json:"packaged,omitempty"` } `json:"os"` Mem struct { Alloc uint64 `json:"alloc"` diff --git a/release/wix/navidrome.wxs b/release/wix/navidrome.wxs index 1392b7dab..9395e387a 100644 --- a/release/wix/navidrome.wxs +++ b/release/wix/navidrome.wxs @@ -61,7 +61,7 @@ Arguments='service execute --configfile "[INSTALLDIR]navidrome.ini" --logfile "[ND_DATAFOLDER]\navidrome.log"' /> - + From 4512cc6096a70512ad7afba75cf1cfeb01e38070 Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Mon, 17 Mar 2025 10:21:34 +0000 Subject: [PATCH 03/21] Packaged implies a bool, this is a description so it should be packaging or just package imo --- core/metrics/insights.go | 2 +- core/metrics/insights/data.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/metrics/insights.go b/core/metrics/insights.go index d2ba6a2ab..965a24797 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -155,7 +155,7 @@ var staticData = sync.OnceValue(func() insights.Data { data.OS.Containerized = consts.InContainer // Install info - data.OS.Packaged = os.Getenv("ND_PACKAGE_TYPE") + data.OS.Package = os.Getenv("ND_PACKAGE_TYPE") // OS info data.OS.Type = runtime.GOOS diff --git a/core/metrics/insights/data.go b/core/metrics/insights/data.go index 5c3c2672f..3fb91e7fb 100644 --- a/core/metrics/insights/data.go +++ b/core/metrics/insights/data.go @@ -16,7 +16,7 @@ type Data struct { Containerized bool `json:"containerized"` Arch string `json:"arch"` NumCPU int `json:"numCPU"` - Packaged string `json:"packaged,omitempty"` + Package string `json:"package,omitempty"` } `json:"os"` Mem struct { Alloc uint64 `json:"alloc"` From 74e8f5c444c4f88242c44d180b036a1aff1f6f64 Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Tue, 18 Mar 2025 09:30:44 +0000 Subject: [PATCH 04/21] wixl currently doesn't support so I'm swapping out to a file next-door to the configuration file, we should be able to reuse this for deb/rpm as well --- core/metrics/insights.go | 6 +++++- release/wix/build_msi.sh | 3 +++ release/wix/navidrome.wxs | 1 - 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/core/metrics/insights.go b/core/metrics/insights.go index 965a24797..8654dc678 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -155,7 +155,11 @@ var staticData = sync.OnceValue(func() insights.Data { data.OS.Containerized = consts.InContainer // Install info - data.OS.Package = os.Getenv("ND_PACKAGE_TYPE") + packageFilename := filepath.Join(filepath.Dir(conf.Server.ConfigFile), "package") + packageFileData, err := os.ReadFile(packageFilename) + if err == nil { + data.OS.Package = string(packageFileData) + } // OS info data.OS.Type = runtime.GOOS diff --git a/release/wix/build_msi.sh b/release/wix/build_msi.sh index 9fc008446..c48e738dc 100755 --- a/release/wix/build_msi.sh +++ b/release/wix/build_msi.sh @@ -49,6 +49,9 @@ cp "${DOWNLOAD_FOLDER}"/extracted_ffmpeg/${FFMPEG_FILE}/bin/ffmpeg.exe "$MSI_OUT cp "$WORKSPACE"/LICENSE "$WORKSPACE"/README.md "$MSI_OUTPUT_DIR" cp "$BINARY" "$MSI_OUTPUT_DIR" +# package type indicator file +echo "msi" > "$MSI_OUTPUT_DIR/package" + # workaround for wixl WixVariable not working to override bmp locations cp "$WORKSPACE"/release/wix/bmp/banner.bmp /usr/share/wixl-*/ext/ui/bitmaps/bannrbmp.bmp cp "$WORKSPACE"/release/wix/bmp/dialogue.bmp /usr/share/wixl-*/ext/ui/bitmaps/dlgbmp.bmp diff --git a/release/wix/navidrome.wxs b/release/wix/navidrome.wxs index 9395e387a..ec8b164e8 100644 --- a/release/wix/navidrome.wxs +++ b/release/wix/navidrome.wxs @@ -61,7 +61,6 @@ Arguments='service execute --configfile "[INSTALLDIR]navidrome.ini" --logfile "[ND_DATAFOLDER]\navidrome.log"' /> - From 198f05ba089ca87eab4268f6f7a6da66ff76427b Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Tue, 18 Mar 2025 09:36:45 +0000 Subject: [PATCH 05/21] Using a file we should be able to add support for linux like this also --- release/goreleaser.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/release/goreleaser.yml b/release/goreleaser.yml index 1a420c927..3fc1a3a83 100644 --- a/release/goreleaser.yml +++ b/release/goreleaser.yml @@ -82,6 +82,12 @@ nfpms: owner: navidrome group: navidrome + - dst: /etc/navidrome/package + contents: {{ .Env.PACKAGE_TYPE }} + file_info: + owner: navidrome + group: navidrome + scripts: preinstall: "release/linux/preinstall.sh" postinstall: "release/linux/postinstall.sh" From 4d29121560a8f4a51d1db8297255abb7c9664c9c Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Tue, 18 Mar 2025 09:46:44 +0000 Subject: [PATCH 06/21] MSI should copy the package into place for us, it's not a KeyPath as older versions won't have it, so it's presence doesn't indicate the installed status of the package --- release/wix/navidrome.wxs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/release/wix/navidrome.wxs b/release/wix/navidrome.wxs index ec8b164e8..c64020fee 100644 --- a/release/wix/navidrome.wxs +++ b/release/wix/navidrome.wxs @@ -63,6 +63,10 @@ + + + + From 3b0fb4851d9270586cc4a3aad3625345a8b93bfe Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Tue, 18 Mar 2025 20:43:45 +0000 Subject: [PATCH 07/21] OK this doesn't exist, need to find another way to do it --- release/goreleaser.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/release/goreleaser.yml b/release/goreleaser.yml index 3fc1a3a83..1a420c927 100644 --- a/release/goreleaser.yml +++ b/release/goreleaser.yml @@ -82,12 +82,6 @@ nfpms: owner: navidrome group: navidrome - - dst: /etc/navidrome/package - contents: {{ .Env.PACKAGE_TYPE }} - file_info: - owner: navidrome - group: navidrome - scripts: preinstall: "release/linux/preinstall.sh" postinstall: "release/linux/postinstall.sh" From 3b062143612f44c8239007b58bcb0978e0d722bb Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Sun, 23 Mar 2025 18:42:11 +0000 Subject: [PATCH 08/21] package to .package and moving to the datadir --- core/metrics/insights.go | 2 +- release/wix/build_msi.sh | 2 +- release/wix/navidrome.wxs | 10 ++++++---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/core/metrics/insights.go b/core/metrics/insights.go index 8654dc678..f1b383977 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -155,7 +155,7 @@ var staticData = sync.OnceValue(func() insights.Data { data.OS.Containerized = consts.InContainer // Install info - packageFilename := filepath.Join(filepath.Dir(conf.Server.ConfigFile), "package") + packageFilename := filepath.Join(conf.Server.DataFolder, ".package") packageFileData, err := os.ReadFile(packageFilename) if err == nil { data.OS.Package = string(packageFileData) diff --git a/release/wix/build_msi.sh b/release/wix/build_msi.sh index c48e738dc..7e595311e 100755 --- a/release/wix/build_msi.sh +++ b/release/wix/build_msi.sh @@ -50,7 +50,7 @@ cp "$WORKSPACE"/LICENSE "$WORKSPACE"/README.md "$MSI_OUTPUT_DIR" cp "$BINARY" "$MSI_OUTPUT_DIR" # package type indicator file -echo "msi" > "$MSI_OUTPUT_DIR/package" +echo "msi" > "$MSI_OUTPUT_DIR/.package" # workaround for wixl WixVariable not working to override bmp locations cp "$WORKSPACE"/release/wix/bmp/banner.bmp /usr/share/wixl-*/ext/ui/bitmaps/bannrbmp.bmp diff --git a/release/wix/navidrome.wxs b/release/wix/navidrome.wxs index c64020fee..080740838 100644 --- a/release/wix/navidrome.wxs +++ b/release/wix/navidrome.wxs @@ -63,10 +63,6 @@ - - - - @@ -75,6 +71,12 @@ + + + + + + Not Installed AND NOT WIX_UPGRADE_DETECTED From 82face2dbd98c5b0b1036af9f630bec4e5d9074a Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 22 Mar 2025 15:07:51 -0400 Subject: [PATCH 09/21] fix(scanner): better log message when AutoImportPlaylists is disabled Fix #3861 Signed-off-by: Deluan --- scanner/phase_4_playlists.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scanner/phase_4_playlists.go b/scanner/phase_4_playlists.go index c3e76cb8c..c98b51ee6 100644 --- a/scanner/phase_4_playlists.go +++ b/scanner/phase_4_playlists.go @@ -45,8 +45,12 @@ func (p *phasePlaylists) producer() ppl.Producer[*model.Folder] { } func (p *phasePlaylists) produce(put func(entry *model.Folder)) error { + if !conf.Server.AutoImportPlaylists { + log.Info(p.ctx, "Playlists will not be imported, AutoImportPlaylists is set to false") + return nil + } u, _ := request.UserFrom(p.ctx) - if !conf.Server.AutoImportPlaylists || !u.IsAdmin { + if !u.IsAdmin { log.Warn(p.ctx, "Playlists will not be imported, as there are no admin users yet, "+ "Please create an admin user first, and then update the playlists for them to be imported") return nil From f8b902d6ae76e285c2214a59991acec92330a1d3 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 22 Mar 2025 15:48:07 -0400 Subject: [PATCH 10/21] fix(scanner): support ID3v2 embedded images in WAV files Fix #3867 Signed-off-by: Deluan --- adapters/taglib/taglib_wrapper.cpp | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/adapters/taglib/taglib_wrapper.cpp b/adapters/taglib/taglib_wrapper.cpp index 188a8b7d7..4c5a9fa1e 100644 --- a/adapters/taglib/taglib_wrapper.cpp +++ b/adapters/taglib/taglib_wrapper.cpp @@ -201,41 +201,42 @@ int taglib_read(const FILENAME_CHAR_T *filename, unsigned long id) { char has_cover(const TagLib::FileRef f) { char hasCover = 0; // ----- MP3 - if (TagLib::MPEG::File * - mp3File{dynamic_cast(f.file())}) { + if (TagLib::MPEG::File * mp3File{dynamic_cast(f.file())}) { if (mp3File->ID3v2Tag()) { const auto &frameListMap{mp3File->ID3v2Tag()->frameListMap()}; hasCover = !frameListMap["APIC"].isEmpty(); } } // ----- FLAC - else if (TagLib::FLAC::File * - flacFile{dynamic_cast(f.file())}) { + else if (TagLib::FLAC::File * flacFile{dynamic_cast(f.file())}) { hasCover = !flacFile->pictureList().isEmpty(); } // ----- MP4 - else if (TagLib::MP4::File * - mp4File{dynamic_cast(f.file())}) { + else if (TagLib::MP4::File * mp4File{dynamic_cast(f.file())}) { auto &coverItem{mp4File->tag()->itemMap()["covr"]}; TagLib::MP4::CoverArtList coverArtList{coverItem.toCoverArtList()}; hasCover = !coverArtList.isEmpty(); } // ----- Ogg - else if (TagLib::Ogg::Vorbis::File * - vorbisFile{dynamic_cast(f.file())}) { + else if (TagLib::Ogg::Vorbis::File * vorbisFile{dynamic_cast(f.file())}) { hasCover = !vorbisFile->tag()->pictureList().isEmpty(); } // ----- Opus - else if (TagLib::Ogg::Opus::File * - opusFile{dynamic_cast(f.file())}) { + else if (TagLib::Ogg::Opus::File * opusFile{dynamic_cast(f.file())}) { hasCover = !opusFile->tag()->pictureList().isEmpty(); } // ----- WMA - if (TagLib::ASF::File * - asfFile{dynamic_cast(f.file())}) { + else if (TagLib::ASF::File * asfFile{dynamic_cast(f.file())}) { const TagLib::ASF::Tag *tag{asfFile->tag()}; hasCover = tag && tag->attributeListMap().contains("WM/Picture"); } + // ----- WAV + else if (TagLib::RIFF::WAV::File * wavFile{ dynamic_cast(f.file()) }) { + if (wavFile->hasID3v2Tag()) { + const auto& frameListMap{ wavFile->ID3v2Tag()->frameListMap() }; + hasCover = !frameListMap["APIC"].isEmpty(); + } + } return hasCover; } From ae801d772cbf15207f5ce38ccb097b06df6c6fee Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 22 Mar 2025 15:48:29 -0400 Subject: [PATCH 11/21] feat(ui): show bitDepth in song info dialog Signed-off-by: Deluan --- resources/i18n/pt.json | 1 + ui/src/common/SongInfo.jsx | 3 ++- ui/src/i18n/en.json | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/resources/i18n/pt.json b/resources/i18n/pt.json index c3c65ca57..d856391ff 100644 --- a/resources/i18n/pt.json +++ b/resources/i18n/pt.json @@ -18,6 +18,7 @@ "size": "Tamanho", "updatedAt": "Últ. Atualização", "bitRate": "Bitrate", + "bitDepth": "Profundidade de bits", "discSubtitle": "Sub-título do disco", "starred": "Favorita", "comment": "Comentário", diff --git a/ui/src/common/SongInfo.jsx b/ui/src/common/SongInfo.jsx index d94685633..5adc1ebf0 100644 --- a/ui/src/common/SongInfo.jsx +++ b/ui/src/common/SongInfo.jsx @@ -74,6 +74,7 @@ export const SongInfo = (props) => { ), compilation: , bitRate: , + bitDepth: , channels: , size: , updatedAt: , @@ -91,7 +92,7 @@ export const SongInfo = (props) => { roles.push([name, record.participants[name].length]) } - const optionalFields = ['discSubtitle', 'comment', 'bpm', 'genre'] + const optionalFields = ['discSubtitle', 'comment', 'bpm', 'genre', 'bitDepth'] optionalFields.forEach((field) => { !record[field] && delete data[field] }) diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index cd377932c..678e42cd4 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -18,6 +18,7 @@ "size": "File size", "updatedAt": "Updated at", "bitRate": "Bit rate", + "bitDepth": "Bit depth", "channels": "Channels", "discSubtitle": "Disc Subtitle", "starred": "Favourite", From dcb69bcf816bd11e4412eaecacda302d47377cfb Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 22 Mar 2025 17:08:03 -0400 Subject: [PATCH 12/21] fix(server): don't break if the ND_CONFIGFILE does not exist Signed-off-by: Deluan --- conf/configuration.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/conf/configuration.go b/conf/configuration.go index 9abb7e163..6f4b2d594 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -577,9 +577,17 @@ func InitConfig(cfgFile string) { } } +// getConfigFile returns the path to the config file, either from the flag or from the environment variable. +// If it is defined in the environment variable, it will check if the file exists. func getConfigFile(cfgFile string) string { if cfgFile != "" { return cfgFile } - return os.Getenv("ND_CONFIGFILE") + cfgFile = os.Getenv("ND_CONFIGFILE") + if cfgFile != "" { + if _, err := os.Stat(cfgFile); err == nil { + return cfgFile + } + } + return "" } From c86816d21eeca56ebe5734ff2bd4510cd4b0ee69 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 22 Mar 2025 17:33:56 -0400 Subject: [PATCH 13/21] feat(docker): automatically loads a navidrome.toml file from /data, if available Signed-off-by: Deluan --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index 0ec64f769..4b4c3d18c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -133,6 +133,7 @@ COPY --from=build /out/navidrome /app/ VOLUME ["/data", "/music"] ENV ND_MUSICFOLDER=/music ENV ND_DATAFOLDER=/data +ENV ND_CONFIGFILE=/data/navidrome.toml ENV ND_PORT=4533 ENV GODEBUG="asyncpreemptoff=1" RUN touch /.nddockerenv From dd63e5a46584def23339e2c095a0bec21d8a6d2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 23 Mar 2025 10:53:21 -0400 Subject: [PATCH 14/21] feat(server): custom ArtistJoiner config (#3873) * feat(server): custom ArtistJoiner config Signed-off-by: Deluan * refactor(ui): organize ArtistLinkField, add tests Signed-off-by: Deluan * feat(ui): use display artist * feat(ui): use display artist Signed-off-by: Deluan --------- Signed-off-by: Deluan --- conf/configuration.go | 2 + model/metadata/map_participants.go | 5 +- ui/src/common/ArtistLinkField.jsx | 78 +++++--- ui/src/common/ArtistLinkField.test.jsx | 238 +++++++++++++++++++++++++ 4 files changed, 297 insertions(+), 26 deletions(-) create mode 100644 ui/src/common/ArtistLinkField.test.jsx diff --git a/conf/configuration.go b/conf/configuration.go index 6f4b2d594..a75581fca 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -129,6 +129,7 @@ type scannerOptions struct { WatcherWait time.Duration ScanOnStartup bool Extractor string + ArtistJoiner string GenreSeparators string // Deprecated: Use Tags.genre.Split instead GroupAlbumReleases bool // Deprecated: Use PID.Album instead } @@ -495,6 +496,7 @@ func init() { viper.SetDefault("scanner.extractor", consts.DefaultScannerExtractor) viper.SetDefault("scanner.watcherwait", consts.DefaultWatcherWait) viper.SetDefault("scanner.scanonstartup", true) + viper.SetDefault("scanner.artistjoiner", consts.ArtistJoiner) viper.SetDefault("scanner.genreseparators", "") viper.SetDefault("scanner.groupalbumreleases", false) diff --git a/model/metadata/map_participants.go b/model/metadata/map_participants.go index a871f64fa..e8be6aaab 100644 --- a/model/metadata/map_participants.go +++ b/model/metadata/map_participants.go @@ -4,6 +4,7 @@ import ( "cmp" "strings" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/str" @@ -210,8 +211,8 @@ func (md Metadata) getArtistValues(single, multi model.TagName) []string { func (md Metadata) mapDisplayName(singularTagName, pluralTagName model.TagName) string { return cmp.Or( - strings.Join(md.tags[singularTagName], consts.ArtistJoiner), - strings.Join(md.tags[pluralTagName], consts.ArtistJoiner), + strings.Join(md.tags[singularTagName], conf.Server.Scanner.ArtistJoiner), + strings.Join(md.tags[pluralTagName], conf.Server.Scanner.ArtistJoiner), ) } diff --git a/ui/src/common/ArtistLinkField.jsx b/ui/src/common/ArtistLinkField.jsx index 053cd25aa..60832eb40 100644 --- a/ui/src/common/ArtistLinkField.jsx +++ b/ui/src/common/ArtistLinkField.jsx @@ -63,38 +63,70 @@ const parseAndReplaceArtists = ( export const ArtistLinkField = ({ record, className, limit, source }) => { const role = source.toLowerCase() - const artists = record['participants'] - ? record['participants'][role] - : [{ name: record[source], id: record[source + 'Id'] }] - // When showing artists for a track, add any remixers to the list of artists - if ( - role === 'artist' && - record['participants'] && - record['participants']['remixer'] - ) { - record['participants']['remixer'].forEach((remixer) => { - artists.push(remixer) - }) - } + // Get artists array with fallback + let artists = record?.participants?.[role] || [] + const remixers = + role === 'artist' && record?.participants?.remixer + ? record.participants.remixer.slice(0, 2) + : [] - if (role === 'albumartist') { + // Use parseAndReplaceArtists for artist and albumartist roles + if ((role === 'artist' || role === 'albumartist') && record[source]) { const artistsLinks = parseAndReplaceArtists( record[source], artists, className, ) + if (artistsLinks.length > 0) { + // For artist role, append remixers if available, avoiding duplicates + if (role === 'artist' && remixers.length > 0) { + // Track which artists are already displayed to avoid duplicates + const displayedArtistIds = new Set( + artists.map((artist) => artist.id).filter(Boolean), + ) + + // Only add remixers that aren't already in the artists list + const uniqueRemixers = remixers.filter( + (remixer) => remixer.id && !displayedArtistIds.has(remixer.id), + ) + + if (uniqueRemixers.length > 0) { + artistsLinks.push(' • ') + uniqueRemixers.forEach((remixer, index) => { + if (index > 0) artistsLinks.push(' • ') + artistsLinks.push( + , + ) + }) + } + } + return
{artistsLinks}
} } - // Dedupe artists, only shows the first 3 + // Fall back to regular handling + if (artists.length === 0 && record[source]) { + artists = [{ name: record[source], id: record[source + 'Id'] }] + } + + // For artist role, combine artists and remixers before deduplication + const allArtists = role === 'artist' ? [...artists, ...remixers] : artists + + // Dedupe artists and collect subroles const seen = new Map() const dedupedArtists = [] let limitedShow = false - for (const artist of artists ?? []) { + for (const artist of allArtists) { + if (!artist?.id) continue + if (!seen.has(artist.id)) { if (dedupedArtists.length < limit) { seen.set(artist.id, dedupedArtists.length) @@ -107,22 +139,20 @@ export const ArtistLinkField = ({ record, className, limit, source }) => { } } else { const position = seen.get(artist.id) - - if (position !== -1) { - const existing = dedupedArtists[position] - if (artist.subRole && !existing.subroles.includes(artist.subRole)) { - existing.subroles.push(artist.subRole) - } + const existing = dedupedArtists[position] + if (artist.subRole && !existing.subroles.includes(artist.subRole)) { + existing.subroles.push(artist.subRole) } } } + // Create artist links const artistsList = dedupedArtists.map((artist) => ( - + )) if (limitedShow) { - artistsList.push(...) + artistsList.push(...) } return <>{intersperse(artistsList, ' • ')} diff --git a/ui/src/common/ArtistLinkField.test.jsx b/ui/src/common/ArtistLinkField.test.jsx new file mode 100644 index 000000000..09fdf64a4 --- /dev/null +++ b/ui/src/common/ArtistLinkField.test.jsx @@ -0,0 +1,238 @@ +import React from 'react' +import { render, screen } from '@testing-library/react' +import { describe, it, expect, beforeEach, vi } from 'vitest' +import { ArtistLinkField } from './ArtistLinkField' +import { intersperse } from '../utils/index.js' + +// Mock dependencies +vi.mock('react-redux', () => ({ + useDispatch: vi.fn(() => vi.fn()), +})) + +vi.mock('./useGetHandleArtistClick', () => ({ + useGetHandleArtistClick: vi.fn(() => (id) => `/artist/${id}`), +})) + +vi.mock('../utils/index.js', () => ({ + intersperse: vi.fn((arr) => arr), +})) + +vi.mock('@material-ui/core', () => ({ + withWidth: () => (Component) => { + const WithWidthComponent = (props) => + WithWidthComponent.displayName = `WithWidth(${Component.displayName || Component.name || 'Component'})` + return WithWidthComponent + }, +})) + +vi.mock('react-admin', () => ({ + Link: ({ children, to, ...props }) => ( + + {children} + + ), +})) + +describe('ArtistLinkField', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + describe('when rendering artists', () => { + it('renders artists from participants when available', () => { + const record = { + participants: { + artist: [ + { id: '1', name: 'Artist 1' }, + { id: '2', name: 'Artist 2' }, + ], + }, + } + + render() + + expect(screen.getByText('Artist 1')).toBeInTheDocument() + expect(screen.getByText('Artist 2')).toBeInTheDocument() + }) + + it('falls back to record[source] when participants not available', () => { + const record = { + artist: 'Fallback Artist', + artistId: '123', + } + + render() + + expect(screen.getByText('Fallback Artist')).toBeInTheDocument() + }) + + it('handles empty artists array', () => { + const record = { + participants: { + artist: [], + }, + } + + render() + + expect(intersperse).toHaveBeenCalledWith([], ' • ') + }) + }) + + describe('when handling remixers', () => { + it('adds remixers when showing artist role', () => { + const record = { + participants: { + artist: [{ id: '1', name: 'Artist 1' }], + remixer: [{ id: '2', name: 'Remixer 1' }], + }, + } + + render() + + expect(screen.getByText('Artist 1')).toBeInTheDocument() + expect(screen.getByText('Remixer 1')).toBeInTheDocument() + }) + + it('limits remixers to maximum of 2', () => { + const record = { + participants: { + artist: [{ id: '1', name: 'Artist 1' }], + remixer: [ + { id: '2', name: 'Remixer 1' }, + { id: '3', name: 'Remixer 2' }, + { id: '4', name: 'Remixer 3' }, + ], + }, + } + + render() + + expect(screen.getByText('Artist 1')).toBeInTheDocument() + expect(screen.getByText('Remixer 1')).toBeInTheDocument() + expect(screen.getByText('Remixer 2')).toBeInTheDocument() + expect(screen.queryByText('Remixer 3')).not.toBeInTheDocument() + }) + + it('deduplicates artists and remixers', () => { + const record = { + participants: { + artist: [{ id: '1', name: 'Duplicate Person' }], + remixer: [{ id: '1', name: 'Duplicate Person' }], + }, + } + + render() + + const links = screen.getAllByRole('link') + expect(links).toHaveLength(1) + expect(links[0]).toHaveTextContent('Duplicate Person') + }) + }) + + describe('when using parseAndReplaceArtists', () => { + it('uses parseAndReplaceArtists when role is albumartist', () => { + const record = { + albumArtist: 'Group Artist', + participants: { + albumartist: [{ id: '1', name: 'Group Artist' }], + }, + } + + render() + + expect(screen.getByText('Group Artist')).toBeInTheDocument() + expect(screen.getByRole('link')).toHaveAttribute('href', '/artist/1') + }) + + it('uses parseAndReplaceArtists when role is artist', () => { + const record = { + artist: 'Main Artist', + participants: { + artist: [{ id: '1', name: 'Main Artist' }], + }, + } + + render() + + expect(screen.getByText('Main Artist')).toBeInTheDocument() + expect(screen.getByRole('link')).toHaveAttribute('href', '/artist/1') + }) + + it('adds remixers after parseAndReplaceArtists for artist role', () => { + const record = { + artist: 'Main Artist', + participants: { + artist: [{ id: '1', name: 'Main Artist' }], + remixer: [{ id: '2', name: 'Remixer 1' }], + }, + } + + render() + + const links = screen.getAllByRole('link') + expect(links).toHaveLength(2) + expect(links[0]).toHaveAttribute('href', '/artist/1') + expect(links[1]).toHaveAttribute('href', '/artist/2') + }) + }) + + describe('when handling artist deduplication', () => { + it('deduplicates artists with the same id', () => { + const record = { + participants: { + artist: [ + { id: '1', name: 'Duplicate Artist' }, + { id: '1', name: 'Duplicate Artist', subRole: 'Vocals' }, + ], + }, + } + + render() + + const links = screen.getAllByRole('link') + expect(links).toHaveLength(1) + expect(links[0]).toHaveTextContent('Duplicate Artist (Vocals)') + }) + + it('aggregates subroles for the same artist', () => { + const record = { + participants: { + artist: [ + { id: '1', name: 'Multi-Role Artist', subRole: 'Vocals' }, + { id: '1', name: 'Multi-Role Artist', subRole: 'Guitar' }, + ], + }, + } + + render() + + expect( + screen.getByText('Multi-Role Artist (Vocals, Guitar)'), + ).toBeInTheDocument() + }) + }) + + describe('when limiting displayed artists', () => { + it('limits the number of artists displayed', () => { + const record = { + participants: { + artist: [ + { id: '1', name: 'Artist 1' }, + { id: '2', name: 'Artist 2' }, + { id: '3', name: 'Artist 3' }, + { id: '4', name: 'Artist 4' }, + ], + }, + } + + render() + + expect(screen.getByText('Artist 1')).toBeInTheDocument() + expect(screen.getByText('Artist 2')).toBeInTheDocument() + expect(screen.getByText('Artist 3')).toBeInTheDocument() + expect(screen.queryByText('Artist 4')).not.toBeInTheDocument() + expect(screen.getByText('...')).toBeInTheDocument() + }) + }) +}) From a316340eb1b7e1a78c6daac5c2bf645ff7c23dbe Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 23 Mar 2025 11:37:20 -0400 Subject: [PATCH 15/21] chore: remove some BFR-related TODOs that are not valid anymore Signed-off-by: Deluan --- conf/configuration.go | 1 - core/artwork/artwork_internal_test.go | 2 +- core/scrobbler/play_tracker_test.go | 1 - model/album.go | 2 +- model/mediafile.go | 4 ++-- persistence/album_repository.go | 1 - persistence/artist_repository.go | 3 +-- persistence/mediafile_repository.go | 1 - persistence/persistence_suite_test.go | 7 ------- persistence/playlist_repository_test.go | 2 +- scanner/controller.go | 1 - server/subsonic/helpers.go | 1 - 12 files changed, 6 insertions(+), 20 deletions(-) diff --git a/conf/configuration.go b/conf/configuration.go index a75581fca..08008105d 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -306,7 +306,6 @@ func Load(noConfigDump bool) { disableExternalServices() } - // BFR Remove before release if Server.Scanner.Extractor != consts.DefaultScannerExtractor { log.Warn(fmt.Sprintf("Extractor '%s' is not implemented, using 'taglib'", Server.Scanner.Extractor)) Server.Scanner.Extractor = consts.DefaultScannerExtractor diff --git a/core/artwork/artwork_internal_test.go b/core/artwork/artwork_internal_test.go index 65228ace5..462027082 100644 --- a/core/artwork/artwork_internal_test.go +++ b/core/artwork/artwork_internal_test.go @@ -15,7 +15,7 @@ import ( . "github.com/onsi/gomega" ) -// BFR Fix tests +// TODO Fix tests var _ = XDescribe("Artwork", func() { var aw *artwork var ds model.DataStore diff --git a/core/scrobbler/play_tracker_test.go b/core/scrobbler/play_tracker_test.go index a4bd7cec2..da1f96864 100644 --- a/core/scrobbler/play_tracker_test.go +++ b/core/scrobbler/play_tracker_test.go @@ -239,7 +239,6 @@ func (f *fakeScrobbler) Scrobble(ctx context.Context, userId string, s Scrobble) return nil } -// BFR This is duplicated in a few places func _p(id, name string, sortName ...string) model.Participant { p := model.Participant{Artist: model.Artist{ID: id, Name: name}} if len(sortName) > 0 { diff --git a/model/album.go b/model/album.go index 4ac976e24..c9dc022cb 100644 --- a/model/album.go +++ b/model/album.go @@ -17,7 +17,7 @@ type Album struct { Name string `structs:"name" json:"name"` EmbedArtPath string `structs:"embed_art_path" json:"-"` AlbumArtistID string `structs:"album_artist_id" json:"albumArtistId"` // Deprecated, use Participants - // BFR Rename to AlbumArtistDisplayName + // AlbumArtist is the display name used for the album artist. AlbumArtist string `structs:"album_artist" json:"albumArtist"` MaxYear int `structs:"max_year" json:"maxYear"` MinYear int `structs:"min_year" json:"minYear"` diff --git a/model/mediafile.go b/model/mediafile.go index 795657466..896442436 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -31,10 +31,10 @@ type MediaFile struct { Title string `structs:"title" json:"title"` Album string `structs:"album" json:"album"` ArtistID string `structs:"artist_id" json:"artistId"` // Deprecated: Use Participants instead - // BFR Rename to ArtistDisplayName + // Artist is the display name used for the artist. Artist string `structs:"artist" json:"artist"` AlbumArtistID string `structs:"album_artist_id" json:"albumArtistId"` // Deprecated: Use Participants instead - // BFR Rename to AlbumArtistDisplayName + // AlbumArtist is the display name used for the album artist. AlbumArtist string `structs:"album_artist" json:"albumArtist"` AlbumID string `structs:"album_id" json:"albumId"` HasCoverArt bool `structs:"has_cover_art" json:"hasCoverArt"` diff --git a/persistence/album_repository.go b/persistence/album_repository.go index b29a44701..0f2a46dec 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -184,7 +184,6 @@ func allRolesFilter(_ string, value interface{}) Sqlizer { func (r *albumRepository) CountAll(options ...model.QueryOptions) (int64, error) { sql := r.newSelect() sql = r.withAnnotation(sql, "album.id") - // BFR WithParticipants (for filtering by name)? return r.count(sql, options...) } diff --git a/persistence/artist_repository.go b/persistence/artist_repository.go index dd3f31b00..7602be381 100644 --- a/persistence/artist_repository.go +++ b/persistence/artist_repository.go @@ -85,7 +85,7 @@ func (a *dbArtist) PostMapArgs(m map[string]any) error { m["full_text"] = formatFullText(a.Name, a.SortArtistName) // Do not override the sort_artist_name and mbz_artist_id fields if they are empty - // BFR: Better way to handle this? + // TODO: Better way to handle this? if v, ok := m["sort_artist_name"]; !ok || v.(string) == "" { delete(m, "sort_artist_name") } @@ -134,7 +134,6 @@ func roleFilter(_ string, role any) Sqlizer { func (r *artistRepository) selectArtist(options ...model.QueryOptions) SelectBuilder { query := r.newSelect(options...).Columns("artist.*") query = r.withAnnotation(query, "artist.id") - // BFR How to handle counts and sizes (per role)? return query } diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go index ef4507877..ebf07ce17 100644 --- a/persistence/mediafile_repository.go +++ b/persistence/mediafile_repository.go @@ -105,7 +105,6 @@ var mediaFileFilter = sync.OnceValue(func() map[string]filterFunc { func (r *mediaFileRepository) CountAll(options ...model.QueryOptions) (int64, error) { query := r.newSelect() query = r.withAnnotation(query, "media_file.id") - // BFR WithParticipants (for filtering by name)? return r.count(query, options...) } diff --git a/persistence/persistence_suite_test.go b/persistence/persistence_suite_test.go index 609904b49..43e4c292b 100644 --- a/persistence/persistence_suite_test.go +++ b/persistence/persistence_suite_test.go @@ -29,13 +29,6 @@ func TestPersistence(t *testing.T) { RunSpecs(t, "Persistence Suite") } -// BFR Test tags -//var ( -// genreElectronic = model.Genre{ID: "gn-1", Name: "Electronic"} -// genreRock = model.Genre{ID: "gn-2", Name: "Rock"} -// testGenres = model.Genres{genreElectronic, genreRock} -//) - func mf(mf model.MediaFile) model.MediaFile { mf.Tags = model.Tags{} mf.LibraryID = 1 diff --git a/persistence/playlist_repository_test.go b/persistence/playlist_repository_test.go index 85a87ece7..5a82964c9 100644 --- a/persistence/playlist_repository_test.go +++ b/persistence/playlist_repository_test.go @@ -145,7 +145,7 @@ var _ = Describe("PlaylistRepository", func() { }) }) - // BFR Validate these tests + // TODO Validate these tests XContext("child smart playlists", func() { When("refresh day has expired", func() { It("should refresh tracks for smart playlist referenced in parent smart playlist criteria", func() { diff --git a/scanner/controller.go b/scanner/controller.go index 84ea8e606..e3e008483 100644 --- a/scanner/controller.go +++ b/scanner/controller.go @@ -98,7 +98,6 @@ type ProgressInfo struct { type scanner interface { scanAll(ctx context.Context, fullScan bool, progress chan<- *ProgressInfo) - // BFR: scanFolders(ctx context.Context, lib model.Lib, folders []string, progress chan<- *ScannerStatus) } type controller struct { diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index fa98a985b..56b65f894 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -235,7 +235,6 @@ func osChildFromMediaFile(ctx context.Context, mf model.MediaFile) *responses.Op child.BitDepth = int32(mf.BitDepth) child.Genres = toItemGenres(mf.Genres) child.Moods = mf.Tags.Values(model.TagMood) - // BFR What if Child is an Album and not a Song? child.DisplayArtist = mf.Artist child.Artists = artistRefs(mf.Participants[model.RoleArtist]) child.DisplayAlbumArtist = mf.AlbumArtist From 0c81fe4938b26b8a91311408f7de704bace76bb6 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 23 Mar 2025 11:53:43 -0400 Subject: [PATCH 16/21] chore: remove more outdated TODOs Signed-off-by: Deluan --- model/criteria/operators_test.go | 1 - model/player.go | 1 - server/subsonic/browsing.go | 4 +--- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/model/criteria/operators_test.go b/model/criteria/operators_test.go index 575b9c3f8..e6082de44 100644 --- a/model/criteria/operators_test.go +++ b/model/criteria/operators_test.go @@ -46,7 +46,6 @@ var _ = Describe("Operators", func() { Entry("notInPlaylist", NotInPlaylist{"id": "deadbeef-dead-beef"}, "media_file.id NOT IN "+ "(SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))", "deadbeef-dead-beef", 1), - // TODO These may be flaky Entry("inTheLast", InTheLast{"lastPlayed": 30}, "annotation.play_date > ?", StartOfPeriod(30, time.Now())), Entry("notInTheLast", NotInTheLast{"lastPlayed": 30}, "(annotation.play_date < ? OR annotation.play_date IS NULL)", StartOfPeriod(30, time.Now())), diff --git a/model/player.go b/model/player.go index ee7346b66..39ea99d1a 100644 --- a/model/player.go +++ b/model/player.go @@ -28,5 +28,4 @@ type PlayerRepository interface { Put(p *Player) error CountAll(...QueryOptions) (int64, error) CountByClient(...QueryOptions) (map[string]int64, error) - // TODO: Add CountAll method. Useful at least for metrics. } diff --git a/server/subsonic/browsing.go b/server/subsonic/browsing.go index d46c7937d..edc45a7c7 100644 --- a/server/subsonic/browsing.go +++ b/server/subsonic/browsing.go @@ -252,9 +252,7 @@ func (api *Router) GetSong(r *http.Request) (*responses.Subsonic, error) { func (api *Router) GetGenres(r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() - // TODO Put back when album_count is available - //genres, err := api.ds.Genre(ctx).GetAll(model.QueryOptions{Sort: "song_count, album_count, name desc", Order: "desc"}) - genres, err := api.ds.Genre(ctx).GetAll(model.QueryOptions{Sort: "song_count, name desc", Order: "desc"}) + genres, err := api.ds.Genre(ctx).GetAll(model.QueryOptions{Sort: "song_count, album_count, name desc", Order: "desc"}) if err != nil { log.Error(r, err) return nil, err From b091bf9eb7821e53f4d545fedddce276c2defdd3 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 23 Mar 2025 12:36:38 -0400 Subject: [PATCH 17/21] fix(scanner): elapsed time for folder processing is wrong in the logs Signed-off-by: Deluan --- scanner/walk_dir_tree.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index 323ba0392..ba87f2628 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -70,7 +70,6 @@ func newFolderEntry(job *scanJob, path string) *folderEntry { albumIDMap: make(map[string]string), updTime: job.popLastUpdate(id), } - f.elapsed.Start() return f } @@ -115,6 +114,8 @@ func walkFolder(ctx context.Context, job *scanJob, currentFolder string, ignoreP "images", maps.Keys(folder.imageFiles), "playlists", folder.numPlaylists, "imagesUpdatedAt", folder.imagesUpdatedAt, "updTime", folder.updTime, "modTime", folder.modTime, "numChildren", len(children)) folder.path = dir + folder.elapsed.Start() + results <- folder return nil From 61339689b053eb4355ec11e1300114042960898e Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Sun, 23 Mar 2025 19:09:55 +0000 Subject: [PATCH 18/21] Should be able to reuse this mechanism with deb and rpm, I think it would be nice to know which specific one it is without guessing based on /etc/debian_version or something; but it doesn't look like that is exposed by goreleaser into an env or anything :/ --- release/linux/postinstall.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/release/linux/postinstall.sh b/release/linux/postinstall.sh index 65f1d208d..85eb0850f 100644 --- a/release/linux/postinstall.sh +++ b/release/linux/postinstall.sh @@ -14,6 +14,11 @@ if [ ! -f /etc/navidrome/navidrome.toml ]; then fi postinstall_flag="/var/lib/navidrome/.installed" +package_file="/var/lib/navidrome/.package" + +if [ ! -f "$package_file" ]; then + echo "deb/rpm" > "$package_file"; +fi if [ ! -f "$postinstall_flag" ]; then # The primary reason why this would fail is if the service was already installed AND From 409a43599bb9739528a5d1416a5eafe2b1d27086 Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Sat, 29 Mar 2025 13:41:17 +0000 Subject: [PATCH 19/21] Need to reference the installed file and I think Id's don't require [] --- release/wix/navidrome.wxs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/release/wix/navidrome.wxs b/release/wix/navidrome.wxs index 080740838..e48d29136 100644 --- a/release/wix/navidrome.wxs +++ b/release/wix/navidrome.wxs @@ -1,4 +1,4 @@ - + @@ -71,7 +71,7 @@ - + @@ -87,6 +87,7 @@ + From 7b373191b6b0a3914b8c54448b402d5d650de228 Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Sat, 29 Mar 2025 14:15:35 +0000 Subject: [PATCH 20/21] Need to add into the root directory for this to work --- release/wix/navidrome.wxs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/release/wix/navidrome.wxs b/release/wix/navidrome.wxs index e48d29136..808b93411 100644 --- a/release/wix/navidrome.wxs +++ b/release/wix/navidrome.wxs @@ -69,12 +69,12 @@ - - - - - + + + + + From eb19ee497d349aa86909d9a9be9508a229bf4358 Mon Sep 17 00:00:00 2001 From: Rob Emery Date: Wed, 2 Apr 2025 08:48:24 +0100 Subject: [PATCH 21/21] That was not deliberately removed --- release/wix/navidrome.wxs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release/wix/navidrome.wxs b/release/wix/navidrome.wxs index 808b93411..8ebba4632 100644 --- a/release/wix/navidrome.wxs +++ b/release/wix/navidrome.wxs @@ -1,4 +1,4 @@ - +