526734 - English string "From update of attachment" in localized installations

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[Max Kanat-Alexander]

bug 317127 handles the "Created an attachment" message, but the "From update of attachment" message still exists as a hardcoded string in attachment.cgi.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

In addition to fixing this bug, this does a few nice things:

1) It changes the message to "Comment on attachment X", which I think is a bit nicer.

2) It shows the description of the comment below the "Comment on attachment X" header, which I think is useful because I never have any idea what attachment is being commented *on*, when people leave attachment comments.

Comment 2

[Max Kanat-Alexander]

Oh, and the patch of course requires the patch from the blocker.

Comment 3

[Frédéric Buclin]

Comment on attachment 410470 [details] [diff] [review]
v1

The blocker has been updated meanwhile, and this patch no longer applies cleanly.

Comment 4

[Max Kanat-Alexander]

Attachment: v2

Okay, here we go.

Comment 5

[Frédéric Buclin]

Comment on attachment 416442 [details] [diff] [review]
v2

>Index: Bugzilla/Install/DB.pm

>+sub _set_attachment_comment_type {

>+    my $what = "update";
>+    if ($type == CMT_ATTACHMENT_CREATED) {
>+        $what = "creation";
>+    }

Nit: my $what = ($type == CMT_ATTACHMENT_CREATED) ? 'creation' : 'update'; would be cleaner IMO.


>+        next if $text !~ /\Q$string\E(\d+)/;

Nit: would be more robust as /^\Q$string\E(\d+)/ I think, so that we don't catch unrelated strings in the middle of a comment.


>+sub _set_attachment_comment_types {

>+    my %comment_ids = map { $_ => 1 } (@{ $created_ids || [] },
>+                                       @{ $updated_ids || [] });

A comment cannot be of both types, so
  my @comment_ids = (@$created_ids, @$updated_ids);
would be cleaner. Just replace
  return if !scalar @comment_ids;
by
  return [] if !scalar @comment_ids;
in _set_attachment_comment_type().


r=LpSolit

Comment 6

[Max Kanat-Alexander]

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.165; previous revision: 1.164
done
Checking in Bugzilla/Comment.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Comment.pm,v  <--  Comment.pm
new revision: 1.3; previous revision: 1.2
done
Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.124; previous revision: 1.123
done
Checking in Bugzilla/Install/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v  <--  DB.pm
new revision: 1.78; previous revision: 1.77
done
Checking in template/en/default/bug/format_comment.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/format_comment.txt.tmpl,v  <--  format_comment.txt.tmpl
new revision: 1.7; previous revision: 1.6
done