Skip to content

Review Requests: prevent reviews after request is resolved#30673

Merged
marcoandredinis merged 4 commits intomasterfrom
marco/review_request_handle_resolved
Aug 18, 2023
Merged

Review Requests: prevent reviews after request is resolved#30673
marcoandredinis merged 4 commits intomasterfrom
marco/review_request_handle_resolved

Conversation

@marcoandredinis
Copy link
Copy Markdown
Contributor

@marcoandredinis marcoandredinis commented Aug 18, 2023

Return an error when trying to approve or deny a request after it was resolved.

Demo
Scenario:

  • nr of required approvers: 1
  • UserA approves and immediately UserB tries to approve as well

Screen from UserB:
image

@marcoandredinis marcoandredinis marked this pull request as ready for review August 18, 2023 14:42
Comment thread lib/services/access_request_test.go Outdated
Comment thread lib/services/access_request_test.go Outdated
@rosstimothy rosstimothy self-requested a review August 18, 2023 14:50
@marcoandredinis marcoandredinis force-pushed the marco/review_request_handle_resolved branch from 48e4b37 to 73d6cde Compare August 18, 2023 15:04
Comment thread lib/services/access_request_test.go Outdated
Comment thread lib/services/access_request_test.go Outdated
@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Aug 18, 2023

Does this fix #5622?

@marcoandredinis
Copy link
Copy Markdown
Contributor Author

Does this fix #5622?

I was not aware of that issue
Just tested and it does not fix the issue
tctl request deny doesn't even add a record to reviewers list:
image

Comment thread lib/services/access_request.go Outdated
@marcoandredinis marcoandredinis force-pushed the marco/review_request_handle_resolved branch from 6ca5000 to 2fd1f96 Compare August 18, 2023 15:51
@marcoandredinis marcoandredinis added this pull request to the merge queue Aug 18, 2023
Merged via the queue into master with commit e748859 Aug 18, 2023
@marcoandredinis marcoandredinis deleted the marco/review_request_handle_resolved branch August 18, 2023 16:25
@public-teleport-github-review-bot
Copy link
Copy Markdown

@marcoandredinis See the table below for backport results.

Branch Result
branch/v11 Create PR
branch/v12 Create PR
branch/v13 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants