mark: A photo of Mark kneeling on top of the Taal Volcano in the Philippines. It was a long hike. (Default)
Mark Smith ([staff profile] mark) wrote in [site community profile] changelog2009-04-26 07:12 pm

[dw-free] Gracefully handle userpic import errors

[commit: http://hg.dwscoalition.org/dw-free/rev/3f7acfee7d4d]

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

Much better errors on userpic import.

Patch by [personal profile] exor674.

Files modified:
  • cgi-bin/DW/Worker/ContentImporter/LiveJournal/Userpics.pm
  • cgi-bin/DW/Worker/ContentImporter/Local/Userpics.pm
--------------------------------------------------------------------------------
diff -r a78476ae5bc3 -r 3f7acfee7d4d cgi-bin/DW/Worker/ContentImporter/LiveJournal/Userpics.pm
--- a/cgi-bin/DW/Worker/ContentImporter/LiveJournal/Userpics.pm	Sun Apr 26 19:10:16 2009 +0000
+++ b/cgi-bin/DW/Worker/ContentImporter/LiveJournal/Userpics.pm	Sun Apr 26 19:12:29 2009 +0000
@@ -83,9 +83,12 @@ sub try_work {
 
     my $errs = [];
     my @imported = DW::Worker::ContentImporter::Local::Userpics->import_userpics( $u, $errs, $default, \@pics, $log );
-    $status->( text => "Your usericon import had some errors:\n\n" . join("\n", map { " * $_" } @$errs) )
-        if @$errs;
+    my $num_imported = scalar( @imported );
+    my $to_import = scalar( @pics );
 
+    # FIXME: Uncomment when "select userpics later is implemented"
+    #my $has_backup = 0;
+    # There's nothing the user can do at this point if Mogile is not available, and any error relating to that will likely confuse them.
     if ( scalar( @imported ) != scalar( @pics ) ) {
         my $mog = LJ::mogclient();
         if ( $mog ) {
@@ -95,11 +98,23 @@ sub try_work {
                 pics => \@pics,
             };
             $mog->store_content( 'import_upi:' . $u->id, 'temp', $data );
-        } else {
-            return $fail->( 'Userpic import failed and MogileFS not available for backup.' );
+            # FIXME: Uncomment when "select userpics later is implemented"
+            #$has_backup = 1;
         }
     }
 
+    # FIXME: Link to "select userpics later" (once it is created) if we have the backup.
+    my $message = "$num_imported out of $to_import usericon" . ( $num_imported == 1 ? "" : "s" ) . " successfully imported.";
+    $message = "None of your usericons imported successfully." if $num_imported == 0;
+
+    my $text;
+    if ( @$errs ) {
+        $text = "The following usericons failed to import:\n\n" . join( "\n", map { " * $_" } @$errs ) . "\n\n$message";
+    } elsif ( scalar( @imported ) != scalar( @pics ) ) {
+        $text = "You did not have enough room to import all your usericons.\n\n$message";
+    }
+
+    $status->( text => $text );
     return $ok->();
 }
 
diff -r a78476ae5bc3 -r 3f7acfee7d4d cgi-bin/DW/Worker/ContentImporter/Local/Userpics.pm
--- a/cgi-bin/DW/Worker/ContentImporter/Local/Userpics.pm	Sun Apr 26 19:10:16 2009 +0000
+++ b/cgi-bin/DW/Worker/ContentImporter/Local/Userpics.pm	Sun Apr 26 19:12:29 2009 +0000
@@ -17,7 +17,6 @@
 
 package DW::Worker::ContentImporter::Local::Userpics;
 use strict;
-
 use Carp qw/ croak /;
 
 =head1 NAME
@@ -139,7 +138,7 @@ sub import_userpic {
 
     my $resp = $ua->get( $upic->{src} );
     unless ( $resp && $resp->is_success ) {
-        push @$errors, "Icon '$identifier': unable to download from server.";
+        push @$errors, $identifier; # unable to download from server.
         return 0;
     }
 
@@ -162,7 +161,7 @@ sub import_userpic {
         } else {
             $userpic = eval { LJ::Userpic->create( $u, data => \$data, nonotify => 1 ); };
             unless ( $userpic ) {
-                push @$errors, "Icon '$identifier': " . $@->as_string;
+                push @$errors, $identifier;
                 return 0;
             }
         }
--------------------------------------------------------------------------------