Skip to content

Commit

Permalink
only use gravatars based on user emails
Browse files Browse the repository at this point in the history 
This removes the use of avatars customizable through the account page.
Instead, gravatars based on the users emails is used, falling back to
identicons.

The customizable avatars did not work well for the multiple sizes we
wanted to display them in.  This was also a problem for our fallback
icon.  The user images would often be served through http, not https,
giving mixed mode alerts.  If the user modified their profile, the
generated gravatar URL would be baked in, preventing updates to their
email address from updating the avatar.
  • Loading branch information
@haarg
haarg committed Jun 4, 2018
1 parent d8702b2 commit 5bc488a
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 90 deletions.
2 changes: 1 addition & 1 deletion lib/MetaCPAN/Web/Controller/Account.pm
View file
Expand Up @@ -85,7 +85,7 @@ sub profile : Local : Args(0) {
? [ $req->params->{latitude}, $req->params->{longitude} ]
: undef;
$data->{$_} = $req->params->{$_} eq q{} ? undef : $req->params->{$_}
for (qw(name asciiname gravatar_url city region country));
for (qw(name asciiname city region country));
$data->{$_} = [ grep {$_} $req->param($_) ] for (qw(website email));

$data->{extra} = $req->param('extra') ? $req->json_param('extra') : undef;
Expand Down
66 changes: 27 additions & 39 deletions lib/MetaCPAN/Web/View/HTML.pm
View file
Expand Up @@ -108,24 +108,37 @@ Template::Alloy->define_vmethod(
},
);

sub gravatar_image {
my ( $emails, $size ) = @_;
my $avatar;
$emails = ['']
if !@$emails;
while ( my $email = pop @$emails ) {
$avatar = Gravatar::URL::gravatar_url(
https => 1,
base => 'https://www.gravatar.com/avatar/',
email => $email,
size => $size || 80,
default => ( $avatar || 'identicon' ),
);
}
return $avatar;
}

Template::Alloy->define_vmethod(
'hash',
gravatar_image => sub {
my ( $author, $size, $default ) = @_;
my ($email)
= ref $author->{email} ? @{ $author->{email} } : $author->{email};
Gravatar::URL::gravatar_url(
email => $email,
size => $size,
default => Gravatar::URL::gravatar_url(

# Fallback to the CPAN address, as used by s.c.o, which will in
# turn fallback to a generated image.
email => $author->{pauseid} . '@cpan.org',
size => $size,
default => $default,
)
my ( $author, $size ) = @_;
my @emails = List::Util::uniq(
map lc,
(
ref $author->{email} ? @{ $author->{email} }
: $author->{email} ? $author->{email}
: ()
),
$author->{pauseid} . '@cpan.org'
);
gravatar_image( \@emails, $size );
}
);

Expand All @@ -142,31 +155,6 @@ Template::Alloy->define_vmethod(
}
);

