From d4bcf5272dffa704cfe97774088e9ecbafa03b47 Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Tue, 21 Nov 2006 02:24:55 +0000 Subject: Bug 353656: User Interface gets corrupted by multiple localized users resolving bugs as duplicates - Patch by Frédéric Buclin r=bkor a=justdave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Bugzilla/Bug.pm | 69 +++++++++++++++++++-------- Bugzilla/BugMail.pm | 32 ++++--------- Bugzilla/Constants.pm | 13 +++++ Bugzilla/DB/Schema.pm | 3 ++ Bugzilla/Install/DB.pm | 5 ++ process_bug.cgi | 27 ++++------- template/en/default/global/messages.html.tmpl | 10 ++++ 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" %] +

[% terms.Bug %] moved to [% Param("move-to-url") FILTER html %].

+

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." %] -- cgit v1.2.3-24-g4f1b