Skip to content

Commit

Permalink
Bug 1917167 - Disable emoji reactions for users without editbugs on b…
Browse files Browse the repository at this point in the history
…ugs that have commenting restricted
  • Loading branch information
kyoshino committed Sep 7, 2024
1 parent 5c53f75 commit 042c227
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 39 deletions.
6 changes: 6 additions & 0 deletions Bugzilla/WebService/Bug.pm
Original file line number Diff line number Diff line change
Expand Up @@ -1429,6 +1429,12 @@ sub update_comment_reactions {
ThrowUserError('comment_reaction_closed');
}

if ($comment->bug->restrict_comments
&& !$user->in_group(Bugzilla->params->{'restrict_comments_group'}))
{
ThrowUserError('comment_reaction_restricted');
}

my $dbh = Bugzilla->dbh;
$dbh->bz_start_transaction();
foreach my $reaction (@{$params->{add} || []}) {
Expand Down
7 changes: 4 additions & 3 deletions Bugzilla/WebService/Constants.pm
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,10 @@ use constant WS_ERROR_CODE => {
comment_invalid_isprivate => 117,

# Comment reactions
comment_reaction_disabled => 136,
comment_reaction_invalid => 137,
comment_reaction_closed => 138,
comment_reaction_disabled => 136,
comment_reaction_invalid => 137,
comment_reaction_closed => 138,
comment_reaction_restricted => 139,

# Comment tagging
comment_tag_disabled => 125,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
[% IF user.id || total_count %]
<div id="cre-[% comment.count FILTER none %]" class="comment-reactions"
[% IF comment.collapsed +%] style="display:none"[% END %]>
[% IF user.id && !bug.is_closed_for({months => 3}) %]
[% IF user.id && !bug.is_closed_for({months => 3})
&& (!bug.restrict_comments || user.in_group(Param('restrict_comments_group'))) %]
<button type="button" class="anchor">
<span class="icon" aria-label="Toggle Reaction Picker"></span>
</button>
Expand Down
37 changes: 36 additions & 1 deletion qa/t/1_test_bug_edit.t
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,49 @@ $sel->is_element_present_ok($reactions_sums_path . $reactions_btn2_path
. '[@data-reaction-count="1"][@aria-pressed="true"]');
logout($sel);

# A logged out user cannot react but can see reactions of other users
# Restrict comments on the bug to users in the editbugs group
log_in($sel, $config, 'admin');
go_to_bug($sel, $bug1_id);
$sel->check_ok('restrict_comments');
$sel->click_ok('bottom-save-btn', 'Save changes');
logout($sel);

# An unprivileged user cannot react but can see reactions of other users
log_in($sel, $config, 'unprivileged');
go_to_bug($sel, $bug1_id);
ok(!$sel->is_element_present($reactions_anchor_path));
ok(!$sel->is_element_present($reactions_picker_path));
$sel->is_element_present_ok($reactions_sums_path . $reactions_btn1_path
. '[@data-reaction-count="1"][@aria-pressed="false"][@disabled]');
$sel->is_element_present_ok($reactions_sums_path . $reactions_btn2_path
. '[@data-reaction-count="1"][@aria-pressed="false"][@disabled]');
logout($sel);

# An editbugs user can still react
log_in($sel, $config, 'editbugs');
go_to_bug($sel, $bug1_id);
$sel->click_ok($reactions_anchor_path);
$sel->click_ok($reactions_picker_path . $reactions_btn1_path
. '[@aria-pressed="false"]');
$sel->is_element_present_ok($reactions_sums_path . $reactions_btn1_path
. '[@data-reaction-count="2"][@aria-pressed="true"]');
logout($sel);

# Restore comment restriction
log_in($sel, $config, 'admin');
go_to_bug($sel, $bug1_id);
$sel->uncheck_ok('restrict_comments');
$sel->click_ok('bottom-save-btn', 'Save changes');
logout($sel);

# A logged out user cannot react but can see reactions of other users
go_to_bug($sel, $bug1_id);
ok(!$sel->is_element_present($reactions_anchor_path));
ok(!$sel->is_element_present($reactions_picker_path));
$sel->is_element_present_ok($reactions_sums_path . $reactions_btn1_path
. '[@data-reaction-count="2"][@aria-pressed="false"][@disabled]');
$sel->is_element_present_ok($reactions_sums_path . $reactions_btn2_path
. '[@data-reaction-count="1"][@aria-pressed="false"][@disabled]');

# Now move the bug into another product, which has a mandatory group.

Expand Down
87 changes: 53 additions & 34 deletions qa/t/rest_bug.t
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@ use MIME::Base64 qw(encode_base64 decode_base64);
use Test::Mojo;
use Test::More;

my $config = get_config();
my $login = $config->{admin_user_login};
my $api_key = $config->{admin_user_api_key};
my $url = Bugzilla->localconfig->urlbase;
my $config = get_config();
my $admin_login = $config->{admin_user_login};
my $admin_api_key = $config->{admin_user_api_key};
my $editbugs_login = $config->{editbugs_user_login};
my $editbugs_api_key = $config->{editbugs_user_api_key};
my $unpriv_api_key = $config->{unprivileged_user_api_key};
my $url = Bugzilla->localconfig->urlbase;

my $t = Test::Mojo->new();

Expand All @@ -46,14 +49,14 @@ $t->post_ok($url . 'rest/bug' => json => $new_bug)->status_is(401)
'/message' => 'You must log in before using this part of Bugzilla.');

# Now try as authenticated user using API key. Should create a new bug.
$t->post_ok(
$url . 'rest/bug' => {'X-Bugzilla-API-Key' => $api_key} => json => $new_bug)
$t->post_ok($url . 'rest/bug' =>
{'X-Bugzilla-API-Key' => $admin_api_key} => json => $new_bug)
->status_is(200)->json_has('/id');

my $bug_id = $t->tx->res->json->{id};

# Retrieve the new bug and verify
$t->get_ok($url . "rest/bug/$bug_id" => {'X-Bugzilla-API-Key' => $api_key})
$t->get_ok($url . "rest/bug/$bug_id" => {'X-Bugzilla-API-Key' => $admin_api_key})
->status_is(200)->json_is('/bugs/0/summary' => $new_bug->{summary});

### Section 2: Make updates to the bug
Expand All @@ -72,13 +75,13 @@ $t->put_ok($url . "rest/bug/$bug_id" => json => $update)->status_is(401)
'/message' => 'You must log in before using this part of Bugzilla.');

