From e83b95790d29393e9711aedeccfd63353bbc3cde Mon Sep 17 00:00:00 2001 From: dklawren Date: Mon, 25 Jun 2018 17:26:53 -0400 Subject: Bug 1457900 - When restricting a revision to a bugzilla group we should tag the revision with the project --- extensions/PhabBugz/lib/Feed.pm | 39 +++------- extensions/PhabBugz/lib/Policy.pm | 18 ++--- extensions/PhabBugz/lib/Revision.pm | 104 +++++++++++++++++++++++---- extensions/Push/lib/Connector/Phabricator.pm | 19 ++--- 4 files changed, 107 insertions(+), 73 deletions(-) (limited to 'extensions') diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm index 31dd8bca0..9fb1dac11 100644 --- a/extensions/PhabBugz/lib/Feed.pm +++ b/extensions/PhabBugz/lib/Feed.pm @@ -337,20 +337,6 @@ sub process_revision_change { blessed $revision_phid ? $revision_phid : Bugzilla::Extension::PhabBugz::Revision->new_from_query({ phids => [ $revision_phid ] }); - - # Project tags/groups that will be used later for policies, etc. - my $secure_revision = - Bugzilla::Extension::PhabBugz::Project->new_from_query( - { - name => 'secure-revision' - } - ); - my $edit_bugs = - Bugzilla::Extension::PhabBugz::Project->new_from_query( - { - name => 'bmo-editbugs-team' - } - ); # NO BUG ID @@ -358,9 +344,7 @@ sub process_revision_change { if ($story_text =~ /\s+created\s+D\d+/) { # If new revision and bug id was omitted, make revision public INFO("No bug associated with new revision. Marking public."); - $revision->set_policy('view', 'public'); - $revision->set_policy('edit', ($edit_bugs ? $edit_bugs->phid : 'users')); - $revision->remove_project($secure_revision->phid); + $revision->make_public(); $revision->update(); INFO("SUCCESS"); return; @@ -388,9 +372,7 @@ sub process_revision_change { # If bug is public then remove privacy policy if (!@{ $bug->groups_in }) { INFO('Bug is public so setting view/edit public'); - $revision->set_policy('view', 'public'); - $revision->set_policy('edit', ($edit_bugs ? $edit_bugs->phid : 'users')); - $revision->remove_project($secure_revision->phid); + $revision->make_public(); } # else bug is private. else { @@ -405,7 +387,7 @@ sub process_revision_change { # Otherwise, we create a new custom policy containing the project # groups that are mapped to bugzilla groups. else { - my @set_projects = map { "bmo-" . $_ } @set_groups; + my $set_project_names = [ map { "bmo-" . $_ } @set_groups ]; # If current policy projects matches what we want to set, then # we leave the current policy alone. @@ -414,13 +396,12 @@ sub process_revision_change { INFO("Loading current policy: " . $revision->view_policy); $current_policy = Bugzilla::Extension::PhabBugz::Policy->new_from_query({ phids => [ $revision->view_policy ]}); - my $current_projects = $current_policy->rule_projects; - INFO("Current policy projects: " . join(", ", @$current_projects)); - my ($added, $removed) = diff_arrays($current_projects, \@set_projects); + my $current_project_names = [ map { $_->name } @{ $current_policy->rule_projects } ]; + INFO("Current policy projects: " . join(", ", @$current_project_names)); + my ($added, $removed) = diff_arrays($current_project_names, $set_project_names); if (@$added || @$removed) { INFO('Project groups do not match. Need new custom policy'); $current_policy = undef; - } else { INFO('Project groups match. Leaving current policy as-is'); @@ -428,13 +409,9 @@ sub process_revision_change { } if (!$current_policy) { - INFO("Creating new custom policy: " . join(", ", @set_projects)); - my $new_policy = Bugzilla::Extension::PhabBugz::Policy->create(\@set_projects); - $revision->set_policy('view', $new_policy->phid); - $revision->set_policy('edit', $new_policy->phid); + INFO("Creating new custom policy: " . join(", ", @$set_project_names)); + $revision->make_private($set_project_names); } - - $revision->add_project($secure_revision->phid); } # Subscriber list of the private revision should always match diff --git a/extensions/PhabBugz/lib/Policy.pm b/extensions/PhabBugz/lib/Policy.pm index 1e925f55a..a86c83036 100644 --- a/extensions/PhabBugz/lib/Policy.pm +++ b/extensions/PhabBugz/lib/Policy.pm @@ -41,7 +41,7 @@ has 'rules' => ( has 'rule_projects' => ( is => 'lazy', - isa => ArrayRef[Str], + isa => ArrayRef[Object], ); # { @@ -88,7 +88,7 @@ sub new_from_query { } sub create { - my ($class, $project_names) = @_; + my ($class, $projects) = @_; my $data = { objectType => 'DREV', @@ -105,19 +105,11 @@ sub create { ] }; - if (@$project_names) { - my $project_phids = []; - foreach my $project_name (@$project_names) { - my $project = Bugzilla::Extension::PhabBugz::Project->new_from_query({ name => $project_name }); - push @$project_phids, $project->phid if $project; - } - - ThrowUserError('invalid_phabricator_projects') unless @$project_phids; - + if (@$projects) { push @{ $data->{policy} }, { action => 'allow', rule => 'PhabricatorProjectsAllPolicyRule', - value => $project_phids, + value => [ map { $_->phid } @$projects ], }; } else { @@ -138,8 +130,6 @@ sub _build_rule_projects { my $rule = first { $_->{rule} =~ /PhabricatorProjects(?:All)?PolicyRule/ } @{ $self->rules }; return [] unless $rule; return [ - map { $_->name } - grep { $_ } map { Bugzilla::Extension::PhabBugz::Project->new_from_query( { phids => [$_] } ) } @{ $rule->{value} } ]; diff --git a/extensions/PhabBugz/lib/Revision.pm b/extensions/PhabBugz/lib/Revision.pm index 854cc48d4..a39e7169c 100644 --- a/extensions/PhabBugz/lib/Revision.pm +++ b/extensions/PhabBugz/lib/Revision.pm @@ -17,6 +17,7 @@ use Type::Utils; use Bugzilla::Bug; use Bugzilla::Error; use Bugzilla::Util qw(trim); +use Bugzilla::Extension::PhabBugz::Project; use Bugzilla::Extension::PhabBugz::User; use Bugzilla::Extension::PhabBugz::Util qw(request); @@ -35,12 +36,12 @@ has author_phid => ( is => 'ro', isa => Str ); has bug_id => ( is => 'ro', isa => Str ); has view_policy => ( is => 'ro', isa => Str ); has edit_policy => ( is => 'ro', isa => Str ); -has projects_raw => ( is => 'ro', isa => ArrayRef [Str] ); has subscriber_count => ( is => 'ro', isa => Int ); has bug => ( is => 'lazy', isa => Object ); has author => ( is => 'lazy', isa => Object ); has reviewers => ( is => 'lazy', isa => ArrayRef [Object] ); has subscribers => ( is => 'lazy', isa => ArrayRef [Object] ); +has projects => ( is => 'lazy', isa => ArrayRef [Object] ); has reviewers_raw => ( is => 'ro', isa => ArrayRef [ @@ -60,6 +61,12 @@ has subscribers_raw => ( viewerIsSubscribed => Bool, ] ); +has projects_raw => ( + is => 'ro', + isa => Dict [ + projectPHIDs => ArrayRef [Str] + ] +); sub new_from_query { my ( $class, $params ) = @_; @@ -91,18 +98,18 @@ sub new_from_query { sub BUILDARGS { my ( $class, $params ) = @_; - $params->{title} = $params->{fields}->{title}; - $params->{summary} = $params->{fields}->{summary}; - $params->{status} = $params->{fields}->{status}->{value}; - $params->{creation_ts} = $params->{fields}->{dateCreated}; - $params->{modification_ts} = $params->{fields}->{dateModified}; - $params->{author_phid} = $params->{fields}->{authorPHID}; - $params->{bug_id} = $params->{fields}->{'bugzilla.bug-id'}; - $params->{view_policy} = $params->{fields}->{policy}->{view}; - $params->{edit_policy} = $params->{fields}->{policy}->{edit}; - $params->{reviewers_raw} = $params->{attachments}->{reviewers}->{reviewers}; - $params->{subscribers_raw} = $params->{attachments}->{subscribers}; - $params->{projects} = $params->{attachments}->{projects}; + $params->{title} = $params->{fields}->{title}; + $params->{summary} = $params->{fields}->{summary}; + $params->{status} = $params->{fields}->{status}->{value}; + $params->{creation_ts} = $params->{fields}->{dateCreated}; + $params->{modification_ts} = $params->{fields}->{dateModified}; + $params->{author_phid} = $params->{fields}->{authorPHID}; + $params->{bug_id} = $params->{fields}->{'bugzilla.bug-id'}; + $params->{view_policy} = $params->{fields}->{policy}->{view}; + $params->{edit_policy} = $params->{fields}->{policy}->{edit}; + $params->{reviewers_raw} = $params->{attachments}->{reviewers}->{reviewers}; + $params->{subscribers_raw} = $params->{attachments}->{subscribers}; + $params->{projects_raw} = $params->{attachments}->{projects}; $params->{subscriber_count} = $params->{attachments}->{subscribers}->{subscriberCount}; @@ -343,6 +350,24 @@ sub _build_subscribers { return $self->{subscribers} = $users; } +sub _build_projects { + my ($self) = @_; + + return $self->{projects} if $self->{projects}; + return [] unless $self->projects_raw->{projectPHIDs}; + + my @projects; + foreach my $phid ( @{ $self->projects_raw->{projectPHIDs} } ) { + push @projects, Bugzilla::Extension::PhabBugz::Project->new_from_query( + { + phids => [ $phid ] + } + ); + } + + return $self->{projects} = \@projects; +} + ######################### # Mutators # ######################### @@ -416,4 +441,57 @@ sub remove_project { push @{ $self->{remove_projects} }, $project_phid; } +sub make_private { + my ( $self, $project_names ) = @_; + + my $secure_revision_project = + Bugzilla::Extension::PhabBugz::Project->new_from_query( + { + name => 'secure-revision' + } + ); + + my @set_projects; + foreach my $name (@$project_names) { + my $set_project = + Bugzilla::Extension::PhabBugz::Project->new_from_query( + { + name => $name + } + ); + push @set_projects, $set_project; + } + + my $new_policy = Bugzilla::Extension::PhabBugz::Policy->create(\@set_projects); + $self->set_policy('view', $new_policy->phid); + $self->set_policy('edit', $new_policy->phid); + + foreach my $project ($secure_revision_project, @set_projects) { + $self->add_project($project->phid); + } + + return $self; +} + +sub make_public { + my ( $self ) = @_; + + my $edit_bugs = + Bugzilla::Extension::PhabBugz::Project->new_from_query( + { + name => 'bmo-editbugs-team' + } + ); + + $self->set_policy('view', 'public'); + $self->set_policy('edit', ($edit_bugs ? $edit_bugs->phid : 'users')); + + my @current_group_projects = grep { $_->name =~ /^(bmo-.*|secure-revision)$/ } @{ $self->projects }; + foreach my $project (@current_group_projects) { + $self->remove_project($project->phid); + } + + return $self; +} + 1; diff --git a/extensions/Push/lib/Connector/Phabricator.pm b/extensions/Push/lib/Connector/Phabricator.pm index aeef32ab4..5d5e4e639 100644 --- a/extensions/Push/lib/Connector/Phabricator.pm +++ b/extensions/Push/lib/Connector/Phabricator.pm @@ -78,19 +78,13 @@ sub send { : 0; 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, $bug->id )); - $revision->set_policy('view', 'public'); - $revision->set_policy('edit', 'users'); - $revision->remove_project($secure_revision->phid); + $revision->make_public(); } elsif ( !$is_public && !@set_groups ) { Bugzilla->audit(sprintf( @@ -99,9 +93,7 @@ sub send { $bug->id, join(', ', @set_groups) )); - $revision->set_policy('view', $secure_revision->phid); - $revision->set_policy('edit', $secure_revision->phid); - $revision->add_project($secure_revision->phid); + $revision->make_private(['secure-revision']); add_security_sync_comments([$revision], $bug); } elsif ( !$is_public && $group_change ) { @@ -110,11 +102,8 @@ sub send { $revision->id, $bug->id )); - 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); + my @set_project_names = map { "bmo-" . $_ } @set_groups; + $revision->make_private(\@set_project_names); } # Subscriber list of the private revision should always match -- cgit v1.2.3-24-g4f1b