diff --git a/api/funkwhale_api/music/migrations/0050_auto_20200129_1344.py b/api/funkwhale_api/music/migrations/0050_auto_20200129_1344.py
new file mode 100644
index 0000000000000000000000000000000000000000..ecd74003d3884844aa42d5f27bd4dd483014868d
--- /dev/null
+++ b/api/funkwhale_api/music/migrations/0050_auto_20200129_1344.py
@@ -0,0 +1,18 @@
+# Generated by Django 2.2.9 on 2020-01-29 13:44
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('music', '0049_auto_20200122_1020'),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name='track',
+ name='mbid',
+ field=models.UUIDField(blank=True, db_index=True, null=True),
+ ),
+ ]
diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py
index e1852870ec548b7c2f5887502e1a67253f78a671..1d156a6c625b560c68a9175462bce02f03c10ef9 100644
--- a/api/funkwhale_api/music/models.py
+++ b/api/funkwhale_api/music/models.py
@@ -475,6 +475,7 @@ def get_artist(release_list):
class Track(APIModelMixin):
+ mbid = models.UUIDField(db_index=True, null=True, blank=True)
title = models.CharField(max_length=MAX_LENGTHS["TRACK_TITLE"])
artist = models.ForeignKey(Artist, related_name="tracks", on_delete=models.CASCADE)
disc_number = models.PositiveIntegerField(null=True, blank=True)
diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py
index 3e95e5325cd1a9e75991ec4a6a5152f6c49121a4..636a54cf40df3fab0cb12d0f43b980f92c556524 100644
--- a/api/funkwhale_api/music/tasks.py
+++ b/api/funkwhale_api/music/tasks.py
@@ -625,9 +625,18 @@ def _get_track(data, attributed_to=None, **forced_values):
else truncate(data.get("copyright"), models.MAX_LENGTHS["COPYRIGHT"])
)
- query = Q(title__iexact=track_title, artist=artist, album=album, position=position)
+ query = Q(
+ title__iexact=track_title,
+ artist=artist,
+ album=album,
+ position=position,
+ disc_number=disc_number,
+ )
if track_mbid:
- query |= Q(mbid=track_mbid)
+ if album_mbid:
+ query |= Q(mbid=track_mbid, album__mbid=album_mbid)
+ else:
+ query |= Q(mbid=track_mbid)
if track_fid:
query |= Q(fid=track_fid)
diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py
index 053f62ede5ba4230e450dd2873312ae7d6564f1c..bbbc9c7458d96022bdd153c10d8cc57997ff0404 100644
--- a/api/tests/music/test_tasks.py
+++ b/api/tests/music/test_tasks.py
@@ -1164,3 +1164,134 @@ def test_can_download_image_file_for_album_mbid(binary_cover, mocker, factories)
assert album.attachment_cover.file.read() == binary_cover
assert album.attachment_cover.mimetype == "image/jpeg"
+
+
+def test_can_import_track_with_same_mbid_in_different_albums(factories, mocker):
+ artist = factories["music.Artist"]()
+ upload = factories["music.Upload"](
+ playable=True, track__artist=artist, track__album__artist=artist
+ )
+ assert upload.track.mbid is not None
+ data = {
+ "title": upload.track.title,
+ "artists": [{"name": artist.name, "mbid": artist.mbid}],
+ "album": {
+ "title": "The Slip",
+ "mbid": uuid.UUID("12b57d46-a192-499e-a91f-7da66790a1c1"),
+ "release_date": datetime.date(2008, 5, 5),
+ "artists": [{"name": artist.name, "mbid": artist.mbid}],
+ },
+ "position": 1,
+ "disc_number": 1,
+ "mbid": upload.track.mbid,
+ }
+
+ mocker.patch.object(metadata.TrackMetadataSerializer, "validated_data", data)
+ mocker.patch.object(tasks, "populate_album_cover")
+
+ new_upload = factories["music.Upload"](library=upload.library)
+
+ tasks.process_upload(upload_id=new_upload.pk)
+
+ new_upload.refresh_from_db()
+
+ assert new_upload.import_status == "finished"
+
+
+def test_import_track_with_same_mbid_in_same_albums_skipped(factories, mocker):
+ artist = factories["music.Artist"]()
+ upload = factories["music.Upload"](
+ playable=True, track__artist=artist, track__album__artist=artist
+ )
+ assert upload.track.mbid is not None
+ data = {
+ "title": upload.track.title,
+ "artists": [{"name": artist.name, "mbid": artist.mbid}],
+ "album": {
+ "title": upload.track.album.title,
+ "mbid": upload.track.album.mbid,
+ "artists": [{"name": artist.name, "mbid": artist.mbid}],
+ },
+ "position": 1,
+ "disc_number": 1,
+ "mbid": upload.track.mbid,
+ }
+
+ mocker.patch.object(metadata.TrackMetadataSerializer, "validated_data", data)
+ mocker.patch.object(tasks, "populate_album_cover")
+
+ new_upload = factories["music.Upload"](library=upload.library)
+
+ tasks.process_upload(upload_id=new_upload.pk)
+
+ new_upload.refresh_from_db()
+
+ assert new_upload.import_status == "skipped"
+
+
+def test_can_import_track_with_same_position_in_different_discs(factories, mocker):
+ upload = factories["music.Upload"](playable=True)
+ artist_data = [
+ {
+ "name": upload.track.album.artist.name,
+ "mbid": upload.track.album.artist.mbid,
+ }
+ ]
+ data = {
+ "title": upload.track.title,
+ "artists": artist_data,
+ "album": {
+ "title": "The Slip",
+ "mbid": upload.track.album.mbid,
+ "release_date": datetime.date(2008, 5, 5),
+ "artists": artist_data,
+ },
+ "position": upload.track.position,
+ "disc_number": 2,
+ "mbid": None,
+ }
+
+ mocker.patch.object(metadata.TrackMetadataSerializer, "validated_data", data)
+ mocker.patch.object(tasks, "populate_album_cover")
+
+ new_upload = factories["music.Upload"](library=upload.library)
+
+ tasks.process_upload(upload_id=new_upload.pk)
+
+ new_upload.refresh_from_db()
+
+ assert new_upload.import_status == "finished"
+
+
+def test_can_import_track_with_same_position_in_same_discs_skipped(factories, mocker):
+ upload = factories["music.Upload"](playable=True)
+ artist_data = [
+ {
+ "name": upload.track.album.artist.name,
+ "mbid": upload.track.album.artist.mbid,
+ }
+ ]
+ data = {
+ "title": upload.track.title,
+ "artists": artist_data,
+ "album": {
+ "title": "The Slip",
+ "mbid": upload.track.album.mbid,
+ "release_date": datetime.date(2008, 5, 5),
+ "artists": artist_data,
+ },
+ "position": upload.track.position,
+ "disc_number": upload.track.disc_number,
+ "mbid": None,
+ }
+
+ mocker.patch.object(metadata.TrackMetadataSerializer, "validated_data", data)
+ mocker.patch.object(tasks, "populate_album_cover")
+
+ new_upload = factories["music.Upload"](library=upload.library)
+
+ tasks.process_upload(upload_id=new_upload.pk)
+
+ new_upload.refresh_from_db()
+
+ assert new_upload.import_status == "skipped"
diff --git a/changes/changelog.d/348.bugfix b/changes/changelog.d/348.bugfix
new file mode 100644
index 0000000000000000000000000000000000000000..10c04b98216c5f000507c1703dd6f58cb65b143a
--- /dev/null
+++ b/changes/changelog.d/348.bugfix
@@ -0,0 +1 @@
+Improved deduplication logic to prevent skipped files during import (#348, #474, #557, #740, #928)
diff --git a/front/src/views/admin/library/ArtistDetail.vue b/front/src/views/admin/library/ArtistDetail.vue
index 0104431eaf2ee9ea16c330b89f07f7fc84946752..3ee193e3547db5e699869970deca3de444d500fc 100644
--- a/front/src/views/admin/library/ArtistDetail.vue
+++ b/front/src/views/admin/library/ArtistDetail.vue
@@ -9,7 +9,7 @@
<div class="ui column">
<div class="segment-content">
<h2 class="ui header">
- <img v-if="object.cover" v-lazy="$store.getters['instance/absoluteUrl'](object.cover.square_crop)">
+ <img v-if="object.cover && object.cover.square_crop" v-lazy="$store.getters['instance/absoluteUrl'](object.cover.square_crop)">
<img v-else src="../../../assets/audio/default-cover.png">
<div class="content">
{{ object.name | truncate(100) }}