# Authenticated request should work fine
$t->put_ok($url
. "rest/bug/$bug_id" => {'X-Bugzilla-API-Key' => $api_key} => json => $update)
$t->put_ok($url . "rest/bug/$bug_id" =>
{'X-Bugzilla-API-Key' => $admin_api_key} => json => $update)
->status_is(200)->json_is('/bugs/0/id' => $bug_id)
->json_has('/bugs/0/changes');

# Retrieve the new bug and verify
$t->get_ok($url . "rest/bug/$bug_id" => {'X-Bugzilla-API-Key' => $api_key})
$t->get_ok($url . "rest/bug/$bug_id" => {'X-Bugzilla-API-Key' => $admin_api_key})
->status_is(200)->json_is('/bugs/0/type' => $update->{type})
->json_is('/bugs/0/severity' => $update->{severity})
->json_is('/bugs/0/status' => $update->{status});
Expand All @@ -94,39 +97,39 @@ $t->post_ok($url . "rest/bug/$bug_id/comment" => json => $update)
'/message' => 'You must log in before using this part of Bugzilla.');

# Authenticated request should work fine
$t->post_ok($url
. "rest/bug/$bug_id/comment" => {'X-Bugzilla-API-Key' => $api_key} => json =>
$update)->status_is(201)->json_has('/id');
$t->post_ok($url . "rest/bug/$bug_id/comment" =>
{'X-Bugzilla-API-Key' => $admin_api_key} => json => $update)
->status_is(201)->json_has('/id');

my $comment_id = $t->tx->res->json->{id};

# Retrieve the new comment and verify
$t->get_ok(
$url . "rest/bug/comment/$comment_id" => {'X-Bugzilla-API-Key' => $api_key})
$t->get_ok($url . "rest/bug/comment/$comment_id" =>
{'X-Bugzilla-API-Key' => $admin_api_key})
->status_is(200)->json_is("/comments/$comment_id/text" => $update->{comment});

### Section 4: Choose emoji comment reactions

# Add reaction
$t->put_ok($url . "rest/bug/comment/$comment_id/reactions" =>
{'X-Bugzilla-API-Key' => $api_key} => json => {add => ['+1', 'tada']})
{'X-Bugzilla-API-Key' => $admin_api_key} => json => {add => ['+1', 'tada']})
->status_is(200)
->json_is('/+1/0/name' => $login)
->json_is('/tada/0/name' => $login);
->json_is('/+1/0/name' => $admin_login)
->json_is('/tada/0/name' => $admin_login);
$t->get_ok($url . "rest/bug/comment/$comment_id/reactions")
->status_is(200)
->json_is('/+1/0/name' => $login)
->json_is('/tada/0/name' => $login);
->json_is('/+1/0/name' => $admin_login)
->json_is('/tada/0/name' => $admin_login);
$t->get_ok($url . "rest/bug/comment/$comment_id")
->status_is(200)
->json_is("/comments/$comment_id/reactions" => {'+1' => 1, 'tada' => 1});

# Multiple attempts are simply ignored
$t->put_ok($url . "rest/bug/comment/$comment_id/reactions" =>
{'X-Bugzilla-API-Key' => $api_key} => json => {add => ['+1', 'tada']})
{'X-Bugzilla-API-Key' => $admin_api_key} => json => {add => ['+1', 'tada']})
->status_is(200)
->json_is('/+1/0/name' => $login)
->json_is('/tada/0/name' => $login);
->json_is('/+1/0/name' => $admin_login)
->json_is('/tada/0/name' => $admin_login);

