From 6710ff41f0efd140818d0c9fd607b279d54b7428 Mon Sep 17 00:00:00 2001 From: Kemal Zebari Date: Wed, 24 Apr 2024 10:56:05 -0700 Subject: [PATCH 1/7] Prevent allow/reject reviews on merged/closed PRs --- routers/web/repo/pull_review.go | 2 ++ services/pull/review.go | 21 +++++++++++++++++++++ templates/repo/diff/new_review.tmpl | 29 +++++++++++++++++------------ 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index a65d4866d0a4..080df9382558 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -264,6 +264,8 @@ func SubmitReview(ctx *context.Context) { if issues_model.IsContentEmptyErr(err) { ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty")) ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) + } else if pull_service.IsSubmitReviewOnClosedPR(err) { + ctx.Error(http.StatusForbidden) } else { ctx.ServerError("SubmitReview", err) } diff --git a/services/pull/review.go b/services/pull/review.go index 5bf1991d1342..9d42976a361f 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -43,6 +43,23 @@ func (err ErrDismissRequestOnClosedPR) Unwrap() error { return util.ErrPermissionDenied } +// ErrSubmitReviewOnClosedPR represents an error when an user tries to submit an approve or reject review associated to a closed or merged PR. +type ErrSubmitReviewOnClosedPR struct{} + +// IsSubmitReviewOnClosedPR checks if an error is an ErrSubmitReviewOnClosedPR. +func IsSubmitReviewOnClosedPR(err error) bool { + _, ok := err.(ErrSubmitReviewOnClosedPR) + return ok +} + +func (err ErrSubmitReviewOnClosedPR) Error() string { + return "can't submit review for a closed or merged PR" +} + +func (err ErrSubmitReviewOnClosedPR) 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 { @@ -293,6 +310,10 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos if reviewType != issues_model.ReviewTypeApprove && reviewType != issues_model.ReviewTypeReject { stale = false } else { + if issue.IsClosed || pr.HasMerged { + return nil, nil, ErrSubmitReviewOnClosedPR{} + } + headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName()) if err != nil { return nil, nil, err diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index a2eae007a56c..2d77119d0f58 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -30,20 +30,25 @@ {{end}}
{{$showSelfTooltip := (and $.IsSigned ($.Issue.IsPoster $.SignedUser.ID))}} - {{if $showSelfTooltip}} - - - - {{else}} - + {{$isOpen := (not (or $.Issue.IsClosed (and $.Issue.IsPull $.Issue.PullRequest.HasMerged)))}} + {{if $isOpen}} + {{if $showSelfTooltip}} + + + + {{else}} + + {{end}} {{end}} - {{if $showSelfTooltip}} - - - - {{else}} - + {{if $isOpen}} + {{if $showSelfTooltip}} + + + + {{else}} + + {{end}} {{end}} From f484e3d44ef28dc01ec706d096cb11ad497873ad Mon Sep 17 00:00:00 2001 From: Kemal Zebari Date: Wed, 24 Apr 2024 11:36:19 -0700 Subject: [PATCH 2/7] Handle error object in API and use HTTP status 422 --- routers/api/v1/repo/pull_review.go | 12 ++++++++++-- routers/web/repo/pull_review.go | 4 ++-- services/pull/review.go | 4 ++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index b527e90f10b8..18683e4a4166 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -372,7 +372,11 @@ func CreatePullReview(ctx *context.APIContext) { // create review and associate all pending review comments review, _, err := pull_service.SubmitReview(ctx, ctx.Doer, ctx.Repo.GitRepo, pr.Issue, reviewType, opts.Body, opts.CommitID, nil) if err != nil { - ctx.Error(http.StatusInternalServerError, "SubmitReview", err) + if pull_service.IsErrSubmitReviewOnClosedPR(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + } else { + ctx.Error(http.StatusInternalServerError, "SubmitReview", err) + } return } @@ -460,7 +464,11 @@ func SubmitPullReview(ctx *context.APIContext) { // create review and associate all pending review comments review, _, err = pull_service.SubmitReview(ctx, ctx.Doer, ctx.Repo.GitRepo, pr.Issue, reviewType, opts.Body, headCommitID, nil) if err != nil { - ctx.Error(http.StatusInternalServerError, "SubmitReview", err) + if pull_service.IsErrSubmitReviewOnClosedPR(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + } else { + ctx.Error(http.StatusInternalServerError, "SubmitReview", err) + } return } diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 080df9382558..245627154767 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -264,8 +264,8 @@ func SubmitReview(ctx *context.Context) { if issues_model.IsContentEmptyErr(err) { ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty")) ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) - } else if pull_service.IsSubmitReviewOnClosedPR(err) { - ctx.Error(http.StatusForbidden) + } else if pull_service.IsErrSubmitReviewOnClosedPR(err) { + ctx.Error(http.StatusUnprocessableEntity) } else { ctx.ServerError("SubmitReview", err) } diff --git a/services/pull/review.go b/services/pull/review.go index 9d42976a361f..52b46e55f64d 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -46,8 +46,8 @@ func (err ErrDismissRequestOnClosedPR) Unwrap() error { // ErrSubmitReviewOnClosedPR represents an error when an user tries to submit an approve or reject review associated to a closed or merged PR. type ErrSubmitReviewOnClosedPR struct{} -// IsSubmitReviewOnClosedPR checks if an error is an ErrSubmitReviewOnClosedPR. -func IsSubmitReviewOnClosedPR(err error) bool { +// IsErrSubmitReviewOnClosedPR checks if an error is an ErrSubmitReviewOnClosedPR. +func IsErrSubmitReviewOnClosedPR(err error) bool { _, ok := err.(ErrSubmitReviewOnClosedPR) return ok } From c1c1be56f78bf7409067696acd13dda87c5faeb6 Mon Sep 17 00:00:00 2001 From: Kemal Zebari Date: Fri, 26 Apr 2024 14:09:06 -0700 Subject: [PATCH 3/7] Add integration tests and just return the HTTP status in the handler --- routers/web/repo/pull_review.go | 2 +- tests/integration/pull_review_test.go | 83 +++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 245627154767..e005f8788b8b 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -265,7 +265,7 @@ func SubmitReview(ctx *context.Context) { ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty")) ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) } else if pull_service.IsErrSubmitReviewOnClosedPR(err) { - ctx.Error(http.StatusUnprocessableEntity) + ctx.Status(http.StatusUnprocessableEntity) } else { ctx.ServerError("SubmitReview", err) } diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index 2d8b3cb4ab2e..5cea27fff1e9 100644 --- a/tests/integration/pull_review_test.go +++ b/tests/integration/pull_review_test.go @@ -5,12 +5,15 @@ package integration import ( "net/http" + "net/http/httptest" "net/url" + "path" "strings" "testing" "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" @@ -176,3 +179,83 @@ func TestPullView_CodeOwner(t *testing.T) { }) }) } + +func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) { + onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) { + user1Session := loginUser(t, "user1") + user2Session := loginUser(t, "user2") + + // Have user1 create a fork of repo1. + testRepoFork(t, user1Session, "user2", "repo1", "user1", "repo1") + + t.Run("Submit approve/reject review on merged PR", func(t *testing.T) { + // Create a merged PR (made by user1) in the upstream repo1. + testEditFile(t, user1Session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") + resp := testPullCreate(t, user1Session, "user1", "repo1", false, "master", "master", "This is a pull title") + elem := strings.Split(test.RedirectURL(resp), "/") + assert.EqualValues(t, "pulls", elem[3]) + testPullMerge(t, user1Session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, true) + + // Grab the CSRF token. + req := NewRequest(t, "GET", path.Join(elem[1], elem[2], "pulls", elem[4])) + resp = user2Session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + // Submit an approve review on the PR. + testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "approve", http.StatusUnprocessableEntity) + + // Submit a reject review on the PR. + testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "reject", http.StatusUnprocessableEntity) + }) + + t.Run("Submit approve/reject review on closed PR", func(t *testing.T) { + // Created a closed PR (made by user1) in the upstream repo1. + testEditFile(t, user1Session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") + resp := testPullCreate(t, user1Session, "user1", "repo1", false, "master", "master", "This is a pull title") + elem := strings.Split(test.RedirectURL(resp), "/") + assert.EqualValues(t, "pulls", elem[3]) + testIssueClose(t, user1Session, elem[1], elem[2], elem[4]) + + // Grab the CSRF token. + req := NewRequest(t, "GET", path.Join(elem[1], elem[2], "pulls", elem[4])) + resp = user2Session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + // Submit an approve review on the PR. + testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "approve", http.StatusUnprocessableEntity) + + // Submit a reject review on the PR. + testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "reject", http.StatusUnprocessableEntity) + + }) + }) +} + +func testSubmitReview(t *testing.T, session *TestSession, csrf, owner, repo, pullNumber, reviewType string, expectedSubmitStatus int) *httptest.ResponseRecorder { + options := map[string]string{ + "_csrf": csrf, + "commit_id": "", + "content": "test", + "type": reviewType, + } + + submitUrl := path.Join(owner, repo, "pulls", pullNumber, "files", "reviews", "submit") + req := NewRequestWithValues(t, "POST", submitUrl, options) + return session.MakeRequest(t, req, expectedSubmitStatus) +} + +func testIssueClose(t *testing.T, session *TestSession, owner, repo, issueNumber string) *httptest.ResponseRecorder { + req := NewRequest(t, "GET", path.Join(owner, repo, "pulls", issueNumber)) + resp := session.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + closeUrl := path.Join(owner, repo, "issues", issueNumber, "comments") + + options := map[string]string{ + "_csrf": htmlDoc.GetCSRF(), + "status": "close", + } + + req = NewRequestWithValues(t, "POST", closeUrl, options) + return session.MakeRequest(t, req, http.StatusOK) +} From 5722bb4f483787aa51975a10993900710a5124e0 Mon Sep 17 00:00:00 2001 From: Kemal Zebari Date: Fri, 26 Apr 2024 14:18:07 -0700 Subject: [PATCH 4/7] CI feedback --- tests/integration/pull_review_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index 5cea27fff1e9..00516913a1b7 100644 --- a/tests/integration/pull_review_test.go +++ b/tests/integration/pull_review_test.go @@ -226,7 +226,6 @@ func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) { // Submit a reject review on the PR. testSubmitReview(t, user2Session, htmlDoc.GetCSRF(), "user2", "repo1", elem[4], "reject", http.StatusUnprocessableEntity) - }) }) } @@ -239,8 +238,8 @@ func testSubmitReview(t *testing.T, session *TestSession, csrf, owner, repo, pul "type": reviewType, } - submitUrl := path.Join(owner, repo, "pulls", pullNumber, "files", "reviews", "submit") - req := NewRequestWithValues(t, "POST", submitUrl, options) + submitURL := path.Join(owner, repo, "pulls", pullNumber, "files", "reviews", "submit") + req := NewRequestWithValues(t, "POST", submitURL, options) return session.MakeRequest(t, req, expectedSubmitStatus) } @@ -249,13 +248,13 @@ func testIssueClose(t *testing.T, session *TestSession, owner, repo, issueNumber resp := session.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) - closeUrl := path.Join(owner, repo, "issues", issueNumber, "comments") + closeURL := path.Join(owner, repo, "issues", issueNumber, "comments") options := map[string]string{ "_csrf": htmlDoc.GetCSRF(), "status": "close", } - req = NewRequestWithValues(t, "POST", closeUrl, options) + req = NewRequestWithValues(t, "POST", closeURL, options) return session.MakeRequest(t, req, http.StatusOK) } From c89d78dfa136f496f03f82f61786185e7173f70b Mon Sep 17 00:00:00 2001 From: Kemal Zebari Date: Fri, 26 Apr 2024 14:39:18 -0700 Subject: [PATCH 5/7] Have the closed PR test create a new branch --- tests/integration/pull_review_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index 00516913a1b7..273332a36b41 100644 --- a/tests/integration/pull_review_test.go +++ b/tests/integration/pull_review_test.go @@ -194,7 +194,7 @@ func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) { resp := testPullCreate(t, user1Session, "user1", "repo1", false, "master", "master", "This is a pull title") elem := strings.Split(test.RedirectURL(resp), "/") assert.EqualValues(t, "pulls", elem[3]) - testPullMerge(t, user1Session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, true) + testPullMerge(t, user1Session, elem[1], elem[2], elem[4], repo_model.MergeStyleMerge, false) // Grab the CSRF token. req := NewRequest(t, "GET", path.Join(elem[1], elem[2], "pulls", elem[4])) @@ -210,8 +210,8 @@ func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) { t.Run("Submit approve/reject review on closed PR", func(t *testing.T) { // Created a closed PR (made by user1) in the upstream repo1. - testEditFile(t, user1Session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n") - resp := testPullCreate(t, user1Session, "user1", "repo1", false, "master", "master", "This is a pull title") + testEditFileToNewBranch(t, user1Session, "user1", "repo1", "master", "a-test-branch", "README.md", "Hello, World (Editied...again)\n") + resp := testPullCreate(t, user1Session, "user1", "repo1", false, "master", "a-test-branch", "This is a pull title") elem := strings.Split(test.RedirectURL(resp), "/") assert.EqualValues(t, "pulls", elem[3]) testIssueClose(t, user1Session, elem[1], elem[2], elem[4]) From a3915c90bd23304bfec803210dac50d15428bafe Mon Sep 17 00:00:00 2001 From: Kemal Zebari Date: Fri, 26 Apr 2024 19:19:51 -0700 Subject: [PATCH 6/7] PR feedback --- routers/api/v1/repo/pull_review.go | 5 +++-- routers/web/repo/pull_review.go | 2 +- services/pull/review.go | 25 ++++++------------------- 3 files changed, 10 insertions(+), 22 deletions(-) diff --git a/routers/api/v1/repo/pull_review.go b/routers/api/v1/repo/pull_review.go index 18683e4a4166..4b481790fb1a 100644 --- a/routers/api/v1/repo/pull_review.go +++ b/routers/api/v1/repo/pull_review.go @@ -4,6 +4,7 @@ package repo import ( + "errors" "fmt" "net/http" "strings" @@ -372,7 +373,7 @@ func CreatePullReview(ctx *context.APIContext) { // create review and associate all pending review comments review, _, err := pull_service.SubmitReview(ctx, ctx.Doer, ctx.Repo.GitRepo, pr.Issue, reviewType, opts.Body, opts.CommitID, nil) if err != nil { - if pull_service.IsErrSubmitReviewOnClosedPR(err) { + if errors.Is(err, pull_service.ErrSubmitReviewOnClosedPR) { ctx.Error(http.StatusUnprocessableEntity, "", err) } else { ctx.Error(http.StatusInternalServerError, "SubmitReview", err) @@ -464,7 +465,7 @@ func SubmitPullReview(ctx *context.APIContext) { // create review and associate all pending review comments review, _, err = pull_service.SubmitReview(ctx, ctx.Doer, ctx.Repo.GitRepo, pr.Issue, reviewType, opts.Body, headCommitID, nil) if err != nil { - if pull_service.IsErrSubmitReviewOnClosedPR(err) { + if errors.Is(err, pull_service.ErrSubmitReviewOnClosedPR) { ctx.Error(http.StatusUnprocessableEntity, "", err) } else { ctx.Error(http.StatusInternalServerError, "SubmitReview", err) diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index e005f8788b8b..62f6d71c5e5c 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -264,7 +264,7 @@ func SubmitReview(ctx *context.Context) { if issues_model.IsContentEmptyErr(err) { ctx.Flash.Error(ctx.Tr("repo.issues.review.content.empty")) ctx.JSONRedirect(fmt.Sprintf("%s/pulls/%d/files", ctx.Repo.RepoLink, issue.Index)) - } else if pull_service.IsErrSubmitReviewOnClosedPR(err) { + } else if errors.Is(err, pull_service.ErrSubmitReviewOnClosedPR) { ctx.Status(http.StatusUnprocessableEntity) } else { ctx.ServerError("SubmitReview", err) diff --git a/services/pull/review.go b/services/pull/review.go index 52b46e55f64d..7f5d056f9e93 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -6,6 +6,7 @@ package pull import ( "context" + "errors" "fmt" "io" "regexp" @@ -44,21 +45,7 @@ func (err ErrDismissRequestOnClosedPR) Unwrap() error { } // ErrSubmitReviewOnClosedPR represents an error when an user tries to submit an approve or reject review associated to a closed or merged PR. -type ErrSubmitReviewOnClosedPR struct{} - -// IsErrSubmitReviewOnClosedPR checks if an error is an ErrSubmitReviewOnClosedPR. -func IsErrSubmitReviewOnClosedPR(err error) bool { - _, ok := err.(ErrSubmitReviewOnClosedPR) - return ok -} - -func (err ErrSubmitReviewOnClosedPR) Error() string { - return "can't submit review for a closed or merged PR" -} - -func (err ErrSubmitReviewOnClosedPR) Unwrap() error { - return util.ErrPermissionDenied -} +var ErrSubmitReviewOnClosedPR = errors.New("can't submit review for a closed or merged PR") // 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. @@ -301,6 +288,10 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo // SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, reviewType issues_model.ReviewType, content, commitID string, attachmentUUIDs []string) (*issues_model.Review, *issues_model.Comment, error) { + if issue.IsClosed { + return nil, nil, ErrSubmitReviewOnClosedPR + } + if err := issue.LoadPullRequest(ctx); err != nil { return nil, nil, err } @@ -310,10 +301,6 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos if reviewType != issues_model.ReviewTypeApprove && reviewType != issues_model.ReviewTypeReject { stale = false } else { - if issue.IsClosed || pr.HasMerged { - return nil, nil, ErrSubmitReviewOnClosedPR{} - } - headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName()) if err != nil { return nil, nil, err From b0e9d8be39d1d13e48b764c4e93dfca6ce223965 Mon Sep 17 00:00:00 2001 From: Kemal Zebari Date: Fri, 26 Apr 2024 19:33:02 -0700 Subject: [PATCH 7/7] Move if check back to correct branch and refactor template file --- services/pull/review.go | 8 ++++---- templates/repo/diff/new_review.tmpl | 5 ++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/services/pull/review.go b/services/pull/review.go index 7f5d056f9e93..e303cd9a9d6e 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -288,10 +288,6 @@ func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_mo // SubmitReview creates a review out of the existing pending review or creates a new one if no pending review exist func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, reviewType issues_model.ReviewType, content, commitID string, attachmentUUIDs []string) (*issues_model.Review, *issues_model.Comment, error) { - if issue.IsClosed { - return nil, nil, ErrSubmitReviewOnClosedPR - } - if err := issue.LoadPullRequest(ctx); err != nil { return nil, nil, err } @@ -301,6 +297,10 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos if reviewType != issues_model.ReviewTypeApprove && reviewType != issues_model.ReviewTypeReject { stale = false } else { + if issue.IsClosed { + return nil, nil, ErrSubmitReviewOnClosedPR + } + headCommitID, err := gitRepo.GetRefCommitID(pr.GetGitRefName()) if err != nil { return nil, nil, err diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index 2d77119d0f58..1b74a230f47d 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -30,8 +30,7 @@ {{end}}
{{$showSelfTooltip := (and $.IsSigned ($.Issue.IsPoster $.SignedUser.ID))}} - {{$isOpen := (not (or $.Issue.IsClosed (and $.Issue.IsPull $.Issue.PullRequest.HasMerged)))}} - {{if $isOpen}} + {{if not $.Issue.IsClosed}} {{if $showSelfTooltip}} @@ -41,7 +40,7 @@ {{end}} {{end}} - {{if $isOpen}} + {{if not $.Issue.IsClosed}} {{if $showSelfTooltip}}