Creator of pull request can chose to start review when commenting but never submit them

[themightychris]

• Gitea version (or commit ref): b9d1fb6
• Git version: n/a
• Operating system: n/a
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes: https://try.gitea.io/themightychris/test/pulls/1/files#issuecomment-12063
  • No
  • Not relevant
• Log gist:

Description

If you create a pull request and then go to comment within that pull request, you're offered the option to just add a comment or start a review. If you start a review, every comment you post after that becomes part of the review and is held in a "Pending" status. But then you can't submit the review because you created the pull request and your comments can never be posted.

At the very least, if submitting a review is not an option then "start review" should not be an option when commenting. Ideally though, any admin or whitelisted review should be able to submit a review even if they created the pull request. I know this behavior was copied from GitHub per #4728 but its problematic: being the creator of a pull request does not mean you're the author of all/any of the commits within it. At the least it should be a branch option whether whitelisted reviewers can review their own pull requests. My use case is that I open develop->master pull requests for each release as the maintainer of the project and want to then review the release. It seems entirely common that maintainers on small teams might open the pull requests for others' pushed branches to create a review context, it seems silly that it should need to be a rule that maintainers wait for people to open their own PR in order to be able to start reviewing a branch

Screenshots

[jamesfancy]

So, how to finish the review?

[lunny]

One should not review himself PR.

[themightychris]

@lunny Gitea's job isn't to force some notion of best practice for teams by having a broken UI

Also I explained in the original post two use cases where it does make sense to be reviewing a PR you created

[lunny]

Maybe this could be a config item to allow user approval/reject himself PR on protected branch and default false.

[themightychris]

Maybe, but this issue is about the UI getting into an unresolvable state by it's own invitation

If there resolution though is going to be Gitea trying to enforce any opinions about team workflow through its UI though I would say it needs to be an option

[lunny]

OKay. I think pull request creator should not be allowed to click start review button. This should be a bug.

[gary-kim]

I would argue there are definite use cases for having the author be able to review their own pull requests. Most of them being covered in the first post of the issue. The person who opened the pull request is not always the person who made the most changes. Pull requests are sometimes opened as a "should we merge this?" or even "let's get this to a workable" rather than "let's merge this" so being able to review a pull request even if you authored it so, for example, if someone else has made a change after you opened the pull request, you can review their changes.

In any case, if the author cannot review their own pull request, following that line of thinking, shouldn't anyone who authored or committed to the branch should not be able to review the pull request? But, depending on the workflow of the organization, it is possible everyone has had some part in contributing to a branch that is going to be merged in meaning noone can review the pull request.

Essentially, the pull request author is not necessarily in a better or worse position then anyone else to be reviewing the pull request.

Maybe opening the pull request is an approval in an of itself but it also often isn't.

For now, there should be a fix for the pull request button but maybe we should create another issue for allowing pull request authors to review the pull request.

[lunny]

@gary-kim see my comment #7289 (comment)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

[themightychris]

There is still a bug here that is pretty serious and has a pretty simple solution: prevent someone from starting a review if they're not going to be allowed to submit it. Without this fix, PRs can be taken into a stuck state via the UI that they can't be recovered from

[lafriks]

You should be able to start review and in review dialog "Comment" button should be allowed

[zeripath]

@lafriks that's the current situation.

[lafriks]

Yes and I don't see a problem with that

[guillep2k]

OK, I couldn't reproduce the problem

I've just tested on my dev server (1.10.0+dev-411-g0e90b506c, 633cd7f).

The steps I did were:

• Log in, create a PR from the UI.
• Go to the code review tab.
• Select a line of code.
• Enter a message, click "Start Review".
• The comment is created, but marked as "Pending".
• Click the "Review" button, the "submit review" dialog shows up.
• The "Comment" button is kinda dim, but I press it.
• My review is added no problem.

Here's a snapshot of the "submit review" dialog:

Testing with my production 1.9.4 gave me the exact same results. Adding more than one comment before submitting worked fine too.

So, perhaps it's just a styling issue.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

[stale]

This issue has been automatically closed because of inactivity. You can re-open it if needed.