# Unauthenticated update should fail
$t->put_ok(
Expand All @@ -137,24 +140,40 @@ $t->put_ok(

# Unsupported reaction should also fail
$t->put_ok($url . "rest/bug/comment/$comment_id/reactions" =>
{'X-Bugzilla-API-Key' => $api_key} => json => {add => ['happy']})
{'X-Bugzilla-API-Key' => $admin_api_key} => json => {add => ['happy']})
->status_is(400)
->json_is('/message' => 'The comment reaction "happy" is not supported.');

# Remove reaction
$t->put_ok($url . "rest/bug/comment/$comment_id/reactions" =>
{'X-Bugzilla-API-Key' => $api_key} => json => {remove => ['+1']})
{'X-Bugzilla-API-Key' => $admin_api_key} => json => {remove => ['+1']})
->status_is(200)
->json_hasnt('/+1')
->json_is('/tada/0/name' => $login);
->json_is('/tada/0/name' => $admin_login);
$t->get_ok($url . "rest/bug/comment/$comment_id/reactions")
->status_is(200)
->json_hasnt('/+1')
->json_is('/tada/0/name' => $login);
->json_is('/tada/0/name' => $admin_login);
$t->get_ok($url . "rest/bug/comment/$comment_id")
->status_is(200)
->json_is("/comments/$comment_id/reactions" => {'tada' => 1});

# Non-editbugs user’s reaction should fail if the bug comments are restricted
$t->put_ok($url . "rest/bug/$bug_id" =>
{'X-Bugzilla-API-Key' => $admin_api_key} => json => {restrict_comments => 1})
->status_is(200);
$t->put_ok($url . "rest/bug/comment/$comment_id/reactions" =>
{'X-Bugzilla-API-Key' => $unpriv_api_key} => json => {add => ['-1']})
->status_is(400)
->json_is('/message' => 'You are not allowed to react on this bug.');
$t->put_ok($url . "rest/bug/comment/$comment_id/reactions" =>
{'X-Bugzilla-API-Key' => $editbugs_api_key} => json => {add => ['-1']})
->status_is(200)
->json_is('/-1/0/name' => $editbugs_login);
$t->put_ok($url . "rest/bug/$bug_id" =>
{'X-Bugzilla-API-Key' => $admin_api_key} => json => {restrict_comments => 0})
->status_is(200);

### Section 5: Attach a file to the bug

my $attach_data = 'XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX';
Expand All @@ -177,15 +196,15 @@ $t->post_ok($url . "rest/bug/$bug_id/attachment" => json => $update)
'/message' => 'You must log in before using this part of Bugzilla.');

# Authenticated request should work fine
$t->post_ok($url
. "rest/bug/$bug_id/attachment" => {'X-Bugzilla-API-Key' => $api_key} => json =>
$update)->status_is(201)->json_has('/attachments');
$t->post_ok($url . "rest/bug/$bug_id/attachment" =>
{'X-Bugzilla-API-Key' => $admin_api_key} => json => $update)
->status_is(201)->json_has('/attachments');

my ($attach_id) = keys %{$t->tx->res->json->{attachments}};

# Retrieve the new attachment and verify
$t->get_ok(
$url . "rest/bug/attachment/$attach_id" => {'X-Bugzilla-API-Key' => $api_key})
$t->get_ok($url . "rest/bug/attachment/$attach_id" =>
{'X-Bugzilla-API-Key' => $admin_api_key})
->status_is(200)->json_is("/attachments/$attach_id/summary" => $update->{summary});

my $got_data = decode_base64($t->tx->res->json->{attachments}->{$attach_id}->{data});
Expand All @@ -195,8 +214,8 @@ ok($attach_data eq $got_data, 'Attachment data received is correct');

$update = {status => 'RESOLVED', resolution => 'FIXED',};

$t->put_ok($url
. "rest/bug/$bug_id" => {'X-Bugzilla-API-Key' => $api_key} => json => $update)
$t->put_ok($url . "rest/bug/$bug_id" =>
{'X-Bugzilla-API-Key' => $admin_api_key} => json => $update)
->status_is(200)->json_is('/bugs/0/id' => $bug_id)
->json_has('/bugs/0/changes');

Expand Down
4 changes: 4 additions & 0 deletions template/en/default/global/user-error.html.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,10 @@
[% title = "Comment Reactions Closed" %]
This [% terms.bug %] no longer accepts comment reactions.

[% ELSIF error == "comment_reaction_restricted" %]
[% title = "Comment Reactions Restricted" %]
You are not allowed to react on this [% terms.bug %].

[% ELSIF error == "comment_tag_disabled" %]
[% title = "Comment Tagging Disabled" %]
The comment tagging is not enabled.
Expand Down

0 comments on commit 042c227

Please sign in to comment.