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-428: Remove Gravatar #399

Merged
merged 3 commits into from Mar 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions admin/schema_changes/20.sql
@@ -0,0 +1,5 @@
BEGIN;

ALTER TABLE "user" DROP COLUMN "show_gravatar";

COMMIT;
1 change: 0 additions & 1 deletion admin/sql/create_tables.sql
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
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
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
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
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
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
58 changes: 3 additions & 55 deletions critiquebrainz/db/users.py
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_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 Down Expand Up @@ -122,7 +82,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 +98,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 +109,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 +124,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 +135,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 +157,6 @@ def get_by_mbid(musicbrainz_username):
"email": (str),
"created": (datetime),
"musicbrainz_username": (str),
"show_gravatar": (bool),
"is_blocked": (bool),
"license_choice": (str)
}
Expand Down Expand Up @@ -234,7 +188,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 +200,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 +246,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 Down Expand Up @@ -587,16 +538,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
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
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
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
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
4 changes: 2 additions & 2 deletions critiquebrainz/frontend/templates/index/index.html
Expand Up @@ -62,7 +62,7 @@ <h3 style="margin-bottom:-5px;margin-top:0px;">{{ _('Popular reviews') }}</h3>
{% set entity = review.entity_id | entity_details(type=review.entity_type) %}
<div class="album">{% include 'entity_review.html' %}</div>
<div class="reviewer">
<p>{{ review_credit(review, user_picture_size=16) }}</p>
<p>{{ review_credit(review) }}</p>
</div>
<div class="preview">{{ review.preview[:250] }}{% if review.preview|length > 250 %}...{% endif %}</div>
</div>
Expand All @@ -89,7 +89,7 @@ <h4>{{ _('Recent reviews') }}</h4>
{% set entity = review.entity_id | entity_details(type=review.entity_type) %}
<div class="album">{% include 'entity_review.html' %}</div>
<div class="reviewer">
<p>{{ review_credit(review, user_picture_size=16) }}</p>
<p>{{ review_credit(review) }}</p>
</div>
</div>
{% endfor %}
Expand Down
2 changes: 1 addition & 1 deletion critiquebrainz/frontend/templates/label/entity.html
Expand Up @@ -45,7 +45,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