Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CB-270: Implemented the reviewal of recordings #265

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f645530
Changes made to SQL scripts in order to support the reviewal of recor…
spellew Jun 16, 2019
1f440a8
Implemented the retrieval of recordings through brainzutils
spellew Jun 16, 2019
e4d1bc4
Implemented the searching of recordings
spellew Jun 16, 2019
37bd421
Implemented the reviewal of recordings
spellew Jun 16, 2019
9af57f6
Fixed bug in recording retrieval
spellew Jun 22, 2019
adbc6bc
Fixed tests
spellew Jul 5, 2019
6465ff4
Commented out AB link
spellew Jul 5, 2019
60512c9
Removed comments and unused imports
spellew Aug 5, 2019
00298fe
Moved alter type sql script to it's own file
spellew Aug 15, 2019
88332e2
Updated recording entity page to display relevant information
spellew Aug 15, 2019
625d03f
Merged with master
spellew Aug 15, 2019
d91d5bf
Merge branch 'master' into recording-entity-support
spellew Aug 15, 2019
81b7535
Removed AcousticBrainz link from release_group entity page
spellew Aug 18, 2019
7b686ce
Added titles for recording, artist, and label to user template
spellew Aug 20, 2019
5fa2056
Moved views/recording limit to __init__.py
spellew Aug 22, 2019
1b57962
Created separate track_length functions for seconds and milliseconds
spellew Aug 22, 2019
7e6159f
Fixed track list in review/modify
spellew Aug 24, 2019
8fa4fc9
Fixed artist in review/modify
spellew Aug 24, 2019
b4ae26b
Removed unnecessary code
spellew Aug 27, 2019
2b41a34
Updated brainzutils
spellew Aug 30, 2019
a3a3bf9
Fixed merge conflicts
spellew Dec 16, 2019
8fd7cf3
Removed leftover tags
spellew Dec 16, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions admin/schema_changes/18.sql
@@ -0,0 +1 @@
ALTER TYPE entity_types ADD VALUE 'recording' AFTER 'label';
3 changes: 2 additions & 1 deletion admin/sql/create_types.sql
Expand Up @@ -10,5 +10,6 @@ CREATE TYPE entity_types AS ENUM (
'event',
'place',
'artist',
'label'
'label',
'recording'
);
1 change: 1 addition & 0 deletions critiquebrainz/db/review.py
Expand Up @@ -20,6 +20,7 @@
"place",
"release_group",
"artist",
"recording",
"label",
]

Expand Down
2 changes: 2 additions & 0 deletions critiquebrainz/frontend/__init__.py
Expand Up @@ -134,6 +134,7 @@ def create_app(debug=None, config_path=None):
from critiquebrainz.frontend.views.label import label_bp
from critiquebrainz.frontend.views.release_group import release_group_bp
from critiquebrainz.frontend.views.release import release_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
Expand All @@ -156,6 +157,7 @@ def create_app(debug=None, config_path=None):
app.register_blueprint(label_bp, url_prefix='/label')
app.register_blueprint(release_group_bp, url_prefix='/release-group')
app.register_blueprint(release_bp, url_prefix='/release')
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')
Expand Down
6 changes: 6 additions & 0 deletions critiquebrainz/frontend/external/musicbrainz.py
Expand Up @@ -46,6 +46,12 @@ def search_places(query='', limit=None, offset=None):
return api_resp.get('place-count'), api_resp.get('place-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')


def search_labels(query='', limit=None, offset=None):
"""Search for labels."""
api_resp = musicbrainzngs.search_labels(query=query, limit=limit, offset=offset)
Expand Down
8 changes: 8 additions & 0 deletions critiquebrainz/frontend/external/musicbrainz_db/entities.py
@@ -1,3 +1,4 @@
from brainzutils.musicbrainz_db.recording import fetch_multiple_recordings
from brainzutils.musicbrainz_db.artist import fetch_multiple_artists
from brainzutils.musicbrainz_db.label import fetch_multiple_labels
from brainzutils.musicbrainz_db.place import fetch_multiple_places
Expand All @@ -8,6 +9,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.artist import get_artist_by_id
from critiquebrainz.frontend.external.musicbrainz_db.recording import get_recording_by_id


