From 79c75164ca70937261b1d9a68420ebfdbdcfa4d4 Mon Sep 17 00:00:00 2001
From: Antonin Delpeuch <antonin@delpeuch.eu>
Date: Tue, 19 Dec 2023 22:18:07 +0100
Subject: [PATCH] [GITEA] pulls: "Edit File" button in "Files Changed" tab
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #1894.
Gitea issue: https://github.com/go-gitea/gitea/issues/23848
---
 routers/web/repo/pull.go              | 12 ++++++++++
 templates/repo/diff/box.tmpl          |  3 +++
 tests/integration/forgejo_git_test.go |  2 +-
 tests/integration/git_test.go         | 34 +++++++++++++++++++++------
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go
index 39f9cefa5c..c52b0116aa 100644
--- a/routers/web/repo/pull.go
+++ b/routers/web/repo/pull.go
@@ -966,6 +966,18 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
 		return
 	}
 
+	// determine if the user viewing the pull request can edit the head branch
+	if ctx.Doer != nil && pull.HeadRepo != nil && !pull.HasMerged {
+		headRepoPerm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer)
+		if err != nil {
+			ctx.ServerError("GetUserRepoPermission", err)
+			return
+		}
+		ctx.Data["HeadBranchIsEditable"] = pull.HeadRepo.CanEnableEditor() && issues_model.CanMaintainerWriteToBranch(ctx, headRepoPerm, pull.HeadBranch, ctx.Doer)
+		ctx.Data["SourceRepoLink"] = pull.HeadRepo.Link()
+		ctx.Data["HeadBranch"] = pull.HeadBranch
+	}
+
 	if ctx.IsSigned && ctx.Doer != nil {
 		if ctx.Data["CanMarkConversation"], err = issues_model.CanMarkConversation(ctx, issue, ctx.Doer); err != nil {
 			ctx.ServerError("CanMarkConversation", err)
diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl
index 1224bbe84c..a5fecce7c1 100644
--- a/templates/repo/diff/box.tmpl
+++ b/templates/repo/diff/box.tmpl
@@ -166,6 +166,9 @@
 										<a class="ui basic tiny button" rel="nofollow" href="{{$.BeforeSourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
 									{{else}}
 										<a class="ui basic tiny button" rel="nofollow" href="{{$.SourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
+										{{if and $.HeadBranchIsEditable (not $file.IsLFSFile)}}
+											<a class="ui basic tiny button" rel="nofollow" href="{{$.SourceRepoLink}}/_edit/{{PathEscapeSegments $.HeadBranch}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.editor.edit_this_file"}}</a>
+										{{end}}
 									{{end}}
 								{{end}}
 								{{if $isReviewFile}}
diff --git a/tests/integration/forgejo_git_test.go b/tests/integration/forgejo_git_test.go
index f81521d735..bd0fb207eb 100644
--- a/tests/integration/forgejo_git_test.go
+++ b/tests/integration/forgejo_git_test.go
@@ -134,6 +134,6 @@ func doActionsUserPR(ctx, doerCtx APITestContext, baseBranch, headBranch string)
 		doerCtx.ExpectedCode = http.StatusCreated
 		t.Run("AutoMergePR", doAPIAutoMergePullRequest(doerCtx, ctx.Username, ctx.Reponame, pr.Index))
 		// Ensure the PR page works
-		t.Run("EnsureCanSeePull", doEnsureCanSeePull(ctx, pr))
+		t.Run("EnsureCanSeePull", doEnsureCanSeePull(ctx, pr, true))
 	}
 }
diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go
index 0c3a8616f0..0afe9fa580 100644
--- a/tests/integration/git_test.go
+++ b/tests/integration/git_test.go
@@ -455,8 +455,19 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun
 			assert.NoError(t, err)
 		})
 
-		// Ensure the PR page works
-		t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
+		// Ensure the PR page works.
+		// For the base repository owner, the PR is not editable (maintainer edits are not enabled):
+		t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr, false))
+		// For the head repository owner, the PR is editable:
+		headSession := loginUser(t, "user2")
+		headToken := getTokenForLoggedInUser(t, headSession, auth_model.AccessTokenScopeReadRepository, auth_model.AccessTokenScopeReadUser)
+		headCtx := APITestContext{
+			Session:  headSession,
+			Token:    headToken,
+			Username: baseCtx.Username,
+			Reponame: baseCtx.Reponame,
+		}
+		t.Run("EnsureCanSeePull", doEnsureCanSeePull(headCtx, pr, true))
 
 		// Then get the diff string
 		var diffHash string
@@ -470,7 +481,9 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun
 
 		// Now: Merge the PR & make sure that doesn't break the PR page or change its diff
 		t.Run("MergePR", doAPIMergePullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index))
-		t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
+		// for both users the PR is still visible but not editable anymore after it was merged
+		t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr, false))
+		t.Run("EnsureCanSeePull", doEnsureCanSeePull(headCtx, pr, false))
 		t.Run("CheckPR", func(t *testing.T) {
 			oldMergeBase := pr.MergeBase
 			pr2, err := doAPIGetPullRequest(baseCtx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t)
@@ -481,12 +494,12 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun
 
 		// Then: Delete the head branch & make sure that doesn't break the PR page or change its diff
 		t.Run("DeleteHeadBranch", doBranchDelete(baseCtx, baseCtx.Username, baseCtx.Reponame, headBranch))
-		t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
+		t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr, false))
 		t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffHash, diffLength))
 
 		// Delete the head repository & make sure that doesn't break the PR page or change its diff
 		t.Run("DeleteHeadRepository", doAPIDeleteRepository(ctx))
-		t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr))
+		t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr, false))
 		t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffHash, diffLength))
 	}
 }
@@ -520,12 +533,19 @@ func doCreatePRAndSetManuallyMerged(ctx, baseCtx APITestContext, dstPath, baseBr
 	}
 }
 
-func doEnsureCanSeePull(ctx APITestContext, pr api.PullRequest) func(t *testing.T) {
+func doEnsureCanSeePull(ctx APITestContext, pr api.PullRequest, editable bool) func(t *testing.T) {
 	return func(t *testing.T) {
 		req := NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
 		ctx.Session.MakeRequest(t, req, http.StatusOK)
 		req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/files", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
-		ctx.Session.MakeRequest(t, req, http.StatusOK)
+		resp := ctx.Session.MakeRequest(t, req, http.StatusOK)
+		doc := NewHTMLParser(t, resp.Body)
+		editButtonCount := doc.doc.Find("div.diff-file-header-actions a[href*='/_edit/']").Length()
+		if editable {
+			assert.Greater(t, editButtonCount, 0, "Expected to find a button to edit a file in the PR diff view but there were none")
+		} else {
+			assert.Equal(t, 0, editButtonCount, "Expected not to find any buttons to edit files in PR diff view but there were some")
+		}
 		req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s/pulls/%d/commits", url.PathEscape(ctx.Username), url.PathEscape(ctx.Reponame), pr.Index))
 		ctx.Session.MakeRequest(t, req, http.StatusOK)
 	}