summaryrefslogtreecommitdiffstats
path: root/services
diff options
context:
space:
mode:
authorKemal Zebari <60799661+kemzeb@users.noreply.github.com>2024-03-28 08:19:24 -0700
committerGitHub <noreply@github.com>2024-03-28 15:19:24 +0000
commit242b331260925e604150346e61329097d5731e77 (patch)
tree7d719a0d19b9b3a0569440ee5f0da89c9c5a7d56 /services
parente40fc75bac65933f2ed3de8fbc5fb336195b59f5 (diff)
downloadgitea-242b331260925e604150346e61329097d5731e77.tar.gz
gitea-242b331260925e604150346e61329097d5731e77.zip
Prevent re-review and dismiss review actions on closed and merged PRs (#30065)
Resolves #29965. --- Manually tested this by: - Following the [installation](https://docs.gitea.com/next/installation/install-with-docker#basics) guide (but built a local Docker image instead) - Creating 2 users, one who is the `Owner` of a newly-created repository and the other a `Collaborator` - Had the `Collaborator` create a PR that the `Owner` reviews - `Collaborator` resolves conversation and `Owner` merges PR And with this change we see that we can no longer see re-request review button for the `Owner`: <img width="1351" alt="Screenshot 2024-03-25 at 12 39 18 AM" src="https://github.com/go-gitea/gitea/assets/60799661/bcd9c579-3cf7-474f-a51e-b436fe1a39a4">
Diffstat (limited to 'services')
-rw-r--r--services/pull/review.go33
-rw-r--r--services/pull/review_test.go48
2 files changed, 81 insertions, 0 deletions
diff --git a/services/pull/review.go b/services/pull/review.go
index de1021c5c0..5bf1991d13 100644
--- a/services/pull/review.go
+++ b/services/pull/review.go
@@ -20,11 +20,29 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting"
+ "code.gitea.io/gitea/modules/util"
notify_service "code.gitea.io/gitea/services/notify"
)
var notEnoughLines = regexp.MustCompile(`fatal: file .* has only \d+ lines?`)
+// ErrDismissRequestOnClosedPR represents an error when an user tries to dismiss a review associated to a closed or merged PR.
+type ErrDismissRequestOnClosedPR struct{}
+
+// IsErrDismissRequestOnClosedPR checks if an error is an ErrDismissRequestOnClosedPR.
+func IsErrDismissRequestOnClosedPR(err error) bool {
+ _, ok := err.(ErrDismissRequestOnClosedPR)
+ return ok
+}
+
+func (err ErrDismissRequestOnClosedPR) Error() string {
+ return "can't dismiss a review associated to a closed or merged PR"
+}
+
+func (err ErrDismissRequestOnClosedPR) Unwrap() error {
+ return util.ErrPermissionDenied
+}
+
// checkInvalidation checks if the line of code comment got changed by another commit.
// If the line got changed the comment is going to be invalidated.
func checkInvalidation(ctx context.Context, c *issues_model.Comment, doer *user_model.User, repo *git.Repository, branch string) error {
@@ -382,6 +400,21 @@ func DismissReview(ctx context.Context, reviewID, repoID int64, message string,
return nil, fmt.Errorf("reviews's repository is not the same as the one we expect")
}
+ issue := review.Issue
+
+ if issue.IsClosed {
+ return nil, ErrDismissRequestOnClosedPR{}
+ }
+
+ if issue.IsPull {
+ if err := issue.LoadPullRequest(ctx); err != nil {
+ return nil, err
+ }
+ if issue.PullRequest.HasMerged {
+ return nil, ErrDismissRequestOnClosedPR{}
+ }
+ }
+
if err := issues_model.DismissReview(ctx, review, isDismiss); err != nil {
return nil, err
}
diff --git a/services/pull/review_test.go b/services/pull/review_test.go
new file mode 100644
index 0000000000..3bce1e523d
--- /dev/null
+++ b/services/pull/review_test.go
@@ -0,0 +1,48 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package pull_test
+
+import (
+ "testing"
+
+ "code.gitea.io/gitea/models/db"
+ issues_model "code.gitea.io/gitea/models/issues"
+ "code.gitea.io/gitea/models/unittest"
+ user_model "code.gitea.io/gitea/models/user"
+ pull_service "code.gitea.io/gitea/services/pull"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestDismissReview(t *testing.T) {
+ assert.NoError(t, unittest.PrepareTestDatabase())
+
+ pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{})
+ assert.NoError(t, pull.LoadIssue(db.DefaultContext))
+ issue := pull.Issue
+ assert.NoError(t, issue.LoadRepo(db.DefaultContext))
+ reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+ review, err := issues_model.CreateReview(db.DefaultContext, issues_model.CreateReviewOptions{
+ Issue: issue,
+ Reviewer: reviewer,
+ Type: issues_model.ReviewTypeReject,
+ })
+
+ assert.NoError(t, err)
+ issue.IsClosed = true
+ pull.HasMerged = false
+ assert.NoError(t, issues_model.UpdateIssueCols(db.DefaultContext, issue, "is_closed"))
+ assert.NoError(t, pull.UpdateCols(db.DefaultContext, "has_merged"))
+ _, err = pull_service.DismissReview(db.DefaultContext, review.ID, issue.RepoID, "", &user_model.User{}, false, false)
+ assert.Error(t, err)
+ assert.True(t, pull_service.IsErrDismissRequestOnClosedPR(err))
+
+ pull.HasMerged = true
+ pull.Issue.IsClosed = false
+ assert.NoError(t, issues_model.UpdateIssueCols(db.DefaultContext, issue, "is_closed"))
+ assert.NoError(t, pull.UpdateCols(db.DefaultContext, "has_merged"))
+ _, err = pull_service.DismissReview(db.DefaultContext, review.ID, issue.RepoID, "", &user_model.User{}, false, false)
+ assert.Error(t, err)
+ assert.True(t, pull_service.IsErrDismissRequestOnClosedPR(err))
+}