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

Fixes #7474 - Handles all redirects for Web UI File CRUD #7478

Merged
merged 10 commits into from
Jul 17, 2019
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,7 @@ editor.delete = Delete '%s'
editor.commit_message_desc = Add an optional extended description…
editor.commit_directly_to_this_branch = Commit directly to the <strong class="branch-name">%s</strong> branch.
editor.create_new_branch = Create a <strong>new branch</strong> for this commit and start a pull request.
editor.propose_file_change = Propose file change
editor.new_branch_name_desc = New branch name…
editor.cancel = Cancel
editor.filename_cannot_be_empty = The filename cannot be empty.
Expand Down
1 change: 1 addition & 0 deletions public/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1306,6 +1306,7 @@ function initEditor() {
$('.quick-pull-branch-name').hide();
$('.quick-pull-branch-name input').prop('required',false);
}
$('#commit-button').text($(this).attr('button_text'));
});

const $editFilename = $("#file-name");
Expand Down
69 changes: 63 additions & 6 deletions routers/repo/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io/ioutil"
"path"
"path/filepath"
"strings"

"code.gitea.io/gitea/models"
Expand Down Expand Up @@ -137,7 +138,7 @@ func editFile(ctx *context.Context, isNewFile bool) {
} else {
ctx.Data["commit_choice"] = frmCommitChoiceNewBranch
}
ctx.Data["new_branch_name"] = ""
ctx.Data["new_branch_name"] = GetUniquePatchBranchName(ctx)
ctx.Data["last_commit"] = ctx.Repo.CommitID
ctx.Data["MarkdownFileExts"] = strings.Join(setting.Markdown.FileExtensions, ",")
ctx.Data["LineWrapExtensions"] = strings.Join(setting.Repository.Editor.LineWrapExtensions, ",")
Expand Down Expand Up @@ -266,6 +267,10 @@ func editFilePost(ctx *context.Context, form auth.EditRepoFileForm, isNewFile bo
} else {
ctx.RenderWithErr(ctx.Tr("repo.editor.fail_to_update_file", form.TreePath, err), tplEditFile, &form)
}
}

if form.CommitChoice == frmCommitChoiceNewBranch {
ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + ctx.Repo.BranchName + "..." + form.NewBranchName)
} else {
ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(branchName) + "/" + util.PathEscapeSegments(form.TreePath))
}
Expand Down Expand Up @@ -335,7 +340,7 @@ func DeleteFile(ctx *context.Context) {
} else {
ctx.Data["commit_choice"] = frmCommitChoiceNewBranch
}
ctx.Data["new_branch_name"] = ""
ctx.Data["new_branch_name"] = GetUniquePatchBranchName(ctx)

ctx.HTML(200, tplDeleteFile)
}
Expand Down Expand Up @@ -426,9 +431,23 @@ func DeleteFilePost(ctx *context.Context, form auth.DeleteRepoFileForm) {
} else {
ctx.ServerError("DeleteRepoFile", err)
}
}

ctx.Flash.Success(ctx.Tr("repo.editor.file_delete_success", ctx.Repo.TreePath))
if form.CommitChoice == frmCommitChoiceNewBranch {
ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + ctx.Repo.BranchName + "..." + form.NewBranchName)
} else {
ctx.Flash.Success(ctx.Tr("repo.editor.file_delete_success", ctx.Repo.TreePath))
ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(branchName))
treePath := filepath.Dir(ctx.Repo.TreePath)
if len(treePath) > 0 && treePath != "." {
// Need to get the latest commit since it changed
commit, err := ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.BranchName)
if err == nil && commit != nil {
treePath = GetClosestParentWithFiles(treePath, commit)
} else {
treePath = ""
}
}
ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(branchName) + "/" + util.PathEscapeSegments(treePath))
}
}

Expand Down Expand Up @@ -467,7 +486,7 @@ func UploadFile(ctx *context.Context) {
} else {
ctx.Data["commit_choice"] = frmCommitChoiceNewBranch
}
ctx.Data["new_branch_name"] = ""
ctx.Data["new_branch_name"] = GetUniquePatchBranchName(ctx)

ctx.HTML(200, tplUploadFile)
}
Expand Down Expand Up @@ -565,7 +584,11 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) {
return
}

ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(branchName) + "/" + util.PathEscapeSegments(form.TreePath))
if form.CommitChoice == frmCommitChoiceNewBranch {
ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + ctx.Repo.BranchName + "..." + form.NewBranchName)
} else {
ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + util.PathEscapeSegments(branchName) + "/" + util.PathEscapeSegments(form.TreePath))
}
}

