diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm index 127c7b304a..e73a3c4867 100644 --- a/Bugzilla/Comment.pm +++ b/Bugzilla/Comment.pm @@ -406,6 +406,8 @@ sub body_full { } else { $body = $self->body; + Bugzilla::Hook::process('comment_body_full', + {comment => $self, body => \$body}); } if ($params->{wrap} and !$self->already_wrapped) { $body = wrap_comment($body); diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm index e230dd92cd..b79a7fd819 100644 --- a/Bugzilla/Hook.pm +++ b/Bugzilla/Hook.pm @@ -735,6 +735,25 @@ spaces. =back +=head2 comment_body_full + +Called by L for a plain comment, before any +wrapping, so that an extension can change the text people read without +changing the stored text. Bugmail, webhook and Pulse payloads use the changed +text, while L and the C REST field keep +returning the stored text. + +Params: + +=over + +=item C - The L being read. + +=item C - A reference to the comment text, which you can modify. + +=back + + =head2 config_add_panels If you want to add new panels to the Parameters administrative interface, diff --git a/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl index c7413b062f..e11a106bb7 100644 --- a/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl +++ b/extensions/BugModal/template/en/default/bug_modal/activity_stream.html.tmpl @@ -167,7 +167,7 @@ [% Hook.process('user', 'bug/comments.html.tmpl') %] [% END %] - + [% Hook.process('before_comment_actions', 'bug_modal/activity_stream.html.tmpl') %]
[% IF user.is_insider && bug.check_can_change_field('longdesc', 0, 1).allowed %] [% IF comment.is_private %] diff --git a/extensions/BugModal/template/en/default/bug_modal/comment_tags.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/comment_tags.html.tmpl index 02e0ca1280..b733084455 100644 --- a/extensions/BugModal/template/en/default/bug_modal/comment_tags.html.tmpl +++ b/extensions/BugModal/template/en/default/bug_modal/comment_tags.html.tmpl @@ -6,8 +6,12 @@ # defined by the Mozilla Public License, v. 2.0. #%] +[% hidden_tags = [] %] +[% Hook.process('before_tags', 'bug_modal/comment_tags.html.tmpl') %] + [% FOREACH tag IN comment.tags ~%] - + [%~ '×' IF user.can_tag_comments %] [% IF comment.tag_url(tag) %] [role="group"] { justify-content: flex-end; flex-wrap: wrap; - gap: 4px 0; - width: 128px; + width: 64px; } .comment .comment-actions > [role="group"] .comment-private { diff --git a/extensions/EditComments/lib/WebService.pm b/extensions/EditComments/lib/WebService.pm index 74932a5154..23ec807f01 100644 --- a/extensions/EditComments/lib/WebService.pm +++ b/extensions/EditComments/lib/WebService.pm @@ -123,10 +123,14 @@ sub update_comment { sync_fulltext => 1, }); + # Render what the bug page shows, which can differ from the stored text: the + # comment_body_full hook lets extensions hide part of it (eg. Hackbot's front + # matter). + my $body = $comment->body_full({exclude_attachment => 1}); my $html = $comment->is_markdown && Bugzilla->params->{use_markdown} - ? Bugzilla->markdown->render_html($new_comment, $bug) - : Bugzilla::Template::quoteUrls($new_comment, $bug); + ? Bugzilla->markdown->render_html($body, $bug) + : Bugzilla::Template::quoteUrls($body, $bug); # Respond with the updated comment and number of revisions return { diff --git a/extensions/Hackbot/Config.pm b/extensions/Hackbot/Config.pm new file mode 100644 index 0000000000..c05acc478f --- /dev/null +++ b/extensions/Hackbot/Config.pm @@ -0,0 +1,20 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Extension::Hackbot; + +use 5.10.1; +use strict; +use warnings; + +use constant NAME => 'Hackbot'; + +use constant REQUIRED_MODULES => []; + +use constant OPTIONAL_MODULES => []; + +__PACKAGE__->NAME; diff --git a/extensions/Hackbot/Extension.pm b/extensions/Hackbot/Extension.pm new file mode 100644 index 0000000000..19d108d9b8 --- /dev/null +++ b/extensions/Hackbot/Extension.pm @@ -0,0 +1,70 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Extension::Hackbot; + +use 5.10.1; +use strict; +use warnings; + +use base qw(Bugzilla::Extension); + +use Bugzilla::Extension::Hackbot::Metadata qw(parse_front_matter); +use Bugzilla::User; + +our $VERSION = '1'; + +BEGIN { + *Bugzilla::Comment::hackbot = \&_comment_hackbot; +} + +# The ID of the account named by the hackbot_user param, or 0 when it is unset +# or names nobody. Bugzilla::User::login_to_id doesn't cache a miss, and this +# runs for every comment that is read. +sub _hackbot_user_id { + my $login = Bugzilla->params->{hackbot_user} or return 0; + my $cache = Bugzilla->request_cache->{hackbot_user_ids} ||= {}; + return $cache->{$login} //= Bugzilla::User::login_to_id($login) || 0; +} + +# Anyone can type a front matter block into a comment, so only the Hackbot +# account's comments are parsed, and only plain ones: the text of other types +# (attachments, duplicates) is formatted without the comment_body_full hook, so +# their block would stay visible. Returns the same list as parse_front_matter. +# A page reads each comment several times (header, badge, tags, text), so the +# result is kept on the comment for as long as its text is unchanged. +sub _parse_comment { + my ($comment) = @_; + my $user_id = _hackbot_user_id() or return; + return if ($comment->{who} // 0) != $user_id || $comment->type; + + my $body = $comment->body; + my $parsed = $comment->{hackbot_front_matter}; + if (!$parsed || $parsed->[0] ne $body) { + $parsed = $comment->{hackbot_front_matter} = [$body, parse_front_matter($body)]; + } + return @$parsed[1 .. $#$parsed]; +} + +sub _comment_hackbot { + my ($comment) = @_; + my ($metadata) = _parse_comment($comment); + return $metadata; +} + +sub comment_body_full { + my ($self, $args) = @_; + my (undef, $body) = _parse_comment($args->{comment}); + ${$args->{body}} = $body if defined $body; +} + +sub config_add_panels { + my ($self, $args) = @_; + $args->{panel_modules}->{Hackbot} = 'Bugzilla::Extension::Hackbot::Config'; +} + +__PACKAGE__->NAME; diff --git a/extensions/Hackbot/lib/Config.pm b/extensions/Hackbot/lib/Config.pm new file mode 100644 index 0000000000..eea1c9076a --- /dev/null +++ b/extensions/Hackbot/lib/Config.pm @@ -0,0 +1,37 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Extension::Hackbot::Config; + +use 5.10.1; +use strict; +use warnings; + +use Bugzilla::Config::Common; +use Bugzilla::User; + +our $sortkey = 1310; + +sub get_param_list { + my ($class) = @_; + + return ({ + name => 'hackbot_user', + type => 't', + default => '', + checker => \&check_hackbot_user, + }); +} + +# Empty is valid: it turns the extension off. +sub check_hackbot_user { + my ($login) = @_; + return '' if $login eq '' || Bugzilla::User::login_to_id($login); + return "There is no user named '$login'."; +} + +1; diff --git a/extensions/Hackbot/lib/Metadata.pm b/extensions/Hackbot/lib/Metadata.pm new file mode 100644 index 0000000000..edfdff4617 --- /dev/null +++ b/extensions/Hackbot/lib/Metadata.pm @@ -0,0 +1,57 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Extension::Hackbot::Metadata; + +use 5.10.1; +use strict; +use warnings; + +use base qw(Exporter); + +our @EXPORT_OK = qw(parse_front_matter); + +# The block hackbot-api puts at the top of a comment: +# +# --- +# hackbot: +# run: 550e8400-e29b-41d4-a716-446655440000 +# action: 3f2a9c1e7b4d4f0a8e6c2d1b5a7f9e30 +# agent: bug-fix +# url: https://hackbot.moz.tools/runs/550e8400-e29b-41d4-a716-446655440000 +# --- +# +# It is only ever generated, so this reads that one shape rather than YAML in +# general. Anything else is an ordinary comment that happens to start with a +# rule, and unknown keys are skipped so that fields can be added. The /a flag +# keeps the character classes to ASCII. +my $FRONT_MATTER + = qr/\A---\nhackbot:\n((?: [[:lower:]_]+: [^\n]*\n)+)---(?:\n+|\z)(.*)\z/as; +my $FIELD = qr/^ ([[:lower:]_]+): ([^\n]*?)\s*$/am; +my $IDENTIFIER = qr/\A[[:alnum:]._-]+\z/a; + +# Returns the metadata hash and the comment text without the block, or an empty +# list when the text doesn't start with a valid block. The url key is left out +# unless it is an https URL, because it ends up in a link's href. +sub parse_front_matter { + my ($text) = @_; + + my ($block, $body) = ($text // '') =~ $FRONT_MATTER or return; + + my %fields = $block =~ /$FIELD/g; + + my %metadata; + foreach my $key (qw(run action agent)) { + return unless ($fields{$key} // '') =~ $IDENTIFIER; + $metadata{$key} = $fields{$key}; + } + $metadata{url} = $fields{url} if ($fields{url} // '') =~ m{\Ahttps://\S+\z}; + + return (\%metadata, $body); +} + +1; diff --git a/extensions/Hackbot/t/bmo/comment-metadata.t b/extensions/Hackbot/t/bmo/comment-metadata.t new file mode 100644 index 0000000000..af89e37ba8 --- /dev/null +++ b/extensions/Hackbot/t/bmo/comment-metadata.t @@ -0,0 +1,258 @@ +#!/usr/bin/env perl +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +# Bug 2086124: Hackbot comments carry a front matter block that links them to +# the run that wrote them. Comments are posted through the real REST API, +# in-process with Test::Mojo, then read back. The parser itself is covered +# without a DB in extensions/Hackbot/t/metadata.t. + +use 5.10.1; +use strict; +use warnings; +use lib qw(. lib local/lib/perl5); + +BEGIN { + $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf'; + $ENV{BUGZILLA_DISABLE_HOSTAGE} = 1; +} + +use Bugzilla; +BEGIN { Bugzilla->extensions } +use Bugzilla::App (); +use Bugzilla::Bug; +use Bugzilla::Comment; +use Bugzilla::Config qw(SetParam write_params); +use Bugzilla::Constants qw(bz_locations); +use Bugzilla::Extension::Hackbot::Config; +use Bugzilla::Extension::Push::Serialize; +use Bugzilla::Test::Util qw(create_user issue_api_key); +use Bugzilla::User; +use Bugzilla::User::Setting qw(clear_settings_cache); + +use Email::MIME; +use Test::More; +use Test::Mojo; + +# The app reads params from the database (through memcached) on every +# request, so set them there, and put them back afterwards. Bugmail is sent +# while the comment is posted and written to a file, so it can be read here. +my %original_params = map { $_ => Bugzilla->params->{$_} // '' } + qw(hackbot_user use_mailer_queue mail_delivery_method edit_comments_admins_group); + +sub set_params { + my (%params) = @_; + SetParam($_, $params{$_}) foreach keys %params; + write_params(); + delete Bugzilla->request_cache->{hackbot_user_ids}; +} + +sub set_hackbot_user { set_params(hackbot_user => $_[0]) } + +END { set_params(%original_params) if %original_params } +set_params( + hackbot_user => '', + use_mailer_queue => 0, + mail_delivery_method => 'Test' +); + +my $admin = Bugzilla::User->check({id => 1}); +Bugzilla->set_user($admin); + +# Reused on a rerun. +my $hackbot = create_user('hackbot@hackbot-metadata.test', '*'); +my $other = create_user('other@hackbot-metadata.test', '*'); + +# Gets bugmail as text and HTML. Bugmail skips fake addresses (.bugs, .tld), +# so it has an ordinary one. Set up before the app runs any request: creating +# a user audits the change with remote_ip(), which wants a controller once a +# request has run. +my $watcher = create_user('watcher@hackbot-metadata.test', '*'); +$watcher->settings->{email_format}->set('html'); +clear_settings_cache($watcher->id); + +my %api_key = map { $_->login => issue_api_key($_->login)->api_key } $admin, + $hackbot, $other; + +# One Test::Mojo for the whole file: building the app twice makes CGI::Compile +# die with "Tried to load admin.cgi more than once". +my $t = Test::Mojo->new('Bugzilla::App'); + +sub api_headers { + my ($user) = @_; + return {'X-Bugzilla-API-Key' => $api_key{$user->login}}; +} + +$t->post_ok( + '/rest/bug' => api_headers($admin) => json => { + product => 'Firefox', + component => 'General', + version => 'Trunk', + type => 'defect', + severity => 'normal', + op_sys => 'Unspecified', + platform => 'Unspecified', + summary => 'Hackbot comment metadata', + description => 'A bug for Hackbot to comment on', + } +)->status_is(200, 'the bug is filed') + or diag $t->tx->res->body; +my $bug_id = $t->tx->res->json->{id}; + +# Post a comment as $user and return it, read back from the database. +sub post_comment { + my ($user, $text) = @_; + $t->post_ok("/rest/bug/$bug_id/comment" => api_headers($user) => json => + {comment => $text, is_markdown => 1})->status_is(201, 'the comment is added') + or diag $t->tx->res->body; + return Bugzilla::Comment->new($t->tx->res->json->{id}); +} + +my $url = 'https://hackbot.example/runs/abc-123'; +my $metadata + = {run => 'abc-123', action => 'f00', agent => 'bug-fix', url => $url}; +my $block = join("\n", + '---', 'hackbot:', ' run: abc-123', + ' action: f00', ' agent: bug-fix', " url: $url", + '---', '', ''); +my $text = 'The failure is here.'; + +# hackbot_user is empty: the feature is off. +my $comment = post_comment($hackbot, "$block$text"); +is($comment->body_full, "$block$text", 'off: the block stays in the text'); +is($comment->hackbot, undef, 'off: no metadata'); + +# hackbot_user is set. +set_hackbot_user($hackbot->login); + +$comment = post_comment($hackbot, "$block$text"); +is($comment->body_full, $text, 'body_full hides the block'); +is($comment->body_full({wrap => 1}), $text, '... when wrapped too'); +is($comment->body, "$block$text", 'body keeps the stored text'); +is_deeply($comment->hackbot, $metadata, 'hackbot returns the metadata'); + +$t->get_ok('/rest/bug/comment/' . $comment->id => api_headers($other)) + ->status_is(200) + ->json_is('/comments/' . $comment->id . '/text', + $text, 'REST text has no block') + ->json_is( + '/comments/' . $comment->id . '/raw_text', + "$block$text", + 'REST raw_text keeps the stored text' + ); + +# Webhook and Pulse payloads carry the text people read, as bugmail does. +my $push = Bugzilla::Extension::Push::Serialize->instance; +is($push->object_to_hash($comment, 1)->{body}, + $text, 'webhook and Pulse payloads have no block'); + +# Bugmail, in both formats, has the text and no block. +$t->put_ok("/rest/bug/$bug_id" => api_headers($admin) => json => + {cc => {add => [$watcher->login]}})->status_is(200, 'the watcher is CCd'); + +my $mail_file = bz_locations()->{datadir} . '/mailer.testfile'; +my $mail_start = -s $mail_file // 0; + +# Bugmail covers the changes since the last mail, to the second, so a comment +# posted in the same second as the CC change could be left out. +sleep 1; +my $mail_text = 'Mail check: the failure is here.'; +post_comment($hackbot, "$block$mail_text"); + +# The text/plain and text/html parts of the mail sent to the watcher, as +# [type, text] pairs. +my @parts; +if (open my $fh, '<', $mail_file) { + seek $fh, $mail_start, 0; + my $new_mail = do { local $/; <$fh> }; + foreach + my $raw ($new_mail =~ /^-{10} begin message\n(.*?)^-{10} end message/msg) + { + my $email = Email::MIME->new($raw); + next unless ($email->header('To') // '') eq $watcher->login; + $email->walk_parts(sub { + my ($part) = @_; + my ($type) = $part->content_type =~ m{^(text/(?:plain|html))}; + push @parts, [$type, $part->body_str] if $type; + }); + } +} +my @comment_parts = grep { $_->[1] =~ /\Q$mail_text\E/ } @parts; +is(scalar @comment_parts, 2, 'bugmail has the comment as text and HTML'); +foreach my $part (@comment_parts) { + unlike($part->[1], qr/hackbot:|run: abc-123/, + "$part->[0] bugmail has no block"); +} + +# The text is parsed again after an edit. +is($comment->hackbot->{run}, 'abc-123', 'metadata before the edit'); +(my $edited_block = $block) =~ s/abc-123/def-456/g; +$comment->update_text({ + text => "${edited_block}Edited.", + timestamp => Bugzilla->dbh->selectrow_array('SELECT NOW()'), + is_hidden => 0, +}); +is($comment->hackbot->{run}, 'def-456', 'an edited block is read again'); +is($comment->body_full, 'Edited.', '... and hidden'); +$comment->update_text({ + text => 'Edited, with no block.', + timestamp => Bugzilla->dbh->selectrow_array('SELECT NOW()'), + is_hidden => 0, +}); +is($comment->hackbot, undef, 'a removed block leaves no metadata'); +is($comment->body_full, 'Edited, with no block.', '... and the text is whole'); + +# Saving an inline edit returns the HTML the page shows, without the block. +# Admins edit other people's comments as edit-comments admins. +set_params(edit_comments_admins_group => 'admin'); +my $edited = post_comment($hackbot, "$block$text"); +my $edit_url = '/rest/editcomments/comment/' . $edited->id; +$t->put_ok($edit_url => api_headers($admin) => json => + {new_comment => "${block}Edited text."})->status_is(200); +my $edit_html = $t->tx->res->json('/html'); +unlike($edit_html, qr/hackbot:|run: abc-123/, 'the edit response has no block'); +like($edit_html, qr/Edited text\./, '... and has the new text'); + +# Anyone can type the block. +my $forged = post_comment($other, "$block$text"); +is($forged->body_full, "$block$text", 'another user: the block stays as text'); +is($forged->hackbot, undef, 'another user: no metadata'); + +my $plain = post_comment($hackbot, 'A plain comment'); +is($plain->body_full, 'A plain comment', 'plain Hackbot comment is unchanged'); +is($plain->hackbot, undef, 'plain Hackbot comment has no metadata'); + +# Attachment comments are shown through their own template, which doesn't hide +# the block, so it isn't read there either. +$t->post_ok( + "/rest/bug/$bug_id/attachment" => api_headers($hackbot) => json => { + ids => [$bug_id], + data => 'cGF0Y2g=', + file_name => 'fix.txt', + summary => 'fix', + content_type => 'text/plain', + comment => "$block$text", + } +)->status_is(201, 'the attachment is added') + or diag $t->tx->res->body; +my $attachment_comment + = Bugzilla::Bug->new($bug_id)->comments({order => 'oldest_to_newest'})->[-1]; +ok($attachment_comment->type, 'the attachment has its own comment type'); +is($attachment_comment->hackbot, undef, + 'an attachment comment has no metadata'); + +# The hackbot_user param. +my $check = \&Bugzilla::Extension::Hackbot::Config::check_hackbot_user; +is($check->(''), '', 'an empty login is valid'); +is($check->($hackbot->login), '', 'an existing login is valid'); +like( + $check->('nobody@hackbot-metadata.test'), + qr/no user named/, + 'an unknown login is rejected' +); + +done_testing; diff --git a/extensions/Hackbot/t/metadata.t b/extensions/Hackbot/t/metadata.t new file mode 100644 index 0000000000..62a642245a --- /dev/null +++ b/extensions/Hackbot/t/metadata.t @@ -0,0 +1,82 @@ +#!/usr/bin/env perl +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +use strict; +use warnings; +use lib qw( . lib local/lib/perl5 ); + +BEGIN { + $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf'; + $ENV{BUGZILLA_DISABLE_HOSTAGE} = 1; +} + +use Bugzilla::Test::MockDB; +use Bugzilla::Test::MockParams; + +use Test2::V0; + +use Bugzilla; +BEGIN { Bugzilla->extensions } + +my $parse = \&Bugzilla::Extension::Hackbot::Metadata::parse_front_matter; + +my $url = 'https://hackbot.moz.tools/runs/abc-123'; + +sub block { + my (%fields) = (run => 'abc-123', action => 'f00', agent => 'bug-fix', @_); + my $lines = join('', map { + defined $fields{$_} ? " $_: $fields{$_}\n" : '' + } qw(run action agent url extra)); + return "---\nhackbot:\n$lines---\n"; +} + +my ($metadata, $body) = $parse->(block(url => $url) . "\nThe failure is here.\n"); +is( + $metadata, + {run => 'abc-123', action => 'f00', agent => 'bug-fix', url => $url}, + 'fields are read' +); +is($body, "The failure is here.\n", 'block and blank line are removed'); + +($metadata, $body) = $parse->(block()); +is($body, '', 'a block with no text after it'); + +($metadata, $body) = $parse->(substr(block(), 0, -1)); +is($metadata->{run}, 'abc-123', 'stored text has its final newline trimmed'); + +($metadata) = $parse->(block(extra => 'value')); +ok(!exists $metadata->{extra}, 'unknown keys are skipped'); + +($metadata, $body) = $parse->(block() . "\nA\n\n---\n\nB"); +is($body, "A\n\n---\n\nB", 'a later rule is part of the text'); + +($metadata, $body) = $parse->(block() . block() . 'text'); +is($body, block() . 'text', 'only the first block counts'); + +($metadata) = $parse->(block(url => 'javascript:alert(1)')); +is($metadata->{run}, 'abc-123', 'a bad url does not reject the block'); +ok(!exists $metadata->{url}, 'only https URLs are kept'); + +($metadata) = $parse->(block(url => 'https://a.test/"x y')); +ok(!exists $metadata->{url}, 'a URL with spaces is dropped'); + +for my $key (qw(run action agent)) { + is([$parse->(block($key => undef))], [], "$key is required"); + is([$parse->(block($key => ''))], [], "$key must be an identifier"); +} + +is([$parse->(undef)], [], 'undef text'); +is([$parse->('')], [], 'empty text'); +is([$parse->('Plain comment')], [], 'plain comment'); +is([$parse->("---\n\nA comment starting with a rule")], [], 'leading rule'); +is([$parse->("---\ntitle: x\n---\ntext")], [], 'other front matter'); +is([$parse->('Text first' . block())], [], 'block not at the top'); +is([$parse->("\n" . block())], [], 'block after a blank line'); +is([$parse->("---\nhackbot:\n run: x\n")], [], 'unterminated block'); + +done_testing; diff --git a/extensions/Hackbot/template/en/default/admin/params/hackbot.html.tmpl b/extensions/Hackbot/template/en/default/admin/params/hackbot.html.tmpl new file mode 100644 index 0000000000..6f056de9a2 --- /dev/null +++ b/extensions/Hackbot/template/en/default/admin/params/hackbot.html.tmpl @@ -0,0 +1,20 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% + title = "Hackbot" + desc = "Settings for the Hackbot integration" +%] + +[% + param_descs = { + hackbot_user => + "The login of the Hackbot account. Metadata at the top of this account's comments is hidden and shown in the " + _ "comment header instead. Leave this empty to turn the integration off.", + } +%] diff --git a/extensions/Hackbot/template/en/default/hook/bug/comments-user.html.tmpl b/extensions/Hackbot/template/en/default/hook/bug/comments-user.html.tmpl new file mode 100644 index 0000000000..08457fafbc --- /dev/null +++ b/extensions/Hackbot/template/en/default/hook/bug/comments-user.html.tmpl @@ -0,0 +1,12 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% hackbot = comment.hackbot %] +[% RETURN UNLESS hackbot %] + +AI diff --git a/extensions/Hackbot/template/en/default/hook/bug_modal/activity_stream-before_comment_actions.html.tmpl b/extensions/Hackbot/template/en/default/hook/bug_modal/activity_stream-before_comment_actions.html.tmpl new file mode 100644 index 0000000000..253a385712 --- /dev/null +++ b/extensions/Hackbot/template/en/default/hook/bug_modal/activity_stream-before_comment_actions.html.tmpl @@ -0,0 +1,23 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% hackbot = comment.hackbot %] +[% RETURN UNLESS hackbot %] + + +
+ + + [% hackbot.agent FILTER html %] + + [% IF hackbot.url %] + View Run + [% END %] +
+ diff --git a/extensions/Hackbot/template/en/default/hook/bug_modal/comment_tags-before_tags.html.tmpl b/extensions/Hackbot/template/en/default/hook/bug_modal/comment_tags-before_tags.html.tmpl new file mode 100644 index 0000000000..82ae5d02fe --- /dev/null +++ b/extensions/Hackbot/template/en/default/hook/bug_modal/comment_tags-before_tags.html.tmpl @@ -0,0 +1,13 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% hackbot = comment.hackbot %] +[% RETURN UNLESS hackbot %] + +[%# Comment tags are case-insensitive. %] +[% hidden_tags.push(hackbot.agent.lower) %] diff --git a/extensions/Hackbot/template/en/default/hook/bug_modal/header-end.html.tmpl b/extensions/Hackbot/template/en/default/hook/bug_modal/header-end.html.tmpl new file mode 100644 index 0000000000..05fa23b00a --- /dev/null +++ b/extensions/Hackbot/template/en/default/hook/bug_modal/header-end.html.tmpl @@ -0,0 +1,9 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% style_urls.push('extensions/Hackbot/web/styles/hackbot.css') %] diff --git a/extensions/Hackbot/web/styles/hackbot.css b/extensions/Hackbot/web/styles/hackbot.css new file mode 100644 index 0000000000..cb8f21be93 --- /dev/null +++ b/extensions/Hackbot/web/styles/hackbot.css @@ -0,0 +1,53 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + * + * This Source Code Form is "Incompatible With Secondary Licenses", as + * defined by the Mozilla Public License, v. 2.0. */ + +/* The header already names the agent, so its comment tag is only kept for search. */ +.comment-tag[hidden] { + display: none; +} + +.hackbot-meta { + display: flex; + overflow: hidden; + margin-inline: 8px; + border: 1px solid var(--primary-region-border-color); + border-radius: 16px; + width: max-content; + font-size: var(--font-size-small); +} + +.hackbot-meta .agent, +.hackbot-meta .run { + display: inline-flex; + gap: 4px; + align-items: center; + padding: 1px 8px; + min-height: 1.5em; +} + +.hackbot-meta .agent { + background-color: var(--control-background-color); +} + +.hackbot-meta .agent .icon::before { + color: rgb(var(--accent-color-yellow-1)); + font-family: var(--icon-font-family); + font-size: var(--icon-size-x-small); + vertical-align: middle; + font-variation-settings: 'FILL' 0, 'wght' 400, 'GRAD' 0, 'opsz' 24; + content: '\e65f'; +} + +@media screen and (max-width: 799px) { + .hackbot-meta { + justify-content: center; + flex-wrap: wrap; + border-radius: 8px; + width: auto; + min-width: 6em; + } +} diff --git a/extensions/Push/lib/Serialize.pm b/extensions/Push/lib/Serialize.pm index dd2d6a2b5e..f96fe79536 100644 --- a/extensions/Push/lib/Serialize.pm +++ b/extensions/Push/lib/Serialize.pm @@ -267,9 +267,15 @@ sub _attachment { sub _comment { my ($self, $comment, $is_shallow) = @_; + + # The text people read, as in bugmail: extensions can hide parts of it with + # the comment_body_full hook. body_full formats the text of other comment + # types (attachments, duplicates), so those keep the stored text. + my $body = $comment->type ? $comment->body : $comment->body_full; + my $rh = { id => _integer($comment->id), - body => _string($comment->body), + body => _string($body), creation_time => _time($comment->creation_ts), is_private => _boolean($comment->is_private), number => _integer($comment->count), diff --git a/qa/config/generate_test_data.pl b/qa/config/generate_test_data.pl index 16367f9bdf..1aab20cac7 100644 --- a/qa/config/generate_test_data.pl +++ b/qa/config/generate_test_data.pl @@ -132,7 +132,7 @@ BEGIN my @usernames = ( 'admin', 'unprivileged', 'QA_Selenium_TEST', 'canconfirm', 'tweakparams', 'permanent', 'editbugs', 'disabled', - 'editusers', 'editusers_target', 'creategroups', + 'editusers', 'editusers_target', 'creategroups', 'hackbot', ); print "creating user accounts...\n"; diff --git a/qa/config/selenium_test.conf b/qa/config/selenium_test.conf index 73976b8936..f5020b9e1b 100644 --- a/qa/config/selenium_test.conf +++ b/qa/config/selenium_test.conf @@ -58,6 +58,10 @@ 'creategroups_user_passwd' => 'bo6aazeKohch', 'creategroups_user_api_key' => 'ReStCrEaTeGrOuPsTestApiKey0123456789abcd', 'protected_group_name' => 'rest-protected-group-test', + 'hackbot_user_login' => 'hackbot@mozilla.test', + 'hackbot_user_passwd' => 'bo6aazeKohch', + 'hackbot_user_username' => 'Hackbot', + 'hackbot_user_api_key' => 'KRIY6KaNDW6sTRL0ej5CDdZVE1wUefLZqVzelRAE', 'tweakparams_user_login' => 'tweakparams@mozilla.test', 'tweakparams_user_login_truncated' => 'tweakparams@mo', 'tweakparams_user_passwd' => 'bo6aazeKohch', diff --git a/qa/t/1_test_hackbot_comments.t b/qa/t/1_test_hackbot_comments.t new file mode 100644 index 0000000000..c35da3ec18 --- /dev/null +++ b/qa/t/1_test_hackbot_comments.t @@ -0,0 +1,210 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +# Bug 2086124: Hackbot comments start with a front matter block that links them +# to the run that wrote them. hackbot-api posts them through REST, so they are +# posted the same way here, and the bug page is checked in the browser: the +# block is hidden, the header shows the agent and a link to the run, the +# agent's comment tag is hidden, and quoting the comment leaves the block out. +# The same block typed by anyone else is an ordinary comment. + +use strict; +use warnings; +use lib qw(lib ../../lib ../../local/lib/perl5); + +use QA::Util; +use QA::Tests qw(create_bug_fields); +use Test::More; +use Test::Mojo; + +my ($sel, $config) = get_selenium(); + +my $rest = "$config->{browser_url}/rest"; +my $t = Test::Mojo->new; + +sub api_key_header { + my ($user) = @_; + return {'X-Bugzilla-API-Key' => $config->{"${user}_user_api_key"}}; +} + +my $agent = 'frontend-triage'; + +# Comment tags are case-insensitive, so the agent's tag may differ in case. +my $agent_tag = 'Frontend-Triage'; + +# Matches the agent only if a tag were read as a pattern ('.' is any character). +my $lookalike_tag = 'frontend.triage'; +my $run = 'e2e-run-1'; +my $url = "https://hackbot.example/runs/$run"; +my $block = join("\n", + '---', 'hackbot:', " run: $run", + ' action: 3f2a9c1e', + " agent: $agent", + " url: $url", '---', '', ''); + +# Turn the integration on for the Hackbot test account, through the UI. +log_in($sel, $config, 'admin'); +set_parameters( + $sel, + { + 'Hackbot' => + {'hackbot_user' => {type => 'text', value => $config->{hackbot_user_login}},} + } +); + +my %bug = %{create_bug_fields($config)}; +delete $bug{creator}; +$t->post_ok( + "$rest/bug" => api_key_header('admin') => json => { + %bug, + summary => 'Hackbot comment metadata', + description => 'A bug for Hackbot to comment on', + } +)->status_is(200, 'The bug is filed'); +my $bug_id = $t->tx->res->json('/id'); + +sub add_comment { + my ($user, $text) = @_; + $t->post_ok("$rest/bug/$bug_id/comment" => api_key_header($user) => json => + {comment => $text, is_markdown => 1})->status_is(201, "$user adds a comment"); + return $t->tx->res->json('/id'); +} + +# The order matters: the field change comes right after the Hackbot comment, +# so a change header that picked up the comment before it would show the badge. +my $forged_id = add_comment('unprivileged', "${block}Typed by someone else."); +my $plain_id = add_comment('hackbot', 'A plain Hackbot comment.'); +my $hackbot_id = add_comment('hackbot', "${block}Triage result: front end."); + +$t->put_ok( + "$rest/bug/comment/$hackbot_id/tags" => api_key_header('admin') => json => { + comment_id => $hackbot_id, + add => [$agent_tag, $lookalike_tag, 'needs-review'] + } +)->status_is(200, 'The comment is tagged'); +$t->put_ok("$rest/bug/$bug_id" => api_key_header('admin') => json => + {whiteboard => 'hackbot-e2e'}) + ->status_is(200, 'A field is changed with no comment'); + +# What the page shows for a comment, read in one go. +sub comment_view { + my ($comment_id) = @_; + return $sel->driver->execute_script(<<"JS"); +const set = document.querySelector('.comment[data-id="$comment_id"]').closest('.change-set'); +const meta = set.querySelector('.hackbot-meta'); +const run = meta && meta.querySelector('.run'); +return { + text: set.querySelector('.comment-text').innerText.trim(), + agent: meta ? meta.querySelector('.agent').textContent.trim() : '', + run: run ? run.getAttribute('href') : '', + badge: [...set.querySelectorAll('.change-author .user-role')] + .some(role => role.textContent.trim() === 'AI'), + tags: Object.fromEntries([...set.querySelectorAll('.comment-tag')] + .map(tag => [tag.dataset.tag, getComputedStyle(tag).display !== 'none'])), +}; +JS +} + +# Poll the page until a JavaScript expression is true. +sub wait_for_js_ok { + my ($expression, $name) = @_; + for (1 .. 20) { + return ok(1, $name) if $sel->driver->execute_script("return !!($expression)"); + sleep 1; + } + return ok(0, $name); +} + +go_to_bug($sel, $bug_id, 1); + +my $view = comment_view($hackbot_id); +is($view->{text}, 'Triage result: front end.', 'The block is hidden'); +is($view->{agent}, $agent, 'The header names the agent'); +is($view->{run}, $url, 'The header links to the run'); +ok($view->{badge}, 'The author has the AI badge'); +ok(!$view->{tags}{$agent_tag}, 'The agent tag is hidden'); +ok($view->{tags}{'needs-review'}, 'Other tags are shown'); +ok($view->{tags}{$lookalike_tag}, + 'A tag that only looks like the agent is shown'); + +$view = comment_view($forged_id); +like( + $view->{text}, + qr/hackbot:.*run: $run/s, + "Another user's block is shown as text" +); +is($view->{agent}, '', '... with no agent in the header'); +ok(!$view->{badge}, '... and no AI badge'); + +$view = comment_view($plain_id); +is( + $view->{text}, + 'A plain Hackbot comment.', + 'A plain Hackbot comment is unchanged' +); +is($view->{agent}, '', '... with no agent in the header'); + +ok( + !$sel->driver->execute_script(<<'JS'), +return [...document.querySelectorAll('.change-set')] + .filter(set => !set.querySelector('.comment')) + .some(set => [...set.querySelectorAll('.change-author .user-role')] + .some(role => role.textContent.trim() === 'AI')); +JS + 'A field change has no AI badge' +); + +# Quoting the comment leaves the block out. +$sel->js_click_ok( + qq{//button[contains(\@class, "reply-btn") and \@data-id="$hackbot_id"]}, + 'Reply to the Hackbot comment'); +wait_for_js_ok( + q{document.querySelector('#comment').value.includes('> Triage result: front end.')}, + 'The reply quotes the comment' +); +unlike( + $sel->driver->execute_script( + q{return document.querySelector('#comment').value}), + qr/hackbot:|run: $run/, + 'The reply has no block' +); +$sel->driver->execute_script(q{document.querySelector('#comment').value = ''}); + +# Changing the tags renders them again, and the agent tag stays hidden. +my $tags = qq{.comment[data-id="$hackbot_id"] .comment-tag}; +$sel->js_click_ok( + qq{//button[contains(\@class, "tag-btn") and \@data-id="$hackbot_id"]}, + 'Open the tag editor'); +$sel->type_ok('ctag-add', 'e2e-added', 'Type a new tag'); +$sel->js_click_ok('ctag-save', 'Save the tag'); +wait_for_js_ok(qq{document.querySelector('$tags\[data-tag="e2e-added"]')}, + 'The new tag is shown'); +$view = comment_view($hackbot_id); +ok($view->{tags}{'e2e-added'}, 'The new tag is visible'); +ok(!$view->{tags}{$agent_tag}, 'The agent tag is still hidden'); + +$sel->js_click_ok( + qq{//div[contains(\@class, "comment") and \@data-id="$hackbot_id"]} + . q{//span[@data-tag="needs-review"]/a[contains(@class, "remove")]}, + 'Remove a tag' +); +wait_for_js_ok(qq{!document.querySelector('$tags\[data-tag="needs-review"]')}, + 'The tag is removed'); +ok(!comment_view($hackbot_id)->{tags}{$agent_tag}, + 'The agent tag is still hidden'); + +# Turn the integration off again: the block is shown as it is stored. +set_parameters($sel, + {'Hackbot' => {'hackbot_user' => {type => 'text', value => ''}}}); +go_to_bug($sel, $bug_id, 1); +$view = comment_view($hackbot_id); +like($view->{text}, qr/hackbot:.*agent: $agent/s, 'Off: the block is shown'); +is($view->{agent}, '', 'Off: no agent in the header'); + +logout($sel); + +done_testing(); diff --git a/skins/standard/global.css b/skins/standard/global.css index e5ab13c697..00356ebdef 100644 --- a/skins/standard/global.css +++ b/skins/standard/global.css @@ -112,6 +112,7 @@ --icon-size-large: 24px; --icon-size-medium: 20px; --icon-size-small: 16px; + --icon-size-x-small: 14px; /** Global */ --global-header-height: 48px;