Skip to content

Commit

Permalink
refactor: Change variable names and fix lint errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@anshg1214
anshg1214 committed Sep 26, 2022
1 parent 73afc1c commit 7f07b3b
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 83 deletions.
21 changes: 12 additions & 9 deletions critiquebrainz/frontend/external/bookbrainz_db/common_entity.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,17 @@
import critiquebrainz.frontend.external.bookbrainz_db as db
from critiquebrainz.frontend.external.bookbrainz_db import DEFAULT_CACHE_EXPIRATION

MB_ARTIST_IDENTIFIER_TYPE = 2
MB_WORK_IDENTIFIER_TYPE = 3

def get_author_for_artist(artist_mbid) -> List:
def get_authors_for_artist(artist_mbid) -> List:
"""
Get the author BBIDs for an artist MBID.
Args:
artist_mbid (str): MusicBrainz ID of the artist.
Returns:
List of author BBIDs.
None if no author BBIDs are found.
Empty List if no author BBIDs are found.
"""

artist_mbid = str(artist_mbid)
Expand All @@ -27,11 +29,11 @@ def get_author_for_artist(artist_mbid) -> List:
LEFT JOIN identifier_set__identifier idens ON idens.set_id = author.identifier_set_id
LEFT JOIN identifier iden ON idens.identifier_id = iden.id
WHERE iden.value = :artist_mbid
AND iden.type_id = 2
AND iden.type_id = :identifier_type
AND master = 't'
AND author.data_id IS NOT NULL
GROUP BY bbid
"""), {'artist_mbid': artist_mbid})
"""), {'artist_mbid': artist_mbid, 'identifier_type': MB_ARTIST_IDENTIFIER_TYPE})
authors = result.fetchall()

author_bbids = []
Expand All @@ -42,17 +44,18 @@ def get_author_for_artist(artist_mbid) -> List:
cache.set(bb_author_mb_artist_key, author_bbids, DEFAULT_CACHE_EXPIRATION)

if not author_bbids:
return None
return []
return author_bbids


def get_literary_work_for_work(work_mbid) -> List:
def get_literary_works_for_work(work_mbid) -> List:
"""
Get the literary work BBIDs for a work MBID.
Args:
work_mbid (str): MusicBrainz ID of the work.
Returns:
List of literary work BBIDs.
Empty List if no literary work BBIDs are found.
"""

work_mbid = str(work_mbid)
Expand All @@ -67,11 +70,11 @@ def get_literary_work_for_work(work_mbid) -> List:
LEFT JOIN identifier_set__identifier idens ON idens.set_id = work.identifier_set_id
LEFT JOIN identifier iden ON idens.identifier_id = iden.id
WHERE iden.value = :work_mbid
AND iden.type_id = 3
AND iden.type_id = :identifier_type
AND master = 't'
AND work.data_id IS NOT NULL
GROUP BY bbid
"""), {'work_mbid': work_mbid})
"""), {'work_mbid': work_mbid, 'identifier_type': MB_WORK_IDENTIFIER_TYPE})

literary_works = result.fetchall()
work_bbids = []
Expand All @@ -82,6 +85,6 @@ def get_literary_work_for_work(work_mbid) -> List:
cache.set(bb_literary_work_mb_work_key, work_bbids, DEFAULT_CACHE_EXPIRATION)

if not work_bbids:
return None
return []

return work_bbids
16 changes: 8 additions & 8 deletions critiquebrainz/frontend/external/bookbrainz_db/test/common_entity_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,18 @@ def setUp(self):
self.bbid3 = 'a99374d5-fa8b-4fab-9fec-9c0c38e8ac7c'
self.bbid4 = '0e5a48f3-7d21-365c-bfb7-98d9865ea1dd'

def test_get_author_for_artist(self):
author_bbids1 = common_entity.get_author_for_artist(self.bbid1)
def test_get_authors_for_artist(self):
author_bbids1 = common_entity.get_authors_for_artist(self.bbid1)
self.assertEqual(len(author_bbids1), 1)
self.assertEqual(author_bbids1[0], 'e5c4e68b-bfce-4c77-9ca2-0f0a2d4d09f0')

