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-394: Do not allow CC BY-NC-SA license for future reviews #474

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions admin/schema_changes/24.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
BEGIN;

INSERT INTO license (id, full_name, info_url)
VALUES ('CC BY-SA 4.0',
'Creative Commons Attribution-ShareAlike 4.0 International',
'https://creativecommons.org/licenses/by-sa/4.0/');

UPDATE "user"
SET license_choice = NULL;

COMMIT;
5 changes: 5 additions & 0 deletions critiquebrainz/data/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ class LicenseData:
full_name="Creative Commons Attribution-NonCommercial-ShareAlike 3.0 Unported",
info_url="https://creativecommons.org/licenses/by-nc-sa/3.0/",
)
cc_by_sa_4 = dict(
id="CC BY-SA 4.0",
full_name="Creative Commons Attribution-ShareAlike 4.0 International",
info_url="https://creativecommons.org/licenses/by-sa/4.0/",
)


# Include all objects into this tuple.
Expand Down
427 changes: 427 additions & 0 deletions critiquebrainz/data/licenses/cc-by-sa-40.txt

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion critiquebrainz/db/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from critiquebrainz.db.user import User

REVIEW_CACHE_NAMESPACE = "Review"
DEFAULT_LICENSE_ID = "CC BY-SA 3.0"
DEFAULT_LICENSE_ID = "CC BY-SA 4.0"
DEFAULT_LANG = "en"

#: list of allowed entity_type's for writing/querying a review
Expand Down
4 changes: 2 additions & 2 deletions critiquebrainz/db/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ def __init__(self, user):
self.musicbrainz_username = user.get('musicbrainz_username')
self.user_ref = user.get('user_ref')
self.is_blocked = user.get('is_blocked', False)
self.license_choice = user.get('license_choice', None)
self.musicbrainz_row_id = user.get('musicbrainz_row_id', None)

@property
Expand Down Expand Up @@ -120,7 +119,8 @@ def to_dict(self, includes=None, confidential=False):
if confidential is True:
response.update(dict(
email=self.email,
license_choice=self.license_choice,
musicbrainz_username=self.musicbrainz_username,
user_ref=self.user_ref,
))

if 'user_type' in includes:
Expand Down
25 changes: 6 additions & 19 deletions critiquebrainz/db/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
'musicbrainz_id as musicbrainz_username',
'musicbrainz_row_id',
'COALESCE(musicbrainz_id, id::text) AS user_ref',
'license_choice',
'is_blocked',
]

Expand Down Expand Up @@ -101,8 +100,7 @@ def get_by_id(user_id):
"email": (str),
"created": (datetime),
"musicbrainz_username": (str),
"is_blocked": (bool),
"license_choice": (str)
"is_blocked": (bool)
}
"""
with db.engine.connect() as connection:
Expand All @@ -118,7 +116,6 @@ def create(**user_data):
musicbrainz_row_id (int): the MusicBrainz row ID of the user,
email(str, optional): email of the user.
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 @@ -128,25 +125,23 @@ def create(**user_data):
"email": (str),
"created": (datetime),
"musicbrainz_username": (str),
"is_blocked": (bool),
"license_choice": (str)
"is_blocked": (bool)
}
"""
display_name = user_data.pop('display_name')
musicbrainz_username = user_data.pop('musicbrainz_username', None)
email = user_data.pop('email', None)
is_blocked = user_data.pop('is_blocked', False)
license_choice = user_data.pop('license_choice', None)
musicbrainz_row_id = user_data.pop('musicbrainz_row_id', None)
if user_data:
raise TypeError('Unexpected **user_data: %r' % user_data)

with db.engine.begin() as connection:
result = connection.execute(sqlalchemy.text("""
INSERT INTO "user" (id, display_name, email, created, musicbrainz_id,
is_blocked, license_choice, musicbrainz_row_id)
is_blocked, musicbrainz_row_id)
VALUES (:id, :display_name, :email, :created, :musicbrainz_id,
:is_blocked, :license_choice, :musicbrainz_row_id)
:is_blocked, :musicbrainz_row_id)
RETURNING id
"""), {
"id": str(uuid.uuid4()),
Expand All @@ -155,7 +150,6 @@ def create(**user_data):
"created": datetime.now(),
"musicbrainz_id": musicbrainz_username,
"is_blocked": is_blocked,
"license_choice": license_choice,
"musicbrainz_row_id": musicbrainz_row_id,
})
new_id = result.fetchone().id
Expand All @@ -176,8 +170,7 @@ def get_by_mbid(musicbrainz_username):
"email": (str),
"created": (datetime),
"musicbrainz_username": (str),
"is_blocked": (bool),
"license_choice": (str)
"is_blocked": (bool)
}
"""
with db.engine.connect() as connection:
Expand Down Expand Up @@ -206,7 +199,6 @@ def get_or_create(musicbrainz_row_id, musicbrainz_username, new_user_data):
"display_name": Display name of the user.
"email": email of the user(optional).
"is_blocked": whether user is blocked(default: false, optional).
"license_choice": preferred license for reviews by the user(optional)
}

Returns:
Expand All @@ -217,8 +209,7 @@ def get_or_create(musicbrainz_row_id, musicbrainz_username, new_user_data):
"email": (str),
"created": (datetime),
"musicbrainz_username": (str),
"is_blocked": (bool),
"license_choice": (str)
"is_blocked": (bool)
}
"""
user = get_by_mb_row_id(musicbrainz_row_id, musicbrainz_username)
Expand Down Expand Up @@ -297,7 +288,6 @@ def list_users(limit=None, offset=0):
"created": (datetime),
"musicbrainz_username": (str),
"is_blocked": (bool),
"license_choice": (str),
"musicbrainz_row_id": (int),
}
"""
Expand Down Expand Up @@ -586,16 +576,13 @@ def update(user_id, user_new_info):
{
"display_name": (str),
"email": (str)
"license_choice": (str)
}
"""
updates = []
if "display_name" in user_new_info:
updates.append("display_name = :display_name")
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: 1 addition & 5 deletions critiquebrainz/frontend/forms/profile.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from flask_wtf import FlaskForm
from flask_babel import lazy_gettext
from wtforms import StringField, BooleanField, RadioField, validators
from wtforms import StringField, validators
from wtforms.fields import EmailField


