From 538790ad1db8bf66942ff2ead4ef78df5ab8b702 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 27 Mar 2024 10:34:10 +0800 Subject: [PATCH] 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. image --------- Co-authored-by: wxiaoguang Co-authored-by: silverwind --- routers/private/hook_post_receive.go | 4 +++ routers/web/repo/editor.go | 12 ++++++-- routers/web/repo/pull.go | 26 ++++++++++++++++ services/pull/pull.go | 19 ++++++++++++ templates/repo/diff/box.tmpl | 3 ++ templates/repo/editor/commit_form.tmpl | 42 +++++++++++++------------- templates/repo/editor/edit.tmpl | 2 +- tests/integration/pull_compare_test.go | 7 +++++ 8 files changed, 91 insertions(+), 24 deletions(-) diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index a09956f738..101ae92302 100644 --- a/routers/private/hook_post_receive.go +++ b/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 { diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 082666276a..474f7ff1da 100644 --- a/routers/web/repo/editor.go +++ b/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) } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 2422be39b8..a0a8e5410c 100644 --- a/routers/web/repo/pull.go +++ b/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) } diff --git a/services/pull/pull.go b/services/pull/pull.go index 4289e2e6e1..c091b8608a 100644 --- a/services/pull/pull.go +++ b/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()) diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 37a4e1e323..555ffafc62 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -166,6 +166,9 @@ {{ctx.Locale.Tr "repo.diff.view_file"}} {{else}} {{ctx.Locale.Tr "repo.diff.view_file"}} + {{if and $.Repository.CanEnableEditor $.CanEditFile (not $file.IsLFSFile) (not $file.IsBin)}} + {{ctx.Locale.Tr "repo.editor.edit_this_file"}} + {{end}} {{end}} {{end}} {{if $isReviewFile}} diff --git a/templates/repo/editor/commit_form.tmpl b/templates/repo/editor/commit_form.tmpl index 0ddbec0e8e..21ef63288f 100644 --- a/templates/repo/editor/commit_form.tmpl +++ b/templates/repo/editor/commit_form.tmpl @@ -39,36 +39,36 @@ - {{if not .Repository.IsEmpty}} -
-
- {{if .CanCreatePullRequest}} - - {{else}} - - {{end}} -
-
-
- {{svg "octicon-git-branch"}} - - +
+
+ {{svg "octicon-git-branch"}} + + +
-
{{end}}
- {{ctx.Locale.Tr "repo.editor.cancel"}} + {{ctx.Locale.Tr "repo.editor.cancel"}} diff --git a/templates/repo/editor/edit.tmpl b/templates/repo/editor/edit.tmpl index 1f5652f6b5..46f82c47d4 100644 --- a/templates/repo/editor/edit.tmpl +++ b/templates/repo/editor/edit.tmpl @@ -21,7 +21,7 @@ {{$v}} {{end}} {{end}} - {{ctx.Locale.Tr "repo.editor.or"}} {{ctx.Locale.Tr "repo.editor.cancel_lower"}} + {{ctx.Locale.Tr "repo.editor.or"}} {{ctx.Locale.Tr "repo.editor.cancel_lower"}} diff --git a/tests/integration/pull_compare_test.go b/tests/integration/pull_compare_test.go index f5baf05965..5ce8ea3031 100644 --- a/tests/integration/pull_compare_test.go +++ b/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") }