Bug 526734: Allow localization of the "From update of attachment" string in comments

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

5 files changed, 66 insertions, 31 deletions

diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm
index f09985426..e81819652 100644
--- a/Bugzilla/Comment.pm
+++ b/Bugzilla/Comment.pm
@@ -96,7 +96,8 @@ sub bug {
 
 sub is_about_attachment {
     my ($self) = @_;
-    return 1 if $self->type == CMT_ATTACHMENT_CREATED;
+    return 1 if ($self->type == CMT_ATTACHMENT_CREATED
+                 or $self->type == CMT_ATTACHMENT_UPDATED);
     return 0;
 }
 
@@ -160,7 +161,9 @@ sub _check_extra_data {
         if ($type == CMT_MOVED_TO) {
             $extra_data = Bugzilla::User->check($extra_data)->login;
         }
-        elsif ($type == CMT_ATTACHMENT_CREATED) {
+        elsif ($type == CMT_ATTACHMENT_CREATED 
+               or $type == CMT_ATTACHMENT_UPDATED) 
+        {
              my $attachment = Bugzilla::Attachment->check({ 
                  id => $extra_data });
              $extra_data = $attachment->id;
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index e052d2ecb..a9340543b 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -93,6 +93,7 @@ use File::Basename;
     CMT_POPULAR_VOTES
     CMT_MOVED_TO
     CMT_ATTACHMENT_CREATED
+    CMT_ATTACHMENT_UPDATED
 
     THROW_ERROR
     
@@ -282,6 +283,7 @@ use constant CMT_HAS_DUPE => 2;
 use constant CMT_POPULAR_VOTES => 3;
 use constant CMT_MOVED_TO => 4;
 use constant CMT_ATTACHMENT_CREATED => 5;
+use constant CMT_ATTACHMENT_UPDATED => 6;
 
 # Determine whether a validation routine should return 0 or throw
 # an error when the validation fails.
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index bada40dad..74668a3bb 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -586,7 +586,7 @@ sub update_table_definitions {
     # 2009-11-01 LpSolit@gmail.com - Bug 525025
     _fix_invalid_custom_field_names();
 
-    _move_attachment_creation_comments_into_comment_type();
+    _set_attachment_comment_types();
 
     ################################################################
     # New --TABLE-- changes should go *** A B O V E *** this point #
@@ -3253,53 +3253,77 @@ sub _fix_invalid_custom_field_names {
     }
 }
 
-sub _move_attachment_creation_comments_into_comment_type {
+sub _set_attachment_comment_type {
+    my ($type, $string) = @_;
     my $dbh = Bugzilla->dbh;
-    # We check if there are any CMT_ATTACHMENT_CREATED comments already,
-    # first, because this is faster than a full LIKE search on the comments,
+    # We check if there are any comments of this type already, first, 
+    # because this is faster than a full LIKE search on the comments,
     # and currently this will run every time we run checksetup.
     my $test = $dbh->selectrow_array(
-        'SELECT 1 FROM longdescs WHERE type = ' . CMT_ATTACHMENT_CREATED
-        . ' ' . $dbh->sql_limit(1));
-    return if $test;
+        "SELECT 1 FROM longdescs WHERE type = $type " . $dbh->sql_limit(1));
+    return [] if $test;
     my %comments = @{ $dbh->selectcol_arrayref(
         "SELECT comment_id, thetext FROM longdescs
-          WHERE thetext LIKE 'Created an attachment (id=%'", 
+          WHERE thetext LIKE '$string%'", 
         {Columns=>[1,2]}) };
     my @comment_ids = keys %comments;
-    return if !scalar @comment_ids;
-    print "Setting the type field on attachment creation comments...\n";
+    return [] if !scalar @comment_ids;
+    my $what = "update";
+    if ($type == CMT_ATTACHMENT_CREATED) {
+        $what = "creation";
+    }
+    print "Setting the type field on attachment $what comments...\n";
     my $sth = $dbh->prepare(
         'UPDATE longdescs SET thetext = ?, type = ?, extra_data = ?
           WHERE comment_id = ?');
     my $count = 0;
     my $total = scalar @comment_ids;
-    $dbh->bz_start_transaction();
     foreach my $id (@comment_ids) {
         $count++;
         my $text = $comments{$id};
-        next if $text !~ /attachment \(id=(\d+)/;
+        next if $text !~ /^\Q$string\E(\d+)/;
         my $attachment_id = $1;
-        # Now we have to remove the text up until we find a line that's
-        # just a single newline, because the old "Created an attachment"
-        # text included the attachment description underneath it, and in
-        # Bugzillas before 2.20, that could be wrapped into multiple lines,
-        # in the database.
         my @lines = split("\n", $text);
-        while (1) {
-            my $line = shift @lines;
-            last if (!defined $line or trim($line) eq '');
+        if ($type == CMT_ATTACHMENT_CREATED) {
+            # Now we have to remove the text up until we find a line that's
+            # just a single newline, because the old "Created an attachment"
+            # text included the attachment description underneath it, and in
+            # Bugzillas before 2.20, that could be wrapped into multiple lines,
+            # in the database.
+            while (1) {
+                my $line = shift @lines;
+                last if (!defined $line or trim($line) eq '');
+            }
+        }
+        else {
+            # However, the "From update of attachment" line is always just
+            # one line--the first line of the comment.
+            shift @lines;
         }
         $text = join("\n", @lines);
-        $sth->execute($text, CMT_ATTACHMENT_CREATED, $attachment_id, $id);
+        $sth->execute($text, $type, $attachment_id, $id);
         indicate_progress({ total => $total, current => $count, 
                             every => 25 });
     }
+    return \@comment_ids;
+}
+
+sub _set_attachment_comment_types {
+    my $dbh = Bugzilla->dbh;
+    $dbh->bz_start_transaction();
+    my $created_ids = _set_attachment_comment_type(
+        CMT_ATTACHMENT_CREATED, 'Created an attachment (id=');
+    my $updated_ids = _set_attachment_comment_type(
+        CMT_ATTACHMENT_UPDATED, '(From update of attachment ');
+    $dbh->bz_commit_transaction();
+    return unless (@$created_ids or @$updated_ids);
+
+    my @comment_ids = (@$created_ids, @$updated_ids);
+
     my $bug_ids = $dbh->selectcol_arrayref(
         'SELECT DISTINCT bug_id FROM longdescs WHERE '
         . $dbh->sql_in('comment_id', \@comment_ids));
     _populate_bugs_fulltext($bug_ids);
-    $dbh->bz_commit_transaction();
 }
 
 1;
diff --git a/attachment.cgi b/attachment.cgi
index a89a46b99..bc1cb90f7 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -616,13 +616,11 @@ sub update {
 
     # If the user submitted a comment while editing the attachment,
     # add the comment to the bug. Do this after having validated isprivate!
-    if ($cgi->param('comment')) {
-        # Prepend a string to the comment to let users know that the comment came
-        # from the "edit attachment" screen.
-        my $comment = "(From update of attachment " . $attachment->id . ")\n" .
-                      $cgi->param('comment');
-
-        $bug->add_comment($comment, { isprivate => $attachment->isprivate });
+    my $comment = $cgi->param('comment');
+    if (trim($comment)) {
+        $bug->add_comment($comment, { isprivate => $attachment->isprivate,
+                                      type => CMT_ATTACHMENT_UPDATED,
+                                      extra_data => $attachment->id });
     }
 
     if ($can_edit) {
diff --git a/template/en/default/bug/format_comment.txt.tmpl b/template/en/default/bug/format_comment.txt.tmpl
index 49ab95ad1..27b72a918 100644
--- a/template/en/default/bug/format_comment.txt.tmpl
+++ b/template/en/default/bug/format_comment.txt.tmpl
@@ -57,6 +57,14 @@ Created attachment [% comment.extra_data %]
 [%+ comment.attachment.description %]
 
 [%+ comment.body %]
+[% ELSIF comment.type == constants.CMT_ATTACHMENT_UPDATED %]
+Comment on attachment [% comment.extra_data %]
+[% IF is_bugmail %]
+  --> [% urlbase _ "attachment.cgi?id=" _ comment.extra_data %]
+[% END %]
+[%+ comment.attachment.description %]
+
+[%+ comment.body %]
 [% ELSE %]
 X[% comment_body %]
 [% END %]