fix(ui): disable PR review button in archived repos (#6729)

* disable the "Finish review" button on PR/files page for archived repos - trying to review PRs in such repos results in JS error * wrap the button in a div and move tooltips here to make them actually display on a disabled button - currently they do not * added simple testing Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6729 Reviewed-by: Gusted <[email protected]> Co-authored-by: 0ko <[email protected]> Co-committed-by: 0ko <[email protected]>
DennisRasey · Jan 31, 2025 · b159006 · b159006
1 parent 4ef00fe
commit b159006
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 5 deletions.
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
@@ -1193,6 +1193,7 @@ archive.title = This repository is archived. You can view files and clone it, bu
 archive.title_date = This repository has been archived on %s. You can view files and clone it, but cannot push or open issues or pull requests.
 archive.issue.nocomment = This repository is archived. You cannot comment on issues.
 archive.pull.nocomment = This repository is archived. You cannot comment on pull requests.
+archive.pull.noreview = This repository is archived. You cannot review pull requests.
 
 form.reach_limit_of_creation_1 = The owner has already reached the limit of %d repository.
 form.reach_limit_of_creation_n = The owner has already reached the limit of %d repositories.

diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl
@@ -1,9 +1,16 @@
 <div id="review-box">
-	<button class="ui tiny primary button tw-pr-1 tw-flex js-btn-review {{if not $.IsShowingAllCommits}}disabled{{end}}" {{if not $.IsShowingAllCommits}}data-tooltip-content="{{ctx.Locale.Tr "repo.pulls.review_only_possible_for_full_diff"}}"{{end}}>
-		{{ctx.Locale.Tr "repo.diff.review"}}
-		<span class="ui small label review-comments-counter" data-pending-comment-number="{{.PendingCodeCommentNumber}}">{{.PendingCodeCommentNumber}}</span>
-		{{svg "octicon-triangle-down" 14 "dropdown icon"}}
-	</button>
+	<div
+		{{if not $.IsShowingAllCommits}}
+			data-tooltip-content="{{ctx.Locale.Tr "repo.pulls.review_only_possible_for_full_diff"}}"
+		{{else if .Repository.IsArchived}}
+			data-tooltip-content="{{ctx.Locale.Tr "repo.archive.pull.noreview"}}"
+		{{end}}>
+		<button class="ui tiny primary button tw-pr-1 tw-flex js-btn-review {{if or (not $.IsShowingAllCommits) .Repository.IsArchived}}disabled{{end}}">
+			{{ctx.Locale.Tr "repo.diff.review"}}
+			<span class="ui small label review-comments-counter" data-pending-comment-number="{{.PendingCodeCommentNumber}}">{{.PendingCodeCommentNumber}}</span>
+			{{svg "octicon-triangle-down" 14 "dropdown icon"}}
+		</button>
+	</div>
 	{{if $.IsShowingAllCommits}}
 	<div class="review-box-panel tippy-target">
 		<div class="ui segment">

diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go
@@ -1,4 +1,5 @@
 // Copyright 2019 The Gitea Authors. All rights reserved.
+// Copyright 2024 The Forgejo Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package integration
@@ -362,6 +363,8 @@ func TestPullView_CodeOwner(t *testing.T) {
 		defer f()
 
 		t.Run("First Pull Request", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
 			// create a new branch to prepare for pull request
 			_, err := files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
 				NewBranch: "codeowner-basebranch",
@@ -409,6 +412,8 @@ func TestPullView_CodeOwner(t *testing.T) {
 		require.NoError(t, err)
 
 		t.Run("Second Pull Request", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
 			// create a new branch to prepare for pull request
 			_, err = files_service.ChangeRepoFiles(db.DefaultContext, repo, user2, &files_service.ChangeRepoFilesOptions{
 				NewBranch: "codeowner-basebranch2",
@@ -431,6 +436,8 @@ func TestPullView_CodeOwner(t *testing.T) {
 		})
 
 		t.Run("Forked Repo Pull Request", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
 			user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
 			forkedRepo, err := repo_service.ForkRepositoryAndUpdates(db.DefaultContext, user2, user5, repo_service.ForkRepoOptions{
 				BaseRepo: repo,
@@ -483,6 +490,8 @@ func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) {
 		defer baseGitRepo.Close()
 
 		t.Run("Submit approve/reject review on merged PR", func(t *testing.T) {
+			defer tests.PrintCurrentTest(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")
@@ -513,6 +522,8 @@ func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) {
 		})
 
 		t.Run("Submit approve/reject review on closed PR", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
 			// Created a closed PR (made by user1) in the upstream repo1.
 			testEditFileToNewBranch(t, user1Session, "user1", "repo1", "master", "a-test-branch", "README.md", "Hello, World (Edited...again)\n")
 			resp := testPullCreate(t, user1Session, "user1", "repo1", false, "master", "a-test-branch", "This is a pull title")
@@ -544,6 +555,41 @@ func TestPullView_GivenApproveOrRejectReviewOnClosedPR(t *testing.T) {
 	})
 }
 
+func TestPullReviewInArchivedRepo(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+		session := loginUser(t, "user2")
+
+		// Open a PR
+		testEditFileToNewBranch(t, session, "user2", "repo1", "master", "for-pr", "README.md", "Hi!\n")
+		resp := testPullCreate(t, session, "user2", "repo1", true, "master", "for-pr", "PR title")
+		elem := strings.Split(test.RedirectURL(resp), "/")
+
+		t.Run("Review box normally", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			// The "Finish review button" must be available
+			resp = session.MakeRequest(t, NewRequest(t, "GET", path.Join(elem[1], elem[2], "pulls", elem[4], "files")), http.StatusOK)
+			button := NewHTMLParser(t, resp.Body).Find("#review-box button")
+			assert.False(t, button.HasClass("disabled"))
+		})
+
+		t.Run("Review box in archived repo", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			// Archive the repo
+			resp = session.MakeRequest(t, NewRequestWithValues(t, "POST", path.Join(elem[1], elem[2], "settings"), map[string]string{
+				"_csrf":  GetCSRF(t, session, path.Join(elem[1], elem[2], "settings")),
+				"action": "archive",
+			}), http.StatusSeeOther)
+
+			// The "Finish review button" must be disabled
+			resp = session.MakeRequest(t, NewRequest(t, "GET", path.Join(elem[1], elem[2], "pulls", elem[4], "files")), http.StatusOK)
+			button := NewHTMLParser(t, resp.Body).Find("#review-box button")
+			assert.True(t, button.HasClass("disabled"))
+		})
+	})
+}
+
 func testNofiticationCount(t *testing.T, session *TestSession, csrf string, expectedSubmitStatus int) *httptest.ResponseRecorder {
 	options := map[string]string{
 		"_csrf": csrf,