Bug 1430905 - Remove legacy phabbugz code that is no longer needed

4 files changed, 81 insertions, 613 deletions

diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm
index a0d620246..ff381ad26 100644
--- a/extensions/PhabBugz/lib/Feed.pm
+++ b/extensions/PhabBugz/lib/Feed.pm
@@ -33,10 +33,8 @@ use Bugzilla::Extension::PhabBugz::Util qw(
     add_security_sync_comments
     create_revision_attachment
     get_bug_role_phids
-    get_project_phid
     get_security_sync_groups
     is_attachment_phab_revision
-    make_revision_public
     request
     set_phab_user
 );
@@ -117,7 +115,7 @@ sub feed_query {
 
     # PROCESS NEW FEED TRANSACTIONS
 
-    INFO("Fetching new transactions");
+    INFO("Fetching new stories");
 
     my $story_last_id = $self->get_last_id('feed');
 
@@ -265,9 +263,6 @@ sub group_query {
 
     INFO("Updating group memberships");
 
-    # Pre setup before making changes
-    my $old_user = set_phab_user();
-
     # Loop through each group and perform the following:
     #
     # 1. Load flattened list of group members
@@ -332,8 +327,6 @@ sub group_query {
         $project->set_members( [ ($phab_user, @group_members) ] );
         $project->update();
     }
-
-    Bugzilla->set_user($old_user);
 }
 
 sub process_revision_change {
@@ -385,6 +378,10 @@ sub process_revision_change {
 
     # REVISION SECURITY POLICY
 
+    my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({
+        name => 'secure-revision'
+    });
+
     # If bug is public then remove privacy policy
     if (!@{ $bug->groups_in }) {
         INFO('Bug is public so setting view/edit public');
diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm
index 916455e24..b24124ede 100644
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -25,56 +25,19 @@ use Taint::Util qw(untaint);
 
 use base qw(Exporter);
 
-our @EXPORT_OK = qw(
-    add_comment_to_revision
+our @EXPORT = qw(
     add_security_sync_comments
-    create_private_revision_policy
-    create_project
     create_revision_attachment
-    edit_revision_policy
     get_attachment_revisions
     get_bug_role_phids
     get_needs_review
-    get_project_phid
-    get_revisions_by_ids
-    get_revisions_by_phids
     get_security_sync_groups
     intersect
     is_attachment_phab_revision
-    make_revision_private
-    make_revision_public
     request
     set_phab_user
-    set_project_members
-    set_revision_subscribers
 );
 
-sub get_revisions_by_ids {
-    my ($ids) = @_;
-    return _get_revisions({ ids => $ids });
-}
-
-sub get_revisions_by_phids {
-    my ($phids) = @_;
-    return _get_revisions({ phids => $phids });
-}
-
-sub _get_revisions {
-    my ($constraints) = @_;
-
-    my $data = {
-        queryKey    => 'all',
-        constraints => $constraints
-    };
-
-    my $result = request('differential.revision.search', $data);
-
-    ThrowUserError('invalid_phabricator_revision_id')
-        unless (exists $result->{result}{data} && @{ $result->{result}{data} });
-
-    return $result->{result}{data};
-}
-
 sub create_revision_attachment {
     my ( $bug, $revision, $timestamp ) = @_;
 
@@ -131,230 +94,14 @@ sub get_bug_role_phids {
     push(@bug_users, $bug->qa_contact) if $bug->qa_contact;
     push(@bug_users, @{ $bug->cc_users }) if @{ $bug->cc_users };
 
-    my $phab_users = Bugzilla::Extension::PhabBugz::User->match(
-      {
-        ids => [ map { $_->id } @bug_users ]
-      }
-    );
-
-    return [ map { $_->phid } @$phab_users ];
-}
-
-sub create_private_revision_policy {
-    my ( $groups ) = @_;
-
-    my $data = {
-        objectType => 'DREV',
-        default    => 'deny',
-        policy     => [
-            {
-                action => 'allow',
-                rule   => 'PhabricatorSubscriptionsSubscribersPolicyRule'
-            },
-            {
-                action => 'allow',
-                rule   => 'PhabricatorDifferentialReviewersPolicyRule'
-            }
-        ]
-    };
-
-    if(scalar @$groups gt 0) {
-        my $project_phids = [];
-        foreach my $group (@$groups) {
-            my $phid = get_project_phid('bmo-' . $group);
-            push(@$project_phids, $phid) if $phid;
-        }
-
-        ThrowUserError('invalid_phabricator_projects') unless @$project_phids;
-
-        push(@{ $data->{policy} },
-            {
-                action => 'allow',
-                rule   => 'PhabricatorProjectsPolicyRule',
-                value  => $project_phids,
-            }
-        );
-    }
-    else {
-        my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({
-            name => 'secure-revision'
-        });
-        push(@{ $data->{policy} },
-            {
-                action => 'allow',
-                value  => $secure_revision->phid,
-            }
-        );
-    }
-
-    my $result = request('policy.create', $data);
-    return $result->{result}{phid};
-}
-
-sub make_revision_public {
-    my ($revision_phid) = @_;
-    return request('differential.revision.edit', {
-        transactions => [
-            {
-                type  => 'view',
-                value => 'public'
-            },
-            {
-                type  => 'edit',
-                value => 'users'
-            }
-        ],
-        objectIdentifier => $revision_phid
-    });
-
-}
-
-sub make_revision_private {
-    my ($revision_phid) = @_;
-
-    # When creating a private policy with no args it
-    # creates one with the secure-revision project.
-    my $private_policy = create_private_revision_policy();
-
-    return request('differential.revision.edit', {
-        transactions => [
-            {
-                type  => "view",
-                value => $private_policy->phid
-            },
-            {
-                type  => "edit",
-                value => $private_policy->phid
-            }
-        ],
-        objectIdentifier => $revision_phid
-    });
-}
-
-sub edit_revision_policy {
-    my ($revision_phid, $policy_phid, $subscribers) = @_;
-
-    my $data = {
-        transactions => [
-            {
-                type  => 'view',
-                value => $policy_phid
-            },
-            {
-                type  => 'edit',
-                value => $policy_phid
-            }
-        ],
-        objectIdentifier => $revision_phid
-    };
-
-    if (@$subscribers) {
-        push(@{ $data->{transactions} }, {
-            type  => 'subscribers.set',
-            value => $subscribers
-        });
-    }
-
-    return request('differential.revision.edit', $data);
-}
-
-sub set_revision_subscribers {
-    my ($revision_phid, $subscribers) = @_;
-
-    my $data = {
-        transactions => [
-            {
-                type  => 'subscribers.set',
-                value => $subscribers
-            }
-        ],
-        objectIdentifier => $revision_phid
-    };
-
-    return request('differential.revision.edit', $data);
-}
-
-sub add_comment_to_revision {
-    my ($revision_phid, $comment) = @_;
-
-    my $data = {
-        transactions => [
-            {
-                type  => 'comment',
-                value => $comment
-            }
-        ],
-        objectIdentifier => $revision_phid
-    };
-    return request('differential.revision.edit', $data);
-}
-
-sub get_project_phid {
-    my $project = shift;
-    my $memcache = Bugzilla->memcached;
-
-    # Check memcache
-    my $project_phid = $memcache->get_config({ key => "phab_project_phid_" . $project });
-    if (!$project_phid) {
-        my $data = {
-            queryKey => 'all',
-            constraints => {
-                name => $project
-            }
-        };
-
-        my $result = request('project.search', $data);
-        return undef
-            unless (exists $result->{result}{data} && @{ $result->{result}{data} });
-
-        # If name is used as a query param, we need to loop through and look
-        # for exact match as Conduit will tokenize the name instead of doing
-        # exact string match :(
-        foreach my $item ( @{ $result->{result}{data} } ) {
-            next if $item->{fields}{name} ne $project;
-            $project_phid = $item->{phid};
+    my $phab_users =
+      Bugzilla::Extension::PhabBugz::User->match(
+        {
+          ids => [ map { $_->id } @bug_users ]
         }
+    );
 
-        $memcache->set_config({ key => "phab_project_phid_" . $project, data => $project_phid });
-    }
-    return $project_phid;
-}
-
-sub create_project {
-    my ($project, $description, $members) = @_;
-
-    my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({
-        name => 'secure-revision'
-    });
-
-    my $data = {
-        transactions => [
-            { type => 'name',  value => $project               },
-            { type => 'description', value => $description     },
-            { type => 'edit',  value => $secure_revision->phid }.
-            { type => 'join',  value => $secure_revision->phid },
-            { type => 'view',  value => $secure_revision->phid },
-            { type => 'icon',  value => 'group'                },
-            { type => 'color', value => 'red'                  }
-        ]
-    };
-
-    my $result = request('project.edit', $data);
-    return $result->{result}{object}{phid};
-}
-
-sub set_project_members {
-    my ($project_id, $phab_user_ids) = @_;
-
-    my $data = {
-        objectIdentifier => $project_id,
-        transactions => [
-            { type => 'members.set',  value => $phab_user_ids }
-        ]
-    };
-
-    my $result = request('project.edit', $data);
-    return $result->{result}{object}{phid};
+    return [ map { $_->phid } @{ $phab_users } ];
 }
 
 sub is_attachment_phab_revision {
@@ -366,26 +113,29 @@ sub is_attachment_phab_revision {
 sub get_attachment_revisions {
     my $bug = shift;
 
-    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) );
-        }
+    return unless @attachments;
 
-        if (@revision_ids) {
-            $revisions = get_revisions_by_ids( \@revision_ids );
-        }
+    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) );
+    }
+
+    return unless @revision_ids;
+
+    my @revisions;
+    foreach my $revision_id (@revision_ids) {
+        push @revisions, Bugzilla::Extension::PhabBugz::Revision->new_from_query({
+            ids => [ $revision_id ]
+        });
     }
 
-    return @$revisions;
+    return \@revisions;
 }
 
 sub request {
@@ -462,7 +212,7 @@ sub add_security_sync_comments {
     my $phab_error_message = 'Revision is being made private due to unknown Bugzilla groups.';
 
     foreach my $revision (@$revisions) {
-        add_comment_to_revision( $revision->{phid}, $phab_error_message );
+        $revision->add_comment($phab_error_message);
     }
 
     my $num_revisions = scalar @$revisions;
diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm
index 56afc93fe..5ca811d58 100644
--- a/extensions/PhabBugz/lib/WebService.pm
+++ b/extensions/PhabBugz/lib/WebService.pm
@@ -13,30 +13,14 @@ use warnings;
 
 use base qw(Bugzilla::WebService);
 
-use Bugzilla::Attachment;
-use Bugzilla::Bug;
-use Bugzilla::BugMail;
 use Bugzilla::Constants;
-use Bugzilla::Error;
-use Bugzilla::Extension::Push::Util qw(is_public);
 use Bugzilla::User;
 use Bugzilla::Util qw(detaint_natural datetime_from time_ago trick_taint);
 use Bugzilla::WebService::Constants;
 
 use Bugzilla::Extension::PhabBugz::Constants;
 use Bugzilla::Extension::PhabBugz::Util qw(
-    add_security_sync_comments
-    create_private_revision_policy
-    create_revision_attachment
-    edit_revision_policy
-    get_bug_role_phids
     get_needs_review
-    get_project_phid
-    get_revisions_by_ids
-    get_security_sync_groups
-    is_attachment_phab_revision
-    make_revision_public
-    request
 );
 
 use DateTime ();
@@ -45,90 +29,28 @@ use List::MoreUtils qw(any);
 use MIME::Base64 qw(decode_base64);
 
 use constant READ_ONLY => qw(
+    check_user_permission_for_bug
     needs_review
 );
 
 use constant PUBLIC_METHODS => qw(
     check_user_permission_for_bug
-    obsolete_attachments
-    revision
-    update_reviewer_statuses
     needs_review
     set_build_target
 );
 
-sub revision {
-    my ($self, $params) = @_;
-
-    # Phabricator only supports sending credentials via HTTP Basic Auth
-    # so we exploit that function to pass in an API key as the password
-    # of basic auth. BMO does not support basic auth but does support
-    # use of API keys.
-    my $http_auth = Bugzilla->cgi->http('Authorization');
-    $http_auth =~ s/^Basic\s+//;
-    $http_auth = decode_base64($http_auth);
-    my ($login, $api_key) = split(':', $http_auth);
-    $params->{'Bugzilla_login'} = $login;
-    $params->{'Bugzilla_api_key'} = $api_key;
-
-    my $user = Bugzilla->login(LOGIN_REQUIRED);
-
-    # Prechecks
-    _phabricator_precheck($user);
-
-    unless (defined $params->{revision} && detaint_natural($params->{revision})) {
-        ThrowCodeError('param_required', { param => 'revision' })
-    }
-
-    # Obtain more information about the revision from Phabricator
-    my $revision_id = $params->{revision};
-    my $revisions = get_revisions_by_ids([$revision_id]);
-    my $revision = $revisions->[0];
-
-    my $revision_phid  = $revision->{phid};
-    my $revision_title = $revision->{fields}{title} || 'Unknown Description';
-    my $bug_id         = $revision->{fields}{'bugzilla.bug-id'};
-
-    my $bug = Bugzilla::Bug->new($bug_id);
-
-    # If bug is public then remove privacy policy
-    my $result;
-    if (is_public($bug)) {
-        $result = make_revision_public($revision_id);
-    }
-    # else bug is private
-    else {
-        my @set_groups = get_security_sync_groups($bug);
-
-        # If bug privacy groups do not have any matching synchronized groups,
-        # then leave revision private and it will have be dealt with manually.
-        if (!@set_groups) {
-            add_security_sync_comments($revisions, $bug);
-        }
-
-        my $policy_phid = create_private_revision_policy($bug, \@set_groups);
-        my $subscribers = get_bug_role_phids($bug);
-        $result = edit_revision_policy($revision_phid, $policy_phid, $subscribers);
-    }
-
-    my $attachment = create_revision_attachment($bug, $revision_id, $revision_title);
-
-    Bugzilla::BugMail::Send($bug_id, { changer => $user });
-
-    return {
-        result          => $result,
-        attachment_id   => $attachment->id,
-        attachment_link => Bugzilla->localconfig->{urlbase} . "attachment.cgi?id=" . $attachment->id
-    };
-}
-
 sub check_user_permission_for_bug {
     my ($self, $params) = @_;
 
     my $user = Bugzilla->login(LOGIN_REQUIRED);
 
-    # Prechecks
-    _phabricator_precheck($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;
 
     # Validate that a bug id and user id are provided
     ThrowUserError('phabricator_invalid_request_params')
@@ -143,161 +65,6 @@ 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 $denied_user_ids = $params->{denied_users};
-    defined $denied_user_ids
-      || ThrowCodeError('param_required', { param => 'denied_users' });
-    $denied_user_ids = [ split(':', $denied_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
-        my (@denied_flags, @new_flags, @removed_flags, %accepted_done, $flag_type);
-        foreach my $flag (@{ $attachment->flags }) {
-            next if $flag->type->name ne 'review';
-            $flag_type = $flag->type if $flag->type->is_active;
-            if (any { $flag->setter->id == $_ } @$denied_user_ids) {
-                push(@denied_flags, { id => $flag->id, setter => $flag->setter, status => 'X' });
-            }
-            if (any { $flag->setter->id == $_ } @$accepted_user_ids) {
-                $accepted_done{$flag->setter->id}++;
-            }
-            if ($flag->status eq '+'
-                && !any { $flag->setter->id == $_ } (@$accepted_user_ids, @$denied_user_ids)) {
-                push(@removed_flags, { id => $flag->id, setter => $flag->setter, status => 'X' });
-            }
-        }
-
-        $flag_type ||= first { $_->name eq 'review' && $_->is_active } @{ $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 => '+' });
-        }
-
-        # Also add comment to for attachment update showing the user's name
-        # that changed the revision.
-        my $comment;
-        foreach my $flag_data (@new_flags) {
-            $comment .= $flag_data->{setter}->name . " has approved the revision.\n";
-        }
-        foreach my $flag_data (@denied_flags) {
-            $comment .= $flag_data->{setter}->name . " has requested changes to the revision.\n";
-        }
-        foreach my $flag_data (@removed_flags) {
-            $comment .= $flag_data->{setter}->name . " has been removed from the revision.\n";
-        }
-
-        if ($comment) {
-            $comment .= "\n" . Bugzilla->params->{phabricator_base_uri} . "D" . $revision_id;
-            # Add transaction_id as anchor if one present
-            $comment .= "#" . $params->{transaction_id} if $params->{transaction_id};
-            $bug->add_comment($comment, {
-                isprivate  => $attachment->isprivate,
-                type       => CMT_ATTACHMENT_UPDATED,
-                extra_data => $attachment->id
-            });
-        }
-
-        $attachment->set_flags([ @denied_flags, @removed_flags ], \@new_flags);
-        $attachment->update($timestamp);
-        $bug->update($timestamp) if $comment;
-
-        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 $make_obsolete = $params->{make_obsolete};
-    unless (defined $make_obsolete) {
-        ThrowCodeError('param_required', { param => 'make_obsolete' })
-    }
-    $make_obsolete = $make_obsolete ? 1 : 0;
-
-    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($make_obsolete);
-        $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 needs_review {
     my ($self, $params) = @_;
     ThrowUserError('phabricator_not_enabled')
@@ -387,18 +154,6 @@ sub needs_review {
     return { result => \@result };
 }
 
-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 set_build_target {
     my ( $self, $params ) = @_;
 
@@ -457,15 +212,6 @@ sub rest_resources {
                 }
             }
         },        
-        # Revision creation
-        qr{^/phabbugz/revision/([^/]+)$}, {
-            POST => {
-                method => 'revision',
-                params => sub {
-                    return { revision => $_[0] };
-                }
-            }
-        },
         # Bug permission checks
         qr{^/phabbugz/check_bug/(\d+)/(\d+)$}, {
             GET => {
@@ -475,24 +221,12 @@ sub rest_resources {
                 }
             }
         },
-        # Update reviewer statuses
-        qr{^/phabbugz/update_reviewer_statuses$}, {
-            PUT => {
-                method => 'update_reviewer_statuses',
-            }
-        },
-        # Obsolete attachments
-        qr{^/phabbugz/obsolete$}, {
-            PUT => {
-                method => 'obsolete_attachments',
-            }
-        },
         # Review requests
         qw{^/phabbugz/needs_review$}, {
             GET => {
                 method => 'needs_review',
             },
-        },
+        }
     ];
 }
 
diff --git a/extensions/Push/lib/Connector/Phabricator.pm b/extensions/Push/lib/Connector/Phabricator.pm
index cea73f410..aeef32ab4 100644
--- a/extensions/Push/lib/Connector/Phabricator.pm
+++ b/extensions/Push/lib/Connector/Phabricator.pm
@@ -15,23 +15,18 @@ use base 'Bugzilla::Extension::Push::Connector::Base';
 
 use Bugzilla::Bug;
 use Bugzilla::Constants;
-use Bugzilla::Error;
-use Bugzilla::User;
 
 use Bugzilla::Extension::PhabBugz::Constants;
+use Bugzilla::Extension::PhabBugz::Policy;
+use Bugzilla::Extension::PhabBugz::Project;
+use Bugzilla::Extension::PhabBugz::Revision;
 use Bugzilla::Extension::PhabBugz::Util qw(
-  add_comment_to_revision
   add_security_sync_comments
-  create_private_revision_policy
-  edit_revision_policy
   get_attachment_revisions
   get_bug_role_phids
-  get_project_phid
   get_security_sync_groups
-  make_revision_public
-  make_revision_private
-  set_revision_subscribers
 );
+
 use Bugzilla::Extension::Push::Constants;
 use Bugzilla::Extension::Push::Util qw(is_public);
 
@@ -75,72 +70,64 @@ sub send {
 
     my @set_groups = get_security_sync_groups($bug);
 
-    my @revisions = get_attachment_revisions($bug);
-
-    if (!$is_public && !@set_groups) {
-        foreach my $revision (@revisions) {
-            Bugzilla->audit(sprintf(
-              'Making revision %s for bug %s private due to unkown Bugzilla groups: %s',
-              $revision->{id},
-              $bug->id,
-              join(', ', @set_groups)
-            ));
-            make_revision_private( $revision->{phid} );
-        }
-
-        add_security_sync_comments(\@revisions, $bug);
-
-        return PUSH_RESULT_OK;
-    }
+    my $revisions = get_attachment_revisions($bug);
 
     my $group_change =
       ($message->routing_key =~ /^(?:attachment|bug)\.modify:.*\bbug_group\b/)
       ? 1
       : 0;
 
-    my $subscribers;
-    if ( !$is_public ) {
-        $subscribers = get_bug_role_phids($bug);
-    }
-
-    my $secure_project_phid = get_project_phid('secure-revision');
-
-    foreach my $revision (@revisions) {
-        my $revision_phid = $revision->{phid};
-
-        my $rev_obj = Bugzilla::Extension::PhabBugz::Revision->new_from_query({ phids => [ $revision_phid ] });
-        my $revision_updated;
+    foreach my $revision (@$revisions) {
+        my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({
+            name => 'secure-revision'
+        });
 
         if ( $is_public && $group_change ) {
             Bugzilla->audit(sprintf(
               'Making revision %s public for bug %s',
-              $revision->{id},
+              $revision->id,
               $bug->id
             ));
-            make_revision_public($revision_phid);
-            $rev_obj->remove_project($secure_project_phid);
-            $revision_updated = 1;
+            $revision->set_policy('view', 'public');
+            $revision->set_policy('edit', 'users');
+            $revision->remove_project($secure_revision->phid);
         }
-        elsif ( !$is_public && $group_change ) {
+        elsif ( !$is_public && !@set_groups ) {
             Bugzilla->audit(sprintf(
-              'Giving revision %s a custom policy for bug %s',
-              $revision->{id},
-              $bug->id
+              'Making revision %s for bug %s private due to unkown Bugzilla groups: %s',
+              $revision->id,
+              $bug->id,
+              join(', ', @set_groups)
             ));
-            my $policy_phid = create_private_revision_policy( \@set_groups );
-            edit_revision_policy( $revision_phid, $policy_phid, $subscribers );
-            $rev_obj->add_project($secure_project_phid);
-            $revision_updated = 1;
+            $revision->set_policy('view', $secure_revision->phid);
+            $revision->set_policy('edit', $secure_revision->phid);
+            $revision->add_project($secure_revision->phid);
+            add_security_sync_comments([$revision], $bug);
         }
-        elsif ( !$is_public && !$group_change ) {
+        elsif ( !$is_public && $group_change ) {
             Bugzilla->audit(sprintf(
-              'Updating subscribers for %s for bug %s',
-              $revision->{id},
+              'Giving revision %s a custom policy for bug %s',
+              $revision->id,
               $bug->id
             ));
-            set_revision_subscribers( $revision_phid, $subscribers );
+            my @set_projects = map { "bmo-" . $_ } @set_groups;
+            my $new_policy = Bugzilla::Extension::PhabBugz::Policy->create(\@set_projects);
+            $revision->set_policy('view', $new_policy->phid);
+            $revision->set_policy('edit', $new_policy->phid);
+            $revision->add_project($secure_revision->phid);
         }
-        $rev_obj->update() if $revision_updated;
+
+        # Subscriber list of the private revision should always match
+        # the bug roles such as assignee, qa contact, and cc members.
+        Bugzilla->audit(sprintf(
+          'Updating subscribers for %s for bug %s',
+          $revision->id,
+          $bug->id
+        ));
+        my $subscribers = get_bug_role_phids($bug);
+        $revision->set_subscribers($subscribers) if $subscribers;
+
+        $revision->update();
     }
 
     return PUSH_RESULT_OK;