sub gravatar_fixup {
my ( $url, $size ) = @_;
if ( $url
=~ m{^https?://([a-z0-9.-]+\.)?gravatar\.com/(avatar|userimage)/}i )
{
my $url = URI->new($url);
$url->scheme('https');
$url->host('secure.gravatar.com');
if ($size) {
$url->query_param( s => $size );
}
else {
$url->query_param_delete('s');
}
if ( my $fallback = $url->query_param('d') ) {
$url->query_param( d => gravatar_fixup( $fallback, $size ) );
}
return $url->as_string;
}
return $url;
}

Template::Alloy->define_vmethod( 'text',
gravatar_fixup => \&gravatar_fixup, );

Template::Alloy->define_vmethod(
'text',
pluralize => sub {
Expand Down
6 changes: 0 additions & 6 deletions root/account/profile.html
View file
Expand Up @@ -100,12 +100,6 @@ <h4 class="alert-heading">Error</h4>
<input type="url" class="form-control" name="blog.feed" value="<% author.blog.0.feed %>" />
</div>
</div>
<div class="form-group">
<label class="col-sm-4 col-md-4 control-label">Avatar</label>
<div class="col-sm-8 col-md-6">
<input type="url" class="form-control" name="gravatar_url" value="<% author.gravatar_url %>" />
</div>
</div>
<div class="form-group">
<label class="col-sm-4 col-md-4"></label>
<%- donation = author.donation.0.id || author.donation.1.id || author.donation.2.id %>
Expand Down
6 changes: 2 additions & 4 deletions root/inc/author-pic.html
View file
Expand Up @@ -6,7 +6,7 @@
<%# display the gravatar url for each pause plusser alongwith their author page as a link. %>
<% FOREACH plusser IN plusser_authors.shuffle.first(author_nums) %>

<a class="display-all" href="/author/<% plusser.pauseid %>"><img src="<% plusser.gravatar_url.gravatar_fixup(20) || '/static/icons/user.png' %>" title="<% plusser.pauseid %>" alt="<% plusser.pauseid %>"></a>
<a class="display-all" href="/author/<% plusser.pauseid %>"><img src="<% plusser.gravatar_image(20) %>" title="<% plusser.pauseid %>" alt="<% plusser.pauseid %>"></a>

<% END %>

Expand All @@ -23,11 +23,9 @@
<% END %>

<div class="author-pic">
<%- IF author.gravatar_url %>
<a href="/author/<% author.pauseid %>">
<img src="<% author.gravatar_url.gravatar_fixup(130) %>">
<img src="<% author.gravatar_image(130) %>">
</a>
<% END %>
<strong>
<a rel="author" href="/author/<% author.pauseid %>"><% author.pauseid %></a>
</strong>
Expand Down
4 changes: 2 additions & 2 deletions root/inc/contributors.html
View file
Expand Up @@ -16,8 +16,8 @@
<a href="/author/<% contributor.pauseid %>"
class="cpan-author"
>
<%- IF contributor.gravatar_url -%>
<img class="gravatar" width="20" height="20" src="<% contributor.gravatar_url.gravatar_fixup(20) %>" />
<%- IF contributor.pauseid || contributor.email.size -%>
<img class="gravatar" width="20" height="20" src="<% contributor.gravatar_image(20) %>" />
<%- END -%>
<% contributor.name %></a>
<% ELSE %>
Expand Down
4 changes: 1 addition & 3 deletions root/inc/twitter/author.html
View file
Expand Up @@ -8,6 +8,4 @@
<meta name="twitter:creator" content="<% profile.id %>" />
<%- END -%>
<%- END %>
<% IF author.gravatar_url -%>
<meta name="twitter:image" content="<% author.gravatar_url %>" />
<%- END %>
<meta name="twitter:image" content="<% author.gravatar_image(400) %>" />
2 changes: 1 addition & 1 deletion root/plussers.html
View file
Expand Up @@ -19,7 +19,7 @@ <h4> PAUSE users who ++ed <a href="/release/<% plusser_data %>"><% plusser_data

<div class="plusser">
<a href="/author/<% plusser.pauseid %>">
<img src="<% plusser.gravatar_url.gravatar_fixup(75) || '/static/icons/user.png' %>" title="<% plusser.pauseid %>" alt="" width="75" height="75">
<img src="<% plusser.gravatar_image(75) %>" title="<% plusser.pauseid %>" alt="" width="75" height="75">
<% plusser.id %>
</a>
</div>
Expand Down
3 changes: 1 addition & 2 deletions root/recent/topuploaders.html
View file
Expand Up @@ -4,10 +4,9 @@
<div class="content toplists row">
<% FOREACH author IN authors %>
<div class="entry col-sm-6 col-md-3">
<%- IF author.gravatar_url %>
<div class="gravatar">
<a href="/author/<% author.pauseid %>">
<img src="<% author.gravatar_url.gravatar_fixup(75) %>" class="author-img" style="width: 75px; height: 75px">
<img src="<% author.gravatar_image(75) %>" class="author-img" style="width: 75px; height: 75px">
</a>
</div>
<%- END %>
Expand Down
4 changes: 1 addition & 3 deletions root/search.html
View file
Expand Up @@ -9,9 +9,7 @@
<%- FOREACH author IN authors.authors %>
<li>
<a href="/author/<% author.id %>" title="Author page for <% author.name | html %>">
<%- IF author.gravatar_url %>
<img src="<% author.gravatar_url.gravatar_fixup(30) %>">
<%- END %>
<img src="<% author.gravatar_image(30) %>">
<% author.name | html %> (<% author.pauseid %>)
</a>
</li>
Expand Down
Binary file removed root/static/icons/user.png
View file
Binary file not shown.
54 changes: 25 additions & 29 deletions t/controller/account.t
View file
Expand Up @@ -68,7 +68,7 @@ test_psgi app, sub {
);

$res_body
= q({"asciiname":"foo","email":["foobar@cpan.org"],"gravatar_url":"https://secure.gravatar.com/avatar/12345678901234567890123456789012?s=130&d=identicon","name":"foo","pauseid":"FOO","updated":"2017-02-15T22:18:19","user":"12345678901234567890","website":[]});
= q({"asciiname":"foo","email":["foobar@cpan.org"],"name":"foo","pauseid":"FOO","updated":"2017-02-15T22:18:19","user":"12345678901234567890","website":[]});
ok(
$res = $cb->( GET '/account/profile' ),
'GET /account/profile happy case'
Expand Down Expand Up @@ -96,7 +96,6 @@ test_psgi app, sub {
'longitude' => '9.7320 E',
'name' => 'Բարեւ',
'asciiname' => 'asciiname1',
'gravatar_url' => 'gravatar_url1',
'city' => 'city1',
'region' => 'region1',
'country' => 'country1',
Expand All @@ -121,9 +120,8 @@ test_psgi app, sub {
'feed' => 'http://example.org/feed1',
'url' => 'http://example.org/blog1',
} ],
'asciiname' => 'asciiname1',
'gravatar_url' => 'gravatar_url1',
'donation' => [ {
'asciiname' => 'asciiname1',
'donation' => [ {
'name' => 'donation.name1',
'id' => 'donation.id1',
} ],
Expand Down Expand Up @@ -156,14 +154,13 @@ test_psgi app, sub {
'Բարեւ', '... and the new user data is in the page' );

$form = [
'name' => 'Բարեւ',
'asciiname' => 'asciiname1',
'utf8' => '🐪',
'gravatar_url' => '',
'city' => '',
'region' => '',
'country' => '',
'extra' => '',
'name' => 'Բարեւ',
'asciiname' => 'asciiname1',
'utf8' => '🐪',
'city' => '',
'region' => '',
'country' => '',
'extra' => '',
];
ok(
$res = $cb->( POST '/account/profile', $form ),
Expand All @@ -172,22 +169,21 @@ test_psgi app, sub {
cmp_deeply(
decode_json( $api_req->content ),
{
'updated' => '2017-02-15T22:18:19',
'user' => '12345678901234567890',
'name' => 'Բարեւ',
'asciiname' => 'asciiname1',
'pauseid' => 'FOO',
'country' => undef,
'blog' => undef,
'city' => undef,
'donation' => undef,
'email' => [],
'gravatar_url' => undef,
'location' => undef,
'profile' => undef,
'website' => [],
'region' => undef,
'extra' => undef,
'updated' => '2017-02-15T22:18:19',
'user' => '12345678901234567890',
'name' => 'Բարեւ',
'asciiname' => 'asciiname1',
'pauseid' => 'FOO',
'country' => undef,
'blog' => undef,
'city' => undef,
'donation' => undef,
'email' => [],
'location' => undef,
'profile' => undef,
'website' => [],
'region' => undef,
'extra' => undef,
},
'... and the API PUT request contains the right stuff'
);
Expand Down

0 comments on commit 5bc488a

Please sign in to comment.