Bug 310450 - Bugzilla should send an email when a comment becomes private or un-private

Bugzilla/Bug.pm

@@ -798,11 +798,15 @@ sub update {
                 Bugzilla->user->id, $delta_ts);
         }
     }
-    
+   
+    # Comment Privacy 
     foreach my $comment_id (keys %{$self->{comment_isprivate} || {}}) {
         $dbh->do("UPDATE longdescs SET isprivate = ? WHERE comment_id = ?",
                  undef, $self->{comment_isprivate}->{$comment_id}, $comment_id);
-        # XXX It'd be nice to track this in the bug activity.
+        my ($from, $to) 
+            = $self->{comment_isprivate}->{$comment_id} ? (0, 1) : (1, 0);
+        LogActivityEntry($self->id, "longdescs.isprivate", $from, $to, 
+                         Bugzilla->user->id, $delta_ts, $comment_id);
     }
 
     # Insert the values into the multiselect value tables
@@ -3142,7 +3146,8 @@ sub GetBugActivity {
 
     my $query = "SELECT fielddefs.name, bugs_activity.attach_id, " .
         $dbh->sql_date_format('bugs_activity.bug_when', '%Y.%m.%d %H:%i:%s') .
-            ", bugs_activity.removed, bugs_activity.added, profiles.login_name
+            ", bugs_activity.removed, bugs_activity.added, profiles.login_name, 
+               bugs_activity.comment_id
           FROM bugs_activity
                $suppjoins
      LEFT JOIN fielddefs
@@ -3163,7 +3168,7 @@ sub GetBugActivity {
     my $incomplete_data = 0;
 
     foreach my $entry (@$list) {
-        my ($fieldname, $attachid, $when, $removed, $added, $who) = @$entry;
+        my ($fieldname, $attachid, $when, $removed, $added, $who, $comment_id) = @$entry;
         my %change;
         my $activity_visible = 1;
 
@@ -3174,7 +3179,14 @@ sub GetBugActivity {
             || $fieldname eq 'deadline')
         {
             $activity_visible = Bugzilla->user->is_timetracker;
-        } else {
+        }
+        elsif ($fieldname eq 'longdescs.isprivate'
+                && !Bugzilla->user->is_insider 
+                && $added) 
+        { 
+            $activity_visible = 0;
+        } 
+        else {
             $activity_visible = 1;
         }
 
@@ -3205,6 +3217,11 @@ sub GetBugActivity {
             $change{'attachid'} = $attachid;
             $change{'removed'} = $removed;
             $change{'added'} = $added;
+            
+            if ($comment_id) {
+                $change{'comment'} = Bugzilla::Comment->new($comment_id);
+            }
+
             push (@$changes, \%change);
         }
     }
@@ -3219,7 +3236,7 @@ sub GetBugActivity {
 
 # Update the bugs_activity table to reflect changes made in bugs.
 sub LogActivityEntry {
-    my ($i, $col, $removed, $added, $whoid, $timestamp) = @_;
+    my ($i, $col, $removed, $added, $whoid, $timestamp, $comment_id) = @_;
     my $dbh = Bugzilla->dbh;
     # in the case of CCs, deps, and keywords, there's a possibility that someone
     # might try to add or remove a lot of them at once, which might take more
@@ -3247,9 +3264,9 @@ sub LogActivityEntry {
         trick_taint($removestr);
         my $fieldid = get_field_id($col);
         $dbh->do("INSERT INTO bugs_activity
-                  (bug_id, who, bug_when, fieldid, removed, added)
-                  VALUES (?, ?, ?, ?, ?, ?)",
-                  undef, ($i, $whoid, $timestamp, $fieldid, $removestr, $addstr));
+                  (bug_id, who, bug_when, fieldid, removed, added, comment_id)
+                  VALUES (?, ?, ?, ?, ?, ?, ?)",
+                  undef, ($i, $whoid, $timestamp, $fieldid, $removestr, $addstr, $comment_id));
     }
 }
 

Bugzilla/BugMail.pm

@@ -238,7 +238,8 @@ sub Send {
     my $diffs = $dbh->selectall_arrayref(
            "SELECT profiles.login_name, profiles.realname, fielddefs.description,
                    bugs_activity.bug_when, bugs_activity.removed, 
-                   bugs_activity.added, bugs_activity.attach_id, fielddefs.name
+                   bugs_activity.added, bugs_activity.attach_id, fielddefs.name,
+                   bugs_activity.comment_id
               FROM bugs_activity
         INNER JOIN fielddefs
                 ON fielddefs.id = bugs_activity.fieldid
@@ -256,7 +257,7 @@ sub Send {
     my $fullwho;
     my @changedfields;
     foreach my $ref (@$diffs) {
-        my ($who, $whoname, $what, $when, $old, $new, $attachid, $fieldname) = (@$ref);
+        my ($who, $whoname, $what, $when, $old, $new, $attachid, $fieldname, $comment_id) = (@$ref);
         my $diffpart = {};
         if ($who ne $lastwho) {
             $lastwho = $who;
@@ -279,6 +280,12 @@ sub Send {
                 'SELECT isprivate FROM attachments WHERE attach_id = ?',
                 undef, ($attachid));
         }
+        if ($fieldname eq 'longdescs.isprivate') {
+            my $comment = Bugzilla::Comment->new($comment_id);
+            my $comment_num = $comment->count;
+            $what =~ s/^(Comment )?/Comment #$comment_num /;
+            $diffpart->{'isprivate'} = $new;
+        }
         $difftext = three_columns($what, $old, $new);
         $diffpart->{'header'} = $diffheader;
         $diffpart->{'fieldname'} = $fieldname;

Bugzilla/Comment.pm

@@ -188,6 +188,22 @@ sub _check_type {
     return $type;
 }
 
+sub count {
+    my ($self) = @_;
+
+    return $self->{'count'} if defined $self->{'count'};
+
+    my $dbh = Bugzilla->dbh;
+    ($self->{'count'}) = $dbh->selectrow_array(
+        "SELECT COUNT(*)
+           FROM longdescs 
+          WHERE bug_id = ? 
+                AND bug_when <= ?",
+        undef, $self->bug_id, $self->creation_ts);
+
+    return --$self->{'count'};
+}   
+
 1;
 
 __END__
@@ -243,6 +259,10 @@ C<0> otherwise.
 
 L<Bugzilla::User> who created the comment.
 
+=item C<count>
+
+C<int> The position this comment is located in the full list of comments for a bug starting from 0.
+
 =item C<body_full>
 
 =over
@@ -273,8 +293,6 @@ A string, the full text of the comment as it would be displayed to an end-user.
 
 =back
 
-
-
 =back
 
 =cut

Bugzilla/DB/Schema.pm

@@ -355,6 +355,10 @@ use constant ABSTRACT_SCHEMA => {
                                              COLUMN =>  'id'}},
             added     => {TYPE => 'varchar(255)'},
             removed   => {TYPE => 'TINYTEXT'},
+            comment_id => {TYPE => 'INT3', 
+                           REFERENCES => { TABLE  => 'longdescs',
+                                           COLUMN => 'comment_id',
+                                           DELETE => 'CASCADE'}},
         ],
         INDEXES => [
             bugs_activity_bug_id_idx  => ['bug_id'],

Bugzilla/Install/DB.pm

@@ -599,6 +599,9 @@ sub update_table_definitions {
     _convert_flagtypes_fks_to_set_null();
     _fix_decimal_types();
 
+    # 2009-11-14 dkl@redhat.com - Bug 310450
+    $dbh->bz_add_column('bugs_activity', 'comment_id', {TYPE => 'INT3'});
+
     ################################################################
     # New --TABLE-- changes should go *** A B O V E *** this point #
     ################################################################

template/en/default/bug/activity/table.html.tmpl

@@ -72,7 +72,13 @@
                 <a href="attachment.cgi?id=[% change.attachid %]">
                 Attachment #[% change.attachid %]</a>
               [% END %]
-              [%+ field_descs.${change.fieldname} FILTER html %]
+              [% IF change.comment.defined %]
+                 [% comment_desc = field_descs.${change.fieldname} %]
+                 [% comment_num = "Comment $change.comment.count" FILTER bug_link(bug.bug_id, comment_num => change.comment.count) %]
+                 [% comment_desc.replace('^(Comment )?', "$comment_num ") FILTER none %]
+              [% ELSE %]
+                [%+ field_descs.${change.fieldname} FILTER html %]
+              [% END %]
             </td>
             <td>
               [% IF change.removed.defined %]