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

Remplacement de Gravatar par Jdenticon pour les avatars par défaut #6609

Merged
merged 3 commits into from
Oct 5, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ function jsPackages() {
require.resolve('chartjs-adapter-moment/dist/chartjs-adapter-moment.min.js'),
require.resolve('chart.js/dist/chart.min.js'),
require.resolve('easymde/dist/easymde.min.js'),
require.resolve('jdenticon/standalone'),
path.resolve('node_modules/mathjax/unpacked/**')
], { sourcemaps: true })
.pipe(gulp.dest('dist/js/', { sourcemaps: '.' }))
Expand Down
2 changes: 1 addition & 1 deletion assets/scss/components/_topic-message.scss
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ div.msg-are-hidden {
overflow: hidden;
}

img {
.avatar {
height: $length-48;
width: $length-48;

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"gulp-postcss": "10.0.0",
"gulp-terser-js": "5.2.2",
"gulp.spritesmith": "6.13.0",
"jdenticon": "3.3.0",
"jquery": "3.7.1",
"mathjax": "2.7.1",
"moment": "2.30.1",
Expand Down
4 changes: 4 additions & 0 deletions templates/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ <h2 class="subtitle" {% if schema %}itemprop="description"{% endif %}>{{ headlin

{# Javascript stuff start #}
<script src="{% static "js/jquery.min.js" %}"></script>
<script>
window.jdenticon_config = {backColor: "#fff"}
</script>
<script src="{% static "js/jdenticon.min.js" %}"></script>
{% block extra_js %}
{% endblock %}
<script src="{% static "js/script.js" %}"></script>
Expand Down
2 changes: 1 addition & 1 deletion templates/forum/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ <h4>{{ period|humane_delta }}</h4>
{% with answer=topic.get_last_answer %}
{% if answer %}
{% with profile=answer.author|profile %}
<img src="{% avatar_url profile %}" alt="" class="avatar">
{% avatar profile %}
{% endwith %}
<span class="topic-last-answer">
{% trans "Dernière réponse" %}
Expand Down
6 changes: 3 additions & 3 deletions templates/header.html
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ <h1>{% trans "Messagerie privée" %}</h1>
<li>
<a href="{{ notification.url }}">
<header>
<img src="{% avatar_url notification.author.profile %}" alt="" class="avatar">
{% avatar notification.author.profile %}
<span class="username">{{ notification.author.username }}</span>
<span class="date">{{ notification.pubdate|format_date:True|capfirst }}</span>
</header>
Expand Down Expand Up @@ -274,7 +274,7 @@ <h1 class="dropdown-title">{% trans "Notifications" %}</h1>
<li>
<a href="{{ first_unread.url }}">
<header>
<img src="{% avatar_url first_unread.author.profile %}" alt="" class="avatar">
{% avatar first_unread.author.profile %}
<span class="username">{{ first_unread.author.username }}</span>
<span class="date">{{ first_unread.pubdate|format_date:True|capfirst }}</span>
</header>
Expand Down Expand Up @@ -344,7 +344,7 @@ <h1 class="dropdown-title">{% trans "Alertes de modération" %}</h1>
data-active="open-my-account"
{% endif %}
>
<img src="{% avatar_url profile %}" alt="Mon compte" class="avatar">
{% avatar profile %}
Copy link
Member

Choose a reason for hiding this comment

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

Ici le texte alternatif peut être pertinent (bien que ce ne soit pas la meilleure manière de faire passer cette information dans ce contexte), car l'image est en réalité un lien vers une page spécifique ; or, il a été supprimé, ce qui fait qu'on ne peut plus savoir, avec des outils d'assistance ou des robots d'exploration, vers quoi pointe cette page.

Une solution propre serait de rendre l'avatar décoratif (texte alternatif vide) et d'ajouter le nom du lien masqué (.visuallyhidden, chez nous, souvent appelé .sr-only ailleurs) ; ou de renommer entièrement le lien englobant avec un aria-label, éventuellement.

Par exemple, quelque chose de cette idée.

{% avatar profile %}
<span class="visuallyhidden">Mon compte</span>
<span class="username label">{{ user.username }}</span>

<span class="username label">{{ user.username }}</span>
</a>
{% endwith %}
Expand Down
2 changes: 1 addition & 1 deletion templates/member/profile.html
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
<div class="flexpage-title-tool">
<div class="avatar">
<figure>
<img src="{% avatar_url profile size=200 %}" alt="{% trans "Avatar" %}" />
{% avatar profile size=200 %}
</figure>
</div>

Expand Down
11 changes: 11 additions & 0 deletions templates/misc/avatar.part.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{% load captureas %}

{# Template used by the templatetag "avatar" defined in zds/utils/templatetags/profile.py #}

{% captureas alt_text %}Avatar de {{ username }}{% endcaptureas %}
Copy link
Member

Choose a reason for hiding this comment

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

Dans la majorité des cas, il ne faudrait pas de texte alternatif ici. En effet, l’avatar est la plupart du temps affiché à côté du nom du membre concerné. Il est dans un tel cas purement décoratif et l'information donnée est en doublon (lecture similaire à “Avatar de Situphen. Image. Situphen.”). Ce n'est pas pour rien que dans l'ancienne version du code, l'attribut alt était vide : c'était une bonne pratique.

La seule alternative serait de permettre aux membres de fournir un texte alternatif pour leur avatar, mais là encore — ce ne serait pas pertinent partout (probablement juste sur leur page de profil ; sinon, ce serait très redondant sur les flux de discussion à force d'être répété à chaque message… ou éventuellement, juste au premier rencontré… et encore).


{% if avatar_url %}
<img src="{{ avatar_url }}" alt="{{ alt_text }}" class="avatar" itemprop="image" aria-hidden="true">
{% else %}
<canvas width="{{ avatar_size }}" height="{{ avatar_size }}" data-jdenticon-value="{{ username }}" class="avatar">{{ alt_text }}</canvas>
{% endif %}
2 changes: 1 addition & 1 deletion templates/misc/member_item.part.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
{% endif %}>

{% if avatar %}
<img src="{% avatar_url profile %}" alt="" class="avatar" itemprop="image" aria-hidden="true" />
{% avatar profile %}
{% endif %}

<span itemprop="name">{{ username }}</span>
Expand Down
2 changes: 1 addition & 1 deletion templates/misc/message_user.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<div class="user">
{% with profile=member|profile %}
<a href="{{ member.get_absolute_url }}" class="avatar-link">
<img src="{% avatar_url profile %}" alt="" class="avatar">
{% avatar profile %}
</a>

{% include 'misc/badge.part.html' %}
Expand Down
14 changes: 14 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,13 @@ caniuse-lite@^1.0.30001587, caniuse-lite@^1.0.30001591:
resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001596.tgz#da06b79c3d9c3d9958eb307aa832ac68ead79bee"
integrity sha512-zpkZ+kEr6We7w63ORkoJ2pOfBwBkY/bJrG/UZ90qNb45Isblu8wzDgevEOrRL1r9dWayHjYiiyCMEXPn4DweGQ==

canvas-renderer@~2.2.0:
version "2.2.1"
resolved "https://registry.yarnpkg.com/canvas-renderer/-/canvas-renderer-2.2.1.tgz#c1d131f78a9799aca8af9679ad0a005052b65550"
integrity sha512-RrBgVL5qCEDIXpJ6NrzyRNoTnXxYarqm/cS/W6ERhUJts5UQtt/XPEosGN3rqUkZ4fjBArlnCbsISJ+KCFnIAg==
dependencies:
"@types/node" "*"

capture-stack-trace@^1.0.0:
version "1.0.2"
resolved "https://registry.yarnpkg.com/capture-stack-trace/-/capture-stack-trace-1.0.2.tgz#1c43f6b059d4249e7f3f8724f15f048b927d3a8a"
Expand Down Expand Up @@ -4809,6 +4816,13 @@ isurl@^1.0.0-alpha5:
has-to-string-tag-x "^1.2.0"
is-object "^1.0.1"

[email protected]:
version "3.3.0"
resolved "https://registry.yarnpkg.com/jdenticon/-/jdenticon-3.3.0.tgz#64bae9f9b3cf5c2a210e183648117afe3a89b367"
integrity sha512-DhuBRNRIybGPeAjMjdHbkIfiwZCCmf8ggu7C49jhp6aJ7DYsZfudnvnTY5/1vgUhrGA7JaDAx1WevnpjCPvaGg==
dependencies:
canvas-renderer "~2.2.0"

[email protected]:
version "0.0.4"
resolved "https://registry.npmjs.org/jpeg-js/-/jpeg-js-0.0.4.tgz"
Expand Down
6 changes: 3 additions & 3 deletions zds/member/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class UserListSerializer(serializers.ModelSerializer):
serializers.
"""

avatar_url = serializers.CharField(source="profile.get_avatar_url")
avatar_url = serializers.CharField(source="profile.get_absolute_avatar_url")
html_url = serializers.CharField(source="get_absolute_url")

class Meta:
Expand All @@ -34,7 +34,7 @@ class ProfileListSerializer(serializers.ModelSerializer):
html_url = serializers.CharField(source="user.get_absolute_url")
is_active = serializers.BooleanField(source="user.is_active")
date_joined = serializers.DateTimeField(source="user.date_joined")
avatar_url = serializers.CharField(source="get_avatar_url")
avatar_url = serializers.CharField(source="get_absolute_avatar_url")
permissions = DRYPermissionsField(additional_actions=["ban"])

class Meta:
Expand Down Expand Up @@ -78,7 +78,7 @@ class ProfileDetailSerializer(serializers.ModelSerializer):
email = serializers.EmailField(source="user.email")
is_active = serializers.BooleanField(source="user.is_active")
date_joined = serializers.DateTimeField(source="user.date_joined")
avatar_url = serializers.CharField(source="get_avatar_url")
avatar_url = serializers.CharField(source="get_absolute_avatar_url")
permissions = DRYPermissionsField(additional_actions=["ban"])

class Meta:
Expand Down
4 changes: 2 additions & 2 deletions zds/member/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ def test_detail_of_the_member(self):
self.assertEqual(profile.user.is_active, response.data.get("is_active"))
self.assertIsNotNone(response.data.get("date_joined"))
self.assertEqual(profile.site, response.data.get("site"))
self.assertEqual(profile.get_avatar_url(), response.data.get("avatar_url"))
self.assertEqual(profile.get_absolute_avatar_url(), response.data.get("avatar_url"))
self.assertEqual(profile.biography, response.data.get("biography"))
self.assertEqual(profile.sign, response.data.get("sign"))
self.assertFalse(response.data.get("show_email"))
Expand Down Expand Up @@ -442,7 +442,7 @@ def test_detail_of_a_member(self):
self.assertEqual(self.profile.user.is_active, response.data.get("is_active"))
self.assertIsNotNone(response.data.get("date_joined"))
self.assertEqual(self.profile.site, response.data.get("site"))
self.assertEqual(self.profile.get_avatar_url(), response.data.get("avatar_url"))
self.assertEqual(self.profile.get_absolute_avatar_url(), response.data.get("avatar_url"))
self.assertEqual(self.profile.biography, response.data.get("biography"))
self.assertEqual(self.profile.sign, response.data.get("sign"))
self.assertFalse(response.data.get("show_email"))
Expand Down
4 changes: 1 addition & 3 deletions zds/member/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,7 @@ class MiniProfileForm(forms.Form):
label="Avatar",
required=False,
max_length=Profile._meta.get_field("avatar_url").max_length,
widget=forms.TextInput(
attrs={"placeholder": _("Lien vers un avatar externe (laissez vide pour utiliser Gravatar).")}
),
widget=forms.TextInput(attrs={"placeholder": _("Lien vers un avatar externe.")}),
)

sign = forms.CharField(
Expand Down
29 changes: 29 additions & 0 deletions zds/member/management/commands/migrate_from_gravatar.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
from hashlib import md5
from time import sleep

import requests
from django.core.management.base import BaseCommand
from django.db.models import Q

from zds.member.models import Profile


class Command(BaseCommand):
help = "Migrate from Gravatar"

def handle(self, *args, **options):
# We have profiles with either NULL or empty avatar_url field
profiles_without_avatar_url = Profile.objects.filter(Q(avatar_url__isnull=True) | Q(avatar_url=""))
total = profiles_without_avatar_url.count()
i = 1
for profile in profiles_without_avatar_url.iterator():
hash = md5(profile.user.email.lower().encode("utf-8")).hexdigest()
gravatar_url = f"https://secure.gravatar.com/avatar/{hash}"
r = requests.get(f"{gravatar_url}?d=404")
if r.status_code == 200:
profile.avatar_url = f"{gravatar_url}?s=200"
profile.save()
self.stdout.write(f"\rProgress: {i}/{total}", ending="")
i += 1
sleep(1)
self.stdout.write(self.style.SUCCESS("\nSuccessfully migrated from Gravatar!"))
24 changes: 8 additions & 16 deletions zds/member/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,22 +105,14 @@ def get_city(self):

return geo_location

def get_avatar_url(self, size=80):
"""Get the avatar URL for this profile.
If the user has defined a custom URL, use it.
If not, use Gravatar.
:return: The avatar URL for this profile
:rtype: str
"""
if self.avatar_url:
if self.avatar_url.startswith(settings.MEDIA_URL):
return "{}{}".format(settings.ZDS_APP["site"]["url"], self.avatar_url)
else:
return self.avatar_url
else:
return "https://secure.gravatar.com/avatar/{}?d=identicon&s={}".format(
md5(self.user.email.lower().encode("utf-8")).hexdigest(), size
)
def get_absolute_avatar_url(self):
"""Gets the avatar URL of this profile.
:return: The absolute URL of this profile's avatar
:rtype: str or None
"""
if self.avatar_url and self.avatar_url.startswith(settings.MEDIA_URL):
return settings.ZDS_APP["site"]["url"] + self.avatar_url
return self.avatar_url

def get_post_count(self):
"""
Expand Down
17 changes: 9 additions & 8 deletions zds/member/tests/tests_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,23 @@ def setUp(self):
def test_get_absolute_url_for_details_of_member(self):
self.assertEqual(self.user1.get_absolute_url(), f"/@{self.user1.user.username}")

def test_get_avatar_url(self):
# if no url was specified -> gravatar !
self.assertIn("gravatar.com", self.user1.get_avatar_url())
def test_get_absolute_avatar_url(self):
# if no url was specified -> nothing !
self.assertEqual(self.user1.get_absolute_avatar_url(), None)

# if an url is specified -> take it !
user2 = ProfileFactory()
testurl = "http://test.com/avatar.jpg"
user2.avatar_url = testurl
self.assertEqual(user2.get_avatar_url(), testurl)
self.assertEqual(user2.get_absolute_avatar_url(), testurl)

# if url is relative, send absolute url
gallerie_avtar = GalleryFactory()
image_avatar = ImageFactory(gallery=gallerie_avtar)
gallerie_avatar = GalleryFactory()
image_avatar = ImageFactory(gallery=gallerie_avatar)
user2.avatar_url = image_avatar.physical.url
self.assertNotEqual(user2.get_avatar_url(), image_avatar.physical.url)
self.assertIn("http", user2.get_avatar_url())
self.assertNotEqual(user2.get_absolute_avatar_url(), image_avatar.physical.url)
self.assertIn(image_avatar.physical.url, user2.get_absolute_avatar_url())
self.assertIn("http", user2.get_absolute_avatar_url())

def test_get_post_count(self):
# Start with 0
Expand Down
2 changes: 1 addition & 1 deletion zds/mp/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ def test_expand_list_of_private_topics_for_author(self):
self.assertEqual(response.status_code, status.HTTP_200_OK)
author = response.data.get("results")[0].get("author")
self.assertEqual(author.get("username"), self.profile.user.username)
self.assertEqual(author.get("avatar_url"), self.profile.get_avatar_url())
self.assertEqual(author.get("avatar_url"), self.profile.get_absolute_avatar_url())

def test_create_private_topics_with_client_unauthenticated(self):
"""
Expand Down
17 changes: 7 additions & 10 deletions zds/utils/templatetags/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from django.core.cache import cache

from zds.member.models import Profile
from zds.utils.templatetags.remove_url_scheme import remove_url_scheme

register = template.Library()

Expand Down Expand Up @@ -73,13 +72,11 @@ def state(current_user):
return user_state


@register.simple_tag
def avatar_url(profile: Profile, size=80) -> str:
"""
Return the URL of the avatar of a profile.
If the profile is None, return an empty string.
"""
@register.inclusion_tag("misc/avatar.part.html")
def avatar(profile: Profile, size=80) -> dict:
if profile is not None:
url = profile.get_avatar_url(size)
return remove_url_scheme(url)
return ""
return {
"avatar_url": profile.avatar_url,
"avatar_size": size,
"username": profile.user.username,
}