Skip to content

Commit 9731239

Browse files
richmahnrichmahn
committed
Fixes go-gitea#7474 - Handles all redirects for Web UI File CRUD (go-gitea#7478)
* Fixes go-gitea#7474 - Handles all redirects for Web UI File CRUD * Fixes lint errors * Typo fix * Adds unit tests for a few helper functions * Fixes per review * Fix for new branch creation and to unit test * Fixes the template used for errors on delete
1 parent a29e667 commit 9731239

File tree

5 files changed

+125
-19
lines changed

5 files changed

+125
-19
lines changed

‎options/locale/locale_en-US.ini‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,7 @@ editor.delete = Delete '%s'
694694
editor.commit_message_desc = Add an optional extended description…
695695
editor.commit_directly_to_this_branch = Commit directly to the <strong class="branch-name">%s</strong> branch.
696696
editor.create_new_branch = Create a <strong>new branch</strong> for this commit and start a pull request.
697+
editor.propose_file_change = Propose file change
697698
editor.new_branch_name_desc = New branch name…
698699
editor.cancel = Cancel
699700
editor.filename_cannot_be_empty = The filename cannot be empty.

‎public/js/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,6 +1275,7 @@ function initEditor() {
12751275
$('.quick-pull-branch-name').hide();
12761276
$('.quick-pull-branch-name input').prop('required',false);
12771277
}
1278+
$('#commit-button').text($(this).attr('button_text'));
12781279
});
12791280

12801281
var $editFilename = $("#file-name");

‎routers/repo/editor.go‎

Lines changed: 80 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"io/ioutil"
1010
"net/http"
1111
"path"
12+
"path/filepath"
1213
"strings"
1314

1415
"code.gitea.io/gitea/models"
@@ -137,7 +138,7 @@ func editFile(ctx *context.Context, isNewFile bool) {
137138
} else {
138139
ctx.Data["commit_choice"] = frmCommitChoiceNewBranch
139140
}
140-
ctx.Data["new_branch_name"] = ""
141+
ctx.Data["new_branch_name"] = GetUniquePatchBranchName(ctx)
141142
ctx.Data["last_commit"] = ctx.Repo.CommitID
142143
ctx.Data["MarkdownFileExts"] = strings.Join(setting.Markdown.FileExtensions, ",")
143144
ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",")
@@ -266,6 +267,10 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo
266267
} else {
267268
ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, err), tplEditFile, &form)
268269
}
270+
}
271+
272+
if form.CommitChoice == frmCommitChoiceNewBranch {
273+
ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + ctx.Repo.BranchName + "..." + form.NewBranchName)
269274
} else {
270275
ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(branchName) + "/" + util.PathEscapeSegments(form.TreePath))
271276
}
@@ -335,7 +340,7 @@ func DeleteFile(ctx *context.Context) {
335340
} else {
336341
ctx.Data["commit_choice"] = frmCommitChoiceNewBranch
337342
}
338-
ctx.Data["new_branch_name"] = ""
343+
ctx.Data["new_branch_name"] = GetUniquePatchBranchName(ctx)
339344

340345
ctx.HTML(200, tplDeleteFile)
341346
}
@@ -362,7 +367,7 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
362367
return
363368
}
364369