func cleanUploadFileName(name string) string {
Expand Down Expand Up @@ -636,3 +659,37 @@ func RemoveUploadFileFromServer(ctx *context.Context, form auth.RemoveUploadFile
log.Trace("Upload file removed: %s", form.File)
ctx.Status(204)
}

// GetUniquePatchBranchName Gets a unique branch name for a new patch branch
// It will be in the form of <lowername>-patch-<num> where <num> is the first branch of this format
// that doesn't already exist
func GetUniquePatchBranchName(ctx *context.Context) string {
prefix := ctx.User.LowerName + "-patch-"
for i := 1; i <= 1000; i++ {
richmahn marked this conversation as resolved.
Show resolved Hide resolved
branchName := fmt.Sprintf("%s%d", prefix, i)
if _, err := ctx.Repo.Repository.GetBranch(branchName); err != nil {
if git.IsErrBranchNotExist(err) {
return branchName
}
return ""
richmahn marked this conversation as resolved.
Show resolved Hide resolved
}
}
return ""
}

// GetClosestParentWithFiles Recursively gets the path of parent in a tree that has files (used when file in a tree is
// deleted). Returns "" for the root if no parents other than the root have files
func GetClosestParentWithFiles(treePath string, commit *git.Commit) string {
if len(treePath) == 0 || treePath == "." {
return ""
}
// see if the tree has entries
if tree, err := commit.SubTree(treePath); err != nil {
// failed to get tree, going up a dir
return GetClosestParentWithFiles(filepath.Dir(treePath), commit)
richmahn marked this conversation as resolved.
Show resolved Hide resolved
} else if entries, err := tree.ListEntries(); err != nil || len(entries) == 0 {
// no files in this dir, going up a dir
return GetClosestParentWithFiles(filepath.Dir(treePath), commit)
}
return treePath
}
32 changes: 32 additions & 0 deletions routers/repo/editor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package repo

import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/test"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should go below with "code.gitea.io/gitea/models" per normal styling preferences

"testing"

"code.gitea.io/gitea/models"
Expand Down Expand Up @@ -37,3 +39,33 @@ func TestCleanUploadName(t *testing.T) {
assert.EqualValues(t, cleanUploadFileName(k), v)
}
}

func TestGetUniquePatchBranchName(t *testing.T) {
models.PrepareTestEnv(t)
ctx := test.MockContext(t, "user2/repo1")
ctx.SetParams(":id", "1")
test.LoadRepo(t, ctx, 1)
test.LoadRepoCommit(t, ctx)
test.LoadUser(t, ctx, 2)
test.LoadGitRepo(t, ctx)
expectedBranchName := "user2-patch-1"
branchName := GetUniquePatchBranchName(ctx)
assert.Equal(t, expectedBranchName, branchName)
}

func TestGetClosestParentWithFiles(t *testing.T) {
models.PrepareTestEnv(t)
ctx := test.MockContext(t, "user2/repo1")
ctx.SetParams(":id", "1")
test.LoadRepo(t, ctx, 1)
test.LoadRepoCommit(t, ctx)
test.LoadUser(t, ctx, 2)
test.LoadGitRepo(t, ctx)
repo := ctx.Repo.Repository
branch := repo.DefaultBranch
gitRepo, _ := git.OpenRepository(repo.RepoPath())
commit, _ := gitRepo.GetBranchCommit(branch)
expectedTreePath := ""
treePath := GetClosestParentWithFiles("dir/dir/dir", commit)
assert.Equal(t, expectedTreePath, treePath)
}
8 changes: 4 additions & 4 deletions templates/repo/editor/commit_form.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<div class="quick-pull-choice js-quick-pull-choice">
<div class="field">
<div class="ui radio checkbox {{if not .CanCommitToBranch}}disabled{{end}}">
<input type="radio" class="js-quick-pull-choice-option" name="commit_choice" value="direct" {{if eq .commit_choice "direct"}}checked{{end}}>
<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}}>
<label>
<i class="octicon octicon-git-commit" height="16" width="14"></i>
{{.i18n.Tr "repo.editor.commit_directly_to_this_branch" (.BranchName|Escape) | Safe}}
Expand All @@ -20,7 +20,7 @@
</div>
<div class="field">
<div class="ui radio checkbox">
<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}}>
<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}}>
<label>
<i class="octicon octicon-git-pull-request" height="16" width="12"></i>
{{.i18n.Tr "repo.editor.create_new_branch" | Safe}}
Expand All @@ -36,8 +36,8 @@
</div>
</div>
</div>
<button type="submit" class="ui green button">
{{.i18n.Tr "repo.editor.commit_changes"}}
<button id="commit-button" type="submit" class="ui green button">
{{if eq .commit_choice "commit-to-new-branch"}}{{.i18n.Tr "repo.editor.propose_file_change"}}{{else}}{{.i18n.Tr "repo.editor.commit_changes"}}{{end}}
</button>
<a class="ui button red" href="{{EscapePound $.BranchLink}}/{{EscapePound .TreePath}}">{{.i18n.Tr "repo.editor.cancel"}}</a>
</div>