Bug 353656: User Interface gets corrupted by multiple localized users resolving bugs as duplicates - Patch by Frédéric Buclin <LpSolit@gmail.com> r=bkor a=justdave

7 files changed, 98 insertions, 61 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index e9c8bf2e9..5e8392d9f 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -1315,12 +1315,16 @@ sub bug_alias_to_id {
 #####################################################################
 
 sub AppendComment {
-    my ($bugid, $whoid, $comment, $isprivate, $timestamp, $work_time) = @_;
+    my ($bugid, $whoid, $comment, $isprivate, $timestamp, $work_time,
+        $type, $extra_data) = @_;
     $work_time ||= 0;
+    $type ||= CMT_NORMAL;
     my $dbh = Bugzilla->dbh;
 
     ValidateTime($work_time, "work_time") if $work_time;
     trick_taint($work_time);
+    detaint_natural($type)
+      || ThrowCodeError('bad_arg', {argument => 'type', function => 'AppendComment'});
 
     # Use the date/time we were given if possible (allowing calling code
     # to synchronize the comment's timestamp with those of other records).
@@ -1329,7 +1333,7 @@ sub AppendComment {
     $comment =~ s/\r\n/\n/g;     # Handle Windows-style line endings.
     $comment =~ s/\r/\n/g;       # Handle Mac-style line endings.
 
-    if ($comment =~ /^\s*$/) {  # Nothin' but whitespace
+    if ($comment =~ /^\s*$/ && !$type) {  # Nothin' but whitespace
         return;
     }
 
@@ -1338,9 +1342,11 @@ sub AppendComment {
     trick_taint($comment); 
     my $privacyval = $isprivate ? 1 : 0 ;
     $dbh->do(q{INSERT INTO longdescs
-                      (bug_id, who, bug_when, thetext, isprivate, work_time)
-               VALUES (?,?,?,?,?,?)}, undef,
-             ($bugid, $whoid, $timestamp, $comment, $privacyval, $work_time));
+                      (bug_id, who, bug_when, thetext, isprivate, work_time,
+                       type, extra_data)
+               VALUES (?, ?, ?, ?, ?, ?, ?, ?)}, undef,
+             ($bugid, $whoid, $timestamp, $comment, $privacyval, $work_time,
+              $type, $extra_data));
     $dbh->do("UPDATE bugs SET delta_ts = ? WHERE bug_id = ?",
              undef, $timestamp, $bugid);
 }
@@ -1402,29 +1408,54 @@ sub ValidateTime {
 }
 
 sub GetComments {
-    my ($id, $comment_sort_order) = (@_);
+    my ($id, $comment_sort_order, $start, $end) = @_;
+    my $dbh = Bugzilla->dbh;
+
     $comment_sort_order = $comment_sort_order ||
         Bugzilla->user->settings->{'comment_sort_order'}->{'value'};
 
     my $sort_order = ($comment_sort_order eq "oldest_to_newest") ? 'asc' : 'desc';
-    my $dbh = Bugzilla->dbh;
+
     my @comments;
-    my $sth = $dbh->prepare(
-            "SELECT  profiles.realname AS name, profiles.login_name AS email,
-            " . $dbh->sql_date_format('longdescs.bug_when', '%Y.%m.%d %H:%i:%s') . "
-               AS time, longdescs.thetext AS body, longdescs.work_time,
-                     isprivate, already_wrapped
-             FROM    longdescs, profiles
-            WHERE    profiles.userid = longdescs.who
-              AND    longdescs.bug_id = ?
-            ORDER BY longdescs.bug_when $sort_order");
-    $sth->execute($id);
+    my @args = ($id);
+
+    my $query = 'SELECT profiles.realname AS name, profiles.login_name AS email, ' .
+                        $dbh->sql_date_format('longdescs.bug_when', '%Y.%m.%d %H:%i:%s') .
+                      ' AS time, longdescs.thetext AS body, longdescs.work_time,
+                        isprivate, already_wrapped, type, extra_data
+                   FROM longdescs
+             INNER JOIN profiles
+                     ON profiles.userid = longdescs.who
+                  WHERE longdescs.bug_id = ?';
+    if ($start) {
+        $query .= ' AND longdescs.bug_when > ?
+                    AND longdescs.bug_when <= ?';
+        push(@args, ($start, $end));
+    }
+    $query .= " ORDER BY longdescs.bug_when $sort_order";
+    my $sth = $dbh->prepare($query);
+    $sth->execute(@args);
 
     while (my $comment_ref = $sth->fetchrow_hashref()) {
         my %comment = %$comment_ref;
 
         $comment{'email'} .= Bugzilla->params->{'emailsuffix'};
         $comment{'name'} = $comment{'name'} || $comment{'email'};
+        if ($comment{'type'} == CMT_DUPE_OF) {
+            $comment{'body'} .= "\n\n" . get_text('bug_duplicate_of',
+                                                  { dupe_of => $comment{'extra_data'} });
+        }
+        elsif ($comment{'type'} == CMT_HAS_DUPE) {
+            $comment{'body'} = get_text('bug_has_duplicate',
+                                        { dupe => $comment{'extra_data'} });
+        }
+        elsif ($comment{'type'} == CMT_POPULAR_VOTES) {
+            $comment{'body'} = get_text('bug_confirmed_by_votes');
+        }
+        elsif ($comment{'type'} == CMT_MOVED_TO) {
+            $comment{'body'} .= "\n\n" . get_text('bug_moved_to',
+                                                  { login => $comment{'extra_data'} });
+        }
 
         push (@comments, \%comment);
     }
@@ -1760,9 +1791,7 @@ sub CheckIfVotedConfirmed {
                  "VALUES (?, ?, ?, ?, ?, ?)",
                  undef, ($id, $who, $timestamp, $fieldid, '0', '1'));
 
-        AppendComment($id, $who,
-                      "*** This bug has been confirmed by popular vote. ***",
-                      0, $timestamp);
+        AppendComment($id, $who, "", 0, $timestamp, 0, CMT_POPULAR_VOTES);
 
         $ret = 1;
     }
diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm
index 9a83b1cd3..6ba6b3c77 100644
--- a/Bugzilla/BugMail.pm
+++ b/Bugzilla/BugMail.pm
@@ -652,17 +652,6 @@ sub get_comments_by_bug {
     my $count = 0;
     my $anyprivate = 0;
 
-    my $query = 'SELECT profiles.login_name, profiles.realname, ' .
-                        $dbh->sql_date_format('longdescs.bug_when', '%Y.%m.%d %H:%i') . ',
-                        longdescs.thetext, longdescs.isprivate,
-                        longdescs.already_wrapped
-                   FROM longdescs
-             INNER JOIN profiles
-                     ON profiles.userid = longdescs.who
-                  WHERE longdescs.bug_id = ? ';
-
-    my @args = ($id);
-
     # $start will be undef for new bugs, and defined for pre-existing bugs.
     if ($start) {
         # If $start is not NULL, obtain the count-index
@@ -670,26 +659,21 @@ sub get_comments_by_bug {
         $count = $dbh->selectrow_array('SELECT COUNT(*) FROM longdescs
                                         WHERE bug_id = ? AND bug_when <= ?',
                                         undef, ($id, $start));
-
-        $query .= ' AND longdescs.bug_when > ?
-                    AND longdescs.bug_when <= ? ';
-        push @args, ($start, $end);
     }
 
-    $query .= ' ORDER BY longdescs.bug_when';
-    my $comments = $dbh->selectall_arrayref($query, undef, @args);
+    my $comments = Bugzilla::Bug::GetComments($id, "oldest_to_newest", $start, $end);
 
-    foreach (@$comments) {
-        my ($who, $whoname, $when, $text, $isprivate, $already_wrapped) = @$_;
+    foreach my $comment (@$comments) {
         if ($count) {
-            $result .= "\n\n--- Comment #$count from $whoname <$who" .
-                       Bugzilla->params->{'emailsuffix'}. ">  " 
-                       . format_time($when) . " ---\n";
+            $result .= "\n\n--- Comment #$count from " . $comment->{'name'} . " <" .
+                       $comment->{'email'} . Bugzilla->params->{'emailsuffix'} . ">  " .
+                       format_time($comment->{'time'}) . " ---\n";
         }
-        if ($isprivate > 0 && Bugzilla->params->{'insidergroup'}) {
+        if ($comment->{'isprivate'} > 0 && Bugzilla->params->{'insidergroup'}) {
             $anyprivate = 1;
         }
-        $result .= ($already_wrapped ? $text : wrap_comment($text));
+        $result .= ($comment->{'already_wrapped'} ? $comment->{'body'}
+                                                  : wrap_comment($comment->{'body'}));
         $count++;
     }
     return ($result, $anyprivate);
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 5186fcbce..89032c0b0 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -81,6 +81,12 @@ use File::Basename;
 
     COMMENT_COLS
 
+    CMT_NORMAL
+    CMT_DUPE_OF
+    CMT_HAS_DUPE
+    CMT_POPULAR_VOTES
+    CMT_MOVED_TO
+
     UNLOCK_ABORT
     THROW_ERROR
     
@@ -231,6 +237,13 @@ use constant LIST_OF_BUGS => 1;
 # The column length for displayed (and wrapped) bug comments.
 use constant COMMENT_COLS => 80;
 
+# The type of bug comments.
+use constant CMT_NORMAL => 0;
+use constant CMT_DUPE_OF => 1;
+use constant CMT_HAS_DUPE => 2;
+use constant CMT_POPULAR_VOTES => 3;
+use constant CMT_MOVED_TO => 4;
+
 # used by Bugzilla::DB to indicate that tables are being unlocked
 # because of error
 use constant UNLOCK_ABORT => 1;
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index c45419a37..7aa77108e 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -276,6 +276,9 @@ use constant ABSTRACT_SCHEMA => {
                                 DEFAULT => 'FALSE'},
             already_wrapped => {TYPE => 'BOOLEAN', NOTNULL => 1,
                                 DEFAULT => 'FALSE'},
+            type            => {TYPE => 'INT2', NOTNULL => 1,
+                                DEFAULT => '0'},
+            extra_data      => {TYPE => 'varchar(255)'}
         ],
         INDEXES => [
             longdescs_bug_id_idx   => ['bug_id'],
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index 9592976b6..5c3c1485a 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -509,6 +509,11 @@ sub update_table_definitions {
     $dbh->bz_add_column('group_control_map', 'canconfirm',
                         {TYPE => 'BOOLEAN', NOTNULL => 1, DEFAULT => 'FALSE'});
 
+    # 2006-11-07 LpSolit@gmail.com - Bug 353656
+    $dbh->bz_add_column('longdescs', 'type',
+                        {TYPE => 'INT2', NOTNULL => 1, DEFAULT => '0'});
+    $dbh->bz_add_column('longdescs', 'extra_data', {TYPE => 'varchar(255)'});
+
     ################################################################
     # New --TABLE-- changes should go *** A B O V E *** this point #
     ################################################################
diff --git a/process_bug.cgi b/process_bug.cgi
index 00594d89e..965e75a9f 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -549,13 +549,8 @@ if ($action eq Bugzilla->params->{'move-button-text'}) {
 
     my $comment = "";
     if (defined $cgi->param('comment') && $cgi->param('comment') !~ /^\s*$/) {
-        $comment = $cgi->param('comment') . "\n\n";
+        $comment = $cgi->param('comment');
     }
-    $comment .= "Bug moved to " . Bugzilla->params->{'move-to-url'} . ".\n\n";
-    $comment .= "If the move succeeded, " . $user->login . " will receive a mail\n";
-    $comment .= "containing the number of the new bug in the other database.\n";
-    $comment .= "If all went well,  please mark this bug verified, and paste\n";
-    $comment .= "in a link to the new bug. Otherwise, reopen this bug.\n";
 
     $dbh->bz_lock_tables('bugs WRITE', 'bugs_activity WRITE', 'duplicates WRITE',
                          'longdescs WRITE', 'profiles READ', 'groups READ',
@@ -574,7 +569,7 @@ if ($action eq Bugzilla->params->{'move-button-text'}) {
         $sth->execute($timestamp, $id);
         $sth2->execute($id);
 
-        AppendComment($id, $whoid, $comment, 0, $timestamp);
+        AppendComment($id, $whoid, $comment, 0, $timestamp, 0, CMT_MOVED_TO, $user->login);
 
         if ($bug->bug_status ne 'RESOLVED') {
             LogActivityEntry($id, 'bug_status', $bug->bug_status,
@@ -942,7 +937,7 @@ if ( defined $cgi->param('id') &&
     }
 }
 
-my $duplicate = 0;
+my $duplicate;
 
 # We need to check the addresses involved in a CC change before we touch any bugs.
 # What we'll do here is formulate the CC data into two hashes of ID's involved
@@ -1190,10 +1185,6 @@ SWITCH: for ($cgi->param('knob')) {
 
         ChangeStatus('RESOLVED');
         ChangeResolution($bug, 'DUPLICATE');
-        my $comment = $cgi->param('comment');
-        $comment .= "\n\n" 
-                    . get_text('bug_duplicate_of', { dupe_of => $duplicate });
-        $cgi->param('comment', $comment);
         last SWITCH;
     };
 
@@ -1605,9 +1596,12 @@ foreach my $id (@idlist) {
         }
     }
 
-    if ($cgi->param('comment') || $work_time) {
+    if ($cgi->param('comment') || $work_time || $duplicate) {
+        my $type = $duplicate ? CMT_DUPE_OF : CMT_NORMAL;
+
         AppendComment($id, $whoid, scalar($cgi->param('comment')),
-                      scalar($cgi->param('commentprivacy')), $timestamp, $work_time);
+                      scalar($cgi->param('commentprivacy')), $timestamp,
+                      $work_time, $type, $duplicate);
         $bug_changed = 1;
     }
 
@@ -2082,10 +2076,9 @@ foreach my $id (@idlist) {
             $dbh->do(q{INSERT INTO cc (who, bug_id) VALUES (?, ?)},
                      undef, $reporter, $duplicate);
         }
-        my $dupe_comment = get_text('bug_has_duplicate',
-                                    { dupe => $cgi->param('id') });
         # Bug 171639 - Duplicate notifications do not need to be private.
-        AppendComment($duplicate, $whoid, $dupe_comment, 0, $timestamp);
+        AppendComment($duplicate, $whoid, "", 0, $timestamp, 0,
+                      CMT_HAS_DUPE, scalar $cgi->param('id'));
 
         $dbh->do(q{INSERT INTO duplicates VALUES (?, ?)}, undef,
                  $duplicate, $cgi->param('id'));
diff --git a/template/en/default/global/messages.html.tmpl b/template/en/default/global/messages.html.tmpl
index b5f736120..eb17fc6e8 100644
--- a/template/en/default/global/messages.html.tmpl
+++ b/template/en/default/global/messages.html.tmpl
@@ -121,12 +121,22 @@
     Please add your attachment by clicking the "Create a New Attachment" link
     below.
 
+  [% ELSIF message_tag == "bug_confirmed_by_votes" %]
+    *** This [% terms.bug %] has been confirmed by popular vote. ***
+
   [% ELSIF message_tag == "bug_duplicate_of" %]
     *** This [% terms.bug %] has been marked as a duplicate of [% terms.bug %] [%+ dupe_of FILTER html %] ***
 
   [% ELSIF message_tag == "bug_has_duplicate" %]
     *** [% terms.Bug %] [%+ dupe FILTER html %] has been marked as a duplicate of this [% terms.bug %]. ***
 
+  [% ELSIF message_tag == "bug_moved_to" %]
+    <p>[% terms.Bug %] moved to [% Param("move-to-url") FILTER html %].</p>
+    <p>If the move succeeded, [% login FILTER html %] will receive a mail
+    containing the number of the new [% terms.bug %] in the other database.
+    If all went well, please mark this [% terms.bug %] verified, and paste
+    in a link to the new [% terms.bug %]. Otherwise, reopen this [% terms.bug %].
+
   [% ELSIF message_tag == "buglist_adding_field" %]
     [% title = "Adding field to search page..." %]
     [% link  = "Click here if the page does not redisplay automatically." %]