Skip to content

Commit

Permalink
Merge pull request #193 from dpmittal/default_license
Browse files Browse the repository at this point in the history
CB-262: Ability to change default license selected when writing a review
  • Loading branch information
paramsingh committed Jun 16, 2018
2 parents 617bef4 + c9ff3c1 commit 5198e02
Show file tree
Hide file tree
Showing 14 changed files with 91 additions and 16 deletions.
11 changes: 11 additions & 0 deletions admin/schema_changes/10.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
BEGIN;

ALTER TABLE "user" ADD COLUMN license_choice VARCHAR;

ALTER TABLE "user"
ADD CONSTRAINT user_license_choice_fkey
FOREIGN KEY (license_choice)
REFERENCES license(id)
ON DELETE CASCADE;

COMMIT;
6 changes: 6 additions & 0 deletions admin/sql/create_foreign_keys.sql
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,10 @@ ALTER TABLE revision
REFERENCES review(id)
ON DELETE CASCADE;

ALTER TABLE "user"
ADD CONSTRAINT user_license_choice_fkey
FOREIGN KEY (license_choice)
REFERENCES license(id)
ON DELETE CASCADE;

COMMIT;
3 changes: 2 additions & 1 deletion admin/sql/create_tables.sql
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ CREATE TABLE "user" (
created TIMESTAMP NOT NULL,
musicbrainz_id VARCHAR,
show_gravatar BOOLEAN NOT NULL DEFAULT False,
is_blocked BOOLEAN NOT NULL DEFAULT False
is_blocked BOOLEAN NOT NULL DEFAULT False,
license_choice VARCHAR
);
ALTER TABLE "user" ADD CONSTRAINT user_musicbrainz_id_key UNIQUE (musicbrainz_id);

Expand Down
1 change: 1 addition & 0 deletions critiquebrainz/data/dump_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"musicbrainz_id",
"show_gravatar",
"is_blocked",
"license_choice",
"spam_reports",
"clients",
"grants",
Expand Down
2 changes: 1 addition & 1 deletion critiquebrainz/db/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from sqlalchemy.pool import NullPool

# This value must be incremented after schema changes on exported tables!
SCHEMA_VERSION = 9
SCHEMA_VERSION = 10
VALID_RATING_VALUES = [None, 1, 2, 3, 4, 5]
REVIEW_RATING_MAX = 5
REVIEW_RATING_MIN = 1
Expand Down
2 changes: 2 additions & 0 deletions critiquebrainz/db/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def __init__(self, user):
self.musicbrainz_username = user.get('musicbrainz_username')
self.show_gravatar = user.get('show_gravatar', False)
self.is_blocked = user.get('is_blocked', False)
self.license_choice = user.get('license_choice', None)

@property
def avatar(self):
Expand Down Expand Up @@ -110,6 +111,7 @@ def to_dict(self, includes=None, confidential=False):
avatar=self.avatar,
show_gravatar=self.show_gravatar,
musicbrainz_username=self.musicbrainz_username,
license_choice=self.license_choice,
))

if 'user_type' in includes:
Expand Down
38 changes: 27 additions & 11 deletions critiquebrainz/db/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ def get_many_by_mb_username(usernames):
email,
created,
musicbrainz_id,
show_gravatar
show_gravatar,
license_choice
FROM "user"
WHERE musicbrainz_id ILIKE ANY(:musicbrainz_usernames)
"""), {
Expand Down Expand Up @@ -91,7 +92,8 @@ def get_by_id(user_id):
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (bool),
"is_blocked": (bool)
"is_blocked": (bool),
"license_choice": (str)
}
"""
with db.engine.connect() as connection:
Expand All @@ -102,7 +104,8 @@ def get_by_id(user_id):
created,
musicbrainz_id,
show_gravatar,
is_blocked
is_blocked,
license_choice
FROM "user"
WHERE id = :user_id
"""), {
Expand All @@ -125,6 +128,7 @@ def create(**user_data):
email(str, optional): email of the user.
show_gravatar(bool, optional): whether to show gravatar(default: false).
is_blocked(bool, optional): whether user is blocked(default: false).
license_choice(str, optional): preferred license for reviews by the user
Returns:
Dictionary with the following structure:
Expand All @@ -135,21 +139,23 @@ def create(**user_data):
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (bool),
"is_blocked": (bool)
"is_blocked": (bool),
"license_choice": (str)
}
"""
display_name = user_data.pop('display_name')
musicbrainz_username = user_data.pop('musicbrainz_username', None)
email = user_data.pop('email', None)
show_gravatar = user_data.pop('show_gravatar', False)
is_blocked = user_data.pop('is_blocked', False)
license_choice = user_data.pop('license_choice', None)
if user_data:
raise TypeError('Unexpected **user_data: %r' % user_data)

