Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1904753: return immediately when approval flag should not be changed #2266

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

cgsheeh
Copy link
Collaborator

@cgsheeh cgsheeh commented Jun 26, 2024

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.

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.
@cgsheeh cgsheeh requested a review from dklawren June 26, 2024 14:00
Copy link
Collaborator

@dklawren dklawren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix.

@dklawren dklawren merged commit 944c50a into mozilla-bteam:master Jun 26, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants