summaryrefslogtreecommitdiffstats
path: root/extensions/PhabBugz
diff options
context:
space:
mode:
authorDylan William Hardison <dylan@hardison.net>2018-08-20 23:44:11 +0200
committerGitHub <noreply@github.com>2018-08-20 23:44:11 +0200
commit77468653f4f3e3285bc68e455b5b4e4265362aeb (patch)
treea67ea16f820e2a07f9706c7fa9772add37ec329d /extensions/PhabBugz
parentec67fc35e552accc2338e322e9128dacb13a9a91 (diff)
downloadbugzilla-77468653f4f3e3285bc68e455b5b4e4265362aeb.tar.gz
bugzilla-77468653f4f3e3285bc68e455b5b4e4265362aeb.tar.xz
Bug 1482145 - PhabBot changes are showing up as from the wrong user
Diffstat (limited to 'extensions/PhabBugz')
-rw-r--r--extensions/PhabBugz/lib/Feed.pm66
-rw-r--r--extensions/PhabBugz/lib/Types.pm7
-rw-r--r--extensions/PhabBugz/lib/Util.pm1
-rw-r--r--extensions/PhabBugz/t/review-flags.t209
4 files changed, 252 insertions, 31 deletions
diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm
index 2be96153e..f2a440bb1 100644
--- a/extensions/PhabBugz/lib/Feed.pm
+++ b/extensions/PhabBugz/lib/Feed.pm
@@ -54,8 +54,10 @@ sub start {
interval => PHAB_FEED_POLL_SECONDS,
reschedule => 'drift',
on_tick => sub {
- try{
- $self->feed_query();
+ try {
+ with_writable_database {
+ $self->feed_query();
+ };
}
catch {
FATAL($_);
@@ -70,8 +72,10 @@ sub start {
interval => PHAB_USER_POLL_SECONDS,
reschedule => 'drift',
on_tick => sub {
- try{
- $self->user_query();
+ try {
+ with_writable_database {
+ $self->user_query();
+ };
}
catch {
FATAL($_);
@@ -86,8 +90,10 @@ sub start {
interval => PHAB_GROUP_POLL_SECONDS,
reschedule => 'drift',
on_tick => sub {
- try{
- $self->group_query();
+ try {
+ with_writable_database {
+ $self->group_query();
+ };
}
catch {
FATAL($_);
@@ -149,23 +155,30 @@ sub feed_query {
}
# Skip changes done by phab-bot user
- my $phab_user = Bugzilla::Extension::PhabBugz::User->new_from_query(
- {
- phids => [ $author_phid ]
- }
+ # If changer does not exist in bugzilla database
+ # we use the phab-bot account as the changer
+ my $author = Bugzilla::Extension::PhabBugz::User->new_from_query(
+ { phids => [ $author_phid ] }
);
- if ($phab_user && $phab_user->bugzilla_id) {
- if ($phab_user->bugzilla_user->login eq PHAB_AUTOMATION_USER) {
+ if ($author && $author->bugzilla_id) {
+ if ($author->bugzilla_user->login eq PHAB_AUTOMATION_USER) {
INFO("SKIPPING: Change made by phabricator user");
$self->save_last_id($story_id, 'feed');
next;
}
}
-
- with_writable_database {
- $self->process_revision_change($object_phid, $story_text);
- };
+ else {
+ my $phab_user = Bugzilla::User->new( { name => PHAB_AUTOMATION_USER } );
+ $author = Bugzilla::Extension::PhabBugz::User->new_from_query(
+ {
+ ids => [ $phab_user->id ]
+ }
+ );
+ }
+ # Load the revision from Phabricator
+ my $revision = Bugzilla::Extension::PhabBugz::Revision->new_from_query({ phids => [ $object_phid ] });
+ $self->process_revision_change($revision, $author, $story_text);
$self->save_last_id($story_id, 'feed');
}
@@ -197,9 +210,7 @@ sub feed_query {
}
);
- with_writable_database {
- $self->process_revision_change($revision, " created D" . $revision->id);
- };
+ $self->process_revision_change( $revision, $revision->author, " created D" . $revision->id );
# Set the build target to a passing status to
# allow the revision to exit draft state
@@ -351,16 +362,10 @@ sub group_query {
}
sub process_revision_change {
- my ($self, $revision_phid, $story_text) = @_;
-
- # Load the revision from Phabricator
- my $revision =
- blessed $revision_phid
- ? $revision_phid
- : Bugzilla::Extension::PhabBugz::Revision->new_from_query({ phids => [ $revision_phid ] });
+ state $check = compile($Invocant, Revision, LinkedPhabUser, Str);
+ my ($self, $revision, $changer, $story_text) = $check->(@_);
# NO BUG ID
-
if (!$revision->bug_id) {
if ($story_text =~ /\s+created\s+D\d+/) {
# If new revision and bug id was omitted, make revision public
@@ -378,10 +383,11 @@ sub process_revision_change {
my $log_message = sprintf(
- "REVISION CHANGE FOUND: D%d: %s | bug: %d | %s",
+ "REVISION CHANGE FOUND: D%d: %s | bug: %d | %s | %s",
$revision->id,
$revision->title,
$revision->bug_id,
+ $changer->name,
$story_text);
INFO($log_message);
@@ -533,6 +539,8 @@ sub process_revision_change {
$flag_type ||= first { $_->name eq 'review' && $_->is_active } @{ $attachment->flag_types };
+ die "Unable to find review flag!" unless $flag_type;
+
# Create new flags
foreach my $user_id (@accepted_user_ids) {
next if $accepted_done{$user_id};
@@ -600,7 +608,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 => $rev_attachment->attacher });
+ Bugzilla::BugMail::Send($bug_id, { changer => $changer->bugzilla_user });
}
INFO('SUCCESS: Revision D' . $revision->id . ' processed');
diff --git a/extensions/PhabBugz/lib/Types.pm b/extensions/PhabBugz/lib/Types.pm
index 44987bfee..493e97fbc 100644
--- a/extensions/PhabBugz/lib/Types.pm
+++ b/extensions/PhabBugz/lib/Types.pm
@@ -13,13 +13,16 @@ use warnings;
use Type::Library
-base,
- -declare => qw( Revision PhabUser Policy Project );
+ -declare => qw( Revision LinkedPhabUser PhabUser Policy Project );
use Type::Utils -all;
-use Types::Standard -types;
+use Types::Standard -all;
class_type Revision, { class => 'Bugzilla::Extension::PhabBugz::Revision' };
class_type Policy, { class => 'Bugzilla::Extension::PhabBugz::Policy' };
class_type Project, { class => 'Bugzilla::Extension::PhabBugz::Project' };
class_type PhabUser, { class => 'Bugzilla::Extension::PhabBugz::User' };
+declare LinkedPhabUser,
+ as PhabUser,
+ where { is_Int($_->bugzilla_id) };
1;
diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm
index 9c79c1855..a7ae98744 100644
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -85,6 +85,7 @@ sub create_revision_attachment {
$bug->add_comment($revision->summary, { type => CMT_ATTACHMENT_CREATED,
extra_data => $attachment->id });
+ delete $bug->{attachments};
return $attachment;
}
diff --git a/extensions/PhabBugz/t/review-flags.t b/extensions/PhabBugz/t/review-flags.t
new file mode 100644
index 000000000..610c46dca
--- /dev/null
+++ b/extensions/PhabBugz/t/review-flags.t
@@ -0,0 +1,209 @@
+#!/usr/bin/perl
+# 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.
+use strict;
+use warnings;
+use 5.10.1;
+use lib qw( . lib local/lib/perl5 );
+BEGIN { $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf' }
+use Test2::V0;
+
+our @EMAILS;
+
+BEGIN {
+ require Bugzilla::Mailer;
+ no warnings 'redefine';
+ *Bugzilla::Mailer::MessageToMTA = sub {
+ push @EMAILS, [@_];
+ };
+}
+use Bugzilla::Test::MockDB;
+use Bugzilla::Test::MockParams;
+use Bugzilla::Test::Util qw(create_user);
+use Test2::Tools::Mock;
+use Try::Tiny;
+use JSON::MaybeXS;
+use Bugzilla::Constants;
+use URI;
+use File::Basename;
+use Digest::SHA qw(sha1_hex);
+use Data::Dumper;
+
+use ok 'Bugzilla::Extension::PhabBugz::Feed';
+use ok 'Bugzilla::Extension::PhabBugz::Constants', 'PHAB_AUTOMATION_USER';
+use ok 'Bugzilla::Config', 'SetParam';
+can_ok('Bugzilla::Extension::PhabBugz::Feed', qw( group_query feed_query user_query ));
+
+SetParam(phabricator_base_uri => 'http://fake.phabricator.tld/');
+SetParam(mailfrom => 'bugzilla-daemon');
+Bugzilla->error_mode(ERROR_MODE_TEST);
+my $nobody = create_user('nobody@mozilla.org', '*');
+my $phab_bot = create_user(PHAB_AUTOMATION_USER, '*');
+
+# Steve Rogers is the revision author
+my $steve = create_user('steverogers@avengers.org', '*', realname => 'Steve Rogers :steve');
+
+# Bucky Barns is the reviewer
+my $bucky = create_user('bucky@avengers.org', '*', realname => 'Bucky Barns :bucky');
+
+my $firefox = Bugzilla::Product->create(
+ {
+ name => 'Firefox',
+ description => 'Fake firefox product',
+ version => 'Unspecified',
+ },
+);
+
+my $general = Bugzilla::Component->create(
+ {
+ product =>$firefox,
+ name => 'General',
+ description => 'The most general description',
+ initialowner => { id => $nobody->id },
+ }
+);
+
+Bugzilla->set_user($steve);
+my $bug = Bugzilla::Bug->create(
+ {
+ short_desc => 'test bug',
+ product => $firefox,
+ component => $general->name,
+ bug_severity => 'normal',
+ op_sys => 'Unspecified',
+ rep_platform => 'Unspecified',
+ version => 'Unspecified',
+ comment => 'first post',
+ priority => 'P1',
+ }
+);
+
+my $recipients = { changer => $steve };
+Bugzilla::BugMail::Send($bug->bug_id, $recipients);
+@EMAILS = ();
+
+my $revision = Bugzilla::Extension::PhabBugz::Revision->new(
+ {
+ id => 1,
+ phid => 'PHID-DREV-uozm3ggfp7e7uoqegmc3',
+ type => 'DREV',
+ fields => {
+ title => "title",
+ summary => "the summary of the revision",
+ status => { value => "not sure" },
+ dateCreated => time() - (60 * 60),
+ dateModified => time() - (60 * 5),
+ authorPHID => 'authorPHID',
+ policy => {
+ view => 'policy.view',
+ edit => 'policy.edit',
+ },
+ 'bugzilla.bug-id' => $bug->id,
+ },
+ attachments => {
+ projects => { projectPHIDs => [] },
+ reviewers => {
+ reviewers => [ ],
+ },
+ subscribers => {
+ subscriberPHIDs => [],
+ subscriberCount => 1,
+ viewerIsSubscribed => 1,
+ }
+ },
+ reviews => [
+ {
+ user => new_phab_user($bucky),
+ status => 'accepted',
+ }
+ ]
+ }
+);
+my $PhabRevisionMock = mock 'Bugzilla::Extension::PhabBugz::Revision' => (
+ override => [
+ make_public => sub { },
+ update => sub { },
+ ]
+);
+my $PhabUserMock = mock 'Bugzilla::Extension::PhabBugz::User' => (
+ override => [
+ match => sub {
+ my ($class, $query) = @_;
+ if ($query && $query->{phids} && $query->{phids}[0]) {
+ my $phid = $query->{phids}[0];
+ if ($phid eq 'authorPHID') {
+ return [ new_phab_user($steve, $phid) ];
+ }
+ }
+ },
+ ]
+);
+
+
+my $feed = Bugzilla::Extension::PhabBugz::Feed->new;
+my $changer = new_phab_user($bucky);
+@EMAILS = ();
+$feed->process_revision_change(
+ $revision, $changer, "story text"
+);
+
+# The first comment, and the comment made when the attachment is attached
+# are made by Steve.
+# The review comment is made by Bucky.
+
+my $sth = Bugzilla->dbh->prepare("select profiles.login_name, thetext from longdescs join profiles on who = userid");
+$sth->execute;
+while (my $row = $sth->fetchrow_hashref) {
+ if ($row->{thetext} =~ /first post/i) {
+ is($row->{login_name}, $steve->login, 'first post author');
+ }
+ elsif ($row->{thetext} =~ /the summary of the revision/i) {
+ is($row->{login_name}, $steve->login, 'the first attachment comment');
+ }
+ elsif ($row->{thetext} =~ /has approved the revision/i) {
+ is($row->{login_name}, $bucky->login);
+ }
+}
+
+diag Dumper(\@EMAILS);
+
+done_testing;
+
+sub new_phab_user {
+ my ($bug_user, $phid) = @_;
+
+ return Bugzilla::Extension::PhabBugz::User->new(
+ {
+ id => $bug_user->id * 1000,
+ type => "USER",
+ phid => $phid // "PHID-USER-" . ( $bug_user->id * 1000 ),
+ fields => {
+ username => $bug_user->nick,
+ realName => $bug_user->name,
+ dateCreated => time() - 60 * 60 * 24,
+ dateModified => time(),
+ roles => [],
+ policy => {
+ view => 'view',
+ edit => 'edit',
+ },
+ },
+ attachments => {
+ 'external-accounts' => {
+ 'external-accounts' => [
+ {
+ type => 'bmo',
+ id => $bug_user->id,
+ }
+ ]
+ }
+ }
+ }
+ );
+
+
+} \ No newline at end of file