From e1ba93b5ca306e2ca6ec16ea3437517ba53e602d Mon Sep 17 00:00:00 2001 From: Connor Sheehan Date: Mon, 11 Dec 2023 11:26:48 -0500 Subject: [PATCH 1/2] Bug 1825961: set approval flags as the revision changer instead of author Add a new positional argument to `set_attachment_approval_flags` that represents the Bugzilla user making a change to the approval flag status. Use the Bugzilla user of the revision changer instead of deriving the flag setter from the author of the revision. --- extensions/PhabBugz/lib/Feed.pm | 9 ++++++++- extensions/PhabBugz/lib/Util.pm | 10 +++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm index 0eca1b230b..4350118c5a 100644 --- a/extensions/PhabBugz/lib/Feed.pm +++ b/extensions/PhabBugz/lib/Feed.pm @@ -631,7 +631,14 @@ sub process_revision_change { INFO('If uplift repository we may need to set approval flags'); if ($revision->repository && $revision->repository->is_uplift_repo()) { INFO('Uplift repository detected. Setting attachment approval flags'); - set_attachment_approval_flags($attachment, $revision); + + # The Bugzilla user for the user who initiated the change should be used to + # 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); } $attachment->update($timestamp); diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm index 4ebc557bf4..cf983d0bc7 100644 --- a/extensions/PhabBugz/lib/Util.pm +++ b/extensions/PhabBugz/lib/Util.pm @@ -45,8 +45,8 @@ our @EXPORT = qw( # Set approval flags on Phabricator revision bug attachments. sub set_attachment_approval_flags { - my ($attachment, $revision) = @_; - my $submitter = $revision->author->bugzilla_user; + my ($attachment, $revision, $flag_setter) = @_; + my $flag_setter = $revision->author->bugzilla_user; my $revision_status_flag_map = { 'abandoned' => '-', @@ -90,7 +90,7 @@ 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 ($submitter->can_change_flag($flag->type, $flag->status, $status)) { + if ($flag_setter->can_change_flag($flag->type, $flag->status, $status)) { INFO("Set existing `$approval_flag_name` flag to `$status`."); push @old_flags, {id => $flag->id, status => $status}; } @@ -108,10 +108,10 @@ 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 ($submitter->can_change_flag($approval_flag, 'X', $status)) { + if ($flag_setter->can_change_flag($approval_flag, 'X', $status)) { INFO("Creating new `$approval_flag_name` flag with status `$status`"); push @new_flags, - {setter => $submitter, status => $status, type_id => $approval_flag->id,}; + {setter => $flag_setter, status => $status, type_id => $approval_flag->id,}; } else { INFO( From 3aeb768505e431bdcb4bcf2bf0da87c1d889f635 Mon Sep 17 00:00:00 2001 From: Connor Sheehan Date: Mon, 11 Dec 2023 12:16:29 -0500 Subject: [PATCH 2/2] remove setting of flag_setter --- extensions/PhabBugz/lib/Util.pm | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm index cf983d0bc7..b9ad00ddfb 100644 --- a/extensions/PhabBugz/lib/Util.pm +++ b/extensions/PhabBugz/lib/Util.pm @@ -46,7 +46,6 @@ our @EXPORT = qw( # Set approval flags on Phabricator revision bug attachments. sub set_attachment_approval_flags { my ($attachment, $revision, $flag_setter) = @_; - my $flag_setter = $revision->author->bugzilla_user; my $revision_status_flag_map = { 'abandoned' => '-',