From 8cfd604e51eab3b1203d214a288f91652e2a39d7 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Fri, 12 Apr 2019 12:04:25 +0200
Subject: [PATCH] Fix #782: Better handling of featuring/multi-artist tracks
 tagged with MusicBrainz

---
 api/funkwhale_api/music/metadata.py | 16 ++++++++++------
 api/tests/music/test_metadata.py    | 15 +++++++++++++++
 api/tests/music/test_tasks.py       | 22 ++++++++++++++++++++++
 changes/changelog.d/782.bugfix      |  1 +
 4 files changed, 48 insertions(+), 6 deletions(-)
 create mode 100644 changes/changelog.d/782.bugfix

diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py
index eca4b4757..387b8ffe7 100644
--- a/api/funkwhale_api/music/metadata.py
+++ b/api/funkwhale_api/music/metadata.py
@@ -381,18 +381,22 @@ class ArtistField(serializers.Field):
 
     def to_internal_value(self, data):
         # we have multiple values that can be separated by various separators
-        separators = [";", "/"]
+        separators = [";"]
         # we get a list like that if tagged via musicbrainz
         # ae29aae4-abfb-4609-8f54-417b1f4d64cc; 3237b5a8-ae44-400c-aa6d-cea51f0b9074;
         raw_mbids = data["mbids"]
         used_separator = None
         mbids = [raw_mbids]
         if raw_mbids:
-            for separator in separators:
-                if separator in raw_mbids:
-                    used_separator = separator
-                    mbids = [m.strip() for m in raw_mbids.split(separator)]
-                    break
+            if "/" in raw_mbids:
+                # it's a featuring, we can't handle this now
+                mbids = []
+            else:
+                for separator in separators:
+                    if separator in raw_mbids:
+                        used_separator = separator
+                        mbids = [m.strip() for m in raw_mbids.split(separator)]
+                        break
 
         # now, we split on artist names, using the same separator as the one used
         # by mbids, if any
diff --git a/api/tests/music/test_metadata.py b/api/tests/music/test_metadata.py
index 1d61901c1..1656ece49 100644
--- a/api/tests/music/test_metadata.py
+++ b/api/tests/music/test_metadata.py
@@ -537,3 +537,18 @@ def test_serializer_album_artist_missing():
     serializer = metadata.TrackMetadataSerializer(data=metadata.FakeMetadata(data))
     assert serializer.is_valid(raise_exception=True) is True
     assert serializer.validated_data == expected
+
+
+def test_artist_field_featuring():
+    data = {
+        "artist": "Santana feat. Chris Cornell",
+        # here is the tricky bit, note the slash
+        "musicbrainz_artistid": "9a3bf45c-347d-4630-894d-7cf3e8e0b632/cbf9738d-8f81-4a92-bc64-ede09341652d",
+    }
+
+    expected = [{"name": "Santana feat. Chris Cornell", "mbid": None}]
+
+    field = metadata.ArtistField()
+    value = field.get_value(data)
+
+    assert field.to_internal_value(value) == expected
diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py
index d897c1a5f..028b10b76 100644
--- a/api/tests/music/test_tasks.py
+++ b/api/tests/music/test_tasks.py
@@ -74,6 +74,28 @@ def test_can_create_track_from_file_metadata_attributed_to(factories, mocker):
     assert track.artist.attributed_to == actor
 
 
+def test_can_create_track_from_file_metadata_featuring(factories):
+    metadata = {
+        "title": "Whole Lotta Love",
+        "position": 1,
+        "disc_number": 1,
+        "mbid": "508704c0-81d4-4c94-ba58-3fc0b7da23eb",
+        "album": {
+            "title": "Guitar Heaven: The Greatest Guitar Classics of All Time",
+            "mbid": "d06f2072-4148-488d-af6f-69ab6539ddb8",
+            "release_date": datetime.date(2010, 9, 17),
+            "artists": [
+                {"name": "Santana", "mbid": "9a3bf45c-347d-4630-894d-7cf3e8e0b632"}
+            ],
+        },
+        "artists": [{"name": "Santana feat. Chris Cornell", "mbid": None}],
+    }
+    track = tasks.get_track_from_import_metadata(metadata)
+
+    assert track.album.artist.name == "Santana"
+    assert track.artist.name == "Santana feat. Chris Cornell"
+
+
 def test_can_create_track_from_file_metadata_mbid(factories, mocker):
     metadata = {
         "title": "Test track",
diff --git a/changes/changelog.d/782.bugfix b/changes/changelog.d/782.bugfix
new file mode 100644
index 000000000..16b0ccdd4
--- /dev/null
+++ b/changes/changelog.d/782.bugfix
@@ -0,0 +1 @@
+Better handling of featuring/multi-artist tracks tagged with MusicBrainz (#782)