with db.engine.connect() as connection:
result = connection.execute(sqlalchemy.text("""
INSERT INTO "user" (id, display_name, email, created, musicbrainz_id, show_gravatar, is_blocked)
VALUES (:id, :display_name, :email, :created, :musicbrainz_id, :show_gravatar, :is_blocked)
INSERT INTO "user" (id, display_name, email, created, musicbrainz_id, show_gravatar, is_blocked, license_choice)
VALUES (:id, :display_name, :email, :created, :musicbrainz_id, :show_gravatar, :is_blocked, :license_choice)
RETURNING id
"""), {
"id": str(uuid.uuid4()),
Expand All @@ -159,6 +165,7 @@ def create(**user_data):
"musicbrainz_id": musicbrainz_username,
"show_gravatar": show_gravatar,
"is_blocked": is_blocked,
"license_choice": license_choice,
})
new_id = result.fetchone()[0]
return get_by_id(new_id)
Expand All @@ -179,7 +186,8 @@ def get_by_mbid(musicbrainz_username):
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (bool),
"is_blocked": (bool)
"is_blocked": (bool),
"license_choice": (str)
}
"""
with db.engine.connect() as connection:
Expand All @@ -190,7 +198,8 @@ def get_by_mbid(musicbrainz_username):
created,
musicbrainz_id,
show_gravatar,
is_blocked
is_blocked,
license_choice
FROM "user"
WHERE musicbrainz_id = :musicbrainz_username
"""), {
Expand All @@ -216,6 +225,7 @@ def get_or_create(musicbrainz_username, new_user_data):
"email": email of the user(optional).
"show_gravatar": whether to show gravatar(default: false, optional).
"is_blocked": whether user is blocked(default: false, optional).
"license_choice": preferred license for reviews by the user(optional)
}
Returns:
Expand All @@ -227,7 +237,8 @@ def get_or_create(musicbrainz_username, new_user_data):
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (bool),
"is_blocked": (bool)
"is_blocked": (bool),
"license_choice": (str)
}
"""
user = get_by_mbid(musicbrainz_username)
Expand Down Expand Up @@ -268,7 +279,8 @@ def list_users(limit=None, offset=0):
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (bool),
"is_blocked": (bool)
"is_blocked": (bool),
"license_choice": (str)
}
"""
with db.engine.connect() as connection:
Expand All @@ -279,7 +291,8 @@ def list_users(limit=None, offset=0):
created,
musicbrainz_id,
show_gravatar,
is_blocked
is_blocked,
license_choice
FROM "user"
LIMIT :limit
OFFSET :offset
Expand Down Expand Up @@ -521,6 +534,7 @@ def update(user_id, user_new_info):
"display_name": (str),
"show_gravatar": (bool),
"email": (str)
"license_choice": (str)
}
"""
updates = []
Expand All @@ -530,6 +544,8 @@ def update(user_id, user_new_info):
updates.append("show_gravatar = :show_gravatar")
if "email" in user_new_info:
updates.append("email = :email")
if "license_choice" in user_new_info:
updates.append("license_choice = :license_choice")

setstr = ", ".join(updates)
query = sqlalchemy.text("""UPDATE "user"
Expand Down
6 changes: 5 additions & 1 deletion critiquebrainz/frontend/forms/profile.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from flask_wtf import Form
from flask_babel import lazy_gettext
from wtforms import StringField, BooleanField, validators
from wtforms import StringField, BooleanField, RadioField, validators
from wtforms.fields.html5 import EmailField


Expand All @@ -13,3 +13,7 @@ class ProfileEditForm(Form):
validators.Optional(strip_whitespace=False),
validators.Email(message=lazy_gettext("Email field is not a valid email address."))])
show_gravatar = BooleanField(lazy_gettext("Show my Gravatar"))
license_choice = RadioField(lazy_gettext("Preferred License Choice"), choices=[
('CC BY-SA 3.0', lazy_gettext('Allow commercial use of my reviews (<a href="https://creativecommons.org/licenses/by-sa/3.0/" target="_blank">CC BY-SA 3.0 license</a>)')), # noqa: E501
('CC BY-NC-SA 3.0', lazy_gettext('Do not allow commercial use of my reviews, unless approved by MetaBrainz Foundation (<a href="https://creativecommons.org/licenses/by-nc-sa/3.0/" target="_blank">CC BY-NC-SA 3.0 license</a>)')), # noqa: E501
])
1 change: 1 addition & 0 deletions critiquebrainz/frontend/forms/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class ReviewEditForm(Form):
('CC BY-NC-SA 3.0', lazy_gettext('Do not allow commercial use of this review, unless approved by MetaBrainz Foundation (<a href="https://creativecommons.org/licenses/by-nc-sa/3.0/" target="_blank">CC BY-NC-SA 3.0 license</a>)')), # noqa: E501
],
validators=[validators.DataRequired(message=lazy_gettext("You need to choose a license!"))])
remember_license = BooleanField(lazy_gettext("Remember this license choice for further preference"))
language = SelectField(lazy_gettext("You need to accept the license agreement!"), choices=languages)
rating = IntegerField(lazy_gettext("Rating"), widget=Input(input_type='number'), validators=[validators.Optional()])

