Bug 1965553 - Uplift approval set in bugzilla without relman approval

Author: dklawren

extensions/PhabBugz/lib/Feed.pm

@@ -651,9 +651,7 @@ sub process_revision_change {
     # set the approval flags. This ensures that users who create revisions will
     # set the flag to `?`, and only approvals from `mozilla-next-drivers` group
     # members will set the flag to `+` or `-`.
-    my $flag_setter = $changer->bugzilla_user;
-
-    set_attachment_approval_flags($attachment, $revision, $flag_setter, $changer);
+    set_attachment_approval_flags($attachment, $revision, $changer, $is_new);
   }
 
   $attachment->update($timestamp);

extensions/PhabBugz/lib/Util.pm

@@ -53,28 +53,46 @@ use constant LEGACY_APPROVAL_MAPPING => {
 
 # Set approval flags on Phabricator revision bug attachments.
 sub set_attachment_approval_flags {
-  my ($attachment, $revision, $flag_setter, $phab_user) = @_;
+  my ($attachment, $revision, $phab_user, $is_new) = @_;
+  my $bmo_user = $phab_user->bugzilla_user;
 
   my $revision_status_flag_map = {
     'abandoned'       => '-',
     'accepted'        => '+',
+    'accepted-prior'  => '+',
     'changes-planned' => 'X',
     'draft'           => '?',
     'needs-review'    => '?',
     'needs-revision'  => '-',
   };
 
-  my $status = $revision_status_flag_map->{$revision->status};
+  # Find the current review status of the revision changer
+  my $status          = undef;
+  my $reviewer_status = undef;
+
+  if ($is_new) {
+    $reviewer_status = $revision->status;
+    $status          = $revision_status_flag_map->{$reviewer_status};
+  }
+  else {
+    foreach my $reviewer (@{$revision->reviews}) {
+      if ($reviewer->{user}->id == $phab_user->id) {
+        $reviewer_status = $reviewer->{status};
+        $status          = $revision_status_flag_map->{$reviewer_status};
+        last;
+      }
+    }
+  }
 
   if (!$status) {
     INFO( "Approval flag status not found for revision status '"
-        . $revision->status
+        . $reviewer_status
         . "'");
     return;
   }
 
   # The repo short name is the appropriate value that aligns with flag names.
-  my $repo_name          = $revision->repository->short_name;
+  my $repo_name = $revision->repository->short_name;
 
   # With the move to git some repository short names in Phabricator changed but
   # we want to use the old approval flags so we map the new names to the old if
@@ -89,7 +107,7 @@ sub set_attachment_approval_flags {
   INFO( 'Setting revision D'
       . $revision->id
       . ' with '
-      . $revision->status
+      . $reviewer_status
       . ' status to '
       . $approval_flag_name
       . $status);
@@ -103,14 +121,14 @@ sub set_attachment_approval_flags {
     # Set the flag to it's new status. If it already has that status,
     # it will be a non-change. We also need to check to make sure the
     # flag change is allowed.
-    if (!$flag_setter->can_change_flag($flag->type, $flag->status, $status)) {
+    if (!$bmo_user->can_change_flag($flag->type, $flag->status, $status)) {
       INFO(
         "Unable to set existing `$approval_flag_name` flag to `$status` due to permissions."
       );
       return;
     }
 
-    # If setting to + or - then user needs to be a release manager in Phab.
+    # If setting to + then the Phabricator user needs to be a release manager.
     if (($status eq '+' || $status eq '-') && !$phab_user->is_release_manager) {
       INFO(
         "Unable to set existing `$approval_flag_name` flag to `$status` due to not being a release manager."
@@ -128,10 +146,19 @@ sub set_attachment_approval_flags {
   if (!@old_flags && $status ne 'X') {
     my $approval_flag = Bugzilla::FlagType->new({name => $approval_flag_name});
     if ($approval_flag) {
-      if ($flag_setter->can_change_flag($approval_flag, 'X', $status)) {
+
+  # If setting to + then at least one accepted reviewer needs to be a release manager.
+      if ($status eq '+' && !$phab_user->is_release_manager) {
+        INFO(
+          "Unable to create new `$approval_flag_name` flag with status `$status` due to not being accepted by a release manager."
+        );
+        return;
+      }
+
+      if ($bmo_user->can_change_flag($approval_flag, 'X', $status)) {
         INFO("Creating new `$approval_flag_name` flag with status `$status`");
         push @new_flags,
-          {setter => $flag_setter, status => $status, type_id => $approval_flag->id,};
+          {setter => $bmo_user, status => $status, type_id => $approval_flag->id,};
       }
       else {
         INFO(