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

Prevent ApprovalController counting mismatch #356

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Feb 18, 2021

The ApprovalController was not great at counting. If you tried to delete an approval that didn't exist, or if you tried to add an approval that already existed, it would miscount the current number of pending approvals.

Validation was in place already to prevent both scenarios, so in practice this shouldn't be possible without calling private functions. But still, preventing it entirely was trivial, so here it is.

The `ApprovalController` was not great at counting. If you tried to
delete an approval that didn't exist, or if you tried to add an
approval that already existed, it would miscount the current number
of pending approvals.

Validation was in place already to prevent both scenarios, so in
practice this shouldn't be possible without calling private functions.
But still, preventing it entirely was trivial, so here it is.
@Gudahtt Gudahtt requested a review from a team as a code owner February 18, 2021 16:06
Copy link
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

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

Good catch!

@Gudahtt Gudahtt merged commit af700d8 into develop Feb 18, 2021
@Gudahtt Gudahtt deleted the fix-approval-store-counting-bug branch February 18, 2021 19:06
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
The `ApprovalController` was not great at counting. If you tried to
delete an approval that didn't exist, or if you tried to add an
approval that already existed, it would miscount the current number
of pending approvals.

Validation was in place already to prevent both scenarios, so in
practice this shouldn't be possible without calling private functions.
But still, preventing it entirely was trivial, so here it is.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
The `ApprovalController` was not great at counting. If you tried to
delete an approval that didn't exist, or if you tried to add an
approval that already existed, it would miscount the current number
of pending approvals.

Validation was in place already to prevent both scenarios, so in
practice this shouldn't be possible without calling private functions.
But still, preventing it entirely was trivial, so here it is.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants