Delete review requests as non-admin

[jpraet]

• Gitea version (or commit ref): 1.14
• Can you reproduce the bug at https://try.gitea.io:
  • Yes

Description

When I request a review, I am not able to delete this review request if I am no administrator. I would expect at least to be able to delete a review request I created myself in case I selected the wrong person by accident. But I think even someone else should be able to delete the review request as well. Everyone that can add reviewers should also be able to delete them in my opinion?

Screenshots

I add a review request:

But I cannot delete it:

Unless when I am admin:

[a1012112796]

That's follow github style.
see:
#10756 (comment)

[PaulBol]

That's follow github style. see: #10756 (comment)

My understanding may be wrong. But to me it seems like the comment refers to review requests that have already been processed, i.e. approved or rejected by the reviewer. This issue mentions a pending review request.

I agree with jpraet that the person who requested the review should be able to delete the review request as long as it's not more than a request. What would be the reason do deny this?

[jpraet]

I want to create a PR to allow the person who requested the review to be able to delete the review request.
But it looks like I cannot easily determine the author of the review request. It's not stored in the review table.

Would a PR get accepted that allows review request deletion to anyone that has permission to add review requests?

[jbartlau]

Any news on this? Just landed here and would support such a change. @jpraet did you ever hear back from anyone?

[lunny]

Maybe we can accept the person himself could remove the review request?
Or everyone who have write permission to pull request could do that since the removal will be recorded in the comment timeline.

[jbartlau]

everyone who have write permission to pull request could do that

That would totally suffice and was indeed what I had expected. And you could even restrict this to reviews that have not yet started. Our use case would be to assign a different reviewer if the originally requested reviewer is not available (e.g. vacation).

[lunny]

Looks like the code is there, but I don't know whether there are some bugs.

https://github.com/go-gitea/gitea/blob/main/routers/web/repo/issue.go#L634-L644

[KevinBaileyCrum]

this would be great, i have opened a pr in gitea, accidentally assigned a reviewer, then i cannot remove the reviewer,

i have had pr's get stuck because i added a reviewer who may be away from their computer for a long time and this completly blocks the merge (merge when all checks pass but an accidentally requested reviewer will always block)

[aleb]

Looks like the code is there, but I don't know whether there are some bugs.

https://github.com/go-gitea/gitea/blob/main/routers/web/repo/issue.go#L634-L644

gitea/routers/web/repo/issue.go

Lines 634 to 644 in e962ade

	if ctx.Repo.IsAdmin() {
	// Admin can dismiss or re-request any review requests
	tmp.CanChange = true
	} else if ctx.Doer != nil && ctx.Doer.ID == review.ReviewerID && review.Type == issues_model.ReviewTypeRequest {
	// A user can refuse review requests
	tmp.CanChange = true
	} else if (canChooseReviewer || (ctx.Doer != nil && ctx.Doer.ID == issue.PosterID)) && review.Type != issues_model.ReviewTypeRequest &&
	ctx.Doer.ID != review.ReviewerID {
	// The poster of the PR, a manager, or official reviewers can re-request review from other reviewers
	tmp.CanChange = true
	}

Which part of that code should allow me to remove a reviewer as long as I'm able to add it back?

[hawicz]

The actual validation check seems to be in IsValidReviewRequest() at https://github.com/go-gitea/gitea/blob/main/services/issue/assignee.go#L157 (called from UpdatePullReviewRequest() at https://github.com/go-gitea/gitea/blob/main/routers/web/repo/issue.go#L2475), and only allows you to remove yourself, unless you're an admin.
This seems unnecessarily restrictive to me too.