From 59f7bec8dd9b87d713b9cd4958d17943fa2486cc Mon Sep 17 00:00:00 2001
From: emeric <emeric.poupon@laposte.net>
Date: Sat, 22 Feb 2025 22:05:25 +0100
Subject: [PATCH] Fixed regression in album grouping, ref #616

---
 src/libs/database/test/Release.cpp            | 30 +++++++++++++++++++
 src/libs/metadata/impl/Parser.cpp             |  2 +-
 src/libs/metadata/test/Parser.cpp             | 15 ++++++++++
 .../impl/scanners/AudioFileScanner.cpp        |  7 ++---
 src/tools/metadata/LmsMetadata.cpp            |  2 +-
 5 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/src/libs/database/test/Release.cpp b/src/libs/database/test/Release.cpp
index 34f7a495..c2e48600 100644
--- a/src/libs/database/test/Release.cpp
+++ b/src/libs/database/test/Release.cpp
@@ -1386,4 +1386,34 @@ namespace lms::db::tests
             EXPECT_EQ(releases.results[0]->getId(), release->getId());
         }
     }
+
+    TEST_F(DatabaseFixture, Release_sortName)
+    {
+        ScopedRelease release1{ session, "MyRelease1" };
+        ScopedRelease release2{ session, "MyRelease2" };
+
+        {
+            auto transaction{ session.createWriteTransaction() };
+            release1.get().modify()->setSortName("BB");
+            release2.get().modify()->setSortName("AA");
+        }
+
+        {
+            auto transaction{ session.createReadTransaction() };
+
+            const auto releases{ Release::find(session, Release::FindParameters{}.setSortMethod(ReleaseSortMethod::Name)) };
+            ASSERT_EQ(releases.results.size(), 2);
+            EXPECT_EQ(releases.results[0]->getId(), release1->getId());
+            EXPECT_EQ(releases.results[1]->getId(), release2->getId());
+        }
+
+        {
+            auto transaction{ session.createReadTransaction() };
+
+            const auto releases{ Release::find(session, Release::FindParameters{}.setSortMethod(ReleaseSortMethod::SortName)) };
+            ASSERT_EQ(releases.results.size(), 2);
+            EXPECT_EQ(releases.results[0]->getId(), release2->getId());
+            EXPECT_EQ(releases.results[1]->getId(), release1->getId());
+        }
+    }
 } // namespace lms::db::tests
\ No newline at end of file
diff --git a/src/libs/metadata/impl/Parser.cpp b/src/libs/metadata/impl/Parser.cpp
index 78a07500..4c922f5a 100644
--- a/src/libs/metadata/impl/Parser.cpp
+++ b/src/libs/metadata/impl/Parser.cpp
@@ -511,7 +511,7 @@ namespace lms::metadata
 
         release.emplace();
         release->name = std::move(*releaseName);
-        release->sortName = getTagValueAs<std::string>(tagReader, TagType::AlbumSortOrder).value_or("");
+        release->sortName = getTagValueAs<std::string>(tagReader, TagType::AlbumSortOrder).value_or(release->name);
         release->artists = getArtists(tagReader, { TagType::AlbumArtists, TagType::AlbumArtist }, { TagType::AlbumArtistsSortOrder, TagType::AlbumArtistSortOrder }, { TagType::MusicBrainzReleaseArtistID }, _artistTagDelimiters, _defaultTagDelimiters);
         release->artistDisplayName = computeArtistDisplayName(release->artists, getTagValueAs<std::string>(tagReader, TagType::AlbumArtist), _artistTagDelimiters);
         release->mbid = getTagValueAs<core::UUID>(tagReader, TagType::MusicBrainzReleaseID);
diff --git a/src/libs/metadata/test/Parser.cpp b/src/libs/metadata/test/Parser.cpp
index 388a202c..5fdffacf 100644
--- a/src/libs/metadata/test/Parser.cpp
+++ b/src/libs/metadata/test/Parser.cpp
@@ -716,6 +716,21 @@ namespace lms::metadata
         EXPECT_EQ(track->composerArtists[0].mbid.value(), core::UUID::fromString("6643f584-5edc-45ce-927d-0a4ab25c2673"));
     }
 
+    TEST(Parser, release_sortNameFallback)
+    {
+        const TestTagReader testTags{
+            {
+                { TagType::Album, { "MyAlbum" } },
+                // No AlbumSortOrder
+            }
+        };
+        std::unique_ptr<Track> track{ Parser{}.parse(testTags) };
+
+        ASSERT_TRUE(track->medium.has_value());
+        ASSERT_TRUE(track->medium->release.has_value());
+        EXPECT_EQ(track->medium->release->sortName, "MyAlbum");
+    }
+
     TEST(Parser, advisory)
     {
         auto doTest = [](std::string_view value, std::optional<Track::Advisory> expectedValue) {
diff --git a/src/libs/services/scanner/impl/scanners/AudioFileScanner.cpp b/src/libs/services/scanner/impl/scanners/AudioFileScanner.cpp
index c22370cf..ba2fc2ca 100644
--- a/src/libs/services/scanner/impl/scanners/AudioFileScanner.cpp
+++ b/src/libs/services/scanner/impl/scanners/AudioFileScanner.cpp
@@ -163,11 +163,8 @@ namespace lms::scanner
         {
             if (release->getName() != releaseInfo.name)
                 release.modify()->setName(releaseInfo.name);
-            {
-                std::string_view sortName{ !releaseInfo.sortName.empty() ? releaseInfo.sortName : releaseInfo.name };
-                if (release->getSortName() != sortName)
-                    release.modify()->setSortName(sortName);
-            }
+            if (release->getSortName() != releaseInfo.sortName)
+                release.modify()->setSortName(releaseInfo.sortName);
             if (release->getGroupMBID() != releaseInfo.groupMBID)
                 release.modify()->setGroupMBID(releaseInfo.groupMBID);
             if (release->getTotalDisc() != releaseInfo.mediumCount)
diff --git a/src/tools/metadata/LmsMetadata.cpp b/src/tools/metadata/LmsMetadata.cpp
index 2317ab93..c147e3ec 100644
--- a/src/tools/metadata/LmsMetadata.cpp
+++ b/src/tools/metadata/LmsMetadata.cpp
@@ -78,7 +78,7 @@ namespace lms::metadata
     std::ostream& operator<<(std::ostream& os, const Release& release)
     {
         os << release.name;
-        if (!release.sortName.empty())
+        if (release.sortName != release.name)
             os << " '" << release.sortName << "'";
         os << std::endl;