Put an edit file button on pull request files to allow a quick operation (#29697)

Resolve #23848

This PR put an edit file button on pull request files to allow a quick
edit for a file. After the edit finished, it will return back to the
viewed file position on pull request files tab.

It also use a branch view file link instead of commit link when it's a
non-commit pull request files view.

<img width="1532" alt="image"
src="https://github.com/go-gitea/gitea/assets/81045/3637ca4c-89d5-4621-847b-79702a44f617">

---------

Co-authored-by: wxiaoguang <[email protected]>
Co-authored-by: silverwind <[email protected]>

Author: 3 people

routers/private/hook_post_receive.go

@@ -20,6 +20,7 @@ import (
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
 	gitea_context "code.gitea.io/gitea/services/context"
+	pull_service "code.gitea.io/gitea/services/pull"
 	repo_service "code.gitea.io/gitea/services/repository"
 )
 
@@ -109,6 +110,9 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
 				}
 			} else {
 				branchesToSync = append(branchesToSync, update)
+
+				// TODO: should we return the error and return the error when pushing? Currently it will log the error and not prevent the pushing
+				pull_service.UpdatePullsRefs(ctx, repo, update)
 			}
 		}
 		if len(branchesToSync) > 0 {

routers/web/repo/editor.go

@@ -80,8 +80,12 @@ func redirectForCommitChoice(ctx *context.Context, commitChoice, newBranchName,
 		}
 	}
 
-	// Redirect to viewing file or folder
-	ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(newBranchName) + "/" + util.PathEscapeSegments(treePath))
+	returnURI := ctx.FormString("return_uri")
+
+	ctx.RedirectToCurrentSite(
+		returnURI,
+		ctx.Repo.RepoLink+"/src/branch/"+util.PathEscapeSegments(newBranchName)+"/"+util.PathEscapeSegments(treePath),
+	)
 }
 
 // getParentTreeFields returns list of parent tree names and corresponding tree paths
@@ -100,6 +104,7 @@ func getParentTreeFields(treePath string) (treeNames, treePaths []string) {
 }
 
 func editFile(ctx *context.Context, isNewFile bool) {
+	ctx.Data["PageIsViewCode"] = true
 	ctx.Data["PageIsEdit"] = true
 	ctx.Data["IsNewFile"] = isNewFile
 	canCommit := renderCommitRights(ctx)
@@ -190,6 +195,9 @@ func editFile(ctx *context.Context, isNewFile bool) {
 	ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",")
 	ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, treePath)
 
+	ctx.Data["IsEditingFileOnly"] = ctx.FormString("return_uri") != ""
+	ctx.Data["ReturnURI"] = ctx.FormString("return_uri")
+
 	ctx.HTML(http.StatusOK, tplEditFile)
 }
 

routers/web/repo/pull.go

@@ -857,6 +857,32 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
 	ctx.Data["CanBlockUser"] = func(blocker, blockee *user_model.User) bool {
 		return user_service.CanBlockUser(ctx, ctx.Doer, blocker, blockee)
 	}
+	if !willShowSpecifiedCommit && !willShowSpecifiedCommitRange && pull.Flow == issues_model.PullRequestFlowGithub {
+		if err := pull.LoadHeadRepo(ctx); err != nil {
+			ctx.ServerError("LoadHeadRepo", err)
+			return
+		}
+
+		if pull.HeadRepo != nil {
+			ctx.Data["SourcePath"] = pull.HeadRepo.Link() + "/src/branch/" + util.PathEscapeSegments(pull.HeadBranch)
+		}
+
+		if !pull.HasMerged && ctx.Doer != nil {
+			perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer)
+			if err != nil {
+				ctx.ServerError("GetUserRepoPermission", err)
+				return
+			}
+
+			if perm.CanWrite(unit.TypeCode) || issues_model.CanMaintainerWriteToBranch(ctx, perm, pull.HeadBranch, ctx.Doer) {
+				ctx.Data["CanEditFile"] = true
+				ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.edit_this_file")
+				ctx.Data["HeadRepoLink"] = pull.HeadRepo.Link()
+				ctx.Data["HeadBranchName"] = pull.HeadBranch
+				ctx.Data["BackToLink"] = setting.AppSubURL + ctx.Req.URL.RequestURI()
+			}
+		}
+	}
 
 	ctx.HTML(http.StatusOK, tplPullFiles)
 }

