summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordklawren <dklawren@users.noreply.github.com>2018-02-07 20:09:25 +0100
committerDylan William Hardison <dylan@hardison.net>2018-02-07 20:09:25 +0100
commit2b916fccae0df60b350369c6fc827c1c9ce1030e (patch)
tree6d2032532ca0e2431b92e7a274d314564dbbdf1f
parentc20774f3a3602a05596c7eea12edd6882097b4e4 (diff)
downloadbugzilla-2b916fccae0df60b350369c6fc827c1c9ce1030e.tar.gz
bugzilla-2b916fccae0df60b350369c6fc827c1c9ce1030e.tar.xz
Bug 1430259 - Update policy code in BMO PhabBugz extension to update custom policy if a private bugs groups have changed.
-rw-r--r--extensions/PhabBugz/lib/Feed.pm147
-rw-r--r--extensions/PhabBugz/lib/Policy.pm142
-rw-r--r--extensions/PhabBugz/lib/Project.pm97
3 files changed, 295 insertions, 91 deletions
diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm
index 8b6801e91..670bc5d24 100644
--- a/extensions/PhabBugz/lib/Feed.pm
+++ b/extensions/PhabBugz/lib/Feed.pm
@@ -14,7 +14,10 @@ use List::MoreUtils qw(any);
use Moo;
use Bugzilla::Constants;
+use Bugzilla::Util qw(diff_arrays);
+
use Bugzilla::Extension::PhabBugz::Constants;
+use Bugzilla::Extension::PhabBugz::Policy;
use Bugzilla::Extension::PhabBugz::Revision;
use Bugzilla::Extension::PhabBugz::Util qw(
add_security_sync_comments
@@ -104,25 +107,7 @@ sub feed_query {
}
}
- my $revision = Bugzilla::Extension::PhabBugz::Revision->new({ phids => [$object_phid] });
-
- if (!$revision->bug_id) {
- if ($story_text =~ /\s+created\s+D\d+/) {
- # If new revision and bug id was omitted, make revision public
- $self->logger->debug("No bug associated with new revision. Marking public.");
- $revision->set_policy('view', 'public');
- $revision->set_policy('edit', 'users');
- $revision->update();
- $self->logger->info("SUCCESS");
- }
- else {
- $self->logger->debug("SKIPPING: No bug associated with revision change");
- }
- $self->save_feed_last_id($story_id);
- next;
- }
-
- $self->process_revision_change($revision, $story_text);
+ $self->process_revision_change($object_phid, $story_text);
$self->save_feed_last_id($story_id);
}
}
@@ -136,18 +121,28 @@ sub save_feed_last_id {
}
sub process_revision_change {
- my ($self, $revision, $story_text) = @_;
+ my ($self, $revision_phid, $story_text) = @_;
- # Pre setup before making changes
- my $old_user = set_phab_user();
+ # Load the revision from Phabricator
+ my $revision = Bugzilla::Extension::PhabBugz::Revision->new({ phids => [ $revision_phid ] });
- my $is_shadow_db = Bugzilla->is_shadow_db;
- Bugzilla->switch_to_main_db if $is_shadow_db;
+ # NO BUG ID
- my $dbh = Bugzilla->dbh;
- $dbh->bz_start_transaction;
-
- my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
+ if (!$revision->bug_id) {
+ if ($story_text =~ /\s+created\s+D\d+/) {
+ # If new revision and bug id was omitted, make revision public
+ $self->logger->debug("No bug associated with new revision. Marking public.");
+ $revision->set_policy('view', 'public');
+ $revision->set_policy('edit', 'users');
+ $revision->update();
+ $self->logger->info("SUCCESS");
+ return;
+ }
+ else {
+ $self->logger->debug("SKIPPING: No bug associated with revision change");
+ return;
+ }
+ }
my $log_message = sprintf(
"REVISION CHANGE FOUND: D%d: %s | bug: %d | %s",
@@ -157,39 +152,73 @@ sub process_revision_change {
$story_text);
$self->logger->info($log_message);
+ # Pre setup before making changes
+ my $old_user = set_phab_user();
+
+ my $is_shadow_db = Bugzilla->is_shadow_db;
+ Bugzilla->switch_to_main_db if $is_shadow_db;
+
+ my $dbh = Bugzilla->dbh;
+ $dbh->bz_start_transaction;
+
my $bug = Bugzilla::Bug->new({ id => $revision->bug_id, cache => 1 });
# REVISION SECURITY POLICY
- # Do not set policy if a custom policy has already been set
- # This keeps from setting new custom policy everytime a change
- # is made.
- unless ($revision->view_policy =~ /^PHID-PLCY/) {
-
- # If bug is public then remove privacy policy
- if (!@{ $bug->groups_in }) {
- $revision->set_policy('view', 'public');
- $revision->set_policy('edit', 'users');
+ # If bug is public then remove privacy policy
+ if (!@{ $bug->groups_in }) {
+ $self->logger->debug('Bug is public so setting view/edit public');
+ $revision->set_policy('view', 'public');
+ $revision->set_policy('edit', 'users');
+ }
+ # 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) {
+ $self->logger->debug('No matching groups. Adding comments to bug and revision');
+ add_security_sync_comments([$revision], $bug);
}
- # else bug is private
+ # Otherwise, we create a new custom policy containing the project
+ # groups that are mapped to bugzilla groups.
else {
- my @set_groups = get_security_sync_groups($bug);
+ my @set_projects = map { "bmo-" . $_ } @set_groups;
+
+ # If current policy projects matches what we want to set, then
+ # we leave the current policy alone.
+ my $current_policy;
+ if ($revision->view_policy =~ /^PHID-PLCY/) {
+ $self->logger->debug("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;
+ $self->logger->debug("Current policy projects: " . join(", ", @$current_projects));
+ my ($added, $removed) = diff_arrays($current_projects, \@set_projects);
+ if (@$added || @$removed) {
+ $self->logger->debug('Project groups do not match. Need new custom policy');
+ $current_policy= undef;
+ }
+ else {
+ $self->logger->debug('Project groups match. Leaving current policy as-is');
+ }
+ }
- # 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([$revision], $bug);
+ if (!$current_policy) {
+ $self->logger->debug("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);
}
- my $policy_phid = create_private_revision_policy($bug, \@set_groups);
my $subscribers = get_bug_role_phids($bug);
-
- $revision->set_policy('view', $policy_phid);
- $revision->set_policy('edit', $policy_phid);
$revision->set_subscribers($subscribers);
}
}
+ my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
+
my $attachment = create_revision_attachment($bug, $revision->id, $revision->title, $timestamp);
# ATTACHMENT OBSOLETES
@@ -203,24 +232,28 @@ sub process_revision_change {
next if $attach_revision_id != $revision->id;
my $make_obsolete = $revision->status eq 'abandoned' ? 1 : 0;
+ $self->logger->debug('Updating obsolete status on attachmment ' . $attachment->id);
$attachment->set_is_obsolete($make_obsolete);
- if ($revision->id == $attach_revision_id
- && $revision->title ne $attachment->description) {
+ if ($revision->title ne $attachment->description) {
+ $self->logger->debug('Updating description on attachment ' . $attachment->id);
$attachment->set_description($revision->title);
}
$attachment->update($timestamp);
- last;
}
# fixup attachments with same revision id but on different bugs
+ my %other_bugs;
my $other_attachments = Bugzilla::Attachment->match({
mimetype => PHAB_CONTENT_TYPE,
filename => 'phabricator-D' . $revision->id . '-url.txt',
WHERE => { 'bug_id != ? AND NOT isobsolete' => $bug->id }
});
foreach my $attachment (@$other_attachments) {
+ $other_bugs{$attachment->bug_id}++;
+ $self->logger->debug('Updating obsolete status on attachment ' .
+ $attachment->id . " for bug " . $attachment->bug_id);
$attachment->set_is_obsolete(1);
$attachment->update($timestamp);
}
@@ -228,9 +261,11 @@ sub process_revision_change {
# REVIEWER STATUSES
my (@accepted_phids, @denied_phids, @accepted_user_ids, @denied_user_ids);
- foreach my $reviewer (@{ $revision->reviewers }) {
- push(@accepted_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'accepted';
- push(@denied_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'rejected';
+ unless ($revision->status eq 'changes-planned' || $revision->status eq 'needs-review') {
+ foreach my $reviewer (@{ $revision->reviewers }) {
+ push(@accepted_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'accepted';
+ push(@denied_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'rejected';
+ }
}
my $phab_users = get_phab_bmo_ids({ phids => \@accepted_phids });
@@ -301,7 +336,11 @@ sub process_revision_change {
$bug->update($timestamp);
$revision->update();
- Bugzilla::BugMail::Send($revision->bug_id, { changer => Bugzilla->user });
+ # 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 });
+ }
$dbh->bz_commit_transaction;
Bugzilla->switch_to_shadow_db if $is_shadow_db;
diff --git a/extensions/PhabBugz/lib/Policy.pm b/extensions/PhabBugz/lib/Policy.pm
new file mode 100644
index 000000000..3205562c3
--- /dev/null
+++ b/extensions/PhabBugz/lib/Policy.pm
@@ -0,0 +1,142 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+package Bugzilla::Extension::PhabBugz::Policy;
+
+use 5.10.1;
+use Moo;
+
+use Bugzilla::Error;
+use Bugzilla::Extension::PhabBugz::Util qw(request);
+use Bugzilla::Extension::PhabBugz::Project;
+
+use List::Util qw(first);
+
+use Types::Standard -all;
+use Type::Utils;
+
+has 'phid' => ( is => 'ro', isa => Str );
+has 'type' => ( is => 'ro', isa => Str );
+has 'name' => ( is => 'ro', isa => Str );
+has 'shortName' => ( is => 'ro', isa => Str );
+has 'fullName' => ( is => 'ro', isa => Str );
+has 'href' => ( is => 'ro', isa => Maybe[Str] );
+has 'workflow' => ( is => 'ro', isa => Maybe[Str] );
+has 'icon' => ( is => 'ro', isa => Str );
+has 'default' => ( is => 'ro', isa => Str );
+has 'rules' => (
+ is => 'ro',
+ isa => ArrayRef[
+ Dict[
+ action => Str,
+ rule => Str,
+ value => Maybe[ArrayRef[Str]]
+ ]
+ ]
+);
+
+has 'rule_projects' => (
+ is => 'lazy',
+ isa => ArrayRef[Str],
+);
+
+# {
+# "data": [
+# {
+# "phid": "PHID-PLCY-l2mt4yeq4byqgcot7x4j",
+# "type": "custom",
+# "name": "Custom Policy",
+# "shortName": "Custom Policy",
+# "fullName": "Custom Policy",
+# "href": null,
+# "workflow": null,
+# "icon": "fa-certificate",
+# "default": "deny",
+# "rules": [
+# {
+# "action": "allow",
+# "rule": "PhabricatorSubscriptionsSubscribersPolicyRule",
+# "value": null
+# },
+# {
+# "action": "allow",
+# "rule": "PhabricatorProjectsPolicyRule",
+# "value": [
+# "PHID-PROJ-cvurjiwfvh756mv2vhvi"
+# ]
+# }
+# ]
+# }
+# ],
+# "cursor": {
+# "limit": 100,
+# "after": null,
+# "before": null
+# }
+# }
+
+sub new_from_query {
+ my ($class, $params) = @_;
+ my $result = request('policy.query', $params);
+ if (exists $result->{result}{data} && @{ $result->{result}{data} }) {
+ return $result->{result}->{data}->[0];
+ }
+ return $class->new($result);
+}
+
+sub create {
+ my ($class, $project_names) = @_;
+
+ my $data = {
+ objectType => 'DREV',
+ default => 'deny',
+ policy => [
+ {
+ action => 'allow',
+ rule => 'PhabricatorSubscriptionsSubscribersPolicyRule',
+ }
+ ]
+ };
+
+ if (@$project_names) {
+ my $project_phids = [];
+ foreach my $project_name (@$project_names) {
+ my $project = Bugzilla::Extension::PhabBugz::Project->new({ name => $project_name });
+ push @$project_phids, $project->phid if $project;
+ }
+
+ ThrowUserError('invalid_phabricator_sync_groups') unless @$project_phids;
+
+ push @{ $data->{policy} }, {
+ action => 'allow',
+ rule => 'PhabricatorProjectsPolicyRule',
+ value => $project_phids,
+ };
+ }
+ else {
+ push @{ $data->{policy} }, { action => 'allow', value => 'admin' };
+ }
+
+ my $result = request('policy.create', $data);
+ return $class->new_from_query({ phids => [ $result->{result}{phid} ] });
+}
+
+sub _build_rule_projects {
+ my ($self) = @_;
+
+ return [] unless $self->rules;
+ my $rule = first { $_->{rule} eq 'PhabricatorProjectsPolicyRule'} @{ $self->rules };
+ return [] unless $rule;
+ return [
+ map { $_->name }
+ grep { $_ }
+ map { Bugzilla::Extension::PhabBugz::Project->new( { phids => [$_] } ) }
+ @{ $rule->{value} }
+ ];
+}
+
+1; \ No newline at end of file
diff --git a/extensions/PhabBugz/lib/Project.pm b/extensions/PhabBugz/lib/Project.pm
index 3ad9558ff..ec00b9532 100644
--- a/extensions/PhabBugz/lib/Project.pm
+++ b/extensions/PhabBugz/lib/Project.pm
@@ -18,37 +18,36 @@ use Bugzilla::Extension::PhabBugz::Util qw(
get_phab_bmo_ids
);
-#########################
-# Initialization #
-#########################
-
-sub new {
- my ($class, $params) = @_;
- my $self = $params ? _load($params) : {};
- bless($self, $class);
- return $self;
-}
-
-sub _load {
- my ($params) = @_;
-
- my $data = {
- queryKey => 'all',
- attachments => {
- projects => 1,
- reviewers => 1,
- subscribers => 1
- },
- constraints => $params
- };
-
- my $result = request('project.search', $data);
- if (exists $result->{result}{data} && @{ $result->{result}{data} }) {
- return $result->{result}->{data}->[0];
- }
-
- return $result;
-}
+use Types::Standard -all;
+use Type::Utils;
+
+my $SearchResult = Dict[
+ id => Int,
+ type => Str,
+ phid => Str,
+ fields => Dict[
+ name => Str,
+ slug => Str,
+ depth => Int,
+ milestone => Maybe[Str],
+ parent => Maybe[Str],
+ icon => Dict[ key => Str, name => Str, icon => Str ],
+ color => Dict[ key => Str, name => Str ],
+ dateCreated => Int,
+ dateModified => Int,
+ policy => Dict[ view => Str, edit => Str, join => Str ],
+ description => Maybe[Str]
+ ],
+ attachments => Dict[
+ members => Dict[
+ members => ArrayRef[
+ Dict[
+ phid => Str
+ ],
+ ],
+ ],
+ ],
+];
# {
# "data": [
@@ -90,12 +89,6 @@ sub _load {
# "phid": "PHID-USER-uif2miph2poiehjeqn5q"
# }
# ]
-# },
-# "ancestors": {
-# "ancestors": []
-# },
-# "watchers": {
-# "watchers": []
# }
# }
# }
@@ -115,6 +108,36 @@ sub _load {
# }
#########################
+# Initialization #
+#########################
+
+sub new {
+ my ($class, $params) = @_;
+ my $self = $params ? _load($params) : {};
+ $SearchResult->assert_valid($self);
+ return bless($self, $class);
+}
+
+sub _load {
+ my ($params) = @_;
+
+ my $data = {
+ queryKey => 'all',
+ attachments => {
+ members => 1
+ },
+ constraints => $params
+ };
+
+ my $result = request('project.search', $data);
+ if (exists $result->{result}{data} && @{ $result->{result}{data} }) {
+ return $result->{result}->{data}->[0];
+ }
+
+ return $result;
+}
+
+#########################
# Modification #
#########################