Bug 562308: importxml.pl should import each comment separately, rather than concatenating them all into a single one

r=ghendricks a=LpSolit

1 files changed, 32 insertions, 53 deletions

diff --git a/importxml.pl b/importxml.pl
index 350bf80c0..e1030b4bb 100755
--- a/importxml.pl
+++ b/importxml.pl
@@ -492,21 +492,17 @@ sub process_bug {
         }
     }
 
-    my @long_descs;
-    my $private = 0;
-
     # Parse long descriptions
+    my @long_descs;
     foreach my $comment ( $bug->children('long_desc') ) {
         Debug( "Parsing Long Description", DEBUG_LEVEL );
-        my %long_desc;
-        $long_desc{'who'}       = $comment->field('who');
-        $long_desc{'bug_when'}  = $comment->field('bug_when');
-        $long_desc{'isprivate'} = $comment->{'att'}->{'isprivate'} || 0;
-
-        # if one of the comments is private we need to set this flag
-        if ( $long_desc{'isprivate'} && $exporter->is_insider) {
-            $private = 1;
-        }
+        my %long_desc = ( who       => $comment->field('who'),
+                          bug_when  => $comment->field('bug_when'),
+                          isprivate => $comment->{'att'}->{'isprivate'} || 0 );
+
+        # If the exporter is not in the insidergroup, keep the comment public.
+        $long_desc{isprivate} = 0 unless $exporter->is_insider;
+
         my $data = $comment->field('thetext');
         if ( defined $comment->first_child('thetext')->{'att'}->{'encoding'}
             && $comment->first_child('thetext')->{'att'}->{'encoding'} =~
@@ -529,40 +525,21 @@ sub process_bug {
         my $url = $urlbase . "show_bug.cgi?id=";
         $data =~ s/([Bb]ugs?\s*\#?\s*(\d+))/$url$2/g;
 
-        $long_desc{'thetext'} = $data;
-        push @long_descs, \%long_desc;
-    }
+        # Keep the original commenter if possible, else we will fall back
+        # to the exporter account.
+        $long_desc{whoid} = login_to_id($long_desc{who});
 
-    # instead of giving each comment its own item in the longdescs
-    # table like it should have, lets cat them all into one big
-    # comment otherwise we would have to lie often about who
-    # authored the comment since commenters in one bugzilla probably
-    # don't have accounts in the other one.
-    # If one of the comments is private the whole comment will be
-    # private since we don't want to expose these unnecessarily
-    sub by_date { my @a; my @b; $a->{'bug_when'} cmp $b->{'bug_when'}; }
-    my @sorted_descs     = sort by_date @long_descs;
-    my $long_description = "";
-    for ( my $z = 0 ; $z <= $#sorted_descs ; $z++ ) {
-        if ( $z == 0 ) {
-            $long_description .= "\n\n\n---- Reported by ";
+        if (!$long_desc{whoid}) {
+            $data = "The original author of this comment is $long_desc{who}.\n\n" . $data;
         }
-        else {
-            $long_description .= "\n\n\n---- Additional Comments From ";
-        }
-        $long_description .= "$sorted_descs[$z]->{'who'} ";
-        $long_description .= "$sorted_descs[$z]->{'bug_when'}";
-        $long_description .= " ----";
-        $long_description .= "\n\n";
-        $long_description .= "THIS COMMENT IS PRIVATE \n"
-          if ( $sorted_descs[$z]->{'isprivate'} );
-        $long_description .= $sorted_descs[$z]->{'thetext'};
-        $long_description .= "\n";
+
+        $long_desc{'thetext'} = $data;
+        push @long_descs, \%long_desc;
     }
 
-    my $comments;
+    my @sorted_descs = sort { $a->{'bug_when'} cmp $b->{'bug_when'} } @long_descs;
 
-    $comments .= "\n\n--- Bug imported by $exporter_login ";
+    my $comments = "\n\n--- Bug imported by $exporter_login ";
     $comments .= format_time(scalar localtime(time()), '%Y-%m-%d %R %Z') . " ";
     $comments .= " ---\n\n";
     $comments .= "This bug was previously known as _bug_ $bug_fields{'bug_id'} at ";
@@ -611,12 +588,12 @@ sub process_bug {
     # Timestamps
     push( @query, "creation_ts" );
     push( @values,
-        format_time( $bug_fields{'creation_ts'}, "%Y-%m-%d %X" )
+        format_time( $bug_fields{'creation_ts'}, "%Y-%m-%d %T" )
           || $timestamp );
 
     push( @query, "delta_ts" );
     push( @values,
-        format_time( $bug_fields{'delta_ts'}, "%Y-%m-%d %X" )
+        format_time( $bug_fields{'delta_ts'}, "%Y-%m-%d %T" )
           || $timestamp );
 
     # Bug Access
@@ -1227,19 +1204,21 @@ sub process_bug {
     # Clear the attachments array for the next bug
     @attachments = ();
 
-    # Insert longdesc and append any errors
+    # Insert comments and append any errors
     my $worktime = $bug_fields{'actual_time'} || 0.0;
     $worktime = 0.0 if (!$exporter->is_timetracker);
-    $long_description .= "\n" . $comments;
-    if ($err) {
-        $long_description .= "\n$err\n";
+    $comments .= "\n$err\n" if $err;
+
+    my $sth_comment =
+      $dbh->prepare('INSERT INTO longdescs (bug_id, who, bug_when, isprivate,
+                                            thetext, work_time)
+                     VALUES (?, ?, ?, ?, ?, ?)');
+
+    foreach my $c (@sorted_descs) {
+        $sth_comment->execute($id, $c->{whoid} || $exporterid, $c->{bug_when},
+                              $c->{isprivate}, $c->{thetext}, 0);
     }
-    trick_taint($long_description);
-    $dbh->do("INSERT INTO longdescs 
-                     (bug_id, who, bug_when, work_time, isprivate, thetext) 
-                     VALUES (?,?,?,?,?,?)", undef,
-        $id, $exporterid, $timestamp, $worktime, $private, $long_description
-    );
+    $sth_comment->execute($id, $exporterid, $timestamp, 0, $comments, $worktime);
     Bugzilla::Bug->new($id)->_sync_fulltext('new_bug');
 
     # Add this bug to each group of which its product is a member.