diff --git a/api/funkwhale_api/music/filters.py b/api/funkwhale_api/music/filters.py
index 1f73fc9b0638df2d15520f5522fb13c013989de0..87537b675ed95dcfe0b3cfa48d792a5ddb7063ce 100644
--- a/api/funkwhale_api/music/filters.py
+++ b/api/funkwhale_api/music/filters.py
@@ -6,19 +6,9 @@ from funkwhale_api.common import fields
from . import models
-class ListenableMixin(filters.FilterSet):
- listenable = filters.BooleanFilter(name="_", method="filter_listenable")
-
- def filter_listenable(self, queryset, name, value):
- queryset = queryset.annotate(files_count=Count("tracks__files"))
- if value:
- return queryset.filter(files_count__gt=0)
- else:
- return queryset.filter(files_count=0)
-
-
-class ArtistFilter(ListenableMixin):
+class ArtistFilter(filters.FilterSet):
q = fields.SearchFilter(search_fields=["name"])
+ listenable = filters.BooleanFilter(name="_", method="filter_listenable")
class Meta:
model = models.Artist
@@ -27,6 +17,13 @@ class ArtistFilter(ListenableMixin):
"listenable": "exact",
}
+ def filter_listenable(self, queryset, name, value):
+ queryset = queryset.annotate(files_count=Count("albums__tracks__files"))
+ if value:
+ return queryset.filter(files_count__gt=0)
+ else:
+ return queryset.filter(files_count=0)
+
class TrackFilter(filters.FilterSet):
q = fields.SearchFilter(search_fields=["title", "album__title", "artist__name"])
@@ -72,10 +69,17 @@ class ImportJobFilter(filters.FilterSet):
}
-class AlbumFilter(ListenableMixin):
+class AlbumFilter(filters.FilterSet):
listenable = filters.BooleanFilter(name="_", method="filter_listenable")
q = fields.SearchFilter(search_fields=["title", "artist__name" "source"])
class Meta:
model = models.Album
fields = ["listenable", "q", "artist"]
+
+ def filter_listenable(self, queryset, name, value):
+ queryset = queryset.annotate(files_count=Count("tracks__files"))
+ if value:
+ return queryset.filter(files_count__gt=0)
+ else:
+ return queryset.filter(files_count=0)
diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py
index d533d852588ac401288c070ada67cb9fcea2b7eb..207b22dfb6745df5124e329736c5990e9ef2b941 100644
--- a/api/funkwhale_api/music/models.py
+++ b/api/funkwhale_api/music/models.py
@@ -319,9 +319,10 @@ class Track(APIModelMixin):
"mbid": {"musicbrainz_field_name": "id"},
"title": {"musicbrainz_field_name": "title"},
"artist": {
- # we use the artist from the release to avoid #237
- "musicbrainz_field_name": "release-list",
- "converter": get_artist,
+ "musicbrainz_field_name": "artist-credit",
+ "converter": lambda v: Artist.get_or_create_from_api(
+ mbid=v[0]["artist"]["id"]
+ )[0],
},
"album": {"musicbrainz_field_name": "release-list", "converter": import_album},
}
@@ -389,19 +390,37 @@ class Track(APIModelMixin):
tracks = [t for m in data["release"]["medium-list"] for t in m["track-list"]]
track_data = None
for track in tracks:
- if track["recording"]["id"] == mbid:
+ if track["recording"]["id"] == str(mbid):
track_data = track
break
if not track_data:
raise ValueError("No track found matching this ID")
+ track_artist_mbid = None
+ for ac in track_data["recording"]["artist-credit"]:
+ try:
+ ac_mbid = ac["artist"]["id"]
+ except TypeError:
+ # it's probably a string, like "feat."
+ continue
+
+ if ac_mbid == str(album.artist.mbid):
+ continue
+
+ track_artist_mbid = ac_mbid
+ break
+ track_artist_mbid = track_artist_mbid or album.artist.mbid
+ if track_artist_mbid == str(album.artist.mbid):
+ track_artist = album.artist
+ else:
+ track_artist = Artist.get_or_create_from_api(track_artist_mbid)[0]
return cls.objects.update_or_create(
mbid=mbid,
defaults={
"position": int(track["position"]),
"title": track["recording"]["title"],
"album": album,
- "artist": album.artist,
+ "artist": track_artist,
},
)
diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py
index c34970d0b5cbcf29155c7b008c0ba4acd6246f1c..14ea54d51a22283e1c82f230da51ecab862b2c14 100644
--- a/api/funkwhale_api/music/serializers.py
+++ b/api/funkwhale_api/music/serializers.py
@@ -60,8 +60,15 @@ class TrackFileSerializer(serializers.ModelSerializer):
return url
+class ArtistSimpleSerializer(serializers.ModelSerializer):
+ class Meta:
+ model = models.Artist
+ fields = ("id", "mbid", "name", "creation_date")
+
+
class AlbumTrackSerializer(serializers.ModelSerializer):
files = TrackFileSerializer(many=True, read_only=True)
+ artist = ArtistSimpleSerializer(read_only=True)
class Meta:
model = models.Track
@@ -77,12 +84,6 @@ class AlbumTrackSerializer(serializers.ModelSerializer):
)
-class ArtistSimpleSerializer(serializers.ModelSerializer):
- class Meta:
- model = models.Artist
- fields = ("id", "mbid", "name", "creation_date")
-
-
class AlbumSerializer(serializers.ModelSerializer):
tracks = serializers.SerializerMethodField()
artist = ArtistSimpleSerializer(read_only=True)
diff --git a/api/tests/music/test_models.py b/api/tests/music/test_models.py
index df18a09096fc147454f4fbf1905a84ff185213b2..1bd4282fe36b9bde03094a8e6304283ea97e9b59 100644
--- a/api/tests/music/test_models.py
+++ b/api/tests/music/test_models.py
@@ -44,6 +44,7 @@ def test_import_album_stores_release_group(factories):
def test_import_track_from_release(factories, mocker):
album = factories["music.Album"](mbid="430347cb-0879-3113-9fde-c75b658c298e")
+ artist = factories["music.Artist"](mbid="a5211c65-2465-406b-93ec-213588869dc1")
album_data = {
"release": {
"id": album.mbid,
@@ -64,6 +65,9 @@ def test_import_track_from_release(factories, mocker):
"id": "2109e376-132b-40ad-b993-2bb6812e19d4",
"title": "Teen Age Riot",
"length": "417973",
+ "artist-credit": [
+ {"artist": {"id": artist.mbid, "name": artist.name}}
+ ],
},
"track_or_recording_length": "417973",
}
@@ -84,10 +88,66 @@ def test_import_track_from_release(factories, mocker):
assert track.title == track_data["recording"]["title"]
assert track.mbid == track_data["recording"]["id"]
assert track.album == album
- assert track.artist == album.artist
+ assert track.artist == artist
assert track.position == int(track_data["position"])
+def test_import_track_with_different_artist_than_release(factories, mocker):
+ album = factories["music.Album"](mbid="430347cb-0879-3113-9fde-c75b658c298e")
+ recording_data = {
+ "recording": {
+ "id": "94ab07eb-bdf3-4155-b471-ba1dc85108bf",
+ "title": "Flaming Red Hair",
+ "length": "159000",
+ "artist-credit": [
+ {
+ "artist": {
+ "id": "a5211c65-2465-406b-93ec-213588869dc1",
+ "name": "Plan 9",
+ "sort-name": "Plan 9",
+ "disambiguation": "New Zealand group",
+ }
+ }
+ ],
+ "release-list": [
+ {
+ "id": album.mbid,
+ "title": "The Lord of the Rings: The Fellowship of the Ring - The Complete Recordings",
+ "status": "Official",
+ "quality": "normal",
+ "text-representation": {"language": "eng", "script": "Latn"},
+ "artist-credit": [
+ {
+ "artist": {
+ "id": "9b58672a-e68e-4972-956e-a8985a165a1f",
+ "name": "Howard Shore",
+ "sort-name": "Shore, Howard",
+ }
+ }
+ ],
+ "date": "2005-12-13",
+ "country": "US",
+ "release-event-count": 1,
+ "barcode": "093624945420",
+ "artist-credit-phrase": "Howard Shore",
+ }
+ ],
+ "release-count": 3,
+ "artist-credit-phrase": "Plan 9",
+ }
+ }
+ artist = factories["music.Artist"](mbid="a5211c65-2465-406b-93ec-213588869dc1")
+ mocker.patch(
+ "funkwhale_api.musicbrainz.api.recordings.get", return_value=recording_data
+ )
+
+ track = models.Track.get_or_create_from_api(recording_data["recording"]["id"])[0]
+ assert track.title == recording_data["recording"]["title"]
+ assert track.mbid == recording_data["recording"]["id"]
+ assert track.album == album
+ assert track.artist == artist
+
+
def test_import_job_is_bound_to_track_file(factories, mocker):
track = factories["music.Track"]()
job = factories["music.ImportJob"](mbid=track.mbid)
diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py
index 51ca96b5d713af1c1e39f2c1872a9bbea9809216..0d7400dfc7c70092a406574ea23754c531313c65 100644
--- a/api/tests/music/test_serializers.py
+++ b/api/tests/music/test_serializers.py
@@ -43,7 +43,7 @@ def test_album_track_serializer(factories, to_api_date):
expected = {
"id": track.id,
- "artist": track.artist.id,
+ "artist": serializers.ArtistSimpleSerializer(track.artist).data,
"album": track.album.id,
"mbid": str(track.mbid),
"title": track.title,
diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py
index 43e596ff7c14a27b6b2db69f1ab7c7206632355c..f63e69b635cad1b4534a8f7c1a3aabc6b636fd15 100644
--- a/api/tests/test_import_audio_file.py
+++ b/api/tests/test_import_audio_file.py
@@ -37,6 +37,7 @@ def test_can_create_track_from_file_metadata_no_mbid(db, mocker):
def test_can_create_track_from_file_metadata_mbid(factories, mocker):
album = factories["music.Album"]()
+ artist = factories["music.Artist"]()
mocker.patch(
"funkwhale_api.music.models.Album.get_or_create_from_api",
return_value=(album, True),
@@ -55,6 +56,9 @@ def test_can_create_track_from_file_metadata_mbid(factories, mocker):
"recording": {
"id": "2109e376-132b-40ad-b993-2bb6812e19d4",
"title": "Teen Age Riot",
+ "artist-credit": [
+ {"artist": {"id": artist.mbid, "name": artist.name}}
+ ],
},
}
],
@@ -79,7 +83,7 @@ def test_can_create_track_from_file_metadata_mbid(factories, mocker):
assert track.mbid == track_data["recording"]["id"]
assert track.position == 4
assert track.album == album
- assert track.artist == album.artist
+ assert track.artist == artist
def test_management_command_requires_a_valid_username(factories, mocker):
diff --git a/changes/changelog.d/237.enhancement b/changes/changelog.d/237.enhancement
new file mode 100644
index 0000000000000000000000000000000000000000..1b5eed8f2c83215b936e89d2a144e25fba8c54ed
--- /dev/null
+++ b/changes/changelog.d/237.enhancement
@@ -0,0 +1,11 @@
+Store track artist and album artist separately (#237)
+
+Better handling of tracks with a different artist than the album artist
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Some tracks involve a different artist than the album artist (e.g. a featuring)
+and Funkwhale has been known to do weird things when importing such tracks, resulting
+in albums that contained a single track, for instance.
+
+The situation should be improved with this release, as Funkwhale is now able to
+store separately the track and album artist, and display it properly in the interface.
diff --git a/front/src/audio/backend.js b/front/src/audio/backend.js
index 5a82719a3a0d403ce6ee264eed47e2f5926068f0..c371566758631cf2a6f8c58547d01a47c318e3dd 100644
--- a/front/src/audio/backend.js
+++ b/front/src/audio/backend.js
@@ -2,7 +2,6 @@ var Album = {
clean (album) {
// we manually rebind the album and artist to each child track
album.tracks = album.tracks.map((track) => {
- track.artist = album.artist
track.album = album
return track
})
diff --git a/front/src/components/audio/track/Row.vue b/front/src/components/audio/track/Row.vue
index bd3ceb2aaa869350fa013ca36c709f16cc97cdbc..5870ac799cf2808baf03cf029a8ff166ed767ac5 100644
--- a/front/src/components/audio/track/Row.vue
+++ b/front/src/components/audio/track/Row.vue
@@ -16,9 +16,18 @@
</router-link>
</td>
<td colspan="6">
- <router-link class="artist discrete link" :to="{name: 'library.artists.detail', params: {id: track.artist.id }}">
+ <router-link v-if="track.artist.id === albumArtist.id" class="artist discrete link" :to="{name: 'library.artists.detail', params: {id: track.artist.id }}">
{{ track.artist.name }}
</router-link>
+ <template v-else>
+ <router-link class="artist discrete link" :to="{name: 'library.artists.detail', params: {id: albumArtist.id }}">
+ {{ albumArtist.name }}
+ </router-link>
+ /
+ <router-link class="artist discrete link" :to="{name: 'library.artists.detail', params: {id: track.artist.id }}">
+ {{ track.artist.name }}
+ </router-link>
+ </template>
</td>
<td colspan="6">
<router-link class="album discrete link" :to="{name: 'library.albums.detail', params: {id: track.album.id }}">
@@ -35,8 +44,6 @@
</template>
<script>
-import backend from '@/audio/backend'
-
import TrackFavoriteIcon from '@/components/favorites/TrackFavoriteIcon'
import TrackPlaylistIcon from '@/components/playlists/TrackPlaylistIcon'
import PlayButton from '@/components/audio/PlayButton'
@@ -44,6 +51,7 @@ import PlayButton from '@/components/audio/PlayButton'
export default {
props: {
track: {type: Object, required: true},
+ artist: {type: Object, required: false},
displayPosition: {type: Boolean, default: false}
},
components: {
@@ -51,9 +59,13 @@ export default {
TrackPlaylistIcon,
PlayButton
},
- data () {
- return {
- backend: backend
+ computed: {
+ albumArtist () {
+ if (this.artist) {
+ return this.artist
+ } else {
+ return this.track.album.artist
+ }
}
}
}
diff --git a/front/src/components/audio/track/Table.vue b/front/src/components/audio/track/Table.vue
index 40d33df732b26adf9ba76181092ea57d6970b453..03e9398f8a6b9ff9518bd9874db331bc5fabee6c 100644
--- a/front/src/components/audio/track/Table.vue
+++ b/front/src/components/audio/track/Table.vue
@@ -14,6 +14,7 @@
<track-row
:display-position="displayPosition"
:track="track"
+ :artist="artist"
:key="index + '-' + track.id"
v-for="(track, index) in tracks"></track-row>
</tbody>
@@ -63,6 +64,7 @@ import Modal from '@/components/semantic/Modal'
export default {
props: {
tracks: {type: Array, required: true},
+ artist: {type: Object, required: false},
displayPosition: {type: Boolean, default: false}
},
components: {
diff --git a/front/src/components/library/Album.vue b/front/src/components/library/Album.vue
index ced69edebd2f3abf69d79b8d5489db30fc33313c..698f07ae28abcf4a611327d41fedfa21e3295d91 100644
--- a/front/src/components/library/Album.vue
+++ b/front/src/components/library/Album.vue
@@ -43,7 +43,7 @@
<h2>
<translate>Tracks</translate>
</h2>
- <track-table v-if="album" :display-position="true" :tracks="album.tracks"></track-table>
+ <track-table v-if="album" :artist="album.artist" :display-position="true" :tracks="album.tracks"></track-table>
</div>
</template>
</div>
diff --git a/front/src/components/library/Artist.vue b/front/src/components/library/Artist.vue
index 89eb7459c0f85fd95aeb32ea8c1ecbfa1e77c286..6e3ad638d01a0dcdb6685c04ac036ba09d79d61b 100644
--- a/front/src/components/library/Artist.vue
+++ b/front/src/components/library/Artist.vue
@@ -15,7 +15,7 @@
tag="div"
translate-plural="%{ count } tracks in %{ albumsCount } albums"
:translate-n="totalTracks"
- :translate-params="{count: totalTracks, albumsCount: albums.length}">
+ :translate-params="{count: totalTracks, albumsCount: totalAlbums}">
%{ count } track in %{ albumsCount } albums
</translate>
</div>
@@ -40,7 +40,7 @@
<div v-if="isLoadingAlbums" class="ui vertical stripe segment">
<div :class="['ui', 'centered', 'active', 'inline', 'loader']"></div>
</div>
- <div v-else-if="albums" class="ui vertical stripe segment">
+ <div v-else-if="albums && albums.length > 0" class="ui vertical stripe segment">
<h2>
<translate>Albums by this artist</translate>
</h2>
@@ -50,33 +50,41 @@
</div>
</div>
</div>
+ <div v-if="tracks.length > 0" class="ui vertical stripe segment">
+ <h2>
+ <translate>Tracks by this artist</translate>
+ </h2>
+ <track-table :display-position="true" :tracks="tracks"></track-table>
+ </div>
</template>
</div>
</template>
<script>
+import _ from 'lodash'
import axios from 'axios'
import logger from '@/logging'
import backend from '@/audio/backend'
import AlbumCard from '@/components/audio/album/Card'
import RadioButton from '@/components/radios/Button'
import PlayButton from '@/components/audio/PlayButton'
-
-const FETCH_URL = 'artists/'
+import TrackTable from '@/components/audio/track/Table'
export default {
props: ['id'],
components: {
AlbumCard,
RadioButton,
- PlayButton
+ PlayButton,
+ TrackTable
},
data () {
return {
isLoading: true,
isLoadingAlbums: true,
artist: null,
- albums: null
+ albums: null,
+ tracks: []
}
},
created () {
@@ -86,14 +94,17 @@ export default {
fetchData () {
var self = this
this.isLoading = true
- let url = FETCH_URL + this.id + '/'
logger.default.debug('Fetching artist "' + this.id + '"')
- axios.get(url).then((response) => {
+ axios.get('tracks/', {params: {artist: this.id}}).then((response) => {
+ self.tracks = response.data.results
+ })
+ axios.get('artists/' + this.id + '/').then((response) => {
self.artist = response.data
self.isLoading = false
self.isLoadingAlbums = true
axios.get('albums/', {params: {artist: this.id, ordering: '-release_date'}}).then((response) => {
- self.albums = JSON.parse(JSON.stringify(response.data.results)).map((album) => {
+ let parsed = JSON.parse(JSON.stringify(response.data.results))
+ self.albums = parsed.map((album) => {
return backend.Album.clean(album)
})
@@ -108,12 +119,21 @@ export default {
title: this.$gettext('Artist')
}
},
+ totalAlbums () {
+ let trackAlbums = _.uniqBy(this.tracks, (t) => {
+ return t.album.id
+ })
+ return this.albums.length + trackAlbums.length
+ },
totalTracks () {
+ if (this.albums.length === 0) {
+ return 0 + this.tracks.length
+ }
return this.albums.map((album) => {
return album.tracks.length
}).reduce((a, b) => {
return a + b
- })
+ }) + this.tracks.length
},
wikipediaUrl () {
return 'https://en.wikipedia.org/w/index.php?search=' + this.artist.name
diff --git a/front/src/components/library/Home.vue b/front/src/components/library/Home.vue
index 5610198a466478412be47e5d42bed4ebbf418f9d..ce6627e18bc66d50950a53b007c84d442c13d9ff 100644
--- a/front/src/components/library/Home.vue
+++ b/front/src/components/library/Home.vue
@@ -72,7 +72,8 @@ export default {
var self = this
this.isLoadingArtists = true
let params = {
- ordering: '-creation_date'
+ ordering: '-creation_date',
+ listenable: true
}
let url = ARTISTS_URL
logger.default.time('Loading latest artists')
diff --git a/front/src/components/requests/Form.vue b/front/src/components/requests/Form.vue
index b6752a6353989acdc8bad0feeb4fbb888204d5b1..667bd5f5acb7b7c6e0c0b00a3e5905d6f127f53c 100644
--- a/front/src/components/requests/Form.vue
+++ b/front/src/components/requests/Form.vue
@@ -13,7 +13,7 @@
</div>
<div class="field">
<label><translate>Comment</translate></label>
- <textarea v-model="currentComment" rows="3" :placeholder="comentPlaceholder" maxlength="2000"></textarea>
+ <textarea v-model="currentComment" rows="3" :placeholder="labels.commentPlaceholder" maxlength="2000"></textarea>
</div>
<button class="ui submit button" type="submit"><translate>Submit</translate></button>
</form>