Merge branch 'mojo-poc'

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 => {