def get_multiple_entities(entities):
Expand All @@ -28,6 +30,7 @@ def get_multiple_entities(entities):
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)]
recording_mbids = [entity[0] for entity in filter(lambda entity: entity[1] == 'recording', 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)]
Expand All @@ -38,6 +41,9 @@ def get_multiple_entities(entities):
entities_info.update(fetch_multiple_artists(
artist_mbids,
))
entities_info.update(fetch_multiple_recordings(
recording_mbids,
))
entities_info.update(fetch_multiple_labels(
label_mbids,
))
Expand All @@ -56,6 +62,8 @@ def get_entity_by_id(id, type='release_group'):
entity = get_release_group_by_id(str(id))
elif type == 'artist':
entity = get_artist_by_id(str(id))
elif type == 'recording':
entity = get_recording_by_id(str(id))
elif type == 'label':
entity = get_label_by_id(str(id))
elif type == 'place':
Expand Down
23 changes: 23 additions & 0 deletions critiquebrainz/frontend/external/musicbrainz_db/recording.py
@@ -0,0 +1,23 @@
from brainzutils import cache
from brainzutils.musicbrainz_db.recording import fetch_multiple_recordings
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(mbid)
recording = cache.get(key)
if not recording:
recording = fetch_multiple_recordings(
[mbid],
includes=['artists', 'work-rels', 'url-rels'],
).get(mbid)
recording.update({'length': recording['length'] * 1000.0})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spellew Why do you need to multiply by 1000? Also, I see that similar change was made in work/entity.html. Could you move both calls to a either a template function or a python function. We should try to stay consistent in order to reduce changes in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brainzutils returns the length in seconds, but we need milliseconds for the track_length function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated here and in the pull request regarding works.

cache.set(key=key, val=recording, time=DEFAULT_CACHE_EXPIRATION)
return recording
5 changes: 5 additions & 0 deletions critiquebrainz/frontend/static/styles/main.less
Expand Up @@ -11,6 +11,7 @@
// Entity colors
@rg-color: @blue;
@artist-color: @blue;
@recording-color: @blue;
@event-color: @green;
@place-color: @yellow;
@label-color: @blue;
Expand Down Expand Up @@ -128,6 +129,8 @@ ul.sharing {
}
&.artist {
background-color: fade(@artist-color, 70%);
&.recording {
background-color: fade(@recording-color, 70%);
}
&.place {
background-color: fade(@place-color, 70%);
Expand Down Expand Up @@ -493,6 +496,8 @@ a#edit-review { margin-top: 20px; }
}
&.artist {
background-color: fade(@artist-color, 70%);
&.recording {
background-color: fade(@recording-color, 70%);
}
&.place {
background-color: fade(@place-color, 70%);
Expand Down
2 changes: 2 additions & 0 deletions critiquebrainz/frontend/templates/entity_review.html
Expand Up @@ -5,6 +5,8 @@
artist = entity['artist-credit-phrase'] | default(_('[Unknown artist]'))) }}
{% elif review.entity_type == 'artist' %}
{{ _('%(artist)s', artist = '<b>'|safe + entity.name | default(_('[Unknown artist]')) + '</b>'|safe) }}
{% elif review.entity_type == 'recording' %}
{{ _('%(recording)s', recording = '<b>'|safe + entity.name | default(_('[Unknown recording]')) + '</b>'|safe) }}
{% elif review.entity_type == 'label' %}
{{ _('%(label)s', label = '<b>'|safe + entity.name | default(_('[Unknown label]')) + '</b>'|safe) }}
{% elif review.entity_type == 'event' %}
Expand Down
7 changes: 7 additions & 0 deletions critiquebrainz/frontend/templates/macros.html
Expand Up @@ -53,6 +53,13 @@
{{ _('Artist') }}
</span>
{% endif %}
{% elif entity_type == 'recording' %}
<img src="{{ get_static_path('images/placeholder_disc.svg') }}" {{ attributes }} />
{% if overlay_type %}
<span class="entity-type recording">
{{ _('Recording') }}
</span>
{% endif %}
{% elif entity_type == 'label' %}
<img src="{{ get_static_path('images/placeholder_disc.svg') }}" {{ attributes }} />
{% if overlay_type %}
Expand Down
1 change: 1 addition & 0 deletions critiquebrainz/frontend/templates/navbar.html
Expand Up @@ -95,6 +95,7 @@
<div class="form-group">
<select id="type-selector" name="type" class="form-control input-sm">
<option value="artist">{{ _('Artist') }}</option>
<option value="recording">{{ _('Recording') }}</option>
<option value="label">{{ _('Label') }}</option>
<option value="event">{{ _('Event') }}</option>
<option value="place">{{ _('Place') }}</option>
Expand Down
140 changes: 140 additions & 0 deletions critiquebrainz/frontend/templates/recording/entity.html
@@ -0,0 +1,140 @@
{% 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 %}
<div class="clearfix">
<h2 class="pull-left">
{% set artist = [] %}
{% for credit in recording['artists'] %}
{% if credit.name %}
{% do artist.append('<a href="%s">'|safe % url_for('artist.entity', mbid=credit.id) ~ credit.name ~ '</a>'|safe) %}
{% if credit.join_phrase %}
{% do artist.append(credit.join_phrase) %}
{% endif %}
{% else %}
{% do artist.append(credit) %}
{% endif %}
{% endfor %}

