From 5460e9be3a80d353f7c49ca94833c08455440ce8 Mon Sep 17 00:00:00 2001 From: dklawren Date: Wed, 30 May 2018 22:12:59 -0400 Subject: Bug 1430905 - Remove legacy phabbugz code that is no longer needed --- extensions/PhabBugz/lib/Feed.pm | 13 +- extensions/PhabBugz/lib/Util.pm | 300 +++------------------------ extensions/PhabBugz/lib/WebService.pm | 284 +------------------------ extensions/Push/lib/Connector/Phabricator.pm | 97 ++++----- 4 files changed, 81 insertions(+), 613 deletions(-) (limited to 'extensions') 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; -- cgit v1.2.3-24-g4f1b