Skip to content

Commit

Permalink
Merge branch 'CB-428'
Browse files Browse the repository at this point in the history
  • Loading branch information
alastair committed Mar 8, 2022
2 parents 431b91b + 3ebeedb commit 517aa1c
Show file tree
Hide file tree
Showing 30 changed files with 32 additions and 146 deletions.
5 changes: 5 additions & 0 deletions admin/schema_changes/20.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
BEGIN;

ALTER TABLE "user" DROP COLUMN "show_gravatar";

COMMIT;
1 change: 0 additions & 1 deletion admin/sql/create_tables.sql
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ CREATE TABLE "user" (
created TIMESTAMP NOT NULL,
musicbrainz_id VARCHAR,
musicbrainz_row_id INTEGER,
show_gravatar BOOLEAN NOT NULL DEFAULT False,
is_blocked BOOLEAN NOT NULL DEFAULT False,
license_choice VARCHAR
);
Expand Down
1 change: 0 additions & 1 deletion critiquebrainz/data/dump_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"email",
"created",
"musicbrainz_id",
"show_gravatar",
"is_blocked",
"license_choice",
"spam_reports",
Expand Down
4 changes: 0 additions & 4 deletions critiquebrainz/db/comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ def get_by_id(comment_id):
"user".email,
"user".created as user_created,
"user".display_name,
"user".show_gravatar,
"user".musicbrainz_id,
"user".is_blocked
FROM comment c
Expand Down Expand Up @@ -115,7 +114,6 @@ def get_by_id(comment_id):
'display_name': comment.pop('display_name'),
'email': comment.pop('email'),
'created': comment.pop('user_created'),
'show_gravatar': comment.pop('show_gravatar'),
'musicbrainz_username': comment.pop('musicbrainz_id'),
'is_blocked': comment.pop('is_blocked')
})
Expand Down Expand Up @@ -168,7 +166,6 @@ def list_comments(*, review_id=None, user_id=None, limit=20, offset=0, inc_hidde
"user".email,
"user".created as user_created,
"user".display_name,
"user".show_gravatar,
"user".musicbrainz_id,
"user".is_blocked,
MIN(comment_revision.timestamp) as created,
Expand Down Expand Up @@ -209,7 +206,6 @@ def list_comments(*, review_id=None, user_id=None, limit=20, offset=0, inc_hidde
row['user'] = User({
'id': row['user_id'],
'display_name': row.pop('display_name'),
'show_gravatar': row.pop('show_gravatar'),
'is_blocked': row.pop('is_blocked'),
'musicbrainz_username': row.pop('musicbrainz_id'),
'email': row.pop('email'),
Expand Down
4 changes: 0 additions & 4 deletions critiquebrainz/db/review.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ def get_by_id(review_id):
"user".email,
"user".created as user_created,
"user".display_name,
"user".show_gravatar,
"user".musicbrainz_id,
"user".is_blocked,
license.full_name,
Expand Down Expand Up @@ -133,7 +132,6 @@ def get_by_id(review_id):
"id": review["user_id"],
"display_name": review.pop("display_name", None),
"is_blocked": review.pop("is_blocked", False),
"show_gravatar": review.pop("show_gravatar", False),
"musicbrainz_username": review.pop("musicbrainz_id"),
"email": review.pop("email"),
"created": review.pop("user_created"),
Expand Down Expand Up @@ -438,7 +436,6 @@ def get_reviews_list(connection, *, inc_drafts=False, inc_hidden=False, entity_i
review.source_url,
review.user_id,
"user".display_name,
"user".show_gravatar,
"user".is_blocked,
"user".email,
"user".created as user_created,
Expand Down Expand Up @@ -495,7 +492,6 @@ def get_reviews_list(connection, *, inc_drafts=False, inc_hidden=False, entity_i
row["user"] = User({
"id": row["user_id"],
"display_name": row.pop("display_name"),
"show_gravatar": row.pop("show_gravatar"),
"is_blocked": row.pop("is_blocked"),
"musicbrainz_username": row.pop("musicbrainz_id"),
"email": row.pop("email"),
Expand Down
8 changes: 1 addition & 7 deletions critiquebrainz/db/test/users_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import critiquebrainz.db.vote as db_vote
from critiquebrainz.data.testing import DataTestCase
from critiquebrainz.db.user import User
from critiquebrainz.db.users import gravatar_url, get_many_by_mb_username
from critiquebrainz.db.users import get_many_by_mb_username


class UserTestCase(DataTestCase):
Expand Down Expand Up @@ -51,12 +51,6 @@ def test_get_many_by_mb_username(self):
self.assertEqual(users, dbresults)
self.assertEqual(get_many_by_mb_username([]), [])

def test_avatar(self):
self.assertEqual(
gravatar_url("test2"),
"https://gravatar.com/avatar/ad0234829205b9033196ba818f7a872b?d=identicon&r=pg"
)

def test_get_by_id(self):
user = db_users.get_by_id(self.user1.id)
self.assertEqual(user['display_name'], "test")
Expand Down
10 changes: 0 additions & 10 deletions critiquebrainz/db/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,10 @@ def __init__(self, user):
self.email = user.get('email')
self.created = user.get('created')
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)
self.musicbrainz_row_id = user.get('musicbrainz_row_id', None)

@property
def avatar(self):
"""Link to user's avatar image."""
if self.show_gravatar and self.email:
return db_users.gravatar_url(self.email)
return db_users.gravatar_url(self.id)

@property
def is_vote_limit_exceeded(self):
return self.votes_today_count() >= self.user_type.votes_per_day
Expand Down Expand Up @@ -125,8 +117,6 @@ def to_dict(self, includes=None, confidential=False):
if confidential is True:
response.update(dict(
email=self.email,
avatar=self.avatar,
show_gravatar=self.show_gravatar,
musicbrainz_username=self.musicbrainz_username,
license_choice=self.license_choice,
))
Expand Down
64 changes: 4 additions & 60 deletions critiquebrainz/db/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,45 +8,13 @@
from critiquebrainz.db import revision as db_revision


def gravatar_url(source, default="identicon", rating="pg"):
"""Generates Gravatar URL from a source ID.
Source string is hashed using the MD5 algorithm and included into a
resulting URL. User's privacy should be considered when using this. For
example, if using an email address, make sure that user explicitly allowed
that.
See https://en.gravatar.com/site/implement/images/ for implementation
details.
Args:
source (str): String to be hashed and used as an avatar ID with the
Gravatar service. Can be email, MusicBrainz username, or any other
unique identifier.
default (str): Default image to use if image for specified source is
not found. Can be one of defaults provided by Gravatar (referenced
by a keyword) or a publicly available image (as a full URL).
rating (string): One of predefined audience rating values. Current
default is recommended.
Returns:
URL to the Gravatar image.
"""
return "https://gravatar.com/avatar/{hash}?d={default}&r={rating}".format(
hash=md5(source.encode('utf-8')).hexdigest(),
default=default,
rating=rating,
)


USER_GET_COLUMNS = [
'id',
'display_name',
'email',
'created',
'musicbrainz_id',
'musicbrainz_id as musicbrainz_username',
'musicbrainz_row_id',
'show_gravatar',
'license_choice',
'is_blocked',
]
Expand All @@ -59,8 +27,7 @@ def get_many_by_mb_username(usernames):
usernames (list): A list of MusicBrainz usernames. This lookup is case-insensetive.
Returns:
All columns of 'user' table (list of dictionaries)
and avatar_url (Gravatar url).
All columns of 'user' table (list of dictionaries).
If the 'usernames' variable is an empty list function returns it back.
"""
if not usernames:
Expand All @@ -76,13 +43,6 @@ def get_many_by_mb_username(usernames):
})
row = result.fetchall()
users = [dict(r) for r in row]
for user in users:
default_gravatar_src = "%s@cb" % user["id"]
user["musicbrainz_username"] = user.pop("musicbrainz_id")
if user["show_gravatar"]:
user["avatar_url"] = gravatar_url(user.get("email") or default_gravatar_src)
else:
user["avatar_url"] = gravatar_url(default_gravatar_src)
return users


Expand All @@ -104,7 +64,6 @@ def get_user_by_id(connection, user_id):
if not row:
return None
row = dict(row)
row['musicbrainz_username'] = row.pop('musicbrainz_id')
return row


Expand All @@ -122,7 +81,6 @@ def get_by_id(user_id):
"email": (str),
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (bool),
"is_blocked": (bool),
"license_choice": (str)
}
Expand All @@ -139,7 +97,6 @@ def create(**user_data):
musicbrainz_username(str, optional): musicbrainz username of user.
musicbrainz_row_id (int): the MusicBrainz row ID of the user,
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
Expand All @@ -151,15 +108,13 @@ def create(**user_data):
"email": (str),
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (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)
musicbrainz_row_id = user_data.pop('musicbrainz_row_id', None)
Expand All @@ -168,9 +123,9 @@ def create(**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,
INSERT INTO "user" (id, display_name, email, created, musicbrainz_id,
is_blocked, license_choice, musicbrainz_row_id)
VALUES (:id, :display_name, :email, :created, :musicbrainz_id, :show_gravatar,
VALUES (:id, :display_name, :email, :created, :musicbrainz_id,
:is_blocked, :license_choice, :musicbrainz_row_id)
RETURNING id
"""), {
Expand All @@ -179,7 +134,6 @@ def create(**user_data):
"email": email,
"created": datetime.now(),
"musicbrainz_id": musicbrainz_username,
"show_gravatar": show_gravatar,
"is_blocked": is_blocked,
"license_choice": license_choice,
"musicbrainz_row_id": musicbrainz_row_id,
Expand All @@ -202,7 +156,6 @@ def get_by_mbid(musicbrainz_username):
"email": (str),
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (bool),
"is_blocked": (bool),
"license_choice": (str)
}
Expand All @@ -219,7 +172,6 @@ def get_by_mbid(musicbrainz_username):
if not row:
return None
row = dict(row)
row['musicbrainz_username'] = row.pop('musicbrainz_id')
return row


Expand All @@ -234,7 +186,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).
"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)
}
Expand All @@ -247,7 +198,6 @@ def get_or_create(musicbrainz_row_id, musicbrainz_username, new_user_data):
"email": (str),
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (bool),
"is_blocked": (bool),
"license_choice": (str)
}
Expand Down Expand Up @@ -294,7 +244,6 @@ def list_users(limit=None, offset=0):
"email": (str),
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (bool),
"is_blocked": (bool),
"license_choice": (str),
"musicbrainz_row_id": (int),
Expand All @@ -312,8 +261,6 @@ def list_users(limit=None, offset=0):
})
rows = result.fetchall()
rows = [dict(row) for row in rows]
for row in rows:
row['musicbrainz_username'] = row.pop('musicbrainz_id')
return rows


Expand Down Expand Up @@ -587,16 +534,13 @@ def update(user_id, user_new_info):
user_new_info: Dictionary containing the new information for the user
{
"display_name": (str),
"show_gravatar": (bool),
"email": (str)
"license_choice": (str)
}
"""
updates = []
if "display_name" in user_new_info:
updates.append("display_name = :display_name")
if "show_gravatar" in 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:
Expand Down
1 change: 0 additions & 1 deletion critiquebrainz/frontend/forms/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ 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."))])
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
Expand Down
15 changes: 0 additions & 15 deletions critiquebrainz/frontend/static/styles/main.less
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,6 @@ body {
}
}

img.avatar {
display: inline;
vertical-align: bottom;
border: 1px solid #ddd;
padding: 1px;
background: #fff;
}

img.cover-art {
max-width: 250px;
}
Expand Down Expand Up @@ -577,13 +569,6 @@ a#edit-review { margin-top: 20px; }


// Moderators list page
#moderators-list {
img.avatar {
margin-top: 2px;
height: 20px;
}
}

nav {
.glyphicon {
padding-right: 5px;
Expand Down
2 changes: 1 addition & 1 deletion critiquebrainz/frontend/templates/artist/entity.html
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ <h4 style="margin-bottom:0;">{{ _('Reviews') }}</h4>
<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) }}
{{ _('by %(reviewer)s', reviewer=review.user.display_name) }}
</a>
</td>
<td>{{ review.published_on | date }}</td>
Expand Down
2 changes: 1 addition & 1 deletion critiquebrainz/frontend/templates/event/entity.html
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ <h4 style="margin-bottom:0;">{{ _('Reviews') }}</h4>
<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) }}
{{ _('by %(reviewer)s', reviewer=review.user.display_name) }}
</a>
</td>
<td>{{ review.published_on | date }}</td>
Expand Down
Loading

0 comments on commit 517aa1c

Please sign in to comment.