author_bbids2 = common_entity.get_author_for_artist(self.bbid2)
self.assertEqual(author_bbids2, None)
author_bbids2 = common_entity.get_authors_for_artist(self.bbid2)
self.assertEqual(author_bbids2, [])

def test_get_literary_work_for_work(self):
work_bbids1 = common_entity.get_literary_work_for_work(self.bbid3)
def test_get_literary_works_for_work(self):
work_bbids1 = common_entity.get_literary_works_for_work(self.bbid3)
self.assertEqual(len(work_bbids1), 1)
self.assertEqual(work_bbids1[0], 'f89e85c0-e341-4b0e-ada6-36655f5dae07')

work_bbids2 = common_entity.get_literary_work_for_work(self.bbid4)
self.assertEqual(work_bbids2, None)
work_bbids2 = common_entity.get_literary_works_for_work(self.bbid4)
self.assertEqual(work_bbids2, [])
60 changes: 29 additions & 31 deletions critiquebrainz/frontend/templates/artist/entity.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,38 +93,36 @@ <h4 style="margin-bottom:0;">{{ _('Reviews') }}</h4>
</div>
{% endif %}

{% if author_info is defined and author_info %}
{% for author in author_info %}
<h4> {{ _('This entity is also in the BookBrainz Database as ') }}
<a href="{{ url_for('bb_author.entity', id=author.bbid ) }}">{{ author.name }}</a>.
</h4>
{% if author.reviews_count %}
<h4 >{{ _('Reviews') }}</h4>
<table class="table table-condensed table-hover">
<thead>
<tr>
<th></th>
<th>{{ _('Published on') }}</th>
<th>{{ _('Votes (+/-)') }}</th>
{% for author in author_info %}
<h4> {{ _('This entity is also in the BookBrainz Database as ') }}
<a href="{{ url_for('bb_author.entity', id=author.bbid ) }}">{{ author.name }}</a>.
</h4>
{% if author.reviews_count %}
<h4 >{{ _('Reviews') }}</h4>
<table class="table table-condensed table-hover">
<thead>
<tr>
<th></th>
<th>{{ _('Published on') }}</th>
<th>{{ _('Votes (+/-)') }}</th>
</tr>
</thead>
<tbody>
{% for review in author.reviews %}
<tr data-review-id="{{ review.id }}">
<td>
<a href="{{ url_for('review.entity', id=review.id) }}">
{{ _('by %(reviewer)s', reviewer=review.user.display_name) }}
</a>
</td>
<td>{{ review.published_on | date }}</td>
<td>{{ review.votes_positive_count }}/{{ review.votes_negative_count }}</td>
</tr>
</thead>
<tbody>
{% for review in author.reviews %}
<tr data-review-id="{{ review.id }}">
<td>
<a href="{{ url_for('review.entity', id=review.id) }}">
{{ _('by %(reviewer)s', reviewer=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>
{% endif %}
{% endfor %}
{% endif %}
{% endfor %}
</tbody>
</table>
{% endif %}
{% endfor %}

<ul class="pagination"></ul>
<h4>{{ _('Discography') }}</h4>
Expand Down
60 changes: 29 additions & 31 deletions critiquebrainz/frontend/templates/work/entity.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,38 +54,36 @@ <h4 style="margin-bottom:0;">{{ _('Reviews') }}</h4>
</div>
{% endif %}

{% if literary_work_info is defined and literary_work_info %}
{% for literary_work in literary_work_info %}
<h4> {{ _('This entity is also in the BookBrainz Database as ') }}
<a href="{{ url_for('bb_literary_work.entity', id=literary_work.bbid ) }}">{{ literary_work.name }}</a>.
</h4>
{% if literary_work.reviews_count %}
<h4 >{{ _('Reviews') }}</h4>
<table class="table table-condensed table-hover">
<thead>
<tr>
<th></th>
<th>{{ _('Published on') }}</th>
<th>{{ _('Votes (+/-)') }}</th>
{% for literary_work in literary_work_info %}
<h4> {{ _('This entity is also in the BookBrainz Database as ') }}
<a href="{{ url_for('bb_literary_work.entity', id=literary_work.bbid ) }}">{{ literary_work.name }}</a>.
</h4>
{% if literary_work.reviews_count %}
<h4 >{{ _('Reviews') }}</h4>
<table class="table table-condensed table-hover">
<thead>
<tr>
<th></th>
<th>{{ _('Published on') }}</th>
<th>{{ _('Votes (+/-)') }}</th>
</tr>
</thead>
<tbody>
{% for review in literary_work.reviews %}
<tr data-review-id="{{ review.id }}">
<td>
<a href="{{ url_for('review.entity', id=review.id) }}">
{{ _('by %(reviewer)s', reviewer=review.user.display_name) }}
</a>
</td>
<td>{{ review.published_on | date }}</td>
<td>{{ review.votes_positive_count }}/{{ review.votes_negative_count }}</td>
</tr>
</thead>
<tbody>
{% for review in literary_work.reviews %}
<tr data-review-id="{{ review.id }}">
<td>
<a href="{{ url_for('review.entity', id=review.id) }}">
{{ _('by %(reviewer)s', reviewer=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>
{% endif %}
{% endfor %}
{% endif %}
{% endfor %}
</tbody>
</table>
{% endif %}
{% endfor %}

<ul class="pagination"></ul>
<h4>{{ _('Recordings') }}</h4>
Expand Down
4 changes: 2 additions & 2 deletions critiquebrainz/frontend/views/artist.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from critiquebrainz.frontend.views import get_avg_rating, BROWSE_RELEASE_GROUPS_LIMIT, ARTIST_REVIEWS_LIMIT
from critiquebrainz.frontend.forms.rate import RatingEditForm
from critiquebrainz.frontend.external.bookbrainz_db import author as bb_author
from critiquebrainz.frontend.external.bookbrainz_db.common_entity import get_author_for_artist
from critiquebrainz.frontend.external.bookbrainz_db.common_entity import get_authors_for_artist

artist_bp = Blueprint('artist', __name__)

Expand All @@ -27,7 +27,7 @@ def entity(id):
if artist is None:
raise NotFound(gettext("Sorry, we couldn't find an artist with that MusicBrainz ID."))

author_bbids = get_author_for_artist(id)
author_bbids = get_authors_for_artist(id)
author_info = {}
if author_bbids:
author_info = bb_author.fetch_multiple_authors(author_bbids)
Expand Down
4 changes: 2 additions & 2 deletions critiquebrainz/frontend/views/work.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import critiquebrainz.frontend.external.musicbrainz_db.work as mb_work
from critiquebrainz.frontend.forms.rate import RatingEditForm
from critiquebrainz.frontend.views import get_avg_rating, WORK_REVIEWS_LIMIT, BROWSE_RECORDING_LIMIT
from critiquebrainz.frontend.external.bookbrainz_db.common_entity import get_literary_work_for_work
from critiquebrainz.frontend.external.bookbrainz_db.common_entity import get_literary_works_for_work
from critiquebrainz.frontend.external.bookbrainz_db import literary_work as bb_literary_work

work_bp = Blueprint('work', __name__)
Expand All @@ -20,7 +20,7 @@ def entity(id):
if work is None:
raise NotFound(gettext("Sorry, we couldn't find a work with that MusicBrainz ID."))

literary_work_bbids = get_literary_work_for_work(id)
literary_work_bbids = get_literary_works_for_work(id)
literary_work_info = {}
if literary_work_bbids:
literary_work_info = bb_literary_work.fetch_multiple_literary_works(literary_work_bbids)
Expand Down

0 comments on commit 7f07b3b

Please sign in to comment.