Bug 1426518 - Revisions can optionally not have a bug id so we need to make it optional in the type constraints of Revision.pm

5 files changed, 43 insertions, 26 deletions

diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm
index 66b05e4ec..8ea343f96 100644
--- a/extensions/PhabBugz/lib/Feed.pm
+++ b/extensions/PhabBugz/lib/Feed.pm
@@ -74,7 +74,6 @@ sub feed_query {
 
     # Process each story
     foreach my $story_data (@$transactions) {
-        my $skip = 0;
         my $story_id    = $story_data->{id};
         my $story_phid  = $story_data->{phid};
         my $author_phid = $story_data->{authorPHID};
@@ -89,32 +88,43 @@ sub feed_query {
 
         # Only interested in changes to revisions for now.
         if ($object_phid !~ /^PHID-DREV/) {
-            $self->logger->debug("SKIP: Not a revision change");
-            $skip = 1;
+            $self->logger->debug("SKIPPING: Not a revision change");
+            $self->save_feed_last_id($story_id);
+            next;
         }
 
         # Skip changes done by phab-bot user
         my $phab_users = get_phab_bmo_ids({ phids => [$author_phid] });
-        if (!$skip && @$phab_users) {
+        if (@$phab_users) {
             my $user = Bugzilla::User->new({ id => $phab_users->[0]->{id}, cache => 1 });
-            $skip = 1 if $user->login eq PHAB_AUTOMATION_USER;
+            if ($user->login eq PHAB_AUTOMATION_USER) {
+                $self->logger->debug("SKIPPING: Change made by phabricator user");
+                $self->save_feed_last_id($story_id);
+                next;
+            }
         }
 
-        if (!$skip) {
-            my $revision = Bugzilla::Extension::PhabBugz::Revision->new({ phids => [$object_phid] });
-            $self->process_revision_change($revision, $story_text);
-        }
-        else {
-            $self->logger->info('SKIPPING');
+        my $revision = Bugzilla::Extension::PhabBugz::Revision->new({ phids => [$object_phid] });
+
+        if (!$revision->bug_id) {
+            $self->logger->debug("SKIPPING: No bug associated with revision");
+            $self->save_feed_last_id($story_id);
+            next;
         }
 
-        # Store the largest last key so we can start from there in the next session
-        $self->logger->debug("UPDATING FEED_LAST_ID: $story_id");
-        $dbh->do("REPLACE INTO phabbugz (name, value) VALUES ('feed_last_id', ?)",
-                 undef, $story_id);
+        $self->process_revision_change($revision, $story_text);
+        $self->save_feed_last_id($story_id);
     }
 }
 
+sub save_feed_last_id {
+    my ($self, $story_id) = @_;
+    # Store the largest last key so we can start from there in the next session
+    $self->logger->debug("UPDATING FEED_LAST_ID: $story_id");
+    Bugzilla->dbh->do("REPLACE INTO phabbugz (name, value) VALUES ('feed_last_id', ?)",
+                      undef, $story_id);
+}
+
 sub process_revision_change {
     my ($self, $revision, $story_text) = @_;
 
diff --git a/extensions/PhabBugz/lib/Revision.pm b/extensions/PhabBugz/lib/Revision.pm
index f3a56a656..e229043e2 100644
--- a/extensions/PhabBugz/lib/Revision.pm
+++ b/extensions/PhabBugz/lib/Revision.pm
@@ -20,6 +20,13 @@ use Bugzilla::Extension::PhabBugz::Util qw(
 );
 
 use Types::Standard -all;
+use Type::Utils;
+
+my $EmptyStr = declare "EmptyStr",
+    as Str,
+    where { length($_) == 0 },
+    inline_as { $_[0]->parent->inline_check($_) . " && length($_) == 0" },
+    message { "String is not empty" };
 
 my $SearchResult = Dict[
     id     => Int,
@@ -35,7 +42,7 @@ my $SearchResult = Dict[
         repositoryPHID    => Maybe[Str],
         status            => HashRef,
         summary           => Str,
-        "bugzilla.bug-id" => Int,
+        "bugzilla.bug-id" => Int | $EmptyStr,
     ],
     attachments => Dict[
         reviewers => Dict[
@@ -49,8 +56,8 @@ my $SearchResult = Dict[
             ],
         ],
         subscribers => Dict[
-            subscriberPHIDs => ArrayRef[Str],
-            subscriberCount => Int,
+            subscriberPHIDs    => ArrayRef[Str],
+            subscriberCount    => Int,
             viewerIsSubscribed => Bool,
         ],
         projects => Dict[ projectPHIDs => ArrayRef[Str] ],
@@ -87,7 +94,7 @@ sub _load {
 
     my $result = request('differential.revision.search', $data);
     if (exists $result->{result}{data} && @{ $result->{result}{data} }) {
-        return $result->{result}->{data}->[0];
+        $result = $result->{result}->{data}->[0];
     }
 
     return $result;
diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm
index 1f2b21d55..838283f97 100644
--- a/extensions/PhabBugz/lib/Util.pm
+++ b/extensions/PhabBugz/lib/Util.pm
@@ -408,7 +408,7 @@ sub is_attachment_phab_revision {
 sub get_attachment_revisions {
     my $bug = shift;
 
-    my @revisions;
+    my $revisions;
 
     my @attachments =
       grep { is_attachment_phab_revision($_) } @{ $bug->attachments() };
@@ -423,11 +423,11 @@ sub get_attachment_revisions {
         }
 
         if (@revision_ids) {
-            @revisions = get_revisions_by_ids( \@revision_ids );
+            $revisions = get_revisions_by_ids( \@revision_ids );
         }
     }
 
-    return @revisions;
+    return @$revisions;
 }
 
 sub request {
diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm
index 80a7df9b1..5b6310de6 100644
--- a/extensions/PhabBugz/lib/WebService.pm
+++ b/extensions/PhabBugz/lib/WebService.pm
@@ -75,8 +75,8 @@ sub 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 $revisions = get_revisions_by_ids([$revision_id]);
+    my $revision = $revisions->[0];
 
     my $revision_phid  = $revision->{phid};
     my $revision_title = $revision->{fields}{title} || 'Unknown Description';
@@ -96,7 +96,7 @@ sub revision {
         # 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);
+            add_security_sync_comments($revisions, $bug);
         }
 
         my $policy_phid = create_private_revision_policy($bug, \@set_groups);
diff --git a/extensions/Push/lib/Connector/Phabricator.pm b/extensions/Push/lib/Connector/Phabricator.pm
index 988403727..8c3b56112 100644
--- a/extensions/Push/lib/Connector/Phabricator.pm
+++ b/extensions/Push/lib/Connector/Phabricator.pm
@@ -22,7 +22,7 @@ use Bugzilla::Extension::PhabBugz::Constants;
 use Bugzilla::Extension::PhabBugz::Util qw(
   add_comment_to_revision create_private_revision_policy
   edit_revision_policy get_attachment_revisions get_bug_role_phids
-  get_revisions_by_ids intersect make_revision_public
+  intersect make_revision_public
   make_revision_private set_revision_subscribers
   get_security_sync_groups add_security_sync_comments);
 use Bugzilla::Extension::Push::Constants;