fu: Close-up of Fu, bringing a scoop of water to her mouth (Default)
fu ([personal profile] fu) wrote in [site community profile] changelog2010-09-08 07:10 am

[dw-free] cleaning up userpics code

[commit: http://hg.dwscoalition.org/dw-free/rev/93050be9e5bf]

http://bugs.dwscoalition.org/show_bug.cgi?id=513

More functions under LJ::Userpic and on the user, rather than in package LJ
(this batch focuses on the calls to get the userpic or the picid from a
keyword)

Patch by [personal profile] kareila.

Files modified:
  • cgi-bin/LJ/Comment.pm
  • cgi-bin/LJ/Entry.pm
  • cgi-bin/LJ/S2.pm
  • cgi-bin/LJ/Talk.pm
  • cgi-bin/LJ/User.pm
  • cgi-bin/LJ/Userpic.pm
  • cgi-bin/ljuserpics.pl
  • htdocs/community/moderate.bml
  • htdocs/support/see_request.bml
  • htdocs/talkpost.bml
--------------------------------------------------------------------------------
diff -r e84aad104e41 -r 93050be9e5bf cgi-bin/LJ/Comment.pm
--- a/cgi-bin/LJ/Comment.pm	Wed Sep 08 14:47:35 2010 +0800
+++ b/cgi-bin/LJ/Comment.pm	Wed Sep 08 15:10:03 2010 +0800
@@ -1591,11 +1591,8 @@ sub userpic {
     my $key = $self->prop('picture_keyword');
 
     # return the picture from keyword, if defined
-    my $picid = LJ::get_picid_from_keyword($up, $key);
-    return LJ::Userpic->new($up, $picid) if $picid;
-
     # else return poster's default userpic
-    return $up->userpic;
+    return LJ::Userpic->new_from_keyword( $up, $key ) || $up->userpic;
 }
 
 sub poster_ip {
diff -r e84aad104e41 -r 93050be9e5bf cgi-bin/LJ/Entry.pm
--- a/cgi-bin/LJ/Entry.pm	Wed Sep 08 14:47:35 2010 +0800
+++ b/cgi-bin/LJ/Entry.pm	Wed Sep 08 15:10:03 2010 +0800
@@ -871,11 +871,8 @@ sub userpic {
         DW::Mood->mood_name( $self->prop('current_moodid') );
 
     # return the picture from keyword, if defined
-    my $picid = LJ::get_picid_from_keyword($up, $key);
-    return LJ::Userpic->new($up, $picid) if $picid;
-
     # else return poster's default userpic
-    return $up->userpic;
+    return LJ::Userpic->new_from_keyword( $up, $key ) || $up->userpic;
 }
 
 sub userpic_kw_from_props {
diff -r e84aad104e41 -r 93050be9e5bf cgi-bin/LJ/S2.pm
--- a/cgi-bin/LJ/S2.pm	Wed Sep 08 14:47:35 2010 +0800
+++ b/cgi-bin/LJ/S2.pm	Wed Sep 08 15:10:03 2010 +0800
@@ -2237,7 +2237,7 @@ sub Image_userpic
 {
     my ( $u, $picid, $kw, $width, $height ) = @_;
 
-    $picid ||= LJ::get_picid_from_keyword($u, $kw);
+    $picid ||= $u->get_picid_from_keyword( $kw ) if LJ::isu( $u );
     return Null("Image") unless $picid;
 
     # get the Userpic object
diff -r e84aad104e41 -r 93050be9e5bf cgi-bin/LJ/Talk.pm
--- a/cgi-bin/LJ/Talk.pm	Wed Sep 08 14:47:35 2010 +0800
+++ b/cgi-bin/LJ/Talk.pm	Wed Sep 08 15:10:03 2010 +0800
@@ -1164,7 +1164,7 @@ sub load_comments
                     $kw = $post->{'props'}->{'picture_keyword'};
                 }
                 my $pu = $opts->{'userref'}->{$post->{'posterid'}};
-                my $id = LJ::get_picid_from_keyword($pu, $kw);
+                my $id = $pu ? $pu->get_picid_from_keyword( $kw ) : undef;
                 $post->{'picid'} = $id;
                 push @load_pic, [ $pu, $id ];
             }
diff -r e84aad104e41 -r 93050be9e5bf cgi-bin/LJ/User.pm
--- a/cgi-bin/LJ/User.pm	Wed Sep 08 14:47:35 2010 +0800
+++ b/cgi-bin/LJ/User.pm	Wed Sep 08 15:10:03 2010 +0800
@@ -6227,6 +6227,27 @@ sub expunge_userpic {
     return ( $u->userid, map {$_->[0]} grep {$_ && @$_ && $_->[0]} @rval );
 }
 
+sub get_picid_from_keyword {
+    my ( $u, $kw, $default ) = @_;
+    $default ||= ref $u ? $u->{defaultpicid} : 0;
+    return $default unless $kw;
+
+    my $info = LJ::isu( $u ) ? $u->get_userpic_info : undef;
+    return $default unless $info;
+
+    my $pr = $info->{'kw'}{$kw};
+    # normal keyword
+    return $pr->{picid} if $pr->{picid};
+
+    # the silly "pic#2343" thing when they didn't assign a keyword
+    if ( $kw =~ /^pic\#(\d+)$/ ) {
+        my $picid = $1;
+        return $picid if $info->{'pic'}{$picid};
+    }
+
+    return $default;
+}
+
 sub get_userpic_count {
     my $u = shift or return undef;
     my $count = scalar LJ::Userpic->load_user_userpics($u);
@@ -7893,7 +7914,7 @@ sub user_search_display {
     my $get_picid = sub {
         my $u = shift;
         return $u->{'defaultpicid'} unless $args{'pickwd'};
-        return LJ::get_picid_from_keyword($u, $args{'pickwd'});
+        return $u->get_picid_from_keyword( $args{pickwd} );
     };
 
     my $ret;
diff -r e84aad104e41 -r 93050be9e5bf cgi-bin/LJ/Userpic.pm
--- a/cgi-bin/LJ/Userpic.pm	Wed Sep 08 14:47:35 2010 +0800
+++ b/cgi-bin/LJ/Userpic.pm	Wed Sep 08 15:10:03 2010 +0800
@@ -149,12 +149,12 @@ sub new_from_row {
 
 sub new_from_keyword
 {
-    my ($class, $u, $kw) = @_;
+    my ( $class, $u, $kw ) = @_;
+    return undef unless LJ::isu( $u );
 
-    my $picid = LJ::get_picid_from_keyword($u, $kw) or
-        return undef;
+    my $picid = $u->get_picid_from_keyword( $kw );
 
-    return $class->new($u, $picid);
+    return $picid ? $class->new( $u, $picid ) : undef;
 }
 
 # instance methods
@@ -1149,10 +1149,10 @@ sub set_and_rename_keywords {
 
         #make sure that none of the target keywords already exist.
         my $u = $self->owner;
-        foreach my $kw (keys %keywordmap) {
-            if (LJ::get_picid_from_keyword($u, $keywordmap{$kw}, -1) != -1) {
-                LJ::errobj("Userpic::RenameKeywordExisting",
-                           keyword => $keywordmap{$kw})->throw;
+        foreach my $kw ( values %keywordmap ) {
+            if ( $u && $u->get_picid_from_keyword( $kw, -1 ) != -1 ) {
+                LJ::errobj( "Userpic::RenameKeywordExisting",
+                            keyword => $kw )->throw;
             }
         }
         
diff -r e84aad104e41 -r 93050be9e5bf cgi-bin/ljuserpics.pl
--- a/cgi-bin/ljuserpics.pl	Wed Sep 08 14:47:35 2010 +0800
+++ b/cgi-bin/ljuserpics.pl	Wed Sep 08 15:10:03 2010 +0800
@@ -108,27 +108,5 @@ sub load_userpics {
     }
 }
 
-sub get_picid_from_keyword
-{
-    my ($u, $kw, $default) = @_;
-    $default ||= (ref $u ? $u->{'defaultpicid'} : 0);
-    return $default unless $kw;
-
-    my $info = LJ::isu( $u ) ? $u->get_userpic_info : undef;
-    return $default unless $info;
-
-    my $pr = $info->{'kw'}{$kw};
-    # normal keyword
-    return $pr->{picid} if $pr->{picid};
-
-    # the lame "pic#2343" thing when they didn't assign a keyword
-    if ($kw =~ /^pic\#(\d+)$/) {
-        my $picid = $1;
-        return $picid if $info->{'pic'}{$picid};
-    }
-
-    return $default;
-}
-
 1;
 
diff -r e84aad104e41 -r 93050be9e5bf htdocs/community/moderate.bml
--- a/htdocs/community/moderate.bml	Wed Sep 08 14:47:35 2010 +0800
+++ b/htdocs/community/moderate.bml	Wed Sep 08 15:10:03 2010 +0800
@@ -411,20 +411,19 @@ body<=
         $ret .= "<p>";
         $ret .= "<table><tr valign='middle'>";
 
-        my $picid = LJ::get_picid_from_keyword($up, $props->{'picture_keyword'});
+        my $picid = $up ? $up->get_picid_from_keyword( $props->{picture_keyword} ) : undef;
 
         my %userpics;
-        if ($picid) {
+        if ( $picid ) {
             LJ::load_userpics(\%userpics, [ $up, $picid ]);
-              my $alt = $up->{'name'};
-              if ($props->{'picture_keyword'}) {
-                  $alt .= ": $props->{'picture_keyword'}";
-              }
-              $alt = LJ::ehtml($alt);
-              $ret .= "<td><img src='$LJ::USERPIC_ROOT/$picid/$up->{'userid'}' width='$userpics{$picid}->{'width'}' ".
-                  "height='$userpics{$picid}->{'height'}' align='absmiddle' ".
-                  "hspace='3' title='$alt' alt=''></td>";
-          }
+            my $alt = $up->{name};
+            $alt .= ": $props->{picture_keyword}"
+                if $props->{picture_keyword};
+            $alt = LJ::ehtml( $alt );
+            $ret .= "<td><img src='$LJ::USERPIC_ROOT/$picid/$up->{'userid'}' width='$userpics{$picid}->{'width'}' ".
+                "height='$userpics{$picid}->{'height'}' align='absmiddle' ".
+                "hspace='3' title='$alt' alt=''></td>";
+        }
 
         $ret .= "<td>";
         $ret .= BML::ml("talk.somebodywrote_comm", { 'realname' => BML::eall($up->{'name'}),
diff -r e84aad104e41 -r 93050be9e5bf htdocs/support/see_request.bml
--- a/htdocs/support/see_request.bml	Wed Sep 08 14:47:35 2010 +0800
+++ b/htdocs/support/see_request.bml	Wed Sep 08 15:10:03 2010 +0800
@@ -591,7 +591,7 @@ body<=
         # reply header
         my $header = "";
         if ($up && LJ::Support::can_see_helper($sp, $remote)) {
-            my $picid = LJ::get_picid_from_keyword($up, '_support') || $up->{defaultpicid};
+            my $picid = $up->get_picid_from_keyword( '_support' ) || $up->{defaultpicid};
             $userpics{$picid} or LJ::load_userpics(\%userpics, [[ $up, $picid ]]);
             $header = "<table style='margin-top: 15px;'><tr valign='bottom'>";
             if ($picid && !$up->is_suspended) {
diff -r e84aad104e41 -r 93050be9e5bf htdocs/talkpost.bml
--- a/htdocs/talkpost.bml	Wed Sep 08 14:47:35 2010 +0800
+++ b/htdocs/talkpost.bml	Wed Sep 08 15:10:03 2010 +0800
@@ -226,9 +226,8 @@ body<=
     LJ::Hooks::run_hook('notify_event_displayed', $entry);
 
     my $userpic;
-    if ($init->{'replyto'}) {
-        my $picid = LJ::get_picid_from_keyword($ur, $pickw);
-        $userpic = LJ::Userpic->new($ur, $picid) if $picid;
+    if ( $init->{replyto} ) {
+        $userpic = LJ::Userpic->new_from_keyword( $ur, $pickw );
     } else {
         $userpic = $entry->userpic;
     }
--------------------------------------------------------------------------------