services/pull/pull.go

@@ -526,6 +526,25 @@ func pushToBaseRepoHelper(ctx context.Context, pr *issues_model.PullRequest, pre
 	return nil
 }
 
+// UpdatePullsRefs update all the PRs head file pointers like /refs/pull/1/head so that it will be dependent by other operations
+func UpdatePullsRefs(ctx context.Context, repo *repo_model.Repository, update *repo_module.PushUpdateOptions) {
+	branch := update.RefFullName.BranchName()
+	// GetUnmergedPullRequestsByHeadInfo() only return open and unmerged PR.
+	prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repo.ID, branch)
+	if err != nil {
+		log.Error("Find pull requests [head_repo_id: %d, head_branch: %s]: %v", repo.ID, branch, err)
+	} else {
+		for _, pr := range prs {
+			log.Trace("Updating PR[%d]: composing new test task", pr.ID)
+			if pr.Flow == issues_model.PullRequestFlowGithub {
+				if err := PushToBaseRepo(ctx, pr); err != nil {
+					log.Error("PushToBaseRepo: %v", err)
+				}
+			}
+		}
+	}
+}
+
 // UpdateRef update refs/pull/id/head directly for agit flow pull request
 func UpdateRef(ctx context.Context, pr *issues_model.PullRequest) (err error) {
 	log.Trace("UpdateRef[%d]: upgate pull request ref in base repo '%s'", pr.ID, pr.GetGitRefName())

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 $.Repository.CanEnableEditor $.CanEditFile (not $file.IsLFSFile) (not $file.IsBin)}}
+											<a class="ui basic tiny button" rel="nofollow" href="{{$.HeadRepoLink}}/_edit/{{PathEscapeSegments $.HeadBranchName}}/{{PathEscapeSegments $file.Name}}?return_uri={{print $.BackToLink "#diff-" $file.NameHash | QueryEscape}}">{{ctx.Locale.Tr "repo.editor.edit_this_file"}}</a>
+										{{end}}
 									{{end}}
 								{{end}}
 								{{if $isReviewFile}}

templates/repo/editor/commit_form.tmpl

@@ -39,36 +39,36 @@
 					</label>
 				</div>
 			</div>
-			{{if not .Repository.IsEmpty}}
-			<div class="field">
-				<div class="ui radio checkbox">
-					{{if .CanCreatePullRequest}}
-						<input type="radio" class="js-quick-pull-choice-option" name="commit_choice" value="commit-to-new-branch" button_text="{{ctx.Locale.Tr "repo.editor.propose_file_change"}}" {{if eq .commit_choice "commit-to-new-branch"}}checked{{end}}>
-					{{else}}
-						<input type="radio" class="js-quick-pull-choice-option" name="commit_choice" value="commit-to-new-branch" button_text="{{ctx.Locale.Tr "repo.editor.commit_changes"}}" {{if eq .commit_choice "commit-to-new-branch"}}checked{{end}}>
-					{{end}}
-					<label>
-						{{svg "octicon-git-pull-request"}}
+			{{if and (not .Repository.IsEmpty) (not .IsEditingFileOnly)}}
+				<div class="field">
+					<div class="ui radio checkbox">
 						{{if .CanCreatePullRequest}}
-							{{ctx.Locale.Tr "repo.editor.create_new_branch"}}
+							<input type="radio" class="js-quick-pull-choice-option" name="commit_choice" value="commit-to-new-branch" button_text="{{ctx.Locale.Tr "repo.editor.propose_file_change"}}" {{if eq .commit_choice "commit-to-new-branch"}}checked{{end}}>
 						{{else}}
-							{{ctx.Locale.Tr "repo.editor.create_new_branch_np"}}
+							<input type="radio" class="js-quick-pull-choice-option" name="commit_choice" value="commit-to-new-branch" button_text="{{ctx.Locale.Tr "repo.editor.commit_changes"}}" {{if eq .commit_choice "commit-to-new-branch"}}checked{{end}}>
 						{{end}}
-					</label>
+						<label>
+							{{svg "octicon-git-pull-request"}}
+							{{if .CanCreatePullRequest}}
+								{{ctx.Locale.Tr "repo.editor.create_new_branch"}}
+							{{else}}
+								{{ctx.Locale.Tr "repo.editor.create_new_branch_np"}}
+							{{end}}
+						</label>
+					</div>
 				</div>
