Bug 401957: Move comment "isprivate" updating from process_bug to Bugzilla::Bug

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

5 files changed, 38 insertions, 30 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 7a4c02e89..e79a7a1ca 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -501,6 +501,12 @@ sub update {
                 Bugzilla->user->id, $delta_ts);
         }
     }
+    
+    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.
+    }
 
     # Insert the values into the multiselect value tables
     my @multi_selects = Bugzilla->get_fields(
@@ -1339,6 +1345,19 @@ sub _set_global_validator {
 
 sub set_alias { $_[0]->set('alias', $_[1]); }
 sub set_cclist_accessible { $_[0]->set('cclist_accessible', $_[1]); }
+sub set_comment_is_private {
+    my ($self, $comment_id, $isprivate) = @_;
+    return unless Bugzilla->user->is_insider;
+    my ($comment) = grep($comment_id eq $_->{id}, @{$self->longdescs});
+    ThrowUserError('comment_invalid_isprivate', { id => $comment_id }) 
+        if !$comment;
+
+    $isprivate = $isprivate ? 1 : 0;
+    if ($isprivate != $comment->{isprivate}) {
+        $self->{comment_isprivate} ||= {};
+        $self->{comment_isprivate}->{$comment_id} = $isprivate;
+    }
+}
 sub set_component  {
     my ($self, $name) = @_;
     my $old_comp  = $self->component_obj;
diff --git a/process_bug.cgi b/process_bug.cgi
index 027965f05..99ee5ed57 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -451,28 +451,14 @@ if (defined $cgi->param('id')) {
         $bug->set_reporter_accessible($cgi->param('reporter_accessible'))
             if $bug->check_can_change_field('reporter_accessible', 0, 1);
     }
-}
-
-if ( defined $cgi->param('id') &&
-     (Bugzilla->params->{"insidergroup"} 
-      && Bugzilla->user->in_group(Bugzilla->params->{"insidergroup"})) ) 
-{
-
-    my $sth = $dbh->prepare('UPDATE longdescs SET isprivate = ?
-                             WHERE bug_id = ? AND bug_when = ?');
-
-    foreach my $field ($cgi->param()) {
-        if ($field =~ /when-([0-9]+)/) {
-            my $sequence = $1;
-            my $private = $cgi->param("isprivate-$sequence") ? 1 : 0 ;
-            if ($private != $cgi->param("oisprivate-$sequence")) {
-                my $field_data = $cgi->param("$field");
-                # Make sure a valid date is given.
-                $field_data = format_time($field_data, '%Y-%m-%d %T');
-                $sth->execute($private, $cgi->param('id'), $field_data);
-            }
-        }
-
+    
+    # You can only mark/unmark comments as private on single bugs. If
+    # you're not in the insider group, this code won't do anything.
+    foreach my $field (grep(/^defined_isprivate/, $cgi->param())) {
+        $field =~ /(\d+)$/;
+        my $comment_id = $1;
+        $bug->set_comment_is_private($comment_id,
+                                     $cgi->param("isprivate_$comment_id"));
     }
 }
 
diff --git a/template/en/default/bug/comments.html.tmpl b/template/en/default/bug/comments.html.tmpl
index 895ab5306..7a0020d47 100644
--- a/template/en/default/bug/comments.html.tmpl
+++ b/template/en/default/bug/comments.html.tmpl
@@ -191,14 +191,14 @@
 
       [% IF mode == "edit" && isinsider %]
         <i>
-          <input type="hidden" name="oisprivate-[% count %]" 
-                 value="[% comment.isprivate %]">
-          <input type="hidden" name="when-[% count %]" value="[% comment.time %]">
-          <input type="checkbox" name="isprivate-[% count %]" value="1"
+          <input type="hidden" value="1"
+                 name="defined_isprivate_[% comment.id %]">
+          <input type="checkbox"
+                 name="isprivate_[% comment.id %]" value="1"
+                 id="isprivate_[% comment.id %]"
                  onClick="updateCommentPrivacy(this, [% count %])"
-                 id="isprivate-[% count %]"
                  [% " checked=\"checked\"" IF comment.isprivate %]>
-          <label for="isprivate-[% count %]">Private</label>
+          <label for="isprivate_[% comment.id %]">Private</label>
         </i>
       [% END %]
       [% IF user.in_group(Param('timetrackinggroup')) &&
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index 1ae80430a..80957e6c7 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -275,8 +275,7 @@
 ],
 
 'bug/comments.html.tmpl' => [
-  'comment.isprivate', 
-  'comment.time', 
+  'comment.id',
   'bug.bug_id',
 ],
 
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 16bc0f7af..b0c087530 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -246,6 +246,10 @@
     [% title = "Chart Too Large" %]
     Sorry, but 2000 x 2000 is the maximum size for a chart.
 
+  [% ELSIF error == "comment_invalid_isprivate" %]
+    You tried to modify the privacy of comment id [% id FILTER html %],
+    but that is not a valid comment on this [% terms.bug %].
+
   [% ELSIF error == "comment_required" %]
     [% title = "Comment Required" %]
     You have to specify a <b>comment</b>