summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordklawren <dklawren@users.noreply.github.com>2017-08-30 17:15:14 +0200
committerDavid Walsh <davidwalsh83@gmail.com>2017-08-30 17:15:14 +0200
commit6e06bb255acdf53d2bbed3911378bd6859a0e3f0 (patch)
tree7a0b8caaf25fb968bcf79dd945b29f9dde2a62c9
parenta4adf15c47119113ab2e93752042528e15fdecfd (diff)
downloadbugzilla-6e06bb255acdf53d2bbed3911378bd6859a0e3f0.tar.gz
bugzilla-6e06bb255acdf53d2bbed3911378bd6859a0e3f0.tar.xz
Bug 1393023: Approving a revision creates an r+ flag on the revision attachment in the associated bug (#219)
Approving a revision creates an r+ flag on the revision attachment in the associated bug * - Working version - Splits accepted_users on ':' instead of accepting a list (phab issue)
-rw-r--r--extensions/PhabBugz/lib/Util.pm28
-rw-r--r--extensions/PhabBugz/lib/WebService.pm159
-rw-r--r--extensions/Push/lib/Connector/Phabricator.pm35
3 files changed, 183 insertions, 39 deletions
diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm
index 57dc1d7d0..5a8385d0c 100644
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -27,6 +27,7 @@ our @EXPORT = qw(
create_private_revision_policy
create_project
edit_revision_policy
+ get_attachment_revisions
get_bug_role_phids
get_members_by_bmo_id
get_project_phid
@@ -297,7 +298,32 @@ sub is_attachment_phab_revision {
my ($attachment, $include_obsolete) = @_;
return ($attachment->contenttype eq PHAB_CONTENT_TYPE
&& ($include_obsolete || !$attachment->isobsolete)
- && $attachment->attacher->login eq 'phab-bot@bmo.tld') ? 1 : 0;
+ && $attachment->attacher->login eq PHAB_AUTOMATION_USER) ? 1 : 0;
+}
+
+sub get_attachment_revisions {
+ my ($self, $bug) = @_;
+
+ my @revisions;
+
+ my @attachments =
+ grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
+
+ if (@attachments) {
+ my @revision_ids;
+ foreach my $attachment (@attachments) {
+ my ($revision_id) =
+ ( $attachment->filename =~ PHAB_ATTACHMENT_PATTERN );
+ next if !$revision_id;
+ push( @revision_ids, int($revision_id) );
+ }
+
+ if (@revision_ids) {
+ @revisions = get_revisions_by_ids( \@revision_ids );
+ }
+ }
+
+ return @revisions;
}
sub request {
diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm
index 84561c3db..5668eac81 100644
--- a/extensions/PhabBugz/lib/WebService.pm
+++ b/extensions/PhabBugz/lib/WebService.pm
@@ -32,17 +32,22 @@ use Bugzilla::Extension::PhabBugz::Util qw(
get_project_phid
get_revisions_by_ids
intersect
+ is_attachment_phab_revision
make_revision_public
request
);
+use List::Util qw(first);
+use List::MoreUtils qw(any);
use MIME::Base64 qw(decode_base64);
use constant PUBLIC_METHODS => qw(
+ check_user_permission_for_bug
+ obsolete_attachments
revision
+ update_reviewer_statuses
);
-
sub revision {
my ($self, $params) = @_;
@@ -59,6 +64,9 @@ sub revision {
my $user = Bugzilla->login(LOGIN_REQUIRED);
+ # Prechecks
+ _phabricator_precheck($user);
+
unless (defined $params->{revision} && detaint_natural($params->{revision})) {
ThrowCodeError('param_required', { param => 'revision' })
}
@@ -117,13 +125,8 @@ sub check_user_permission_for_bug {
my $user = Bugzilla->login(LOGIN_REQUIRED);
- # Ensure PhabBugz is on
- ThrowUserError('phabricator_not_enabled')
- unless Bugzilla->params->{phabricator_enabled};
-
- # Validate that the requesting user's email matches phab-bot
- ThrowUserError('phabricator_unauthorized_user')
- unless $user->login eq PHAB_AUTOMATION_USER;
+ # Prechecks
+ _phabricator_precheck($user);
# Validate that a bug id and user id are provided
ThrowUserError('phabricator_invalid_request_params')
@@ -138,6 +141,134 @@ sub check_user_permission_for_bug {
};
}
+sub update_reviewer_statuses {
+ my ($self, $params) = @_;
+
+ my $user = Bugzilla->login(LOGIN_REQUIRED);
+
+ # Prechecks
+ _phabricator_precheck($user);
+
+ my $revision_id = $params->{revision_id};
+ unless (defined $revision_id && detaint_natural($revision_id)) {
+ ThrowCodeError('param_required', { param => 'revision_id' })
+ }
+
+ my $bug_id = $params->{bug_id};
+ unless (defined $bug_id && detaint_natural($bug_id)) {
+ ThrowCodeError('param_required', { param => 'bug_id' })
+ }
+
+ my $accepted_user_ids = $params->{accepted_users};
+ defined $accepted_user_ids
+ || ThrowCodeError('param_required', { param => 'accepted_users' });
+ $accepted_user_ids = [ split(':', $accepted_user_ids) ];
+
+ my $bug = Bugzilla::Bug->check($bug_id);
+
+ my @attachments =
+ grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
+
+ return { result => [] } if !@attachments;
+
+ my $dbh = Bugzilla->dbh;
+ my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
+
+ my @updated_attach_ids;
+ foreach my $attachment (@attachments) {
+ my ($curr_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
+ next if $revision_id != $curr_revision_id;
+
+ # Clear old flags if no longer accepted or a previous
+ # acceptor is not in the new list.
+ my (@old_flags, @new_flags, %accepted_done, $flag_type);
+ foreach my $flag (@{ $attachment->flags }) {
+ next if $flag->type->name ne 'review';
+ $flag_type = $flag->type;
+ unless (any { $flag->setter->id == $_ } @$accepted_user_ids) {
+ push(@old_flags, { id => $flag->id, status => 'X' });
+ }
+ else {
+ $accepted_done{$flag->setter->id}++; # so we do not set it again as new
+ }
+ }
+
+ $flag_type ||= first { $_->name eq 'review' } @{ $attachment->flag_types };
+
+ # Create new flags
+ foreach my $user_id (@$accepted_user_ids) {
+ next if $accepted_done{$user_id};
+ my $user = Bugzilla::User->check({ id => $user_id, cache => 1 });
+ push(@new_flags, { type_id => $flag_type->id, setter => $user, status => '+' });
+ }
+
+ $attachment->set_flags(\@old_flags, \@new_flags);
+ $attachment->update($timestamp);
+
+ push(@updated_attach_ids, $attachment->id);
+ }
+
+ Bugzilla::BugMail::Send($bug_id, { changer => $user }) if @updated_attach_ids;
+
+ return { result => \@updated_attach_ids };
+}
+
+sub obsolete_attachments {
+ my ($self, $params) = @_;
+
+ my $user = Bugzilla->login(LOGIN_REQUIRED);
+
+ # Prechecks
+ _phabricator_precheck($user);
+
+ my $revision_id = $params->{revision_id};
+ unless (defined $revision_id && detaint_natural($revision_id)) {
+ ThrowCodeError('param_required', { param => 'revision' })
+ }
+
+ my $bug_id= $params->{bug_id};
+ unless (defined $bug_id && detaint_natural($bug_id)) {
+ ThrowCodeError('param_required', { param => 'bug_id' })
+ }
+
+ my $bug = Bugzilla::Bug->check($bug_id);
+
+ my @attachments =
+ grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
+
+ return { result => [] } if !@attachments;
+
+ my $dbh = Bugzilla->dbh;
+ my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
+
+ my @updated_attach_ids;
+ foreach my $attachment (@attachments) {
+ my ($curr_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
+ next if $revision_id != $curr_revision_id;
+
+ $attachment->set_is_obsolete(1);
+ $attachment->update($timestamp);
+
+ push(@updated_attach_ids, $attachment->id);
+ }
+
+ Bugzilla::BugMail::Send($bug_id, { changer => $user }) if @updated_attach_ids;
+
+ return { result => \@updated_attach_ids };
+}
+
+sub _phabricator_precheck {
+ my ($user) = @_;
+
+ # Ensure PhabBugz is on
+ ThrowUserError('phabricator_not_enabled')
+ unless Bugzilla->params->{phabricator_enabled};
+
+ # Validate that the requesting user's email matches phab-bot
+ ThrowUserError('phabricator_unauthorized_user')
+ unless $user->login eq PHAB_AUTOMATION_USER;
+}
+
sub rest_resources {
return [
# Revision creation
@@ -157,6 +288,18 @@ sub rest_resources {
return { bug_id => $_[0], user_id => $_[1] };
}
}
+ },
+ # Update reviewer statuses
+ qr{^/phabbugz/update_reviewer_statuses$}, {
+ PUT => {
+ method => 'update_reviewer_statuses',
+ }
+ },
+ # Obsolete attachments
+ qr{^/phabbugz/obsolete$}, {
+ PUT => {
+ method => 'obsolete_attachments',
+ }
}
];
}
diff --git a/extensions/Push/lib/Connector/Phabricator.pm b/extensions/Push/lib/Connector/Phabricator.pm
index 2571c6d37..c92df3173 100644
--- a/extensions/Push/lib/Connector/Phabricator.pm
+++ b/extensions/Push/lib/Connector/Phabricator.pm
@@ -21,9 +21,9 @@ use Bugzilla::User;
use Bugzilla::Extension::PhabBugz::Constants;
use Bugzilla::Extension::PhabBugz::Util qw(
add_comment_to_revision create_private_revision_policy
- edit_revision_policy get_bug_role_phids get_revisions_by_ids
- intersect is_attachment_phab_revision make_revision_public
- make_revision_private);
+ edit_revision_policy get_attachment_revisions get_bug_role_phids
+ get_revisions_by_ids intersect is_attachment_phab_revision
+ make_revision_public make_revision_private);
use Bugzilla::Extension::Push::Constants;
use Bugzilla::Extension::Push::Util qw(is_public);
@@ -77,7 +77,7 @@ sub send {
my $phab_error_message =
'Revision is being made private due to unknown Bugzilla groups.';
- my @revisions = $self->_get_attachment_revisions($bug);
+ my @revisions = get_attachment_revisions($bug);
foreach my $revision (@revisions) {
add_comment_to_revision( $revision->{phid}, $phab_error_message );
make_revision_private( $revision->{phid} );
@@ -110,7 +110,7 @@ sub send {
$subscribers = get_bug_role_phids($bug);
}
- my @revisions = $self->_get_attachment_revisions($bug);
+ my @revisions = get_attachment_revisions($bug);
foreach my $revision (@revisions) {
my $revision_phid = $revision->{phid};
@@ -125,31 +125,6 @@ sub send {
return PUSH_RESULT_OK;
}
-sub _get_attachment_revisions() {
- my ( $self, $bug ) = @_;
-
- my @revisions;
-
- my @attachments =
- grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
-
- if (@attachments) {
- my @revision_ids;
- foreach my $attachment (@attachments) {
- my ($revision_id) =
- ( $attachment->filename =~ PHAB_ATTACHMENT_PATTERN );
- next if !$revision_id;
- push( @revision_ids, int($revision_id) );
- }
-
- if (@revision_ids) {
- @revisions = get_revisions_by_ids( \@revision_ids );
- }
- }
-
- return @revisions;
-}
-
sub _get_bug_by_data {
my ( $self, $data ) = @_;
my $bug_data = $self->_get_bug_data($data) || return 0;