365-
if branchName != ctx.Repo.BranchName && !canCommit {
370+
if branchName == ctx.Repo.BranchName && !canCommit {
366371
ctx.Data["Err_NewBranchName"] = true
367372
ctx.Data["commit_choice"] = frmCommitChoiceNewBranch
368373
ctx.RenderWithErr(ctx.Tr("repo.editor.cannot_commit_to_protected_branch", branchName), tplDeleteFile, &form)
@@ -387,20 +392,20 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
387392
}); err != nil {
388393
// This is where we handle all the errors thrown by repofiles.DeleteRepoFile
389394
if git.IsErrNotExist(err) || models.IsErrRepoFileDoesNotExist(err) {
390-
ctx.RenderWithErr(ctx.Tr("repo.editor.file_deleting_no_longer_exists", ctx.Repo.TreePath), tplEditFile, &form)
395+
ctx.RenderWithErr(ctx.Tr("repo.editor.file_deleting_no_longer_exists", ctx.Repo.TreePath), tplDeleteFile, &form)
391396
} else if models.IsErrFilenameInvalid(err) {
392397
ctx.Data["Err_TreePath"] = true
393-
ctx.RenderWithErr(ctx.Tr("repo.editor.filename_is_invalid", ctx.Repo.TreePath), tplEditFile, &form)
398+
ctx.RenderWithErr(ctx.Tr("repo.editor.filename_is_invalid", ctx.Repo.TreePath), tplDeleteFile, &form)
394399
} else if models.IsErrFilePathInvalid(err) {
395400
ctx.Data["Err_TreePath"] = true
396401
if fileErr, ok := err.(models.ErrFilePathInvalid); ok {
397402
switch fileErr.Type {
398403
case git.EntryModeSymlink:
399-
ctx.RenderWithErr(ctx.Tr("repo.editor.file_is_a_symlink", fileErr.Path), tplEditFile, &form)
404+
ctx.RenderWithErr(ctx.Tr("repo.editor.file_is_a_symlink", fileErr.Path), tplDeleteFile, &form)
400405
case git.EntryModeTree:
401-
ctx.RenderWithErr(ctx.Tr("repo.editor.filename_is_a_directory", fileErr.Path), tplEditFile, &form)
406+
ctx.RenderWithErr(ctx.Tr("repo.editor.filename_is_a_directory", fileErr.Path), tplDeleteFile, &form)
402407
case git.EntryModeBlob:
403-
ctx.RenderWithErr(ctx.Tr("repo.editor.directory_is_a_file", fileErr.Path), tplEditFile, &form)
408+
ctx.RenderWithErr(ctx.Tr("repo.editor.directory_is_a_file", fileErr.Path), tplDeleteFile, &form)
404409
default:
405410
ctx.ServerError("DeleteRepoFile", err)
406411
}
@@ -410,25 +415,44 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
410415
} else if git.IsErrBranchNotExist(err) {
411416
// For when a user deletes a file to a branch that no longer exists
412417
if branchErr, ok := err.(git.ErrBranchNotExist); ok {
413-
ctx.RenderWithErr(ctx.Tr("repo.editor.branch_does_not_exist", branchErr.Name), tplEditFile, &form)
418+
ctx.RenderWithErr(ctx.Tr("repo.editor.branch_does_not_exist", branchErr.Name), tplDeleteFile, &form)
414419
} else {
415420
ctx.Error(500, err.Error())
416421
}
417422
} else if models.IsErrBranchAlreadyExists(err) {
418423
// For when a user specifies a new branch that already exists
419424
if branchErr, ok := err.(models.ErrBranchAlreadyExists); ok {
420-
ctx.RenderWithErr(ctx.Tr("repo.editor.branch_already_exists", branchErr.BranchName), tplEditFile, &form)
425+
ctx.RenderWithErr(ctx.Tr("repo.editor.branch_already_exists", branchErr.BranchName), tplDeleteFile, &form)
421426
} else {
422427
ctx.Error(500, err.Error())
423428
}
424429
} else if models.IsErrCommitIDDoesNotMatch(err) {
425-
ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_editing", ctx.Repo.RepoLink+"/compare/"+form.LastCommit+"..."+ctx.Repo.CommitID), tplEditFile, &form)
430+
ctx.RenderWithErr(ctx.Tr("repo.editor.file_changed_while_deleting", ctx.Repo.RepoLink+"/compare/"+form.LastCommit+"..."+ctx.Repo.CommitID), tplDeleteFile, &form)
426431
} else {
427432
ctx.ServerError("DeleteRepoFile", err)
428433
}
434+
}
435+
436+
ctx.Flash.Success(ctx.Tr("repo.editor.file_delete_success", ctx.Repo.TreePath))
437+
if form.CommitChoice == frmCommitChoiceNewBranch {
438+
ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + ctx.Repo.BranchName + "..." + form.NewBranchName)
429439
} else {
430-
ctx.Flash.Success(ctx.Tr("repo.editor.file_delete_success", ctx.Repo.TreePath))
431-
ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(branchName))
440+
treePath := filepath.Dir(ctx.Repo.TreePath)
441+
if treePath == "." {
442+
treePath = "" // the file deleted was in the root, so we return the user to the root directory
443+
}
444+
if len(treePath) > 0 {
445+
// Need to get the latest commit since it changed
446+
commit, err := ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.BranchName)
447+
if err == nil && commit != nil {
448+
// We have the comment, now find what directory we can return the user to
449+
// (must have entries)
450+
treePath = GetClosestParentWithFiles(treePath, commit)
451+
} else {
452+
treePath = "" // otherwise return them to the root of the repo
453+
}
454+
}
455+
ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(branchName) + "/" + util.PathEscapeSegments(treePath))
432456
}
433457
}
434458

