diff --git a/models/git/branch.go b/models/git/branch.go index 7954efb1cb7..2418c45aa19 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -208,23 +208,11 @@ func AddBranches(ctx context.Context, branches []*Branch) error { func GetDeletedBranchByID(ctx context.Context, repoID, branchID int64) (*Branch, error) { var branch Branch - has, err := db.GetEngine(ctx).ID(branchID).Get(&branch) + has, err := db.GetEngine(ctx).ID(branchID).Where("repo_id=? AND is_deleted=?", repoID, true).Get(&branch) if err != nil { return nil, err } else if !has { - return nil, ErrBranchNotExist{ - RepoID: repoID, - } - } - if branch.RepoID != repoID { - return nil, ErrBranchNotExist{ - RepoID: repoID, - } - } - if !branch.IsDeleted { - return nil, ErrBranchNotExist{ - RepoID: repoID, - } + return nil, ErrBranchNotExist{RepoID: repoID} } return &branch, nil } diff --git a/modules/git/gitcmd/pipe.go b/modules/git/gitcmd/pipe.go index d0ce3e2dc63..95b1a9205f5 100644 --- a/modules/git/gitcmd/pipe.go +++ b/modules/git/gitcmd/pipe.go @@ -9,14 +9,6 @@ import ( ) type PipeBufferReader interface { - // Read should be used in the same goroutine as command's Wait - // When Reader in one goroutine, command's Wait in another goroutine, then the command exits, the pipe will be closed: - // * If the Reader goroutine reads faster, it will read all remaining data and then get io.EOF - // * But this io.EOF doesn't mean the Reader has gotten complete data, the data might still be corrupted - // * If the Reader goroutine reads slower, it will get os.ErrClosed because the os.Pipe is closed ahead when the command exits - // - // When using 2 goroutines, no clear solution to distinguish these two cases or make Reader knows whether the data is complete - // It should avoid using Reader in a different goroutine than the command if the Read error needs to be handled. Read(p []byte) (n int, err error) Bytes() []byte } @@ -26,6 +18,15 @@ type PipeBufferWriter interface { Bytes() []byte } +// PipeReader should be used in the same goroutine as command's Wait +// When Reader in one goroutine, command's Wait in another goroutine, then the command exits, the pipe will be closed: +// * If the Reader goroutine reads faster, it will read all remaining data and then get io.EOF +// - But this io.EOF doesn't mean the Reader has gotten complete data, the data might still be corrupted +// +// * If the Reader goroutine reads slower, it will get os.ErrClosed because the os.Pipe is closed ahead when the command exits +// +// When using 2 goroutines, no clear solution to distinguish these two cases or make Reader knows whether the data is complete +// It should avoid using Reader in a different goroutine than the command if the Read error needs to be handled. type PipeReader interface { io.ReadCloser internalOnly() diff --git a/routers/web/admin/packages.go b/routers/web/admin/packages.go index e81f66d1b8a..64322ac1823 100644 --- a/routers/web/admin/packages.go +++ b/routers/web/admin/packages.go @@ -5,7 +5,6 @@ package admin import ( "net/http" - "net/url" "time" "gitea.dev/models/db" @@ -93,7 +92,7 @@ func DeletePackageVersion(ctx *context.Context) { } ctx.Flash.Success(ctx.Tr("packages.settings.delete.version.success")) - ctx.JSONRedirect(setting.AppSubURL + "/-/admin/packages?page=" + url.QueryEscape(ctx.FormString("page")) + "&q=" + url.QueryEscape(ctx.FormString("q")) + "&type=" + url.QueryEscape(ctx.FormString("type"))) + ctx.JSONRedirect("") } func CleanupExpiredData(ctx *context.Context) { diff --git a/routers/web/admin/repos.go b/routers/web/admin/repos.go index 9eea7e08717..16c9bebaca3 100644 --- a/routers/web/admin/repos.go +++ b/routers/web/admin/repos.go @@ -47,10 +47,6 @@ func DeleteRepo(ctx *context.Context) { return } - if ctx.Repo != nil && ctx.Repo.GitRepo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == repo.ID { - ctx.Repo.GitRepo.Close() - } - if err := repo_service.DeleteRepository(ctx, ctx.Doer, repo, true); err != nil { ctx.ServerError("DeleteRepository", err) return @@ -58,7 +54,7 @@ func DeleteRepo(ctx *context.Context) { log.Trace("Repository deleted: %s", repo.FullName()) ctx.Flash.Success(ctx.Tr("repo.settings.deletion_success")) - ctx.JSONRedirect(setting.AppSubURL + "/-/admin/repos?page=" + url.QueryEscape(ctx.FormString("page")) + "&sort=" + url.QueryEscape(ctx.FormString("sort"))) + ctx.JSONRedirect("") } // UnadoptedRepos lists the unadopted repositories diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index 3ba0b8a869b..b3c7cc0eb67 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "net/http" - "net/url" "strings" git_model "gitea.dev/models/git" @@ -87,48 +86,31 @@ func Branches(ctx *context.Context) { ctx.HTML(http.StatusOK, tplBranch) } -// DeleteBranchPost responses for delete merged branch func DeleteBranchPost(ctx *context.Context) { - defer jsonRedirectBranches(ctx) branchName := ctx.FormString("name") - - if err := repo_service.DeleteBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, branchName); err != nil { - switch { - case git.IsErrBranchNotExist(err): - log.Debug("DeleteBranch: Can't delete non existing branch '%s'", branchName) - ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName)) - case errors.Is(err, repo_service.ErrBranchIsDefault): - log.Debug("DeleteBranch: Can't delete default branch '%s'", branchName) - ctx.Flash.Error(ctx.Tr("repo.branch.default_deletion_failed", branchName)) - case errors.Is(err, git_model.ErrBranchIsProtected): - log.Debug("DeleteBranch: Can't delete protected branch '%s'", branchName) - ctx.Flash.Error(ctx.Tr("repo.branch.protected_deletion_failed", branchName)) - default: - log.Error("DeleteBranch: %v", err) - ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", branchName)) - } - - return + err := repo_service.DeleteBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, branchName) + switch { + case err == nil: + ctx.Flash.Success(ctx.Tr("repo.branch.deletion_success", branchName)) + ctx.JSONRedirect("") + case git.IsErrBranchNotExist(err): + ctx.JSONError(ctx.Tr("repo.branch.deletion_failed", branchName)) + case errors.Is(err, repo_service.ErrBranchIsDefault): + ctx.JSONError(ctx.Tr("repo.branch.default_deletion_failed", branchName)) + case errors.Is(err, git_model.ErrBranchIsProtected): + ctx.JSONError(ctx.Tr("repo.branch.protected_deletion_failed", branchName)) + default: + log.Error("DeleteBranch: %v", err) + ctx.JSONError(ctx.Tr("repo.branch.deletion_failed", branchName)) } - - ctx.Flash.Success(ctx.Tr("repo.branch.deletion_success", branchName)) } -// RestoreBranchPost responses for delete merged branch func RestoreBranchPost(ctx *context.Context) { - defer jsonRedirectBranches(ctx) - branchID := ctx.FormInt64("branch_id") - branchName := ctx.FormString("name") deletedBranch, err := git_model.GetDeletedBranchByID(ctx, ctx.Repo.Repository.ID, branchID) if err != nil { - log.Error("GetDeletedBranchByID: %v", err) - ctx.Flash.Error(ctx.Tr("repo.branch.restore_failed", branchName)) - return - } else if deletedBranch == nil { - log.Debug("RestoreBranch: Can't restore branch[%d] '%s', as it does not exist", branchID, branchName) - ctx.Flash.Error(ctx.Tr("repo.branch.restore_failed", branchName)) + ctx.JSONErrorAuto(err) return } @@ -138,11 +120,11 @@ func RestoreBranchPost(ctx *context.Context) { }); err != nil { if strings.Contains(err.Error(), "already exists") { log.Debug("RestoreBranch: Can't restore branch '%s', since one with same name already exist", deletedBranch.Name) - ctx.Flash.Error(ctx.Tr("repo.branch.already_exists", deletedBranch.Name)) + ctx.JSONError(ctx.Tr("repo.branch.already_exists", deletedBranch.Name)) return } log.Error("RestoreBranch: CreateBranch: %v", err) - ctx.Flash.Error(ctx.Tr("repo.branch.restore_failed", deletedBranch.Name)) + ctx.JSONError(ctx.Tr("repo.branch.restore_failed", deletedBranch.Name)) return } @@ -163,10 +145,7 @@ func RestoreBranchPost(ctx *context.Context) { } ctx.Flash.Success(ctx.Tr("repo.branch.restore_success", deletedBranch.Name)) -} - -func jsonRedirectBranches(ctx *context.Context) { - ctx.JSONRedirect(ctx.Repo.RepoLink + "/branches?page=" + url.QueryEscape(ctx.FormString("page"))) + ctx.JSONRedirect("") } // CreateBranch creates new branch in repository diff --git a/routers/web/user/notification.go b/routers/web/user/notification.go index 3d315d6df23..21eb08a34e0 100644 --- a/routers/web/user/notification.go +++ b/routers/web/user/notification.go @@ -140,6 +140,7 @@ func prepareUserNotificationsData(ctx *context.Context) { pager.RemoveParam(container.SetOf("div-only", "sequence-number")) ctx.Data["Page"] = pager + ctx.Data["PageQueryParams"] = templates.QueryBuild(pager.GetParams(), "page", page) } func filterNotificationsByRepoAccess(ctx stdCtx.Context, doer *user_model.User, notifications activities_model.NotificationList) (activities_model.NotificationList, []int, error) { diff --git a/templates/admin/packages/list.tmpl b/templates/admin/packages/list.tmpl index df322294d30..1b3bcb46177 100644 --- a/templates/admin/packages/list.tmpl +++ b/templates/admin/packages/list.tmpl @@ -51,15 +51,10 @@ - {{range .PackageDescriptors}} + {{range $pd := .PackageDescriptors}} {{.Version.ID}} - - {{.Owner.Name}} - {{if .Owner.Visibility.IsPrivate}} - {{svg "octicon-lock"}} - {{end}} - + {{template "admin/shared/owner" dict "OwnerUser" $pd.Owner}} {{.Package.Type.Name}} {{.Package.Name}} {{.Version.Version}} @@ -73,8 +68,8 @@ {{DateUtils.AbsoluteShort .Version.CreatedUnix}} {{svg "octicon-trash"}} diff --git a/templates/admin/repo/list.tmpl b/templates/admin/repo/list.tmpl index 244974d14b8..8e9b8937968 100644 --- a/templates/admin/repo/list.tmpl +++ b/templates/admin/repo/list.tmpl @@ -43,15 +43,10 @@ - {{range .Repos}} + {{range $repo := .Repos}} {{.ID}} - - {{.Owner.Name}} - {{if .Owner.Visibility.IsPrivate}} - {{svg "octicon-lock"}} - {{end}} - + {{template "admin/shared/owner" dict "OwnerUser" $repo.Owner}} {{.Name}} {{if .IsArchived}} @@ -59,7 +54,7 @@ {{end}} {{if .IsPrivate}} {{ctx.Locale.Tr "repo.desc.private"}} - {{else}} + {{else if .Owner}} {{if .Owner.Visibility.IsPrivate}} {{ctx.Locale.Tr "repo.desc.internal"}} {{end}} @@ -86,8 +81,8 @@ {{DateUtils.AbsoluteShort .CreatedUnix}} {{svg "octicon-trash"}} diff --git a/templates/admin/shared/owner.tmpl b/templates/admin/shared/owner.tmpl new file mode 100644 index 00000000000..9e2667d95e7 --- /dev/null +++ b/templates/admin/shared/owner.tmpl @@ -0,0 +1,5 @@ +{{$ownerUser := $.OwnerUser}} +{{if $ownerUser}} + {{$ownerUser.Name}} + {{if $ownerUser.Visibility.IsPrivate}}{{svg "octicon-lock"}}{{end}} +{{end}} diff --git a/templates/org/settings/options.tmpl b/templates/org/settings/options.tmpl index 614e861b09f..939f5072f6b 100644 --- a/templates/org/settings/options.tmpl +++ b/templates/org/settings/options.tmpl @@ -1,6 +1,6 @@ {{template "org/settings/layout_head" (dict "pageClass" "organization settings options")}} -
+

{{ctx.Locale.Tr "org.settings.options"}}

@@ -65,8 +65,8 @@
- {{template "org/settings/options_dangerzone" .}} + {{template "org/settings/layout_footer" .}} diff --git a/templates/repo/branch/list.tmpl b/templates/repo/branch/list.tmpl index 28b40e2ebf0..f95361b3502 100644 --- a/templates/repo/branch/list.tmpl +++ b/templates/repo/branch/list.tmpl @@ -54,7 +54,7 @@ {{end}} {{if and $.IsWriter $.Repository.CanContentChange (not .IsDeleted)}} - {{else}} - {{end}} @@ -256,11 +256,9 @@
{{ctx.Locale.Tr "repo.branch.warning_rename_default_branch"}}
+
- -
- -
+
diff --git a/templates/user/notification/notification_div.tmpl b/templates/user/notification/notification_div.tmpl index 4305f3fdd6f..258473c354b 100644 --- a/templates/user/notification/notification_div.tmpl +++ b/templates/user/notification/notification_div.tmpl @@ -53,7 +53,7 @@ {{DateUtils.TimeSince $one.UpdatedUnix}} {{end}} -
diff --git a/tests/integration/api_helper_for_declarative_test.go b/tests/integration/api_helper_for_declarative_test.go index f9d36d29985..4822512e213 100644 --- a/tests/integration/api_helper_for_declarative_test.go +++ b/tests/integration/api_helper_for_declarative_test.go @@ -212,12 +212,12 @@ func doAPICreateDeployKey(ctx APITestContext, keyname, keyFile string, readOnly } } -func doAPICreatePullRequest(ctx APITestContext, owner, repo, baseBranch, headBranch string) func(*testing.T) (api.PullRequest, error) { +func doAPICreatePullRequest(ctx APITestContext, owner, repo, baseBranch, headOwnerBranch string) func(*testing.T) (api.PullRequest, error) { return func(t *testing.T) (api.PullRequest, error) { req := NewRequestWithJSON(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls", owner, repo), &api.CreatePullRequestOption{ - Head: headBranch, + Head: headOwnerBranch, Base: baseBranch, - Title: fmt.Sprintf("create a pr from %s to %s", headBranch, baseBranch), + Title: fmt.Sprintf("create a pr from %s to %s", headOwnerBranch, baseBranch), }).AddTokenAuth(ctx.Token) expected := http.StatusCreated diff --git a/tests/integration/git_general_test.go b/tests/integration/git_general_test.go index 43fbb78bfef..f96e5ea752f 100644 --- a/tests/integration/git_general_test.go +++ b/tests/integration/git_general_test.go @@ -32,6 +32,7 @@ import ( "gitea.dev/modules/lfs" "gitea.dev/modules/setting" api "gitea.dev/modules/structs" + "gitea.dev/modules/test" "gitea.dev/tests" "github.com/kballard/go-shellquote" @@ -543,15 +544,15 @@ func doProtectBranchExt(ctx APITestContext, ruleName string, opts doProtectBranc } } -func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) func(t *testing.T) { +func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headOwnerBranch string) func(t *testing.T) { return func(t *testing.T) { defer tests.PrintCurrentTest(t)() var pr api.PullRequest var err error - // Create a test pullrequest + // Create a test pull request t.Run("CreatePullRequest", func(t *testing.T) { - pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headBranch)(t) + pr, err = doAPICreatePullRequest(ctx, baseCtx.Username, baseCtx.Reponame, baseBranch, headOwnerBranch)(t) assert.NoError(t, err) }) @@ -578,10 +579,14 @@ func doMergeFork(ctx, baseCtx APITestContext, baseBranch, headBranch string) fun t.Run("EnsurDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffContent)) // Then: Delete the head branch & make sure that doesn't break the PR page or change its diff - t.Run("DeleteHeadBranch", doBranchDelete(baseCtx, baseCtx.Username, baseCtx.Reponame, headBranch)) - t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr)) - t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffContent)) - + // FIXME: this test (from #10936) is not right, the "master" branch can't be deleted + _ = doBranchDelete + /* + _, headBranch, _ := strings.Cut(headOwnerBranch, ":") + t.Run("DeleteHeadBranch", doBranchDelete(baseCtx, baseCtx.Username, baseCtx.Reponame, headBranch)) + t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr)) + t.Run("EnsureDiffNoChange", doEnsureDiffNoChange(baseCtx, pr, diffContent)) + */ // Delete the head repository & make sure that doesn't break the PR page or change its diff t.Run("DeleteHeadRepository", doAPIDeleteRepository(ctx)) t.Run("EnsureCanSeePull", doEnsureCanSeePull(baseCtx, pr)) @@ -621,11 +626,14 @@ func doCreatePRAndSetManuallyMerged(ctx, baseCtx APITestContext, dstPath, baseBr func doEnsureCanSeePull(ctx APITestContext, pr api.PullRequest) func(t *testing.T) { return 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) + resp := ctx.Session.MakeRequest(t, req, http.StatusOK) + assert.True(t, test.IsNormalPageCompleted(resp.Body.String())) 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) + assert.True(t, test.IsNormalPageCompleted(resp.Body.String())) 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) + resp = ctx.Session.MakeRequest(t, req, http.StatusOK) + assert.True(t, test.IsNormalPageCompleted(resp.Body.String())) } } diff --git a/web_src/js/features/repo-branch.ts b/web_src/js/features/repo-branch.ts index befcebfc486..837ed79b3ba 100644 --- a/web_src/js/features/repo-branch.ts +++ b/web_src/js/features/repo-branch.ts @@ -1,6 +1,7 @@ import {toggleElem} from '../utils/dom.ts'; import {showFomanticModal} from '../modules/fomantic/modal.ts'; import {trString} from '../modules/i18n.ts'; +import {registerGlobalEventFunc} from '../modules/observer.ts'; export function initRepoBranchButton() { initRepoCreateBranchButton(); @@ -25,19 +26,17 @@ function initRepoCreateBranchButton() { } function initRepoRenameBranchButton() { - for (const el of document.querySelectorAll('.show-rename-branch-modal')) { - el.addEventListener('click', () => { - const target = el.getAttribute('data-modal')!; - const modal = document.querySelector(target)!; - const oldBranchName = el.getAttribute('data-old-branch-name')!; - modal.querySelector('input[name=from]')!.value = oldBranchName; + registerGlobalEventFunc('click', 'showRenameBranchModal', (el) => { + const target = el.getAttribute('data-modal')!; + const modal = document.querySelector(target)!; + const oldBranchName = el.getAttribute('data-old-branch-name')!; + modal.querySelector('input[name=from]')!.value = oldBranchName; - // display the warning that the branch which is chosen is the default branch - const warn = modal.querySelector('.default-branch-warning')!; - toggleElem(warn, el.getAttribute('data-is-default-branch') === 'true'); + // display the warning that the branch which is chosen is the default branch + const warn = modal.querySelector('.default-branch-warning')!; + toggleElem(warn, el.getAttribute('data-is-default-branch') === 'true'); - const text = modal.querySelector('[data-rename-branch-to]')!; - text.textContent = trString(text.getAttribute('data-rename-branch-to')!, oldBranchName); - }); - } + const text = modal.querySelector('[data-rename-branch-to]')!; + text.textContent = trString(text.getAttribute('data-rename-branch-to')!, oldBranchName); + }); } diff --git a/web_src/js/modules/fomantic/modal.ts b/web_src/js/modules/fomantic/modal.ts index 58fdb3d6905..e05e7607ea7 100644 --- a/web_src/js/modules/fomantic/modal.ts +++ b/web_src/js/modules/fomantic/modal.ts @@ -67,8 +67,8 @@ function onModalApproveDefault(this: HTMLElement) { const $modal = $(this); const selectors = $modal.modal('setting', 'selector'); const elModal = $modal[0]; - const elApprove = elModal.querySelector(selectors.approve); - const elForm = elApprove?.closest('form'); + const elApprove = elModal.querySelector(selectors.approve); + const elForm = elApprove?.closest('form'); if (!elForm) return true; // no form, just allow closing the modal // "form-fetch-action" can handle network errors gracefully, @@ -78,6 +78,7 @@ function onModalApproveDefault(this: HTMLElement) { // There is an abuse for the "modal" + "form" combination, the "Approve" button is a traditional form submit button in the form. // Then "approve" and "submit" occur at the same time, the modal will be closed immediately before the form is submitted. // So here we prevent the modal from closing automatically by returning false, add the "is-loading" class to the form element. + if (!elForm.reportValidity()) return false; elForm.classList.add('is-loading'); return false; }