Bug 2069143 - When retrieving comments for a bug(s) via REST API, remove comments that would be collapsed in the web UI unless explicitly asking for them

Bugzilla/Comment.pm

@@ -22,7 +22,7 @@ use Bugzilla::Hook;
 use Bugzilla::User;
 use Bugzilla::Util;
 
-use List::Util qw(first);
+use List::Util qw(first any);
 use Scalar::Util qw(blessed weaken isweak);
 use Role::Tiny::With;
 use Tie::IxHash;
@@ -327,8 +327,19 @@ sub tag_url {
 sub collapsed {
   my ($self) = @_;
   return $self->{collapsed} if exists $self->{collapsed};
-  return 0 unless Bugzilla->params->{'comment_taggers_group'};
   $self->{collapsed} = 0;
+
+  # treeherder is so spammy we hide its comments by default. treeherder_users
+  # is added by the BugModal extension, which may be disabled.
+  if (Bugzilla->can('treeherder_users')
+    && any { $_->id == $self->author->id } @{Bugzilla->treeherder_users})
+  {
+    $self->{collapsed}        = 1;
+    $self->{collapsed_reason} = $self->author->name;
+    return $self->{collapsed};
+  }
+
+  return $self->{collapsed} unless Bugzilla->params->{'comment_taggers_group'};
   Bugzilla->request_cache->{comment_tags_collapsed}
     ||= [split(/\s*,\s*/, lc(Bugzilla->params->{'collapsed_comment_tags'}))];
   my @collapsed_tags = @{Bugzilla->request_cache->{comment_tags_collapsed}};

Bugzilla/WebService/Bug.pm

@@ -364,11 +364,14 @@ sub comments {
   # request would run one aggregation query per bug.
   $self->_preload_comment_edit_info([map { @{$_->[1]} } @bug_comments]);
 
+  my $want_collapsed = _wants_collapsed_comments($params);
+
   foreach my $bug_comment (@bug_comments) {
     my ($bug, $comments) = @$bug_comment;
     my @result;
     foreach my $comment (@$comments) {
       next if $comment->is_private && !$user->is_insider;
+      next if $comment->collapsed && !$want_collapsed;
       push(@result, $self->_translate_comment($comment, $params));
     }
     $bugs{$bug->id}{'comments'} = \@result;
@@ -475,6 +478,18 @@ sub _preload_bugs_comment_edit_info {
   $self->_preload_comment_edit_info([map { @{$_->comments} } @$bugs]);
 }
 
+# Comments that Bugzilla::Comment::collapsed flags (tagged with one of the
+# 'collapsed_comment_tags' tags, or authored by treeherder) are hidden behind a
+# click in the web UI, so they are left out of the API response too unless the
+# caller asks for them with
+# include_fields=_collapsed_comments. Like the other underscore-prefixed
+# include_fields values, it does not imply _default, so callers that want the
+# usual fields as well need include_fields=_default,_collapsed_comments.
+sub _wants_collapsed_comments {
+  my ($params) = @_;
+  return grep { $_ eq '_collapsed_comments' } @{$params->{include_fields} || []};
+}
+
 # Helper for Bug.comments
 sub _translate_comment {
   my ($self, $comment, $filters, $types, $prefix) = @_;
@@ -512,6 +527,7 @@ sub _translate_comment {
   # Don't load comment tags unless enabled
   if (Bugzilla->params->{'comment_taggers_group'}) {
     $comment_hash->{tags} = [map { $self->type('string', $_) } @{$comment->tags}];
+    $comment_hash->{collapsed} = $self->type('boolean', $comment->collapsed);
   }
 
   return filter($filters, $comment_hash, $types, $prefix);
@@ -1676,8 +1692,10 @@ sub _bug_to_hash {
     my $comments
       = $bug->comments({order => 'oldest_to_newest', after => $params->{new_since}});
     $self->_preload_comment_edit_info($comments);
+    my $want_collapsed = _wants_collapsed_comments($params);
     foreach my $comment (@$comments) {
       next if $comment->is_private && !$user->is_insider;
+      next if $comment->collapsed && !$want_collapsed;
       push(@result,
            $self->_translate_comment($comment, $params, ['extra'], 'comments'));
     }
@@ -2773,6 +2791,20 @@ than this time. This only affects comments returned from the C<ids>
 argument. You will always be returned all comments you request in the
 C<comment_ids> argument, even if they are older than this date.
 
+=item C<_collapsed_comments>
+
+Comments that are collapsed in the web UI -- those tagged with one of the tags
+listed in the C<collapsed_comment_tags> parameter (C<spam>, C<abusive>, etc), and
+those authored by treeherder -- are left out of the comments returned for C<ids>
+entirely. Passing
+C<_collapsed_comments> in C<include_fields> includes them in the response.
+
+As with the other underscore-prefixed C<include_fields> values, it does not
+imply C<_default>, so use C<include_fields=_default,_collapsed_comments> to get
+the usual comment fields as well.
+
+Comments requested by C<comment_ids> are always returned, collapsed or not.
+
 =item C<skip_private> B<EXPERIMENTAL>
 
 C<boolean> Normally, if you request any inaccessible or invalid bug ids, this
@@ -2875,6 +2907,14 @@ comment, or null if the comment has never been edited.
 This key is only present for users who are allowed to edit other people's
 comments, and follows the same rules as C<edit_count> for hidden revisions.
 
+=item collapsed
+
+C<boolean> True if this comment is collapsed in the web UI, either because one
+of its tags is listed in the C<collapsed_comment_tags> parameter or because it
+was authored by treeherder. False otherwise.
+
+This key is only present when comment tagging is enabled.
+
 =back
 
 =item B<Errors>
@@ -3047,6 +3087,10 @@ section above for the object format.
 This is an B<extra> field returned only by specifying C<comments> or C<_extra>
 in C<include_fields>.
 
+Comments that are collapsed in the web UI are left out unless
+C<_collapsed_comments> is also passed in C<include_fields>, as described under
+L</comments>.
+
 =item C<component>
 
 C<string> The name of the current component of this bug.

docs/en/rst/api/core/v1/bug.rst

@@ -256,6 +256,9 @@ attachments               array     Each array item is an Attachment object. See
                                     :ref:`rest_attachments` for details of the object.
 comments                  array     Each array item is a Comment object. See
                                     :ref:`rest_comments` for details of the object.
+                                    Comments that are collapsed in the web UI are
+                                    left out unless ``_collapsed_comments`` is also
+                                    passed in ``include_fields``.
 counts                    object    An object containing the numbers of the items in the
                                     following fields: ``attachments``, ``cc``,
                                     ``comments``, ``keywords``, ``blocks``,

docs/en/rst/api/core/v1/comment.rst

@@ -33,8 +33,20 @@ new_since        datetime  If specified, the method will only return comments
                            returned all comments you request in the
                            ``comment_ids`` argument, even if they are older than
                            this date.
+include_fields   array     Pass ``_collapsed_comments`` to include comments that
+                           are collapsed in the web UI (see below). As with the
+                           other underscore-prefixed values it does not imply
+                           ``_default``, so use
+                           ``include_fields=_default,_collapsed_comments`` to get
+                           the usual comment fields as well.
 ===============  ========  ======================================================
 
+Comments that are collapsed in the web UI -- those tagged with one of the tags
+listed in the ``collapsed_comment_tags`` parameter (``spam``, ``abusive``, etc),
+and those authored by treeherder -- are left out of the comments returned for a
+bug entirely unless ``_collapsed_comments`` is requested. Comments requested by
+``comment_id`` are always returned, collapsed or not.
+
 **Response**
 
 .. code-block:: js
@@ -132,6 +144,12 @@ last_change_time  datetime  The time (in Bugzilla's timezone) of the most recent
 reactions         object    An object containing reacted emoji names and
                             corresponding counts. To retrieve reacted users, use
                             :ref:`rest_get_comment_reactions`.
+collapsed         boolean   ``true`` if this comment is collapsed in the web UI,
+                            either because one of its tags is listed in the
+                            ``collapsed_comment_tags`` parameter or because it was
+                            authored by treeherder. ``false`` otherwise.
+
+                            Only present when comment tagging is enabled.
 ================  ========  =====================================================
 
 **Errors**

extensions/BugModal/lib/ActivityStream.pm

@@ -17,7 +17,6 @@ use warnings;
 use Bugzilla::Extension::BugModal::Util qw(date_str_to_time);
 use Bugzilla::User;
 use Bugzilla::Constants;
-use List::MoreUtils qw(any);
 
 # returns an arrayref containing all changes to the bug - comments, field
 # changes, and duplicates
@@ -170,7 +169,6 @@ sub _add_activity_to_stream {
 sub _add_comments_to_stream {
   my ($bug, $stream) = @_;
   my $user = Bugzilla->user;
-  my @treeherder_ids = map { $_->id } @{Bugzilla->treeherder_users};
 
   my $raw_comments = $bug->comments();
   foreach my $comment (@$raw_comments) {
@@ -182,12 +180,6 @@ sub _add_comments_to_stream {
       && ($comment->work_time - 0) != 0
       && $user->is_timetracker;
 
-    # treeherder is so spammy we hide its comments by default
-    if (any { $_ == $author_id } @treeherder_ids) {
-      $comment->{collapsed}        = 1;
-      $comment->{collapsed_reason} = $comment->author->name;
-    }
-
     # If comment type is resolved as duplicate, do not add '...marked as
     # duplicate...' string to comment body
     if ($comment->type == CMT_DUPE_OF) {

qa/t/rest_bug_comments.t

@@ -195,4 +195,60 @@ foreach my $test (@comment_tests) {
   }
 }
 
+###########################
+# Collapsed Comment Tests #
+###########################
+
+# Comments tagged with one of the collapsed_comment_tags are collapsed in the
+# web UI, so they are left out of the API response unless the caller asks for
+# them with include_fields=_collapsed_comments.
+
+my $admin_key = $config->{admin_user_api_key};
+
+$t->post_ok($url
+    . 'rest/bug/public_bug/comment' => {'X-Bugzilla-API-Key' => $admin_key} =>
+    json => {comment => 'spammy comment'})->status_is(201);
+my $spam_id = $t->tx->res->json->{id};
+
+$t->put_ok($url
+    . "rest/bug/comment/$spam_id/tags" =>
+    {'X-Bugzilla-API-Key' => $admin_key} => json => {add => ['spam']})
+  ->status_is(200);
+
+my $bug_comment_path = $url . 'rest/bug/public_bug/comment';
+
+$t->get_ok($bug_comment_path => api_headers($admin_key))->status_is(200);
+my @ids = map { $_->{id} } map { @{$_->{comments}} }
+  values %{$t->tx->res->json->{bugs}};
+ok(!grep({ $_ == $spam_id } @ids), 'collapsed comment is omitted by default');
+
+$t->get_ok($bug_comment_path
+    . '?include_fields=_default,_collapsed_comments' => api_headers($admin_key))
+  ->status_is(200);
+my ($spam_comment)
+  = grep { $_->{id} == $spam_id } map { @{$_->{comments}} }
+  values %{$t->tx->res->json->{bugs}};
+ok($spam_comment, 'collapsed comment is returned with _collapsed_comments');
+ok($spam_comment->{collapsed}, 'collapsed comment is flagged as collapsed');
+
+# GET /rest/bug?include_fields=comments is a separate code path (_bug_to_hash).
+$t->get_ok($url
+    . 'rest/bug/public_bug?include_fields=comments' => api_headers($admin_key))
+  ->status_is(200);
+@ids = map { $_->{id} } @{$t->tx->res->json->{bugs}->[0]->{comments}};
+ok(!grep({ $_ == $spam_id } @ids),
+  'collapsed comment is omitted by default from GET /rest/bug');
+
+$t->get_ok($url
+    . 'rest/bug/public_bug?include_fields=comments,_collapsed_comments' =>
+    api_headers($admin_key))->status_is(200);
+@ids = map { $_->{id} } @{$t->tx->res->json->{bugs}->[0]->{comments}};
+ok(grep({ $_ == $spam_id } @ids),
+  'collapsed comment is returned from GET /rest/bug with _collapsed_comments');
+
+# Requesting the comment by id returns it either way.
+$t->get_ok($url . "rest/bug/comment/$spam_id" => api_headers($admin_key))
+  ->status_is(200)->json_is("/comments/$spam_id/id", $spam_id,
+  'collapsed comment is returned when requested by id');
+
 done_testing();