@@ -467,7 +491,7 @@ func UploadFile(ctx *context.Context) {
467491
} else {
468492
ctx.Data["commit_choice"] = frmCommitChoiceNewBranch
469493
}
470-
ctx.Data["new_branch_name"] = ""
494+
ctx.Data["new_branch_name"] = GetUniquePatchBranchName(ctx)
471495

472496
ctx.HTML(200, tplUploadFile)
473497
}
@@ -565,7 +589,11 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) {
565589
return
566590
}
567591

568-
ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(branchName) + "/" + util.PathEscapeSegments(form.TreePath))
592+
if form.CommitChoice == frmCommitChoiceNewBranch {
593+
ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + ctx.Repo.BranchName + "..." + form.NewBranchName)
594+
} else {
595+
ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(branchName) + "/" + util.PathEscapeSegments(form.TreePath))
596+
}
569597
}
570598

571599
func cleanUploadFileName(name string) string {
@@ -645,3 +673,40 @@ func RemoveUploadFileFromServer(ctx *context.Context, form auth.RemoveUploadFile
645673
log.Trace("Upload file removed: %s", form.File)
646674
ctx.Status(204)
647675
}
676+
677+
// GetUniquePatchBranchName Gets a unique branch name for a new patch branch
678+
// It will be in the form of <username>-patch-<num> where <num> is the first branch of this format
679+
// that doesn't already exist. If we exceed 1000 tries or an error is thrown, we just return "" so the user has to
680+
// type in the branch name themselves (will be an empty field)
681+
func GetUniquePatchBranchName(ctx *context.Context) string {
682+
prefix := ctx.User.LowerName + "-patch-"
683+
for i := 1; i <= 1000; i++ {
684+
branchName := fmt.Sprintf("%s%d", prefix, i)
685+
if _, err := ctx.Repo.Repository.GetBranch(branchName); err != nil {
686+
if git.IsErrBranchNotExist(err) {
687+
return branchName
688+
}
689+
log.Error("GetUniquePatchBranchName: %v", err)
690+
return ""
691+
}
692+
}
693+
return ""
694+
}
695+
696+
// GetClosestParentWithFiles Recursively gets the path of parent in a tree that has files (used when file in a tree is
697+
// deleted). Returns "" for the root if no parents other than the root have files. If the given treePath isn't a
698+
// SubTree or it has no entries, we go up one dir and see if we can return the user to that listing.
699+
func GetClosestParentWithFiles(treePath string, commit *git.Commit) string {
700+
if len(treePath) == 0 || treePath == "." {
701+
return ""
702+
}
703+
// see if the tree has entries
704+
if tree, err := commit.SubTree(treePath); err != nil {
705+
// failed to get tree, going up a dir
706+
return GetClosestParentWithFiles(filepath.Dir(treePath), commit)
707+
} else if entries, err := tree.ListEntries(); err != nil || len(entries) == 0 {
708+
// no files in this dir, going up a dir
709+
return GetClosestParentWithFiles(filepath.Dir(treePath), commit)
710+
}
711+
return treePath
712+
}

‎routers/repo/editor_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
package repo
66

77
import (
8+
"code.gitea.io/gitea/modules/git"
9+
"code.gitea.io/gitea/modules/test"
810
"testing"
911

1012
"code.gitea.io/gitea/models"
@@ -37,3 +39,40 @@ func TestCleanUploadName(t *testing.T) {
3739
assert.EqualValues(t, cleanUploadFileName(k), v)
3840
}
3941
}
42+
43+
func TestGetUniquePatchBranchName(t *testing.T) {
44+
models.PrepareTestEnv(t)
45+
ctx := test.MockContext(t, "user2/repo1")
46+
ctx.SetParams(":id", "1")
47+
test.LoadRepo(t, ctx, 1)
48+
test.LoadRepoCommit(t, ctx)
49+
test.LoadUser(t, ctx, 2)
50+
test.LoadGitRepo(t, ctx)
51+
expectedBranchName := "user2-patch-1"
52+
branchName := GetUniquePatchBranchName(ctx)
53+
assert.Equal(t, expectedBranchName, branchName)
54+
}
55+
56+
func TestGetClosestParentWithFiles(t *testing.T) {
57+
models.PrepareTestEnv(t)
58+
ctx := test.MockContext(t, "user2/repo1")
59+
ctx.SetParams(":id", "1")
60+
test.LoadRepo(t, ctx, 1)
61+
test.LoadRepoCommit(t, ctx)
62+
test.LoadUser(t, ctx, 2)
63+
test.LoadGitRepo(t, ctx)
64+
repo := ctx.Repo.Repository
65+
branch := repo.DefaultBranch
66+
gitRepo, _ := git.OpenRepository(repo.RepoPath())
67+
commit, _ := gitRepo.GetBranchCommit(branch)
68+
expectedTreePath := ""
69+
70+
expectedTreePath = "" // Should return the root dir, empty string, since there are no subdirs in this repo
71+
for _, deletedFile := range []string{
72+
"dir1/dir2/dir3/file.txt",
73+
"file.txt",
74+
} {
75+
treePath := GetClosestParentWithFiles(deletedFile, commit)
76+
assert.Equal(t, expectedTreePath, treePath)
77+
}
78+
}

‎templates/repo/editor/commit_form.tmpl‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
<div class="quick-pull-choice js-quick-pull-choice">
1212
<div class="field">
1313
<div class="ui radio checkbox {{if not .CanCommitToBranch}}disabled{{end}}">
14-
<input type="radio" class="js-quick-pull-choice-option" name="commit_choice" value="direct" {{if eq .commit_choice "direct"}}checked{{end}}>
14+
<input type="radio" class="js-quick-pull-choice-option" name="commit_choice" value="direct" button_text="{{.i18n.Tr "repo.editor.commit_changes"}}" {{if eq .commit_choice "direct"}}checked{{end}}>
1515
<label>
1616
<i class="octicon octicon-git-commit" height="16" width="14"></i>
1717
{{.i18n.Tr "repo.editor.commit_directly_to_this_branch" (.BranchName|Escape) | Safe}}
@@ -20,7 +20,7 @@
2020
</div>
2121
<div class="field">
2222
<div class="ui radio checkbox">
23-
<input type="radio" class="js-quick-pull-choice-option" name="commit_choice" value="commit-to-new-branch" {{if eq .commit_choice "commit-to-new-branch"}}checked{{end}}>
23+
<input type="radio" class="js-quick-pull-choice-option" name="commit_choice" value="commit-to-new-branch" button_text="{{.i18n.Tr "repo.editor.propose_file_change"}}" {{if eq .commit_choice "commit-to-new-branch"}}checked{{end}}>
2424
<label>
2525
<i class="octicon octicon-git-pull-request" height="16" width="12"></i>
2626
{{.i18n.Tr "repo.editor.create_new_branch" | Safe}}
@@ -36,8 +36,8 @@
3636
</div>
3737
</div>
3838
</div>
39-
<button type="submit" class="ui green button">
40-
{{.i18n.Tr "repo.editor.commit_changes"}}
39+
<button id="commit-button" type="submit" class="ui green button">
40+
{{if eq .commit_choice "commit-to-new-branch"}}{{.i18n.Tr "repo.editor.propose_file_change"}}{{else}}{{.i18n.Tr "repo.editor.commit_changes"}}{{end}}
4141
</button>
4242
<a class="ui button red" href="{{EscapePound $.BranchLink}}/{{EscapePound .TreePath}}">{{.i18n.Tr "repo.editor.cancel"}}</a>
4343
</div>

0 commit comments

Comments
 (0)