Skip to content

Commit

Permalink
Bug 1904753: return immediately when approval flag should not be changed
Browse files Browse the repository at this point in the history
In `set_attachment_approval_flags` we check for several conditions
that indicate the flag cannot be updated by the user, but we do not
immediately `return` when these conditions are hit. This causes the
code to attempt to create the flag once it exits the loop, which
is not allowed since approval flags are not multiplicable. Add `return`
statements to branches which indicate the flag should not be updated.

Since we are now returning early from these branches, we can reverse the
logic of the main `if` branch to clean up the loop by de-denting the code.
  • Loading branch information
cgsheeh committed Jun 26, 2024
1 parent e37bbe8 commit aaa0b3a
Showing 1 changed file with 12 additions and 14 deletions.
26 changes: 12 additions & 14 deletions extensions/PhabBugz/lib/Util.pm
Original file line number Diff line number Diff line change
Expand Up @@ -89,25 +89,23 @@ 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 setting to + or - then user needs to be a release manager in Phab
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."
);
}
else {
INFO("Set existing `$approval_flag_name` flag to `$status`.");
push @old_flags, {id => $flag->id, status => $status};
}
}
else {
if (!$flag_setter->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 (($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."
);
return;
}

INFO("Set existing `$approval_flag_name` flag to `$status`.");
push @old_flags, {id => $flag->id, status => $status};
last;
}

Expand Down

0 comments on commit aaa0b3a

Please sign in to comment.