Merge branch '782-artists-metadata' into 'develop'

Fix #782: Better handling of featuring/multi-artist tracks tagged with MusicBrainz

Closes #782

See merge request funkwhale/funkwhale!717

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 "/" in raw_mbids:
-                if separator in raw_mbids:
+                # it's a featuring, we can't handle this now
-                    used_separator = separator
+                mbids = []
-                    mbids = [m.strip() for m in raw_mbids.split(separator)]
+            else:
-                    break
+                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

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

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",

changes/changelog.d/782.bugfix

@@ -0,0 +1 @@
+Better handling of featuring/multi-artist tracks tagged with MusicBrainz (#782)