Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 30 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
bf1e779
Put an edit file button on pull request files to allow a quick operation
lunny Mar 9, 2024
470f893
Remove unnecessary test code
lunny Mar 9, 2024
4298a7a
Return back to the viewed file of pull request files tab after edit t…
lunny Mar 10, 2024
c49b896
Merge branch 'main' into lunny/edit_on_pr
lunny Mar 10, 2024
f9db1ed
Some improvements
lunny Mar 10, 2024
ec6ab8d
Update pull request head sync to fix the redirecto problem and do som…
lunny Mar 11, 2024
2d75787
Merge branch 'main' into lunny/edit_on_pr
lunny Mar 11, 2024
06fb88b
Use redirect to
lunny Mar 11, 2024
21a8260
Merge branch 'main' into lunny/edit_on_pr
lunny Mar 11, 2024
a9ce74b
Fix test
lunny Mar 11, 2024
87f9389
Fix test
lunny Mar 11, 2024
7cf364e
Fix bug
lunny Mar 12, 2024
efcb8e2
Merge branch 'main' into lunny/edit_on_pr
lunny Mar 12, 2024
22130ef
Merge branch 'main' into lunny/edit_on_pr
lunny Mar 14, 2024
afe9aaf
Follow wxiaoguang's suggestion
lunny Mar 14, 2024
6a0064b
Update routers/web/repo/editor.go
lunny Mar 14, 2024
9c89597
Fix lint error and Use util.PathEscapeSegments instead of url.PathEscape
lunny Mar 14, 2024
c0dabd9
Merge branch 'main' into lunny/edit_on_pr
lunny Mar 14, 2024
4bd286e
Add indents and add editor mode
lunny Mar 14, 2024
3f616b6
Remove unused code
lunny Mar 14, 2024
de2c535
Change cancel button link of editor page when edit from pull request …
lunny Mar 14, 2024
00e41d9
Apply suggestions from code review
lunny Mar 14, 2024
b0efe00
Add comment for the error handling when writting the pull head file w…
lunny Mar 16, 2024
5f15e70
Merge branch 'main' into lunny/edit_on_pr
silverwind Mar 21, 2024
9c1c4a5
Update routers/web/repo/editor.go
wxiaoguang Mar 22, 2024
00954b2
Merge branch 'main' into lunny/edit_on_pr
lunny Mar 26, 2024
e597548
fix merge conflicts
lunny Mar 26, 2024
5656851
Merge branch 'lunny/edit_on_pr' of github.com:lunny/gitea into lunny/…
lunny Mar 26, 2024
6907cdf
Update tests/integration/pull_compare_test.go
silverwind Mar 26, 2024
7cbbeeb
Merge branch 'main' into lunny/edit_on_pr
GiteaBot Mar 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions routers/private/hook_post_receive.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
lunny marked this conversation as resolved.
Show resolved Hide resolved
}
}
if len(branchesToSync) > 0 {
Expand Down
12 changes: 10 additions & 2 deletions routers/web/repo/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,12 @@
}
}

// 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.RedirectToFirst(

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-backend

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-backend

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-backend

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-backend

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / test-e2e

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / checks-backend

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)) (typecheck)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / test-pgsql

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / test-sqlite

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / test-mssql

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / backend

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / test-mysql

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)

Check failure on line 85 in routers/web/repo/editor.go

View workflow job for this annotation

GitHub Actions / test-unit

ctx.RedirectToFirst undefined (type *"code.gitea.io/gitea/services/context".Context has no field or method RedirectToFirst)
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
returnURI,
ctx.Repo.RepoLink+"/src/branch/"+util.PathEscapeSegments(newBranchName)+"/"+util.PathEscapeSegments(treePath),
)
}

// getParentTreeFields returns list of parent tree names and corresponding tree paths
Expand All @@ -100,6 +104,7 @@
}

func editFile(ctx *context.Context, isNewFile bool) {
ctx.Data["PageIsViewCode"] = true
ctx.Data["PageIsEdit"] = true
ctx.Data["IsNewFile"] = isNewFile
canCommit := renderCommitRights(ctx)
Expand Down Expand Up @@ -190,6 +195,9 @@
ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",")
ctx.Data["EditorconfigJson"] = GetEditorConfig(ctx, treePath)

ctx.Data["IsEditingFileOnly"] = ctx.FormString("return_uri") != ""
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
ctx.Data["ReturnURI"] = ctx.FormString("return_uri")

ctx.HTML(http.StatusOK, tplEditFile)
}

Expand Down
26 changes: 26 additions & 0 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
19 changes: 19 additions & 0 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
3 changes: 3 additions & 0 deletions templates/repo/diff/box.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
42 changes: 21 additions & 21 deletions templates/repo/editor/commit_form.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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")}}gt-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 gt-mr-2 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")}}gt-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 gt-mr-2 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>
2 changes: 1 addition & 1 deletion templates/repo/editor/edit.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
7 changes: 7 additions & 0 deletions tests/integration/pull_compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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("div.diff-file-header-actions a[href*='/_edit/']").Length()
silverwind marked this conversation as resolved.
Show resolved Hide resolved
assert.Greater(t, editButtonCount, 0, "Expected to find a button to edit a file in the PR diff view but there were none")
}
Loading