From 77468653f4f3e3285bc68e455b5b4e4265362aeb Mon Sep 17 00:00:00 2001 From: Dylan William Hardison Date: Mon, 20 Aug 2018 17:44:11 -0400 Subject: Bug 1482145 - PhabBot changes are showing up as from the wrong user --- extensions/PhabBugz/lib/Feed.pm | 66 ++++++++++++++++++++++------------------ extensions/PhabBugz/lib/Types.pm | 7 +++-- extensions/PhabBugz/lib/Util.pm | 1 + 3 files changed, 43 insertions(+), 31 deletions(-) (limited to 'extensions/PhabBugz/lib') 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; } -- cgit v1.2.3-24-g4f1b