Expand All @@ -12,7 +12,3 @@ class ProfileEditForm(FlaskForm):
email = EmailField(lazy_gettext("Email"), [
validators.Optional(strip_whitespace=False),
validators.Email(message=lazy_gettext("Email field is not a valid email address."))])
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
])
9 changes: 1 addition & 8 deletions critiquebrainz/frontend/forms/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,11 @@ class ReviewEditForm(FlaskForm):
StateAndLength(min=MIN_REVIEW_LENGTH, max=MAX_REVIEW_LENGTH,
message=lazy_gettext("Text length needs to be between %(min)d and %(max)d characters.",
min=MIN_REVIEW_LENGTH, max=MAX_REVIEW_LENGTH))])
license_choice = RadioField(
choices=[
('CC BY-SA 3.0', lazy_gettext('Allow commercial use of this review(<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 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.InputRequired(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()])

def __init__(self, default_license_id='CC BY-SA 3.0', default_language='en', **kwargs):
kwargs.setdefault('license_choice', default_license_id)
def __init__(self, default_language='en', **kwargs):
kwargs.setdefault('language', default_language)
FlaskForm.__init__(self, **kwargs)

Expand Down
10 changes: 0 additions & 10 deletions critiquebrainz/frontend/templates/profile/edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,6 @@ <h2>{{ _('Edit profile') }}</h2>
<label class="col-sm-2 control-label">{{ form.email.label.text }}</label>
<div class="col-sm-4">{{ form.email(class="form-control") }}</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
22 changes: 6 additions & 16 deletions critiquebrainz/frontend/templates/review/modify/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,15 @@
</div>
<hr />
{% if review is not defined or review.is_draft %}
<div class="form-group">
<div class="col-sm-12">
{% for choice in form.license_choice %}
<div class="radio">
<label>{{ choice }}{{ choice.label.text | safe }}</label>
</div>
{% endfor %}
<br /><small class="text-warning"><em>{{ _('You cannot change the license after the review is published.') }}</em></small>
{% if not current_user.license_choice %}
<br /><br />
<div class="checkbox">
<label>{{ form.remember_license }}{{ form.remember_license.label.text }}</label>
</div>
{% endif %}
</div>
</div>
{% if review is defined and review['license'] %}
<span>{{ _('You will be posting this review under ') }}<a href="{{ review['license']['info_url'] }}" target="_blank">{{review['license']['id']}}</a> license.</span>
{% else %}
<span>{{ _('You will be posting this review under ') }}<a href="https://creativecommons.org/licenses/by-sa/4.0/" target="_blank">CC BY-SA 4.0</a> license.</span>
{% endif %}
{% endif %}
{% block license_agreement_input %}{% endblock %}
</form>
<br/>
{% block buttons %}{% endblock %}
{% endblock %}

Expand Down
3 changes: 1 addition & 2 deletions critiquebrainz/frontend/templates/review/modify/write.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ <h2>{{ _('Write review') }}</h2>
<label id="agreement">
{{ form.agreement }}
{{ _('You acknowledge and agree that your contributed reviews to CritiqueBrainz are licensed under a
Creative Commons Attribution-ShareAlike 3.0 Unported or Creative Commons Attribution-ShareAlike-NonCommercial
license as per your choice above. You agree to license your work under this license.
Creative Commons Attribution-ShareAlike 4.0 International license. You agree to license your work under this license.
You represent and warrant that you own or control all rights in and to the work, that nothing in the work
infringes the rights of any third-party, and that you have the permission to use and to license the work
under the selected Creative Commons license. Finally, you give the MetaBrainz Foundation permission to license
Expand Down
4 changes: 1 addition & 3 deletions critiquebrainz/frontend/views/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ def edit():
if form.validate_on_submit():
db_users.update(current_user.id, user_new_info={
"display_name": form.display_name.data,
"email": form.email.data,
"license_choice": form.license_choice.data,
"email": form.email.data
})
flash.success(gettext("Profile updated."))
return redirect(url_for('user.reviews', user_ref= current_user.user_ref))

form.display_name.data = current_user.display_name
form.email.data = current_user.email
form.license_choice.data = current_user.license_choice
return render_template('profile/edit.html', form=form)


Expand Down
13 changes: 3 additions & 10 deletions critiquebrainz/frontend/views/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,19 +326,15 @@ def create(entity_type=None, entity_id=None):
flash.error(gettext("You have exceeded your limit of reviews per day."))
return redirect(url_for('user.reviews', user_ref=current_user.user_ref))

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

if form.validate_on_submit():
is_draft = form.state.data == 'draft'
if form.text.data == '':
form.text.data = None
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,
text=form.text.data, rating=form.rating.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 Expand Up @@ -382,9 +378,6 @@ def edit(id):
"entity_type": review["entity_type"],
"review": review,
}
if not review["is_draft"]:
Copy link
Member

Choose a reason for hiding this comment

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

IIUC, this PR currently doesn't show users the option to upgrade from CC SA v3 to CC SA v4. I don't remember whether we decided the move should be opt-in or mandatory but in any case a dialog should be shown to the user. Also, even when doing that need to confirm with @alastair whether CC NC can be upgraded to CC SA or not and accordingly a check may be needed.

Till that is implemented in this or a future PR, we should keep this if check.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We decided to not worry about asking to upgrade. We'll leave old ones as the current license. If we find that people are asking to upgrade their licenses, the we can add the feature

# Can't change license if review is published.
del form.license_choice

# The purpose of the check is to not create unnecessary revisions. So updating a draft review
# without changes or editing a published review without changes is not allowed. But publishing
Expand All @@ -397,7 +390,7 @@ def edit(id):
form.form_errors.append("You must edit either text or rating to update the review.")
elif form.validate_on_submit():
if review["is_draft"]:
license_choice = form.license_choice.data
license_choice = db_review.DEFAULT_LICENSE_ID
else:
license_choice = None
if form.text.data == '':
Expand Down
2 changes: 1 addition & 1 deletion critiquebrainz/frontend/views/test/test_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def setUp(self):
"display_name": u"Commenter",
}))
self.license = db_license.create(
id="CC BY-SA 3.0",
id="CC BY-SA 4.0",
full_name="Test License.",
)
self.review = db_review.create(
Expand Down
5 changes: 2 additions & 3 deletions critiquebrainz/frontend/views/test/test_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class ProfileViewsTestCase(FrontendTestCase):
def setUp(self):
super(ProfileViewsTestCase, self).setUp()
self.license = db_license.create(
id="CC BY-SA 3.0",
id="CC BY-SA 4.0",
full_name="Created so we can fill the form correctly.",
)
self.user = User(db_users.get_or_create(1, "Tester", new_user_data={
Expand All @@ -19,8 +19,7 @@ def setUp(self):
def test_edit(self):
data = dict(
display_name="Some User",
email='someuser@somesite.com',
license_choice=self.license["id"]
email='someuser@somesite.com'
)

response = self.client.post('/profile/edit', data=data,
Expand Down
2 changes: 1 addition & 1 deletion critiquebrainz/frontend/views/test/test_rate.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def setUp(self):
"display_name": u"Reviewer",
}))
self.license = db_license.create(
id="CC BY-SA 3.0",
id="CC BY-SA 4.0",
full_name="Test License.",
)

Expand Down
5 changes: 1 addition & 4 deletions critiquebrainz/frontend/views/test/test_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def setUp(self):
"display_name": u"Hacker!",
}))
self.license = db_license.create(
id="CC BY-SA 3.0",
id="CC BY-SA 4.0",
full_name="Created so we can fill the form correctly.",
)
self.review_text = "Testing! This text should be on the page."
Expand Down Expand Up @@ -105,7 +105,6 @@ def test_create(self):
entity_type='release_group',
state='draft',
text=self.review_text,
license_choice=self.license["id"],
language='en',
agreement='True'
)
Expand Down Expand Up @@ -153,7 +152,6 @@ def test_edit_draft_without_changes(self):
data = {
review["entity_type"]: review["entity_id"],
"state": "draft",
"license_choice": self.license["id"],
"language": 'en',
"agreement": 'True',
"text": review["text"],
Expand Down Expand Up @@ -186,7 +184,6 @@ def test_edit(self):
release_group="0cef1de0-6ff1-38e1-80cb-ff11ee2c69e2",
state='publish',
text=updated_text,
license_choice=self.license["id"],
language='en',
agreement='True'
)
Expand Down
2 changes: 1 addition & 1 deletion critiquebrainz/frontend/views/test/test_statistics.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def setUp(self):
"display_name": u"Tester",
}))
self.license = db_license.create(
id="CC BY-SA 3.0",
id="CC BY-SA 4.0",
full_name="Test License.",
)

Expand Down
Loading