Expand Down
10 changes: 10 additions & 0 deletions critiquebrainz/frontend/templates/profile/edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ <h2>{{ _('Edit profile') }}</h2>
</div>
</div>
</div>
<div class="form-group">
<label class="col-sm-2 control-label">{{ form.license_choice.label.text }}</label>
<div class="col-sm-6">
{% for choice in form.license_choice %}
<div class="radio">
<label>{{ choice }}{{ choice.label.text | safe }}</label>
</div>
{% endfor %}
</div>
</div>
</fieldset>
<div class="form-group">
<div class="col-sm-offset-2 col-sm-10">
Expand Down
10 changes: 10 additions & 0 deletions critiquebrainz/frontend/templates/review/modify/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@
<br /><small class="text-warning"><em>{{ _('You cannot change the license after the review is published.') }}</em></small>
</div>
</div>
{% if not current_user.license_choice %}
<div class="form-group">
<div class="col-sm-12">
<div class="checkbox">
{{ form.remember_license }}
</div>
<label class="col-sm-4"> {{ _('Remember this license choice for future reviews') }} </label>
</div>
</div>
{% endif %}
{% endif %}
{% block license_agreement_input %}{% endblock %}
</form>
Expand Down
2 changes: 2 additions & 0 deletions critiquebrainz/frontend/views/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ def edit():
"display_name": form.display_name.data,
"email": form.email.data,
"show_gravatar": form.show_gravatar.data,
"license_choice": form.license_choice.data,
})
flash.success(gettext("Profile updated."))
return redirect(url_for('user.reviews', user_id=current_user.id))
else:
form.display_name.data = current_user.display_name
form.email.data = current_user.email
form.show_gravatar.data = current_user.show_gravatar
form.license_choice.data = current_user.license_choice
return render_template('profile/edit.html', form=form)


Expand Down
7 changes: 6 additions & 1 deletion critiquebrainz/frontend/views/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import critiquebrainz.db.spam_report as db_spam_report
import critiquebrainz.db.review as db_review
import critiquebrainz.db.moderation_log as db_moderation_log
import critiquebrainz.db.users as db_users
from critiquebrainz.frontend.external.musicbrainz_db.entities import get_multiple_entities, get_entity_by_id


Expand Down Expand Up @@ -223,7 +224,7 @@ def create():
flash.error(gettext("You have already published a review for this entity!"))
return redirect(url_for('review.entity', id=review["id"]))

form = ReviewCreateForm(default_language=get_locale())
form = ReviewCreateForm(default_license_id=current_user.license_choice, default_language=get_locale())

if form.validate_on_submit():
if current_user.is_review_limit_exceeded:
Expand All @@ -236,6 +237,10 @@ def create():
review = db_review.create(user_id=current_user.id, entity_id=entity_id, entity_type=entity_type,
text=form.text.data, rating=form.rating.data, license_id=form.license_choice.data,
language=form.language.data, is_draft=is_draft)
if form.remember_license.data:
db_users.update(current_user.id, user_new_info={
"license_choice": form.license_choice.data,
})
if is_draft:
flash.success(gettext("Review has been saved!"))
else:
Expand Down
8 changes: 7 additions & 1 deletion critiquebrainz/frontend/views/test/test_profile.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
from critiquebrainz.frontend.testing import FrontendTestCase
from critiquebrainz.db.user import User
import critiquebrainz.db.users as db_users
import critiquebrainz.db.license as db_license


class ProfileViewsTestCase(FrontendTestCase):

def setUp(self):
super(ProfileViewsTestCase, self).setUp()
self.license = db_license.create(
id="CC BY-SA 3.0",
full_name="Created so we can fill the form correctly.",
)
self.user = User(db_users.get_or_create("aef06569-098f-4218-a577-b413944d9493", new_user_data={
"display_name": "Tester",
}))
Expand All @@ -15,7 +20,8 @@ def test_edit(self):
data = dict(
display_name="Some User",
email='someuser@somesite.com',
show_gravatar='True'
show_gravatar='True',
license_choice=self.license["id"]
)

response = self.client.post('/profile/edit', data=data,
Expand Down

0 comments on commit 5198e02

Please sign in to comment.