summaryrefslogtreecommitdiffstats
path: root/extensions/PhabBugz
diff options
context:
space:
mode:
Diffstat (limited to 'extensions/PhabBugz')
-rw-r--r--extensions/PhabBugz/lib/Feed.pm36
-rw-r--r--extensions/PhabBugz/lib/Project.pm8
-rw-r--r--extensions/PhabBugz/lib/Revision.pm2
-rw-r--r--extensions/PhabBugz/lib/User.pm16
-rw-r--r--extensions/PhabBugz/lib/Util.pm3
-rw-r--r--extensions/PhabBugz/lib/WebService.pm71
6 files changed, 97 insertions, 39 deletions
diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm
index c46d36c13..7d6b4e0ed 100644
--- a/extensions/PhabBugz/lib/Feed.pm
+++ b/extensions/PhabBugz/lib/Feed.pm
@@ -12,7 +12,7 @@ use 5.10.1;
use IO::Async::Timer::Periodic;
use IO::Async::Loop;
use List::Util qw(first);
-use List::MoreUtils qw(any);
+use List::MoreUtils qw(any uniq);
use Moo;
use Scalar::Util qw(blessed);
use Try::Tiny;
@@ -322,11 +322,27 @@ sub group_query {
# Make sure phab-bot also a member of the new project group so that it can
# make policy changes to the private revisions
- INFO("Setting project members for " . $project->name);
- my $set_members = $self->get_group_members( $group );
- push @$set_members, $phab_user unless grep $_->phid eq $phab_user->phid, @$set_members;
- $project->set_members( $set_members );
- $project->update();
+ INFO( "Checking project members for " . $project->name );
+ my $set_members = $self->get_group_members($group);
+ my @set_member_phids = uniq map { $_->phid } ( @$set_members, $phab_user );
+ my @current_member_phids = uniq map { $_->phid } @{ $project->members };
+ my ( $removed, $added ) = diff_arrays( \@current_member_phids, \@set_member_phids );
+
+ if (@$added) {
+ INFO( 'Adding project members: ' . join( ',', @$added ) );
+ $project->add_member($_) foreach @$added;
+ }
+
+ if (@$removed) {
+ INFO( 'Removing project members: ' . join( ',', @$removed ) );
+ $project->remove_member($_) foreach @$removed;
+ }
+
+ if (@$added || @$removed) {
+ my $result = $project->update();
+ local Bugzilla::Logging->fields->{api_result} = $result;
+ INFO( "Project " . $project->name . " updated" );
+ }
}
}
@@ -424,9 +440,9 @@ sub process_revision_change {
my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
INFO('Checking for revision attachment');
- my $attachment = create_revision_attachment($bug, $revision, $timestamp, $revision->author->bugzilla_user);
- INFO('Attachment ' . $attachment->id . ' created or already exists.');
-
+ my $rev_attachment = create_revision_attachment($bug, $revision, $timestamp, $revision->author->bugzilla_user);
+ INFO('Attachment ' . $rev_attachment->id . ' created or already exists.');
+
# ATTACHMENT OBSOLETES
# fixup attachments on current bug
@@ -567,7 +583,7 @@ sub process_revision_change {
# Email changes for this revisions bug and also for any other
# bugs that previously had these revision attachments
foreach my $bug_id ($revision->bug_id, keys %other_bugs) {
- Bugzilla::BugMail::Send($bug_id, { changer => Bugzilla->user });
+ Bugzilla::BugMail::Send($bug_id, { changer => $rev_attachment->attacher });
}
Bugzilla->set_user($old_user);
diff --git a/extensions/PhabBugz/lib/Project.pm b/extensions/PhabBugz/lib/Project.pm
index c52e1a661..b93a6eb9e 100644
--- a/extensions/PhabBugz/lib/Project.pm
+++ b/extensions/PhabBugz/lib/Project.pm
@@ -26,7 +26,7 @@ has id => ( is => 'ro', isa => Int );
has phid => ( is => 'ro', isa => Str );
has type => ( is => 'ro', isa => Str );
has name => ( is => 'ro', isa => Str );
-has description => ( is => 'ro', isa => Str );
+has description => ( is => 'ro', isa => Maybe[Str] );
has creation_ts => ( is => 'ro', isa => Str );
has modification_ts => ( is => 'ro', isa => Str );
has view_policy => ( is => 'ro', isa => Str );
@@ -307,7 +307,7 @@ sub set_policy {
############
sub _build_members {
- my ($self) = @_;
+ my ( $self ) = @_;
return [] unless $self->members_raw;
my @phids;
@@ -317,13 +317,11 @@ sub _build_members {
return [] if !@phids;
- my $users = Bugzilla::Extension::PhabBugz::User->match(
+ return Bugzilla::Extension::PhabBugz::User->match(
{
phids => \@phids
}
);
-
- return [ map { $_->bugzilla_user } @$users ];
}
1;
diff --git a/extensions/PhabBugz/lib/Revision.pm b/extensions/PhabBugz/lib/Revision.pm
index 900454220..4e82fa500 100644
--- a/extensions/PhabBugz/lib/Revision.pm
+++ b/extensions/PhabBugz/lib/Revision.pm
@@ -93,6 +93,8 @@ sub new_from_query {
: "";
return $class->new($result);
}
+
+ return undef;
}
sub BUILDARGS {
diff --git a/extensions/PhabBugz/lib/User.pm b/extensions/PhabBugz/lib/User.pm
index 9d4e9eef4..1bf1a842d 100644
--- a/extensions/PhabBugz/lib/User.pm
+++ b/extensions/PhabBugz/lib/User.pm
@@ -131,14 +131,18 @@ sub match {
attachments => { 'external-accounts' => 1 }
};
+ # We can only fetch 100 users at a time so we need to do this in lumps
my $phab_users = [];
- my $result = request( 'user.search', $data );
-
- if ( exists $result->{result}{data} && @{ $result->{result}{data} } ) {
- foreach my $user ( @{ $result->{result}{data} } ) {
- push @$phab_users, $class->new($user);
+ my $result;
+ do {
+ $result = request( 'user.search', $data );
+ if ( exists $result->{result}{data} && @{ $result->{result}{data} } ) {
+ foreach my $user ( @{ $result->{result}{data} } ) {
+ push @$phab_users, $class->new($user);
+ }
}
- }
+ $data->{after} = $result->{cursor}->{after};
+ } while ($result->{cursor}->{after});
return $phab_users;
}
diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm
index d25f62f68..091475718 100644
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -146,9 +146,10 @@ sub get_attachment_revisions {
my @revisions;
foreach my $revision_id (@revision_ids) {
- push @revisions, Bugzilla::Extension::PhabBugz::Revision->new_from_query({
+ my $revision = Bugzilla::Extension::PhabBugz::Revision->new_from_query({
ids => [ $revision_id ]
});
+ push @revisions, $revision if $revision;
}
return \@revisions;
diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm
index 0239ccf74..fa9306667 100644
--- a/extensions/PhabBugz/lib/WebService.pm
+++ b/extensions/PhabBugz/lib/WebService.pm
@@ -14,6 +14,7 @@ use warnings;
use base qw(Bugzilla::WebService);
use Bugzilla::Constants;
+use Bugzilla::Error;
use Bugzilla::User;
use Bugzilla::Util qw(detaint_natural datetime_from time_ago trick_taint);
use Bugzilla::WebService::Constants;
@@ -29,34 +30,46 @@ use List::MoreUtils qw(any);
use MIME::Base64 qw(decode_base64);
use constant READ_ONLY => qw(
+ check_user_enter_bug_permission
check_user_permission_for_bug
needs_review
);
use constant PUBLIC_METHODS => qw(
+ check_user_enter_bug_permission
check_user_permission_for_bug
needs_review
set_build_target
);
-sub check_user_permission_for_bug {
- my ($self, $params) = @_;
-
- my $user = Bugzilla->login(LOGIN_REQUIRED);
-
+sub _check_phabricator {
# Ensure PhabBugz is on
ThrowUserError('phabricator_not_enabled')
unless Bugzilla->params->{phabricator_enabled};
+}
+
+sub _validate_phab_user {
+ my ($self, $user) = @_;
+
+ $self->_check_phabricator();
# Validate that the requesting user's email matches phab-bot
ThrowUserError('phabricator_unauthorized_user')
unless $user->login eq PHAB_AUTOMATION_USER;
+}
+
+sub check_user_permission_for_bug {
+ my ($self, $params) = @_;
+
+ my $user = Bugzilla->login(LOGIN_REQUIRED);
+
+ $self->_validate_phab_user($user);
# Validate that a bug id and user id are provided
ThrowUserError('phabricator_invalid_request_params')
unless ($params->{bug_id} && $params->{user_id});
- # Validate that the user and bug exist
+ # Validate that the user exists
my $target_user = Bugzilla::User->check({ id => $params->{user_id}, cache => 1 });
# Send back an object which says { "result": 1|0 }
@@ -65,10 +78,32 @@ sub check_user_permission_for_bug {
};
}
+sub check_user_enter_bug_permission {
+ my ($self, $params) = @_;
+
+ my $user = Bugzilla->login(LOGIN_REQUIRED);
+
+ $self->_validate_phab_user($user);
+
+ # Validate that a product name and user id are provided
+ ThrowUserError('phabricator_invalid_request_params')
+ unless ($params->{product} && $params->{user_id});
+
+ # Validate that the user exists
+ my $target_user = Bugzilla::User->check({ id => $params->{user_id}, cache => 1 });
+
+ # Send back an object with the attribute "result" set to 1 if the user
+ # can enter bugs into the given product, or 0 if not.
+ return {
+ result => $target_user->can_enter_product($params->{product}) ? 1 : 0
+ };
+}
+
sub needs_review {
my ($self, $params) = @_;
- ThrowUserError('phabricator_not_enabled')
- unless Bugzilla->params->{phabricator_enabled};
+
+ $self->_check_phabricator();
+
my $user = Bugzilla->login(LOGIN_REQUIRED);
my $dbh = Bugzilla->dbh;
@@ -169,13 +204,7 @@ sub set_build_target {
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;
+ $self->_validate_phab_user($user);
my $revision_id = $params->{revision_id};
my $build_target = $params->{build_target};
@@ -204,13 +233,13 @@ sub rest_resources {
POST => {
method => 'set_build_target',
params => sub {
- return {
+ return {
revision_id => $_[0],
build_target => $_[1]
};
}
}
- },
+ },
# Bug permission checks
qr{^/phabbugz/check_bug/(\d+)/(\d+)$}, {
GET => {
@@ -220,6 +249,14 @@ sub rest_resources {
}
}
},
+ qr{^/phabbugz/check_enter_bug/([^/]+)/(\d+)$}, {
+ GET => {
+ method => 'check_user_enter_bug_permission',
+ params => sub {
+ return { product => $_[0], user_id => $_[1] };
+ },
+ },
+ },
# Review requests
qw{^/phabbugz/needs_review$}, {
GET => {