{{ _('%(recording)s by %(artist)s', recording = recording.name, artist = artist|join()) }}
</h2>

{% if not my_review %}
<a id="write-review" href="{{ url_for('review.create', recording=id) }}"
role="button" class="btn btn-success pull-right">
<span class="glyphicon glyphicon-asterisk"></span> {{ _('Write a review') }}
</a>
{% else %}
<a id="edit-review" href="{{ url_for('review.edit', id=my_review.id) }}"
role="button" class="btn btn-primary pull-right">
<span class="glyphicon glyphicon-edit"></span> {{ _('Edit my review') }}
</a>
{% endif %}
</div>

<div id="recording-details" class="row">
<div class="col-md-9">
{{ entity_rate_form('recording', 'recording') }}
<br/><br/>
<h4 style="margin-bottom:0;">{{ _('Reviews') }}</h4>
{% if not reviews %}
<p class="lead" style="text-align: center;">{{ _('No reviews found') }}</p>
{% else %}
<table class="table table-condensed table-hover">
<thead>
<tr>
<th></th>
<th>{{ _('Published on') }}</th>
<th>{{ _('Votes (+/-)') }}</th>
</tr>
</thead>
<tbody>
{% for review in reviews %}
<tr data-review-id="{{ review.id }}">
<td>
<a href="{{ url_for('review.entity', id=review.id) }}">
{{ _('by %(reviewer)s', reviewer='<img class="avatar" src="%s&s=16" /> '|safe % review.user.avatar + review.user.display_name) }}
</a>
</td>
<td>{{ review.published_on | date }}</td>
<td>{{ review.votes_positive_count }}/{{ review.votes_negative_count }}</td>
</tr>
{% endfor %}
</tbody>
</table>
<ul class="pagination">
{% 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 %}
<li {% if offset == p_offset %}class="active"{% endif %}>
<a href="{{ url_for('recording.entity', id=id, limit=limit, offset=p*limit) }}">{{ p+1 }}</a>
</li>
{% endfor %}
{% endif %}
</ul>
{% endif %}
</div>

<div class="col-md-3">
<h4>{{ _('Recording information') }}</h4>
{% if avg_rating %}
<div class="avg-rating">
{{ show_avg_rating(avg_rating.rating, avg_rating.count) }}
</div>
{% endif %}
{% if external_reviews %}
<b>{{ _('External reviews') }}</b>
<ul class="list-unstyled external-links">
{{ show_external_reviews(external_reviews) }}
</ul>
{% endif %}
{% if recording['external-urls'] %}
<b>{{ _('External links') }}</b>
<ul class="list-unstyled external-links">
{% for url in recording['external-urls'] %}
<li class="clearfix">
<div class="favicon-container">
{% if url.icon %}
<img src="{{ get_static_path('favicons/'+url.icon) }}" />
{% else %}
<img src="{{ get_static_path('favicons/external-16.png') }}" />
{% endif %}
</div>
<a href="{{ url.url.url }}">{{ url.name }}</a>
{% if url.disambiguation %}<span class="text-muted">({{ url.disambiguation }})</span>{% endif %}
</li>
{% endfor %}
</ul>
{% endif %}

