Skip to content

Commit b6dd87d

Browse files
committed
Bug 1965553 - Uplift approval set in bugzilla without relman approval
1 parent 5fd0ad8 commit b6dd87d

File tree

3 files changed

+339
-10
lines changed

3 files changed

+339
-10
lines changed

extensions/PhabBugz/lib/Feed.pm

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -651,9 +651,7 @@ sub process_revision_change {
651651
# set the approval flags. This ensures that users who create revisions will
652652
# set the flag to `?`, and only approvals from `mozilla-next-drivers` group
653653
# members will set the flag to `+` or `-`.
654-
my $flag_setter = $changer->bugzilla_user;
655-
656-
set_attachment_approval_flags($attachment, $revision, $flag_setter, $changer);
654+
set_attachment_approval_flags($attachment, $revision, $changer);
657655
}
658656

659657
$attachment->update($timestamp);

extensions/PhabBugz/lib/Util.pm

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,26 @@ use constant LEGACY_APPROVAL_MAPPING => {
5353

5454
# Set approval flags on Phabricator revision bug attachments.
5555
sub set_attachment_approval_flags {
56-
my ($attachment, $revision, $flag_setter, $phab_user) = @_;
56+
my ($attachment, $revision, $phab_user) = @_;
57+
my $bmo_user = $phab_user->bugzilla_user;
5758

5859
my $revision_status_flag_map = {
5960
'abandoned' => '-',
6061
'accepted' => '+',
62+
'accepted-prior' => '+',
6163
'changes-planned' => 'X',
6264
'draft' => '?',
6365
'needs-review' => '?',
6466
'needs-revision' => '-',
6567
};
6668

67-
my $status = $revision_status_flag_map->{$revision->status};
69+
# Find the current review status of the revision changer
70+
my $status = undef;
71+
foreach my $reviewer (@{$revision->reviews}) {
72+
if ($reviewer->{user}->id == $phab_user->id) {
73+
$status = $reviewer->{status};
74+
}
75+
}
6876

6977
if (!$status) {
7078
INFO( "Approval flag status not found for revision status '"
@@ -74,7 +82,7 @@ sub set_attachment_approval_flags {
7482
}
7583

7684
# The repo short name is the appropriate value that aligns with flag names.
77-
my $repo_name = $revision->repository->short_name;
85+
my $repo_name = $revision->repository->short_name;
7886

7987
# With the move to git some repository short names in Phabricator changed but
8088
# we want to use the old approval flags so we map the new names to the old if
@@ -110,14 +118,14 @@ sub set_attachment_approval_flags {
110118
# Set the flag to it's new status. If it already has that status,
111119
# it will be a non-change. We also need to check to make sure the
112120
# flag change is allowed.
113-
if (!$flag_setter->can_change_flag($flag->type, $flag->status, $status)) {
121+
if (!$bmo_user->can_change_flag($flag->type, $flag->status, $status)) {
114122
INFO(
115123
"Unable to set existing `$approval_flag_name` flag to `$status` due to permissions."
116124
);
117125
return;
118126
}
119127

120-
# If setting to + or - then user needs to be a release manager in Phab.
128+
# If setting to + or - then user needs to be a release manager.
121129
if (($status eq '+' || $status eq '-') && !$phab_user->is_release_manager) {
122130
INFO(
123131
"Unable to set existing `$approval_flag_name` flag to `$status` due to not being a release manager."
@@ -135,10 +143,18 @@ sub set_attachment_approval_flags {
135143
if (!@old_flags && $status ne 'X') {
136144
my $approval_flag = Bugzilla::FlagType->new({name => $approval_flag_name});
137145
if ($approval_flag) {
138-
if ($flag_setter->can_change_flag($approval_flag, 'X', $status)) {
146+
# If setting to + then at least one accepted reviewer needs to be a release manager.
147+
if ($status eq '+' && !$phab_user->is_release_manager) {
148+
INFO(
149+
"Unable to create new `$approval_flag_name` flag with status `$status` due to not being accepted by a release manager."
150+
);
151+
return;
152+
}
153+
154+
if ($bmo_user->can_change_flag($approval_flag, 'X', $status)) {
139155
INFO("Creating new `$approval_flag_name` flag with status `$status`");
140156
push @new_flags,
141-
{setter => $flag_setter, status => $status, type_id => $approval_flag->id,};
157+
{setter => $bmo_user, status => $status, type_id => $approval_flag->id,};
142158
}
143159
else {
144160
INFO(

0 commit comments

Comments
 (0)