diff --git a/admin/schema_changes/18.sql b/admin/schema_changes/18.sql new file mode 100644 index 000000000..15a154731 --- /dev/null +++ b/admin/schema_changes/18.sql @@ -0,0 +1 @@ +ALTER TYPE entity_types ADD VALUE 'recording' AFTER 'label'; diff --git a/admin/sql/create_types.sql b/admin/sql/create_types.sql index 27a364f85..5c2dde6c0 100644 --- a/admin/sql/create_types.sql +++ b/admin/sql/create_types.sql @@ -11,5 +11,6 @@ CREATE TYPE entity_types AS ENUM ( 'place', 'work', 'artist', - 'label' + 'label', + 'recording' ); diff --git a/critiquebrainz/db/review.py b/critiquebrainz/db/review.py index d04a7a7b0..4071ac688 100644 --- a/critiquebrainz/db/review.py +++ b/critiquebrainz/db/review.py @@ -24,6 +24,7 @@ "work", "artist", "label", + "recording" ] supported_languages = [] diff --git a/critiquebrainz/frontend/__init__.py b/critiquebrainz/frontend/__init__.py index 8c73bb905..23d571819 100644 --- a/critiquebrainz/frontend/__init__.py +++ b/critiquebrainz/frontend/__init__.py @@ -137,6 +137,7 @@ def create_app(debug=None, config_path=None): from critiquebrainz.frontend.views.release_group import release_group_bp from critiquebrainz.frontend.views.release import release_bp from critiquebrainz.frontend.views.work import work_bp + from critiquebrainz.frontend.views.recording import recording_bp from critiquebrainz.frontend.views.event import event_bp from critiquebrainz.frontend.views.mapping import mapping_bp from critiquebrainz.frontend.views.user import user_bp @@ -160,6 +161,7 @@ def create_app(debug=None, config_path=None): app.register_blueprint(release_group_bp, url_prefix='/release-group') app.register_blueprint(release_bp, url_prefix='/release') app.register_blueprint(work_bp, url_prefix='/work') + app.register_blueprint(recording_bp, url_prefix='/recording') app.register_blueprint(event_bp, url_prefix='/event') app.register_blueprint(place_bp, url_prefix='/place') app.register_blueprint(mapping_bp, url_prefix='/mapping') diff --git a/critiquebrainz/frontend/external/musicbrainz.py b/critiquebrainz/frontend/external/musicbrainz.py index a3c625cd4..ec8222aa9 100644 --- a/critiquebrainz/frontend/external/musicbrainz.py +++ b/critiquebrainz/frontend/external/musicbrainz.py @@ -55,3 +55,9 @@ def search_labels(query='', limit=None, offset=None): """Search for labels.""" api_resp = musicbrainzngs.search_labels(query=query, limit=limit, offset=offset) return api_resp.get('label-count'), api_resp.get('label-list') + + +def search_recordings(query='', limit=None, offset=None): + """Search for recordings.""" + api_resp = musicbrainzngs.search_recordings(query=query, limit=limit, offset=offset) + return api_resp.get('recording-count'), api_resp.get('recording-list') diff --git a/critiquebrainz/frontend/external/musicbrainz_db/artist.py b/critiquebrainz/frontend/external/musicbrainz_db/artist.py index 5443a6973..304e59bd5 100644 --- a/critiquebrainz/frontend/external/musicbrainz_db/artist.py +++ b/critiquebrainz/frontend/external/musicbrainz_db/artist.py @@ -16,10 +16,10 @@ def get_artist_by_id(mbid): key = cache.gen_key('artist', mbid) artist = cache.get(key) if not artist: - artist = db.fetch_multiple_artists( - [mbid], + artist = db.get_artist_by_id( + mbid, includes=['artist-rels', 'url-rels'], unknown_entities_for_missing=True, - ).get(mbid) + ) cache.set(key=key, val=artist, time=DEFAULT_CACHE_EXPIRATION) return artist_rel.process(artist) diff --git a/critiquebrainz/frontend/external/musicbrainz_db/entities.py b/critiquebrainz/frontend/external/musicbrainz_db/entities.py index 8453c74f9..b5ea0d974 100644 --- a/critiquebrainz/frontend/external/musicbrainz_db/entities.py +++ b/critiquebrainz/frontend/external/musicbrainz_db/entities.py @@ -4,6 +4,7 @@ from brainzutils.musicbrainz_db.place import fetch_multiple_places from brainzutils.musicbrainz_db.release_group import fetch_multiple_release_groups from brainzutils.musicbrainz_db.work import fetch_multiple_works +from brainzutils.musicbrainz_db.recording import fetch_multiple_recordings from critiquebrainz.frontend.external.musicbrainz_db.artist import get_artist_by_id from critiquebrainz.frontend.external.musicbrainz_db.event import get_event_by_id @@ -11,13 +12,14 @@ from critiquebrainz.frontend.external.musicbrainz_db.place import get_place_by_id from critiquebrainz.frontend.external.musicbrainz_db.release_group import get_release_group_by_id from critiquebrainz.frontend.external.musicbrainz_db.work import get_work_by_id +from critiquebrainz.frontend.external.musicbrainz_db.recording import get_recording_by_id def get_multiple_entities(entities): """Fetch multiple entities using their MBIDs. Args: - entites: List of tuples containing the entity ID and the entity type. + entities: List of tuples containing the entity ID and the entity type. Returns: Dictionary containing the basic information related to the entities. @@ -29,12 +31,13 @@ def get_multiple_entities(entities): coordinates of the places is also included. """ entities_info = {} - release_group_mbids = [entity[0] for entity in filter(lambda entity: entity[1] == 'release_group', entities)] - artist_mbids = [entity[0] for entity in filter(lambda entity: entity[1] == 'artist', entities)] - label_mbids = [entity[0] for entity in filter(lambda entity: entity[1] == 'label', entities)] - place_mbids = [entity[0] for entity in filter(lambda entity: entity[1] == 'place', entities)] - event_mbids = [entity[0] for entity in filter(lambda entity: entity[1] == 'event', entities)] - work_mbids = [entity[0] for entity in filter(lambda entity: entity[1] == 'work', entities)] + release_group_mbids = [entity[0] for entity in entities if entity[1] == 'release_group'] + artist_mbids = [entity[0] for entity in entities if entity[1] == 'artist'] + label_mbids = [entity[0] for entity in entities if entity[1] == 'label'] + recording_mbids = [entity[0] for entity in entities if entity[1] == 'recording'] + place_mbids = [entity[0] for entity in entities if entity[1] == 'place'] + event_mbids = [entity[0] for entity in entities if entity[1] == 'event'] + work_mbids = [entity[0] for entity in entities if entity[1] == 'work'] entities_info.update(fetch_multiple_release_groups( release_group_mbids, includes=['artists'], @@ -57,6 +60,9 @@ def get_multiple_entities(entities): entities_info.update(fetch_multiple_works( work_mbids, )) + entities_info.update(fetch_multiple_recordings( + recording_mbids, + )) return entities_info @@ -74,4 +80,6 @@ def get_entity_by_id(id, type='release_group'): entity = get_event_by_id(str(id)) elif type == 'work': entity = get_work_by_id(str(id)) + elif type == 'recording': + entity = get_recording_by_id(str(id)) return entity diff --git a/critiquebrainz/frontend/external/musicbrainz_db/event.py b/critiquebrainz/frontend/external/musicbrainz_db/event.py index 114c018e9..025c16395 100644 --- a/critiquebrainz/frontend/external/musicbrainz_db/event.py +++ b/critiquebrainz/frontend/external/musicbrainz_db/event.py @@ -15,10 +15,10 @@ def get_event_by_id(mbid): key = cache.gen_key('event', mbid) event = cache.get(key) if not event: - event = db.fetch_multiple_events( - [mbid], + event = db.get_event_by_id( + mbid, includes=['artist-rels', 'place-rels', 'series-rels', 'url-rels', 'release-group-rels'], unknown_entities_for_missing=True, - ).get(mbid) + ) cache.set(key=key, val=event, time=DEFAULT_CACHE_EXPIRATION) return event diff --git a/critiquebrainz/frontend/external/musicbrainz_db/label.py b/critiquebrainz/frontend/external/musicbrainz_db/label.py index 0b34cd7c9..a9bf4c26b 100644 --- a/critiquebrainz/frontend/external/musicbrainz_db/label.py +++ b/critiquebrainz/frontend/external/musicbrainz_db/label.py @@ -16,10 +16,10 @@ def get_label_by_id(mbid): key = cache.gen_key('label', mbid) label = cache.get(key) if not label: - label = db.fetch_multiple_labels( - [mbid], + label = db.get_label_by_id( + mbid, includes=['artist-rels', 'url-rels'], unknown_entities_for_missing=True - ).get(mbid) + ) cache.set(key=key, val=label, time=DEFAULT_CACHE_EXPIRATION) return label_rel.process(label) diff --git a/critiquebrainz/frontend/external/musicbrainz_db/place.py b/critiquebrainz/frontend/external/musicbrainz_db/place.py index 8a86fa0b1..f3c0118be 100644 --- a/critiquebrainz/frontend/external/musicbrainz_db/place.py +++ b/critiquebrainz/frontend/external/musicbrainz_db/place.py @@ -16,10 +16,10 @@ def get_place_by_id(mbid): key = cache.gen_key('place', mbid) place = cache.get(key) if not place: - place = db.fetch_multiple_places( - [mbid], + place = db.get_place_by_id( + mbid, includes=['artist-rels', 'place-rels', 'release-group-rels', 'url-rels'], unknown_entities_for_missing=True, - ).get(mbid) + ) cache.set(key=key, val=place, time=DEFAULT_CACHE_EXPIRATION) return place_rel.process(place) diff --git a/critiquebrainz/frontend/external/musicbrainz_db/recording.py b/critiquebrainz/frontend/external/musicbrainz_db/recording.py new file mode 100644 index 000000000..136929520 --- /dev/null +++ b/critiquebrainz/frontend/external/musicbrainz_db/recording.py @@ -0,0 +1,23 @@ +from brainzutils import cache +from brainzutils.musicbrainz_db import recording as db + +from critiquebrainz.frontend.external.musicbrainz_db import DEFAULT_CACHE_EXPIRATION + + +def get_recording_by_id(mbid): + """Get recording with MusicBrainz ID. + + Args: + mbid (uuid): MBID(gid) of the recording. + Returns: + Dictionary containing the recording information + """ + key = cache.gen_key('recording', mbid) + recording = cache.get(key) + if not recording: + recording = db.get_recording_by_mbid( + mbid, + includes=['artists', 'work-rels', 'url-rels'], + ) + cache.set(key=key, val=recording, time=DEFAULT_CACHE_EXPIRATION) + return recording diff --git a/critiquebrainz/frontend/external/musicbrainz_db/release.py b/critiquebrainz/frontend/external/musicbrainz_db/release.py index ffa1029d9..b09725bdb 100644 --- a/critiquebrainz/frontend/external/musicbrainz_db/release.py +++ b/critiquebrainz/frontend/external/musicbrainz_db/release.py @@ -15,10 +15,10 @@ def get_release_by_id(mbid): key = cache.gen_key('release', mbid) release = cache.get(key) if not release: - release = db.fetch_multiple_releases( - [mbid], + release = db.get_release_by_id( + mbid, includes=['media', 'release-groups'], unknown_entities_for_missing=True, - ).get(mbid) + ) cache.set(key=key, val=release, time=DEFAULT_CACHE_EXPIRATION) return release diff --git a/critiquebrainz/frontend/external/musicbrainz_db/release_group.py b/critiquebrainz/frontend/external/musicbrainz_db/release_group.py index 1da0d473c..f5219df74 100644 --- a/critiquebrainz/frontend/external/musicbrainz_db/release_group.py +++ b/critiquebrainz/frontend/external/musicbrainz_db/release_group.py @@ -9,11 +9,11 @@ def get_release_group_by_id(mbid): key = cache.gen_key('release-group', mbid) release_group = cache.get(key) if not release_group: - release_group = db.fetch_multiple_release_groups( - [mbid], + release_group = db.get_release_group_by_id( + mbid, includes=['artists', 'releases', 'release-group-rels', 'url-rels', 'tags'], unknown_entities_for_missing=True, - )[mbid] + ) cache.set(key=key, val=release_group, time=DEFAULT_CACHE_EXPIRATION) return release_group_rel.process(release_group) diff --git a/critiquebrainz/frontend/external/musicbrainz_db/test/test_cache.py b/critiquebrainz/frontend/external/musicbrainz_db/test/test_cache.py index 062e66425..6abcd1a16 100644 --- a/critiquebrainz/frontend/external/musicbrainz_db/test/test_cache.py +++ b/critiquebrainz/frontend/external/musicbrainz_db/test/test_cache.py @@ -6,6 +6,7 @@ from critiquebrainz.frontend.external.musicbrainz_db.event import get_event_by_id from critiquebrainz.frontend.external.musicbrainz_db.label import get_label_by_id from critiquebrainz.frontend.external.musicbrainz_db.place import get_place_by_id +from critiquebrainz.frontend.external.musicbrainz_db.recording import get_recording_by_id from critiquebrainz.frontend.external.musicbrainz_db.release import get_release_by_id from critiquebrainz.frontend.external.musicbrainz_db.release_group import get_release_group_by_id from critiquebrainz.frontend.external.musicbrainz_db.work import get_work_by_id @@ -16,7 +17,7 @@ class CacheTestCase(DataTestCase): @mock.patch('brainzutils.cache.get') @mock.patch('brainzutils.cache.set') - @mock.patch('brainzutils.musicbrainz_db.artist.fetch_multiple_artists') + @mock.patch('brainzutils.musicbrainz_db.artist.get_artist_by_id') def test_artist_cache(self, artist_fetch, cache_set, cache_get): mbid = "f59c5520-5f46-4d2c-b2c4-822eabf53419" expected_key = b"artist_f59c5520-5f46-4d2c-b2c4-822eabf53419" @@ -26,14 +27,14 @@ def test_artist_cache(self, artist_fetch, cache_set, cache_get): "sort_name": "Linkin Park", "type": "Group" } - artist_fetch.return_value = {mbid: artist} + artist_fetch.return_value = artist cache_get.return_value = None get_artist_by_id(mbid) # Test that first time data is fetched database is queried cache_get.assert_called_with(expected_key) - artist_fetch.assert_called_with([mbid], includes=['artist-rels', 'url-rels'], unknown_entities_for_missing=True) + artist_fetch.assert_called_with(mbid, includes=['artist-rels', 'url-rels'], unknown_entities_for_missing=True) cache_set.assert_called_with(key=expected_key, val=artist, time=DEFAULT_CACHE_EXPIRATION) cache_get.return_value = artist @@ -48,7 +49,7 @@ def test_artist_cache(self, artist_fetch, cache_set, cache_get): @mock.patch('brainzutils.cache.get') @mock.patch('brainzutils.cache.set') - @mock.patch('brainzutils.musicbrainz_db.event.fetch_multiple_events') + @mock.patch('brainzutils.musicbrainz_db.event.get_event_by_id') def test_event_cache(self, event_fetch, cache_set, cache_get): mbid = "ebe6ce0f-22c0-4fe7-bfd4-7a0397c9fe94" expected_key = b"event_ebe6ce0f-22c0-4fe7-bfd4-7a0397c9fe94" @@ -56,15 +57,15 @@ def test_event_cache(self, event_fetch, cache_set, cache_get): 'id': 'ebe6ce0f-22c0-4fe7-bfd4-7a0397c9fe94', 'name': 'Taubertal-Festival 2004, Day 1', } - event_fetch.return_value = {mbid: event} + event_fetch.return_value = event cache_get.return_value = None get_event_by_id(mbid) # Test that first time data is fetched database is queried cache_get.assert_called_with(expected_key) - event_fetch.assert_called_with([mbid], includes=['artist-rels', 'place-rels', - 'series-rels', 'url-rels', 'release-group-rels'], + event_fetch.assert_called_with(mbid, includes=['artist-rels', 'place-rels', + 'series-rels', 'url-rels', 'release-group-rels'], unknown_entities_for_missing=True) cache_set.assert_called_with(key=expected_key, val=event, time=DEFAULT_CACHE_EXPIRATION) @@ -80,7 +81,7 @@ def test_event_cache(self, event_fetch, cache_set, cache_get): @mock.patch('brainzutils.cache.get') @mock.patch('brainzutils.cache.set') - @mock.patch('brainzutils.musicbrainz_db.label.fetch_multiple_labels') + @mock.patch('brainzutils.musicbrainz_db.label.get_label_by_id') def test_label_cache(self, label_fetch, cache_set, cache_get): mbid = "1aed8c3b-8e1e-46f8-b558-06357ff5f298" expected_key = b"label_1aed8c3b-8e1e-46f8-b558-06357ff5f298" @@ -90,14 +91,14 @@ def test_label_cache(self, label_fetch, cache_set, cache_get): "type": "Imprint", "area": "United States", } - label_fetch.return_value = {mbid: label} + label_fetch.return_value = label cache_get.return_value = None get_label_by_id(mbid) # Test that first time data is fetched database is queried cache_get.assert_called_with(expected_key) - label_fetch.assert_called_with([mbid], includes=['artist-rels', 'url-rels'], unknown_entities_for_missing=True) + label_fetch.assert_called_with(mbid, includes=['artist-rels', 'url-rels'], unknown_entities_for_missing=True) cache_set.assert_called_with(key=expected_key, val=label, time=DEFAULT_CACHE_EXPIRATION) cache_get.return_value = label @@ -112,7 +113,7 @@ def test_label_cache(self, label_fetch, cache_set, cache_get): @mock.patch('brainzutils.cache.get') @mock.patch('brainzutils.cache.set') - @mock.patch('brainzutils.musicbrainz_db.place.fetch_multiple_places') + @mock.patch('brainzutils.musicbrainz_db.place.get_place_by_id') def test_place_cache(self, place_fetch, cache_set, cache_get): mbid = "d71ffe38-5eaf-426b-9a2e-e1f21bc84609" expected_key = b"place_d71ffe38-5eaf-426b-9a2e-e1f21bc84609" @@ -128,15 +129,15 @@ def test_place_cache(self, place_fetch, cache_set, cache_get): "name": "Hämeenlinna", } } - place_fetch.return_value = {mbid: place} + place_fetch.return_value = place cache_get.return_value = None get_place_by_id(mbid) # Test that first time data is fetched database is queried cache_get.assert_called_with(expected_key) - place_fetch.assert_called_with([mbid], includes=['artist-rels', 'place-rels', - 'release-group-rels', 'url-rels'], + place_fetch.assert_called_with(mbid, includes=['artist-rels', 'place-rels', + 'release-group-rels', 'url-rels'], unknown_entities_for_missing=True) cache_set.assert_called_with(key=expected_key, val=place, time=DEFAULT_CACHE_EXPIRATION) @@ -152,7 +153,50 @@ def test_place_cache(self, place_fetch, cache_set, cache_get): @mock.patch('brainzutils.cache.get') @mock.patch('brainzutils.cache.set') - @mock.patch('brainzutils.musicbrainz_db.release.fetch_multiple_releases') + @mock.patch('brainzutils.musicbrainz_db.recording.get_recording_by_mbid') + def test_recording_cache(self, recording_fetch, cache_set, cache_get): + mbid = "442ddce2-ffa1-4865-81d2-b42c40fec7c5" + expected_key = b"recording_442ddce2-ffa1-4865-81d2-b42c40fec7c5" + recording = { + 'id': '442ddce2-ffa1-4865-81d2-b42c40fec7c5', + 'name': 'Dream Come True', + 'length': 229.0, + 'artists': [ + { + 'id': '164f0d73-1234-4e2c-8743-d77bf2191051', + 'name': 'Kanye West', + 'join_phrase': ' feat. ' + }, + { + 'id': '75a72702-a5ef-4513-bca5-c5b944903546', + 'name': 'John Legend' + } + ], + 'artist-credit-phrase': 'Kanye West feat. John Legend' + } + recording_fetch.return_value = recording + + cache_get.return_value = None + get_recording_by_id(mbid) + + # Test that first time data is fetched database is queried + cache_get.assert_called_with(expected_key) + recording_fetch.assert_called_with(mbid, includes=['artists', 'work-rels', 'url-rels']) + cache_set.assert_called_with(key=expected_key, val=recording, time=DEFAULT_CACHE_EXPIRATION) + + cache_get.return_value = recording + cache_set.reset_mock() + recording_fetch.reset_mock() + get_recording_by_id(mbid) + + # Test that second time data is fetched from cache + cache_get.assert_called_with(expected_key) + recording_fetch.assert_not_called() + cache_set.assert_not_called() + + @mock.patch('brainzutils.cache.get') + @mock.patch('brainzutils.cache.set') + @mock.patch('brainzutils.musicbrainz_db.release.get_release_by_id') def test_release_cache(self, release_fetch, cache_set, cache_get): mbid = "16bee711-d7ce-48b0-adf4-51f124bcc0df" expected_key = b"release_16bee711-d7ce-48b0-adf4-51f124bcc0df" @@ -171,14 +215,14 @@ def test_release_cache(self, release_fetch, cache_set, cache_get): }] }] } - release_fetch.return_value = {mbid: release} + release_fetch.return_value = release cache_get.return_value = None get_release_by_id(mbid) # Test that first time data is fetched database is queried cache_get.assert_called_with(expected_key) - release_fetch.assert_called_with([mbid], includes=['media', 'release-groups'], unknown_entities_for_missing=True) + release_fetch.assert_called_with(mbid, includes=['media', 'release-groups'], unknown_entities_for_missing=True) cache_set.assert_called_with(key=expected_key, val=release, time=DEFAULT_CACHE_EXPIRATION) cache_get.return_value = release @@ -193,7 +237,7 @@ def test_release_cache(self, release_fetch, cache_set, cache_get): @mock.patch('brainzutils.cache.get') @mock.patch('brainzutils.cache.set') - @mock.patch('brainzutils.musicbrainz_db.release_group.fetch_multiple_release_groups') + @mock.patch('brainzutils.musicbrainz_db.release_group.get_release_group_by_id') def test_release_group_cache(self, release_group_fetch, cache_set, cache_get): mbid = "7c1014eb-454c-3867-8854-3c95d265f8de" expected_key = b"release-group_7c1014eb-454c-3867-8854-3c95d265f8de" @@ -211,15 +255,15 @@ def test_release_group_cache(self, release_group_fetch, cache_set, cache_get): 'join_phrase': '/', }] } - release_group_fetch.return_value = {mbid: release_group} + release_group_fetch.return_value = release_group cache_get.return_value = None get_release_group_by_id(mbid) # Test that first time data is fetched database is queried cache_get.assert_called_with(expected_key) - release_group_fetch.assert_called_with([mbid], includes=['artists', 'releases', - 'release-group-rels', 'url-rels', 'tags'], + release_group_fetch.assert_called_with(mbid, + includes=['artists', 'releases', 'release-group-rels', 'url-rels', 'tags'], unknown_entities_for_missing=True) cache_set.assert_called_with(key=expected_key, val=release_group, time=DEFAULT_CACHE_EXPIRATION) @@ -235,7 +279,7 @@ def test_release_group_cache(self, release_group_fetch, cache_set, cache_get): @mock.patch('brainzutils.cache.get') @mock.patch('brainzutils.cache.set') - @mock.patch('brainzutils.musicbrainz_db.work.fetch_multiple_works') + @mock.patch('brainzutils.musicbrainz_db.work.get_work_by_id') def test_work_cache(self, work_fetch, cache_set, cache_get): mbid = "54ce5e07-2aca-4578-83d8-5a41a7b2f434" expected_key = b"work_54ce5e07-2aca-4578-83d8-5a41a7b2f434" @@ -244,14 +288,14 @@ def test_work_cache(self, work_fetch, cache_set, cache_get): "name": "a lot", "type": "Song", } - work_fetch.return_value = {mbid: work} + work_fetch.return_value = work cache_get.return_value = None get_work_by_id(mbid) # Test that first time data is fetched database is queried cache_get.assert_called_with(expected_key) - work_fetch.assert_called_with([mbid], includes=['artist-rels', 'recording-rels']) + work_fetch.assert_called_with(mbid, includes=['artist-rels', 'recording-rels']) cache_set.assert_called_with(key=expected_key, val=work, time=DEFAULT_CACHE_EXPIRATION) cache_get.return_value = work diff --git a/critiquebrainz/frontend/external/musicbrainz_db/work.py b/critiquebrainz/frontend/external/musicbrainz_db/work.py index bc9238a88..b631826b4 100644 --- a/critiquebrainz/frontend/external/musicbrainz_db/work.py +++ b/critiquebrainz/frontend/external/musicbrainz_db/work.py @@ -15,9 +15,9 @@ def get_work_by_id(mbid): key = cache.gen_key('work', mbid) work = cache.get(key) if not work: - work = db.fetch_multiple_works( - [mbid], + work = db.get_work_by_id( + mbid, includes=['artist-rels', 'recording-rels'], - ).get(mbid) + ) cache.set(key=key, val=work, time=DEFAULT_CACHE_EXPIRATION) return work diff --git a/critiquebrainz/frontend/static/styles/main.less b/critiquebrainz/frontend/static/styles/main.less index 9c5477637..79aaca11d 100644 --- a/critiquebrainz/frontend/static/styles/main.less +++ b/critiquebrainz/frontend/static/styles/main.less @@ -11,6 +11,7 @@ // Entity colors @rg-color: @blue; @artist-color: @blue; +@recording-color: @blue; @event-color: @green; @place-color: @yellow; @work-color: @blue; @@ -136,6 +137,9 @@ ul.sharing { &.artist { background-color: fade(@artist-color, 70%); } + &.recording { + background-color: fade(@recording-color, 70%); + } &.work { background-color: fade(@work-color, 70%); } @@ -510,6 +514,9 @@ a#edit-review { margin-top: 20px; } &.artist { background-color: fade(@artist-color, 70%); } + &.recording { + background-color: fade(@recording-color, 70%); + } &.work { background-color: fade(@work-color, 70%); } diff --git a/critiquebrainz/frontend/templates/entity_review.html b/critiquebrainz/frontend/templates/entity_review.html index c41108387..09ca1ae26 100644 --- a/critiquebrainz/frontend/templates/entity_review.html +++ b/critiquebrainz/frontend/templates/entity_review.html @@ -1,17 +1,7 @@ {% if review.entity_type == 'release_group' %} - {{ _('%(album)s by %(artist)s', - album = ''|safe + entity.title | default(_('[Unknown release group]')) + ''|safe, - artist = entity['artist-credit-phrase'] | default(_('[Unknown artist]'))) }} - {% elif review.entity_type == 'artist' %} - {{ _('%(artist)s', artist = ''|safe + entity.name | default(_('[Unknown artist]')) + ''|safe) }} - {% elif review.entity_type == 'label' %} - {{ _('%(label)s', label = ''|safe + entity.name | default(_('[Unknown label]')) + ''|safe) }} - {% elif review.entity_type == 'event' %} - {{ _('%(event)s', event = ''|safe + entity.name | default(_('[Unknown event]')) + ''|safe) }} - {% elif review.entity_type == 'place' %} - {{ _('%(place)s', place = ''|safe + entity.name | default(_('[Unknown place]')) + ''|safe) }} - {% elif review.entity_type == 'work' %} - {{ _('%(work)s', work = ''|safe + entity.name | default(_('[Unknown work]')) + ''|safe) }} + {{ entity.title }} {{ _('by') }} {{ entity['artist-credit-phrase'] | default(_('[Unknown artist]')) }} + {% else %} + {{ entity.name }} {% endif %} diff --git a/critiquebrainz/frontend/templates/macros.html b/critiquebrainz/frontend/templates/macros.html index c42c4c8f2..fbcf535b7 100644 --- a/critiquebrainz/frontend/templates/macros.html +++ b/critiquebrainz/frontend/templates/macros.html @@ -67,7 +67,14 @@ {{ _('Label') }} {% endif %} - {% else %} {# release-group #} + {% elif entity_type == 'recording' %} + + {% if overlay_type %} + + {{ _('Recording') }} + + {% endif %} + {% elif entity_type == 'release_group' %} {% if overlay_type %} @@ -197,3 +204,19 @@

{{ _('Rate this {}:'.format(entity_type_readable)) {{ _('

Sign in to rate!

', link=url_for('login.index', next=url_for('{}.entity'.format(entity_type), id=id))) }} {% endif %} {% endmacro %} + +{% macro display_artist_credit(result) %} + {% if result['artist-relation-list'] %} + {% if result['artist-credit-phrase'] %} + {{ result['artist-credit-phrase'] }} + {% else %} + {{ result['artist-relation-list'][0]['artist']['name'] or '-' }} + {% set count = result['artist-relation-list'] | length %} + {% if count > 1 %} + + {{ count - 1 }} {{ _("more") }} + {% endif %} + {% endif %} + {% else %} + - + {% endif %} +{% endmacro %} diff --git a/critiquebrainz/frontend/templates/navbar.html b/critiquebrainz/frontend/templates/navbar.html index b2e25b632..77e597904 100644 --- a/critiquebrainz/frontend/templates/navbar.html +++ b/critiquebrainz/frontend/templates/navbar.html @@ -100,6 +100,7 @@ + diff --git a/critiquebrainz/frontend/templates/recording/entity.html b/critiquebrainz/frontend/templates/recording/entity.html new file mode 100644 index 000000000..007e63f75 --- /dev/null +++ b/critiquebrainz/frontend/templates/recording/entity.html @@ -0,0 +1,134 @@ +{% extends 'base.html' %} +{% from 'macros.html' import show_avg_rating, entity_rate_form, show_external_reviews with context %} + +{% block title %} + {{ _('Recording "%(name)s" by %(artist)s', name=recording.name, artist=recording['artist-credit-phrase']) }} + - CritiqueBrainz +{% endblock %} + +{% block content %} +
+

+ {{ recording.name }} {{ _('by') }} + {% for credit in recording['artists'] %} + {% if credit.name %} + {{ credit.name }} + {% if credit.join_phrase %}{{ credit.join_phrase }}{% endif %} + {% endif %} + {% endfor %} +

+ + {% if not my_review %} + + {{ _('Write a review') }} + + {% else %} + + {{ _('Edit my review') }} + + {% endif %} +
+ +
+
+ {{ entity_rate_form('recording', 'recording') }} +

+

{{ _('Reviews') }}

+ {% if not reviews %} +

{{ _('No reviews found') }}

+ {% else %} + + + + + + + + + + {% for review in reviews %} + + + + + + {% endfor %} + +
{{ _('Published on') }}{{ _('Votes (+/-)') }}
+ + {{ _('by') }} + + {{ review.published_on | date }}{{ review.votes_positive_count }}/{{ review.votes_negative_count }}
+
    + {% set pages = count//limit %} + {% if count%limit %} + {% set pages = pages+1 %} + {% endif %} + {% if pages>1 %} + {% for p in range(pages) %} + {% set p_offset = p*limit %} +
  • + {{ p+1 }} +
  • + {% endfor %} + {% endif %} +
+ {% endif %} +
+ +
+

{{ _('Recording information') }}

+ {% if avg_rating %} +
+ {{ show_avg_rating(avg_rating.rating, avg_rating.count) }} +
+ {% endif %} + {% if external_reviews %} + {{ _('External reviews') }} + + {% endif %} + {% if recording['external-urls'] %} + {{ _('External links') }} + + {% endif %} + + +
+
+{% endblock %} + +{% block scripts %} + + + +{% endblock %} diff --git a/critiquebrainz/frontend/templates/release_group/entity.html b/critiquebrainz/frontend/templates/release_group/entity.html index 49cc7f1e7..96ebaa3c7 100644 --- a/critiquebrainz/frontend/templates/release_group/entity.html +++ b/critiquebrainz/frontend/templates/release_group/entity.html @@ -113,7 +113,7 @@

{{ _('Tracklist') }}

{{ track.number }} - + {{ track.recording_title }} diff --git a/critiquebrainz/frontend/templates/review/browse.html b/critiquebrainz/frontend/templates/review/browse.html index 722e7ba6a..f6576799e 100644 --- a/critiquebrainz/frontend/templates/review/browse.html +++ b/critiquebrainz/frontend/templates/review/browse.html @@ -15,6 +15,8 @@

{{ _('Reviews') }}

{{ _('Artist') }}
  • {{ _('Label') }}
  • +
  • + {{ _('Recording') }}
  • {{ _('Event') }}
  • diff --git a/critiquebrainz/frontend/templates/review/entity/recording.html b/critiquebrainz/frontend/templates/review/entity/recording.html new file mode 100644 index 000000000..37290a2d0 --- /dev/null +++ b/critiquebrainz/frontend/templates/review/entity/recording.html @@ -0,0 +1,18 @@ +{% extends 'review/entity/base.html' %} + +{% set recording = review.entity_id | entity_details(type='recording') %} + +{% block title %} + {% set recording_title = recording.name | default(_('[Unknown recording]')) %} + {{ _('Review of "%(recording)s" by %(user)s', recording=recording_title, user=review.user.display_name) }} - CritiqueBrainz +{% endblock %} + +{% block entity_title %} +

    + {% if recording %} + {{ recording.name }} + {% else %} + {{ _('[Unknown recording]') }} + {% endif %} +

    +{% endblock %} diff --git a/critiquebrainz/frontend/templates/review/modify/recording.html b/critiquebrainz/frontend/templates/review/modify/recording.html new file mode 100644 index 000000000..da6a30925 --- /dev/null +++ b/critiquebrainz/frontend/templates/review/modify/recording.html @@ -0,0 +1,26 @@ +{% if entity is not defined %} + {% set entity = review.entity_id | entity_details(type=entity_type) %} +{% endif %} +
    +
    +
    {{ _('Recording') }}
    +
    + {{ entity['name'] | default(_('[Unknown recording]')) }} +
    +
    {{ _('Length') }}
    +
    + {% if entity['length'] %} + {{ entity['length'] | track_length }} + {% else %} + - + {% endif %} +
    +
    {{ _('Artist') }}
    +
    + {{ entity['artist-credit-phrase'] or '-' }} +
    + {% block more_info %} + {# Information like creation date, votes etc. #} + {% endblock %} +
    +
    diff --git a/critiquebrainz/frontend/templates/search/index.html b/critiquebrainz/frontend/templates/search/index.html index 08675df77..8d0969068 100644 --- a/critiquebrainz/frontend/templates/search/index.html +++ b/critiquebrainz/frontend/templates/search/index.html @@ -24,6 +24,7 @@

    {{ _('Search') }}

    +
    + +
    +
    + +
    +
    + + + + + @@ -157,6 +180,7 @@

    {{ _('Label selection') }}

    or request.args.get('place', default=False) or request.args.get('work', default=False) or request.args.get('label', default=False) + or request.args.get('recording', default=False) %}
    {% if not results %} @@ -209,6 +233,14 @@

    {{ _('Label selection') }}

    {{ _('Language') }} + {% elif type=="recording" %} + + {{ _('Title') }} + {{ _('Length') }} + {{ _('Artist') }} + {{ _('Release group') }} + + {% endif %} {% include 'search/selector_results.html' %} diff --git a/critiquebrainz/frontend/templates/search/selector_results.html b/critiquebrainz/frontend/templates/search/selector_results.html index 61dac6819..0cd3a759b 100644 --- a/critiquebrainz/frontend/templates/search/selector_results.html +++ b/critiquebrainz/frontend/templates/search/selector_results.html @@ -131,6 +131,34 @@ + + + + +{% elif type=="recording" %} + + + {{ result['title'] }} + + + {% if result['length'] %} + {{ result['length'] | track_length_ms }} + {% else %} + - + {% endif %} + + + {{ result['artist-credit'][0]['artist']['name'] or '-' }} + + + {% if result['release-list'] %} + {{ result['release-list'][0].title or '-' }} + {% else %} + - + {% endif %} + + + diff --git a/critiquebrainz/frontend/templates/user/reviews.html b/critiquebrainz/frontend/templates/user/reviews.html index 4dc3db463..e6dbdc6ab 100644 --- a/critiquebrainz/frontend/templates/user/reviews.html +++ b/critiquebrainz/frontend/templates/user/reviews.html @@ -60,6 +60,22 @@ {% endif %} ) {% endif %} + + {%- elif review.entity_type == 'recording' -%} + {{ entity.name | default(_('[Unknown recording]')) }} + - {{ entity['artist-credit-phrase'] | default(_('[Unknown artist]')) }} + + {%- elif review.entity_type == 'artist' -%} + {{ entity.name | default(_('[Unknown artist]')) }} + {% if entity['type'] %} + ({{ entity.type }}) + {% endif %} + + {%- elif review.entity_type == 'label' -%} + {{ entity.name | default(_('[Unknown label]')) }} + {% if entity['type'] %} + ({{ entity.type }}) + {% endif %} {%- endif -%} diff --git a/critiquebrainz/frontend/views/__init__.py b/critiquebrainz/frontend/views/__init__.py index 017f648da..9b6cd5b36 100644 --- a/critiquebrainz/frontend/views/__init__.py +++ b/critiquebrainz/frontend/views/__init__.py @@ -4,6 +4,7 @@ ARTIST_REVIEWS_LIMIT = 5 LABEL_REVIEWS_LIMIT = 5 WORK_REVIEWS_LIMIT = 5 +RECORDING_REVIEWS_LIMIT = 10 BROWSE_RELEASE_GROUPS_LIMIT = 20 BROWSE_RECORDING_LIMIT = 10 diff --git a/critiquebrainz/frontend/views/recording.py b/critiquebrainz/frontend/views/recording.py new file mode 100644 index 000000000..b09b3ac6e --- /dev/null +++ b/critiquebrainz/frontend/views/recording.py @@ -0,0 +1,53 @@ +from flask import Blueprint, render_template, request +from flask_login import current_user +from werkzeug.exceptions import NotFound +from flask_babel import gettext +import critiquebrainz.db.review as db_review +import critiquebrainz.frontend.external.musicbrainz_db.recording as mb_recording +import critiquebrainz.frontend.external.musicbrainz_db.exceptions as mb_exceptions +from critiquebrainz.frontend.forms.rate import RatingEditForm +from critiquebrainz.frontend.views import get_avg_rating, RECORDING_REVIEWS_LIMIT + + +recording_bp = Blueprint('recording', __name__) + + +@recording_bp.route('/') +def entity(id): + id = str(id) + try: + recording = mb_recording.get_recording_by_id(id) + except mb_exceptions.NoDataFoundException: + raise NotFound(gettext("Sorry, we couldn't find a recording with that MusicBrainz ID.")) + + if 'url-rels' in recording: + external_reviews = list(filter(lambda rel: rel['type'] == 'review', recording['url-rels'])) + else: + external_reviews = [] + + limit = int(request.args.get('limit', default=RECORDING_REVIEWS_LIMIT)) + offset = int(request.args.get('offset', default=0)) + if current_user.is_authenticated: + my_reviews, my_count = db_review.list_reviews( + entity_id=recording['id'], + entity_type='recording', + user_id=current_user.id, + ) + my_review = my_reviews[0] if my_count else None + else: + my_review = None + reviews, count = db_review.list_reviews( + entity_id=recording['id'], + entity_type='recording', + sort='popularity', + limit=limit, + offset=offset, + ) + avg_rating = get_avg_rating(recording['id'], "recording") + + rating_form = RatingEditForm(entity_id=id, entity_type='recording') + rating_form.rating.data = my_review['rating'] if my_review else None + + return render_template('recording/entity.html', id=recording['id'], recording=recording, reviews=reviews, + my_review=my_review, external_reviews=external_reviews, limit=limit, offset=offset, + count=count, avg_rating=avg_rating, rating_form=rating_form, current_user=current_user) diff --git a/critiquebrainz/frontend/views/search.py b/critiquebrainz/frontend/views/search.py index ae31e1464..b352ed492 100644 --- a/critiquebrainz/frontend/views/search.py +++ b/critiquebrainz/frontend/views/search.py @@ -21,6 +21,8 @@ def search_wrapper(query, type, offset=None): count, results = musicbrainz.search_works(query, limit=RESULTS_LIMIT, offset=offset) elif type == "label": count, results = musicbrainz.search_labels(query, limit=RESULTS_LIMIT, offset=offset) + elif type == "recording": + count, results = musicbrainz.search_recordings(query, limit=RESULTS_LIMIT, offset=offset) else: count, results = 0, [] else: @@ -52,6 +54,7 @@ def more(): def selector(): release_group = request.args.get('release_group') artist = request.args.get('artist') + recording = request.args.get('recording') label = request.args.get('label') event = request.args.get('event') place = request.args.get('place') @@ -68,6 +71,8 @@ def selector(): count, results = musicbrainz.search_labels(label, limit=RESULTS_LIMIT) elif work: count, results = musicbrainz.search_works(work, limit=RESULTS_LIMIT) + elif recording: + count, results = musicbrainz.search_recordings(recording, limit=RESULTS_LIMIT) elif event: count, results = musicbrainz.search_events(event, limit=RESULTS_LIMIT) elif place: @@ -77,12 +82,13 @@ def selector(): return render_template('search/selector.html', next=next, type=type, results=results, count=count, limit=RESULTS_LIMIT, artist=artist, release_group=release_group, event=event, - work=work, place=place, label=label) + work=work, place=place, label=label, recording=recording) @search_bp.route('/selector/more') def selector_more(): artist = request.args.get('artist') + recording = request.args.get('recording') release_group = request.args.get('release_group') label = request.args.get('label') event = request.args.get('event') @@ -100,6 +106,8 @@ def selector_more(): count, results = musicbrainz.search_artists(artist, limit=RESULTS_LIMIT, offset=offset) elif type == 'label': count, results = musicbrainz.search_labels(label, limit=RESULTS_LIMIT, offset=offset) + elif type == 'recording': + count, results = musicbrainz.search_recordings(recording, limit=RESULTS_LIMIT, offset=offset) elif type == 'event': count, results = musicbrainz.search_events(event, limit=RESULTS_LIMIT, offset=offset) elif type == 'place': diff --git a/critiquebrainz/frontend/views/test/test_recording.py b/critiquebrainz/frontend/views/test/test_recording.py new file mode 100644 index 000000000..da80486f3 --- /dev/null +++ b/critiquebrainz/frontend/views/test/test_recording.py @@ -0,0 +1,50 @@ +from unittest.mock import MagicMock + +import critiquebrainz.db.license as db_license +import critiquebrainz.db.review as db_review +import critiquebrainz.db.users as db_users +import critiquebrainz.frontend.external.musicbrainz_db.recording as mb_recording +from critiquebrainz.db.user import User +from critiquebrainz.frontend.testing import FrontendTestCase + + +class RecordingViewsTestCase(FrontendTestCase): + + def setUp(self): + super(RecordingViewsTestCase, self).setUp() + mb_recording.get_recording_by_id = MagicMock() + mb_recording.get_recording_by_id.return_value = { + 'id': '442ddce2-ffa1-4865-81d2-b42c40fec7c5', + 'name': 'Dream Come True', + 'length': 229.0, + 'artists': [ + { + 'id': '164f0d73-1234-4e2c-8743-d77bf2191051', + 'name': 'Kanye West', + 'join_phrase': ' feat. ' + }, + { + 'id': '75a72702-a5ef-4513-bca5-c5b944903546', + 'name': 'John Legend' + } + ], + 'artist-credit-phrase': 'Kanye West feat. John Legend' + } + self.user = User(db_users.get_or_create(1, "Tester", new_user_data={"display_name": "test user"})) + self.license = db_license.create(id='Test', full_name='Test License') + + def test_recording_page(self): + db_review.create( + user_id=self.user.id, + entity_id='442ddce2-ffa1-4865-81d2-b42c40fec7c5', + entity_type='recording', + text='This is a test review', + is_draft=False, + license_id=self.license['id'], + language='en', + ) + response = self.client.get('/recording/8ef859e3-feb2-4dd1-93da-22b91280d768') + self.assert200(response) + self.assertIn('Dream Come True', str(response.data)) + # Test if there is a review from test user + self.assertIn('test user', str(response.data))