<div class="external-links">
<div class="favicon-container"><img src="{{ get_static_path('favicons/musicbrainz-16.svg') }}" /></div>
<a href="https://musicbrainz.org/recording/{{ recording.id }}"><em>{{ _('Edit on MusicBrainz') }}</em></a>
</div>
</div>
</div>
{% endblock %}

{% block scripts %}
<link rel="stylesheet" href="https://cdn.jsdelivr.net/simplemde/latest/simplemde.min.css"/>
<script>
$(document).ready(function() {
$("#ratestars").click(function(e){
// only submit rating form when user clicks on <i></i> tag (see generated HTML for rating stars on this page)
if (e.target.tagName === 'I') {
$("#rating-form").submit();
}
});
});
</script>
<script src="{{ get_static_path('rating.js') }}"></script>
{% endblock %}
Expand Up @@ -113,7 +113,7 @@ <h4>{{ _('Tracklist') }}</h4>
<tr>
<td>{{ track.number }}</td>
<td>
<a href="http://acousticbrainz.org/{{ track.recording_id }}" title="View on AcousticBrainz">
<a href="{{ url_for('recording.entity', id=track.recording_id) }}">
{{ track.recording_title }}
</a>
</td>
Expand Down
2 changes: 2 additions & 0 deletions critiquebrainz/frontend/templates/review/browse.html
Expand Up @@ -13,6 +13,8 @@ <h2>{{ _('Reviews') }}</h2>
<a href="{{ url_for('review.browse', entity_type='release_group') }}">{{ _('Release group') }}</a></li>
<li role="presentation" {{ "class=active" if entity_type == 'artist' }}>
<a href="{{ url_for('review.browse', entity_type='artist') }}">{{ _('Artist') }}</a></li>
<li role="presentation" {{ "class=active" if entity_type == 'recording' }}>
<a href="{{ url_for('review.browse', entity_type='recording') }}">{{ _('Recording') }}</a></li>
<li role="presentation" {{ "class=active" if entity_type == 'label' }}>
<a href="{{ url_for('review.browse', entity_type='label') }}">{{ _('Label') }}</a></li>
<li role="presentation" {{ "class=active" if entity_type == 'event' }}>
Expand Down
24 changes: 24 additions & 0 deletions critiquebrainz/frontend/templates/review/entity/recording.html
@@ -0,0 +1,24 @@
{% 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 %}
<h2 id="title">
{% if recording %}
{% set recording_name = '<a href="%s">' | safe % url_for('recording.entity', id=review.entity_id) ~ recording.name ~ '</a>'|safe %}
{% else %}
{% set recording_name = _('[Unknown recording]') %}
{% endif %}

{{ _('%(recording)s', recording=recording_name) }}

{% if recording['life-span'] %}
<small>{{ recording['life-span']['begin'][:4] }}</small>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for the year of the recording? Please move this as a function in the template just so that it gets easier to read the code. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this was accidentally included, since recordings don't have life-spans.

{% endif %}
</h2>
{% endblock %}
26 changes: 26 additions & 0 deletions 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 %}
<div class="col-md-12">
<dl class="dl-horizontal">
<dt>{{ _('Recording') }}</dt>
<dd>
{{ entity['name'] | default(_('[Unknown recording]')) }}
</dd>
<dt>{{ _('Length') }}</dt>
<dd>
{% if entity['length'] %}
{{ entity['length'] | track_length }}
{% else %}
-
{% endif %}
</dd>
<dt>{{ _('Artist') }}</dt>
<dd>
{{ entity['artist'] or '-' }}
</dd>
{% block more_info %}
{# Information like creation date, votes etc. #}
{% endblock %}
</dl>
</div>