From 163b1cab7daaa9843dffefaab090ed5f60961f72 Mon Sep 17 00:00:00 2001 From: dklawren Date: Fri, 13 Jul 2018 01:05:33 -0400 Subject: Bug 1473798 - Add and remove members of a phabricator project instead of setting exact list --- extensions/PhabBugz/lib/Feed.pm | 19 ++++++++++++++----- extensions/PhabBugz/lib/Project.pm | 6 ++---- 2 files changed, 16 insertions(+), 9 deletions(-) (limited to 'extensions/PhabBugz') diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm index c46d36c13..1f9824ed9 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; @@ -323,10 +323,19 @@ 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(); + 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 ($added, $removed) = diff_arrays(\@set_member_phids, \@current_member_phids); + + INFO('Adding members: ' . join( ',', @$added )); + $project->add_member( $_ ) foreach @$added; + INFO('Removing members: ' . join( ',', @$removed )); + $project->remove_member( $_ ) foreach @$removed; + + my $result = $project->update(); + local Bugzilla::Logging->fields->{api_result} = $result; + INFO("Project " . $project->name . " updated"); } } diff --git a/extensions/PhabBugz/lib/Project.pm b/extensions/PhabBugz/lib/Project.pm index c52e1a661..3d93e2f04 100644 --- a/extensions/PhabBugz/lib/Project.pm +++ b/extensions/PhabBugz/lib/Project.pm @@ -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; -- cgit v1.2.3-24-g4f1b From e8c0dfe232ce1efe78f8dd7cf6b78c24b96786e8 Mon Sep 17 00:00:00 2001 From: byron jones Date: Mon, 23 Jul 2018 22:35:10 +0800 Subject: Bug 1475593 - Bugzilla Emails received when patches are attached in Phabricator --- extensions/PhabBugz/lib/Feed.pm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'extensions/PhabBugz') diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm index 1f9824ed9..d238c6a43 100644 --- a/extensions/PhabBugz/lib/Feed.pm +++ b/extensions/PhabBugz/lib/Feed.pm @@ -433,9 +433,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 @@ -576,7 +576,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); -- cgit v1.2.3-24-g4f1b From 7ded28be567fd52b2ab752c30203e59f307ebdcd Mon Sep 17 00:00:00 2001 From: dklawren Date: Tue, 24 Jul 2018 12:39:02 -0400 Subject: Bug 1477894 - get_attachment_revisions() should be returning an empty list, instead of a list of [0] --- extensions/PhabBugz/lib/Revision.pm | 2 ++ extensions/PhabBugz/lib/Util.pm | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) (limited to 'extensions/PhabBugz') 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/Util.pm b/extensions/PhabBugz/lib/Util.pm index 214bc3fb7..99668289a 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; -- cgit v1.2.3-24-g4f1b From 7dadc66b5c3895ff5d9a83de1dcde49f8f523554 Mon Sep 17 00:00:00 2001 From: Mark Côté Date: Wed, 25 Jul 2018 14:00:06 -0400 Subject: no bug - Include Bugzilla::Error in PhabBugz's WebService.pm. The ThrowUserError() calls in PhabBugz's WebService.pm were causing errors as Bugzilla::Error wasn't being imported. --- extensions/PhabBugz/lib/WebService.pm | 1 + 1 file changed, 1 insertion(+) (limited to 'extensions/PhabBugz') diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm index 0239ccf74..142211c6d 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; -- cgit v1.2.3-24-g4f1b From 2f1edccbe351176eaf69a6042945f84a5417c0b7 Mon Sep 17 00:00:00 2001 From: dklawren Date: Wed, 25 Jul 2018 16:36:47 -0400 Subject: Bug 1478012 Phab allows projects to have empty descriptions so Project.pm in PhabBugz should allow the same --- extensions/PhabBugz/lib/Project.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'extensions/PhabBugz') diff --git a/extensions/PhabBugz/lib/Project.pm b/extensions/PhabBugz/lib/Project.pm index 3d93e2f04..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 ); -- cgit v1.2.3-24-g4f1b From 694a37b9debdf280d279b39b44329adda5caff40 Mon Sep 17 00:00:00 2001 From: dklawren Date: Fri, 27 Jul 2018 10:17:50 -0400 Subject: Bug 1478540 - Update User.pm to load more than 100 users by using the paging functionality of Conduit API --- extensions/PhabBugz/lib/Feed.pm | 29 ++++++++++++++++++----------- extensions/PhabBugz/lib/User.pm | 16 ++++++++++------ 2 files changed, 28 insertions(+), 17 deletions(-) (limited to 'extensions/PhabBugz') diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm index d238c6a43..7d6b4e0ed 100644 --- a/extensions/PhabBugz/lib/Feed.pm +++ b/extensions/PhabBugz/lib/Feed.pm @@ -322,20 +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 ); - my @set_member_phids = uniq map { $_->phid } (@$set_members, $phab_user); + 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 ($added, $removed) = diff_arrays(\@set_member_phids, \@current_member_phids); + my ( $removed, $added ) = diff_arrays( \@current_member_phids, \@set_member_phids ); - INFO('Adding members: ' . join( ',', @$added )); - $project->add_member( $_ ) foreach @$added; - INFO('Removing members: ' . join( ',', @$removed )); - $project->remove_member( $_ ) foreach @$removed; + 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; + } - my $result = $project->update(); - local Bugzilla::Logging->fields->{api_result} = $result; - INFO("Project " . $project->name . " updated"); + if (@$added || @$removed) { + my $result = $project->update(); + local Bugzilla::Logging->fields->{api_result} = $result; + INFO( "Project " . $project->name . " updated" ); + } } } 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; } -- cgit v1.2.3-24-g4f1b From 8b75f8e691309ec68e5de1cbdf77f6e8b2b305f8 Mon Sep 17 00:00:00 2001 From: Mark Côté Date: Fri, 27 Jul 2018 17:19:57 -0400 Subject: Bug 1478983 - WebService endpoint to check if a user can enter a bug into a given product There is some common code, which we will need again for another WebService call, that can be refactored into their own functions. This WebService endpoint takes a product name and user ID in the form `check_enter_bug//`. It returns an object with one attribute, `result`, that is set to 1 if the product exists and the user is allowed to enter bugs in it, or 0 otherwise. --- extensions/PhabBugz/lib/WebService.pm | 70 ++++++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 17 deletions(-) (limited to 'extensions/PhabBugz') diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm index 142211c6d..fa9306667 100644 --- a/extensions/PhabBugz/lib/WebService.pm +++ b/extensions/PhabBugz/lib/WebService.pm @@ -30,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 } @@ -66,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; @@ -170,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}; @@ -205,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 => { @@ -221,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 => { -- cgit v1.2.3-24-g4f1b