From bf1e7791c16a82e9230a8e1710c9a31aa04fde56 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 10 Mar 2024 00:25:15 +0800 Subject: [PATCH 01/20] Put an edit file button on pull request files to allow a quick operation --- routers/web/repo/pull.go | 19 +++++++++++++++++++ templates/repo/diff/box.tmpl | 11 +++++++---- tests/integration/git_test.go | 13 +++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index ed063715e50d3..7a152de601521 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1071,6 +1071,25 @@ 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 { + ctx.Data["SourcePath"] = pull.HeadRepo.Link() + "/src/branch/" + url.PathEscape(pull.HeadBranch) + + if !pull.HasMerged && ctx.Doer != nil { + if err := pull.LoadHeadRepo(ctx); err != nil { + ctx.ServerError("LoadHeadRepo", err) + return + } + perm, err := access_model.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer) + if err != nil { + ctx.ServerError("GetUserRepoPermission", err) + return + } + ctx.Data["CanEditFile"] = perm.CanWrite(unit.TypeCode) || issues_model.CanMaintainerWriteToBranch(ctx, perm, pull.HeadBranch, ctx.Doer) + ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.edit_this_file") + ctx.Data["HeadRepoLink"] = pull.HeadRepo.Link() + ctx.Data["HeadBranchName"] = pull.HeadBranch + } + } ctx.HTML(http.StatusOK, tplPullFiles) } diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 39df6faea50dc..0fbe28b77a347 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -130,6 +130,10 @@ {{if $file.IsRenamed}}{{$file.OldName}} → {{end}}{{$file.Name}}{{if .IsLFSFile}} ({{ctx.Locale.Tr "repo.stored_lfs"}}){{end}} + {{if not (or $file.IsIncomplete $file.IsBin $file.IsSubmodule)}} + + + {{end}} {{if $file.IsGenerated}} {{ctx.Locale.Tr "repo.diff.generated"}} {{end}} @@ -157,14 +161,13 @@ {{if and $isReviewFile $file.HasChangedSinceLastReview}} {{ctx.Locale.Tr "repo.pulls.has_changed_since_last_review"}} {{end}} - {{if not (or $file.IsIncomplete $file.IsBin $file.IsSubmodule)}} - - - {{end}} {{if and (not $file.IsSubmodule) (not $.PageIsWiki)}} {{if $file.IsDeleted}} {{ctx.Locale.Tr "repo.diff.view_file"}} {{else}} + {{if and $.Repository.CanEnableEditor $.CanEditFile (not $file.IsLFSFile)}} + {{ctx.Locale.Tr "repo.editor.edit_this_file"}} + {{end}} {{ctx.Locale.Tr "repo.diff.view_file"}} {{end}} {{end}} diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index 818e1fa65350a..173518295c1b0 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -461,6 +461,19 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun // Ensure the PR page works t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr)) + t.Run("EnsureCanEditPullFile", 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() + assert.Greater(t, editButtonCount, 0, "Expected to find a button to edit a file in the PR diff view but there were none") + 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) + }) + // Then get the diff string var diffHash string var diffLength int From 470f89324210326e1b36c09b37d839e307bea73b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 10 Mar 2024 00:26:28 +0800 Subject: [PATCH 02/20] Remove unnecessary test code --- tests/integration/git_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index 173518295c1b0..2704a1262b749 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -462,16 +462,12 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr)) t.Run("EnsureCanEditPullFile", 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)) + 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() assert.Greater(t, editButtonCount, 0, "Expected to find a button to edit a file in the PR diff view but there were none") - 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) }) // Then get the diff string From 4298a7a4bcbd93c6b5bddc051f19235ae5b95675 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 10 Mar 2024 11:27:54 +0800 Subject: [PATCH 03/20] Return back to the viewed file of pull request files tab after edit the file --- routers/web/repo/editor.go | 15 +++++++++++++++ templates/repo/diff/box.tmpl | 10 +++++----- templates/repo/editor/commit_form.tmpl | 4 ++++ tests/integration/git_test.go | 1 - 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 6146ce4ce4a38..c61015a6870f3 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -80,6 +80,18 @@ func redirectForCommitChoice(ctx *context.Context, commitChoice, newBranchName, } } + prIndex := ctx.FormInt64("back_to_pr") + if prIndex > 0 { + // Redirect to the PR's files tab + link := ctx.Repo.RepoLink + fmt.Sprintf("/pulls/%d/files", prIndex) + diffHash := ctx.FormString("back_to_diff_hash") + if len(diffHash) > 0 { + link += "#diff-" + diffHash + ctx.Redirect(link) + return + } + } + // Redirect to viewing file or folder ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(newBranchName) + "/" + util.PathEscapeSegments(treePath)) } @@ -190,6 +202,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["CanCreateNewBranch"] = !ctx.FormBool("hide_create_new_branch") + ctx.Data["CanCreatePullRequest"] = !ctx.FormBool("hide_create_pr") + ctx.HTML(http.StatusOK, tplEditFile) } diff --git a/templates/repo/diff/box.tmpl b/templates/repo/diff/box.tmpl index 0fbe28b77a347..53efbedcd8b25 100644 --- a/templates/repo/diff/box.tmpl +++ b/templates/repo/diff/box.tmpl @@ -130,10 +130,6 @@ {{if $file.IsRenamed}}{{$file.OldName}} → {{end}}{{$file.Name}}{{if .IsLFSFile}} ({{ctx.Locale.Tr "repo.stored_lfs"}}){{end}} - {{if not (or $file.IsIncomplete $file.IsBin $file.IsSubmodule)}} - - - {{end}} {{if $file.IsGenerated}} {{ctx.Locale.Tr "repo.diff.generated"}} {{end}} @@ -161,12 +157,16 @@ {{if and $isReviewFile $file.HasChangedSinceLastReview}} {{ctx.Locale.Tr "repo.pulls.has_changed_since_last_review"}} {{end}} + {{if not (or $file.IsIncomplete $file.IsBin $file.IsSubmodule)}} + + + {{end}} {{if and (not $file.IsSubmodule) (not $.PageIsWiki)}} {{if $file.IsDeleted}} {{ctx.Locale.Tr "repo.diff.view_file"}} {{else}} {{if and $.Repository.CanEnableEditor $.CanEditFile (not $file.IsLFSFile)}} - {{ctx.Locale.Tr "repo.editor.edit_this_file"}} + {{ctx.Locale.Tr "repo.editor.edit_this_file"}} {{end}} {{ctx.Locale.Tr "repo.diff.view_file"}} {{end}} diff --git a/templates/repo/editor/commit_form.tmpl b/templates/repo/editor/commit_form.tmpl index 94429452dd116..1421ca091354a 100644 --- a/templates/repo/editor/commit_form.tmpl +++ b/templates/repo/editor/commit_form.tmpl @@ -40,6 +40,7 @@ {{if not .Repository.IsEmpty}} + {{if .CanCreatePullRequest}}
{{if .CanCreatePullRequest}} @@ -57,6 +58,8 @@
+ {{end}} + {{if .CanCreateNewBranch}}
{{svg "octicon-git-branch"}} @@ -65,6 +68,7 @@
{{end}} + {{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 05a8d966815b0..e9c52fb84e2b9 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"}} From 00e41d95e8097826099869da4382a72918ec8055 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Mar 2024 21:22:11 +0800 Subject: [PATCH 16/20] Apply suggestions from code review Co-authored-by: wxiaoguang --- templates/repo/editor/commit_form.tmpl | 2 +- templates/repo/editor/edit.tmpl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/repo/editor/commit_form.tmpl b/templates/repo/editor/commit_form.tmpl index a1265f47e3320..8a674b1555f7c 100644 --- a/templates/repo/editor/commit_form.tmpl +++ b/templates/repo/editor/commit_form.tmpl @@ -70,5 +70,5 @@ - {{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 e9c52fb84e2b9..12bcde01443b9 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"}} From b0efe0053f0fd9f3f09c8b2e899da7c17194df57 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 16 Mar 2024 16:57:30 +0800 Subject: [PATCH 17/20] Add comment for the error handling when writting the pull head file when pushing. --- routers/private/hook_post_receive.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index a3fa052a6ceef..2601e77474a0b 100644 --- a/routers/private/hook_post_receive.go +++ b/routers/private/hook_post_receive.go @@ -107,6 +107,7 @@ 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) } } From 9c1c4a5f3bd0bdd4c39547225f58c7b6a2fc66cf Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 22 Mar 2024 09:51:53 +0800 Subject: [PATCH 18/20] Update routers/web/repo/editor.go --- routers/web/repo/editor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 9cc8664521269..f9918b32733d9 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -82,7 +82,7 @@ func redirectForCommitChoice(ctx *context.Context, commitChoice, newBranchName, returnURI := ctx.FormString("return_uri") - ctx.RedirectToFirst( + ctx.RedirectToCurrentSite( returnURI, ctx.Repo.RepoLink+"/src/branch/"+util.PathEscapeSegments(newBranchName)+"/"+util.PathEscapeSegments(treePath), ) From e59754817c93c1f0487171899c8bbde88cd67ade Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 26 Mar 2024 14:04:11 +0800 Subject: [PATCH 19/20] fix merge conflicts --- routers/web/repo/editor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index a90e0ab74e746..474f7ff1da428 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -82,7 +82,7 @@ func redirectForCommitChoice(ctx *context.Context, commitChoice, newBranchName, returnURI := ctx.FormString("return_uri") - ctx.RedirectToFirst( + ctx.RedirectToCurrentSite( returnURI, ctx.Repo.RepoLink+"/src/branch/"+util.PathEscapeSegments(newBranchName)+"/"+util.PathEscapeSegments(treePath), ) From 6907cdf6b6f722deffb2268b1ae96bb012f42e20 Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 26 Mar 2024 18:46:01 +0100 Subject: [PATCH 20/20] Update tests/integration/pull_compare_test.go --- tests/integration/pull_compare_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/pull_compare_test.go b/tests/integration/pull_compare_test.go index e399e65475c05..5ce8ea3031e13 100644 --- a/tests/integration/pull_compare_test.go +++ b/tests/integration/pull_compare_test.go @@ -30,6 +30,6 @@ func TestPullCompare(t *testing.T) { 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("div.diff-file-header-actions a[href*='/_edit/']").Length() + 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") }