diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm index e14130b66d..127c7b304a 100644 --- a/Bugzilla/Comment.pm +++ b/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}}; diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index 0d3ba14a1c..d8211e94bd 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/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 argument. You will always be returned all comments you request in the C 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 parameter (C, C, etc), and +those authored by treeherder -- are left out of the comments returned for C +entirely. Passing +C<_collapsed_comments> in C includes them in the response. + +As with the other underscore-prefixed C values, it does not +imply C<_default>, so use C to get +the usual comment fields as well. + +Comments requested by C are always returned, collapsed or not. + =item C B C 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 for hidden revisions. +=item collapsed + +C True if this comment is collapsed in the web UI, either because one +of its tags is listed in the C parameter or because it +was authored by treeherder. False otherwise. + +This key is only present when comment tagging is enabled. + =back =item B @@ -3047,6 +3087,10 @@ section above for the object format. This is an B field returned only by specifying C or C<_extra> in C. +Comments that are collapsed in the web UI are left out unless +C<_collapsed_comments> is also passed in C, as described under +L. + =item C C The name of the current component of this bug. diff --git a/docs/en/rst/api/core/v1/bug.rst b/docs/en/rst/api/core/v1/bug.rst index b75516eee4..516d59d907 100644 --- a/docs/en/rst/api/core/v1/bug.rst +++ b/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``, diff --git a/docs/en/rst/api/core/v1/comment.rst b/docs/en/rst/api/core/v1/comment.rst index e7709f2572..29a0fb5d4a 100644 --- a/docs/en/rst/api/core/v1/comment.rst +++ b/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** diff --git a/extensions/BugModal/lib/ActivityStream.pm b/extensions/BugModal/lib/ActivityStream.pm index 5d2e980303..5745511763 100644 --- a/extensions/BugModal/lib/ActivityStream.pm +++ b/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) { diff --git a/qa/t/rest_bug_comments.t b/qa/t/rest_bug_comments.t index 5f0aa37e3d..53b521ce42 100644 --- a/qa/t/rest_bug_comments.t +++ b/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();