Skip to content

Commit

Permalink
Merge pull request #458 from anshg1214/CB-444
Browse files Browse the repository at this point in the history
CB-444: API error messages are music-specific
  • Loading branch information
alastair authored Aug 30, 2022
2 parents 841bd89 + 0880585 commit 48bb19a
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 46 deletions.
42 changes: 22 additions & 20 deletions critiquebrainz/db/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import sqlalchemy
from brainzutils import cache
from flask import current_app
from flask_babel import lazy_gettext

from critiquebrainz import db
from critiquebrainz.db import (exceptions as db_exceptions,
Expand All @@ -21,24 +22,25 @@
DEFAULT_LANG = "en"

#: list of allowed entity_type's for writing/querying a review
MUSICBRAINZ_ENTITY_TYPES = [
"event",
"place",
"release_group",
"work",
"artist",
"label",
"recording"
]

BOOKBRAINZ_ENTITY_TYPES = [
"bb_edition_group",
"bb_literary_work",
"bb_author",
"bb_series",
]

ENTITY_TYPES = MUSICBRAINZ_ENTITY_TYPES + BOOKBRAINZ_ENTITY_TYPES
MUSICBRAINZ_ENTITY_TYPES = {
"event": lazy_gettext("Event"),
"place": lazy_gettext("Place"),
"release_group": lazy_gettext("Release Group"),
"work": lazy_gettext("Work"),
"artist": lazy_gettext("Artist"),
"label": lazy_gettext("Label"),
"recording": lazy_gettext("Recording"),
}

BOOKBRAINZ_ENTITY_TYPES = {
"bb_edition_group": lazy_gettext("Edition Group"),
"bb_literary_work": lazy_gettext("Literary Work"),
"bb_author": lazy_gettext("Author"),
"bb_series": lazy_gettext("Series"),
}

ENTITY_TYPES_MAPPING = {**MUSICBRAINZ_ENTITY_TYPES, **BOOKBRAINZ_ENTITY_TYPES}
ENTITY_TYPES = list(ENTITY_TYPES_MAPPING.keys())

supported_languages = []
for lang in list(pycountry.languages):
Expand Down Expand Up @@ -427,10 +429,10 @@ def get_reviews_list(connection, *, inc_drafts=False, inc_hidden=False, entity_i
if entity_type is not None:
if entity_type == 'musicbrainz':
filters.append("entity_type in :entity_type")
filter_data["entity_type"] = tuple(MUSICBRAINZ_ENTITY_TYPES)
filter_data["entity_type"] = tuple(MUSICBRAINZ_ENTITY_TYPES.keys())
elif entity_type == 'bookbrainz':
filters.append("entity_type in :entity_type")
filter_data["entity_type"] = tuple(BOOKBRAINZ_ENTITY_TYPES)
filter_data["entity_type"] = tuple(BOOKBRAINZ_ENTITY_TYPES.keys())
else:
filters.append("entity_type = :entity_type")
filter_data["entity_type"] = entity_type
Expand Down
1 change: 0 additions & 1 deletion critiquebrainz/frontend/babel.cfg
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
[python: **.py]
[jinja2: **/templates/**.html]
extensions=jinja2.ext.autoescape,jinja2.ext.with_
4 changes: 2 additions & 2 deletions critiquebrainz/frontend/babel.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
from flask_babel import Babel, Locale


def init_app(app):
babel = Babel(app)
def init_app(app, domain='messages'):
babel = Babel(app, default_domain=domain)

app.config['LANGUAGES'] = {}
for language in app.config['SUPPORTED_LANGUAGES']:
Expand Down
16 changes: 2 additions & 14 deletions critiquebrainz/frontend/views/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,8 @@ def reviews(user_id):
# happens so infrequently that we don't bother to back-fill it.
retrieved_entity_mbids = entities_info.keys()
reviews = [r for r in reviews if str(r["entity_id"]) in retrieved_entity_mbids]

entity_names = {
'artist': gettext('Artist'),
'release_group': gettext('Release Group'),
'label': gettext('Label'),
'recording': gettext('Recording'),
'place': gettext('Place'),
'event': gettext('Event'),
'work': gettext('Work'),
'bb_edition_group': gettext('Edition Group'),
'bb_literary_work': gettext('Literary Work'),
'bb_author': gettext('Author'),
'bb_series': gettext('Series'),
}

entity_names = db_review.ENTITY_TYPES_MAPPING


return render_template('user/reviews.html', section='reviews', user=user,
Expand Down
3 changes: 3 additions & 0 deletions critiquebrainz/ws/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ def create_app(debug=None, config_path=None):
else:
logging.warning("Redis is not defined in config file. Skipping initialization.")

from critiquebrainz.frontend import babel
babel.init_app(app, domain='cb_webservice')

# OAuth
from critiquebrainz.ws.oauth import oauth
oauth.init_app(app)
Expand Down
20 changes: 20 additions & 0 deletions critiquebrainz/ws/review/test/views_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,26 @@ def test_review_post(self):
resp = self.client.post('/review/', headers=self.header(self.another_user), data=json.dumps(review_3))
self.assert200(resp)

def test_review_same_entity_type(self):
"""Submitting a review for the same entity id from the same user results in an error
even if the review is drafted or hidden
"""
review = dict(
entity_id=self.review['entity_id'],
entity_type='release_group',
text=self.review['text'],
rating=str(self.review['rating']),
license_choice=self.license["id"],
language='en',
is_draft=True
)
resp = self.client.post('/review/', headers=self.header(self.user), data=json.dumps(review))
self.assert200(resp)

resp = self.client.post('/review/', headers=self.header(self.user), data=json.dumps(review))
self.assert400(resp)
self.assertEqual(resp.json['description'], 'You have already published a review for this Release Group')

def test_review_vote_entity(self):
review = self.create_dummy_review()
resp = self.client.get('/review/%s/vote' % review["id"], headers=self.header(self.user))
Expand Down
16 changes: 7 additions & 9 deletions critiquebrainz/ws/review/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,6 @@ def review_list_handler():

reviews = [db_review.to_dict(p) for p in reviews]


if include_metadata == 'true':
entities = [(str(review["entity_id"]), review["entity_type"]) for review in reviews]
entities_info = mbstore.get_multiple_entities(entities)
Expand All @@ -429,11 +428,10 @@ def review_list_handler():
for review in reviews:
review[review['entity_type']] = entities_info[str(review["entity_id"])]


if include_avg_rating:
if reviews and not entity_type:
entity_type = reviews[0]["entity_type"] if reviews else None
entity_type = reviews[0]["entity_type"] if reviews else None

if entity_type:
avg_rating_data = get_avg_rating(entity_id, entity_type)
if avg_rating_data:
Expand Down Expand Up @@ -470,10 +468,9 @@ def review_list_handler():
expirein=REVIEW_CACHE_TIMEOUT,
namespace=REVIEW_CACHE_NAMESPACE)


result = {"limit": limit, "offset": offset, "count": count, "reviews": reviews}
result = {"limit": limit, "offset": offset, "count": count, "reviews": reviews}
if include_avg_rating:
result["average_rating"] = avg_rating_data
result["average_rating"] = avg_rating_data
return jsonify(**result)


Expand Down Expand Up @@ -515,8 +512,9 @@ def fetch_params():
raise InvalidRequest(desc='Review must have either text or rating')
if language and language not in supported_languages:
raise InvalidRequest(desc='Unsupported language')
if db_review.list_reviews(user_id=user.id, entity_id=entity_id)[1]:
raise InvalidRequest(desc='You have already published a review for this album')
if db_review.list_reviews(inc_drafts=True, inc_hidden=True, entity_id=entity_id, user_id=user.id)[1]:
raise InvalidRequest(desc='You have already published a review for this {entity_name}'.format(
entity_name=db_review.ENTITY_TYPES_MAPPING[entity_type]))
return entity_id, entity_type, text, rating, license_choice, language, is_draft

if user.is_review_limit_exceeded:
Expand Down

0 comments on commit 48bb19a

Please sign in to comment.