-			</div>
-			<div class="quick-pull-branch-name {{if not (eq .commit_choice "commit-to-new-branch")}}tw-hidden{{end}}">
-				<div class="new-branch-name-input field {{if .Err_NewBranchName}}error{{end}}">
-					{{svg "octicon-git-branch"}}
-					<input type="text" name="new_branch_name" maxlength="100" value="{{.new_branch_name}}" class="input-contrast tw-mr-1 js-quick-pull-new-branch-name" placeholder="{{ctx.Locale.Tr "repo.editor.new_branch_name_desc"}}" {{if eq .commit_choice "commit-to-new-branch"}}required{{end}} title="{{ctx.Locale.Tr "repo.editor.new_branch_name"}}">
-					<span class="text-muted js-quick-pull-normalization-info"></span>
+				<div class="quick-pull-branch-name {{if not (eq .commit_choice "commit-to-new-branch")}}tw-hidden{{end}}">
+					<div class="new-branch-name-input field {{if .Err_NewBranchName}}error{{end}}">
+						{{svg "octicon-git-branch"}}
+						<input type="text" name="new_branch_name" maxlength="100" value="{{.new_branch_name}}" class="input-contrast tw-mr-1 js-quick-pull-new-branch-name" placeholder="{{ctx.Locale.Tr "repo.editor.new_branch_name_desc"}}" {{if eq .commit_choice "commit-to-new-branch"}}required{{end}} title="{{ctx.Locale.Tr "repo.editor.new_branch_name"}}">
+						<span class="text-muted js-quick-pull-normalization-info"></span>
+					</div>
 				</div>
-			</div>
 			{{end}}
 		</div>
 	</div>
 	<button id="commit-button" type="submit" class="ui primary button">
 		{{if eq .commit_choice "commit-to-new-branch"}}{{ctx.Locale.Tr "repo.editor.propose_file_change"}}{{else}}{{ctx.Locale.Tr "repo.editor.commit_changes"}}{{end}}
 	</button>
-	<a class="ui button red" href="{{$.BranchLink}}/{{PathEscapeSegments .TreePath}}">{{ctx.Locale.Tr "repo.editor.cancel"}}</a>
+	<a class="ui button red" href="{{if .ReturnURI}}{{.ReturnURI}}{{else}}{{$.BranchLink}}/{{PathEscapeSegments .TreePath}}{{end}}">{{ctx.Locale.Tr "repo.editor.cancel"}}</a>
 </div>

templates/repo/editor/edit.tmpl

@@ -21,7 +21,7 @@
 							<span class="section"><a href="{{$.BranchLink}}/{{index $.TreePaths $i | PathEscapeSegments}}">{{$v}}</a></span>
 						{{end}}
 					{{end}}
-					<span>{{ctx.Locale.Tr "repo.editor.or"}} <a href="{{$.BranchLink}}{{if not .IsNewFile}}/{{PathEscapeSegments .TreePath}}{{end}}">{{ctx.Locale.Tr "repo.editor.cancel_lower"}}</a></span>
+					<span>{{ctx.Locale.Tr "repo.editor.or"}} <a href="{{if .ReturnURI}}{{.ReturnURI}}{{else}}{{$.BranchLink}}{{if not .IsNewFile}}/{{PathEscapeSegments .TreePath}}{{end}}{{end}}">{{ctx.Locale.Tr "repo.editor.cancel_lower"}}</a></span>
 					<input type="hidden" id="tree_path" name="tree_path" value="{{.TreePath}}" required>
 				</div>
 			</div>

tests/integration/pull_compare_test.go

@@ -25,4 +25,11 @@ func TestPullCompare(t *testing.T) {
 	req = NewRequest(t, "GET", link)
 	resp = session.MakeRequest(t, req, http.StatusOK)
 	assert.EqualValues(t, http.StatusOK, resp.Code)
+
+	// test the edit button in the PR diff view
+	req = NewRequest(t, "GET", "/user2/repo1/pulls/3/files")
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	doc := NewHTMLParser(t, resp.Body)
+	editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length()
+	assert.Greater(t, editButtonCount, 0, "Expected to find a button to edit a file in the PR diff view but there were none")
 }