Fix Repository transferring page (#37277)

While editing frontend, I found some inconsistencies while testing
transferring repositories:

- No button for accepting/rejecting/cancelling the transfer of an empty
repository.
- The `redirect_to` in `templates/repo/header.tmpl` is useless.
- There's no redirection when there's an error from `handleActionError`
in `routers/web/repo/repo.go`. Therefore, instead of flash message, a
blank page will be displayed.

This pr adds some commits to resolve all these issues.

Update: see the new changes
https://github.com/go-gitea/gitea/pull/37277#issuecomment-4276150232

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Giteabot <teabot@gitea.io>

options/locale/locale_en-US.json

@@ -224,6 +224,7 @@
   "error.occurred": "An error occurred",
   "error.report_message": "If you believe that this is a Gitea bug, please search for issues on <a href=\"%s\" target=\"_blank\">GitHub</a> or open a new issue if necessary.",
   "error.not_found": "The target couldn't be found.",
+  "error.permission_denied": "Permission denied.",
   "error.network_error": "Network error",
   "startpage.app_desc": "A painless, self-hosted Git service",
   "startpage.install": "Easy to install",
@@ -1066,8 +1067,8 @@
   "repo.transfer.accept_desc": "Transfer to \"%s\"",
   "repo.transfer.reject": "Reject Transfer",
   "repo.transfer.reject_desc": "Cancel transfer to \"%s\"",
-  "repo.transfer.no_permission_to_accept": "You do not have permission to accept this transfer.",
-  "repo.transfer.no_permission_to_reject": "You do not have permission to reject this transfer.",
+  "repo.transfer.is_transferring": "Transferring…",
+  "repo.transfer.is_transferring_prompt": "The repository is being transferred to %s",
   "repo.desc.private": "Private",
   "repo.desc.public": "Public",
   "repo.desc.public_access": "Public Access",

routers/web/repo/repo.go

@@ -302,12 +302,12 @@ func CreatePost(ctx *context.Context) {
 func handleActionError(ctx *context.Context, err error) {
 	switch {
 	case errors.Is(err, user_model.ErrBlockedUser):
-		ctx.Flash.Error(ctx.Tr("repo.action.blocked_user"))
+		ctx.JSONError(ctx.Tr("repo.action.blocked_user"))
 	case repo_service.IsRepositoryLimitReached(err):
 		limit := err.(repo_service.LimitReachedError).Limit
-		ctx.Flash.Error(ctx.TrN(limit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", limit))
+		ctx.JSONError(ctx.TrN(limit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", limit))
 	case errors.Is(err, util.ErrPermissionDenied):
-		ctx.HTTPError(http.StatusNotFound)
+		ctx.JSONError(ctx.Tr("error.permission_denied"))
 	default:
 		ctx.ServerError(fmt.Sprintf("Action (%s)", ctx.PathParam("action")), err)
 	}

routers/web/repo/transfer.go

@@ -12,7 +12,7 @@ func acceptTransfer(ctx *context.Context) {
 	err := repo_service.AcceptTransferOwnership(ctx, ctx.Repo.Repository, ctx.Doer)
 	if err == nil {
 		ctx.Flash.Success(ctx.Tr("repo.settings.transfer.success"))
-		ctx.Redirect(ctx.Repo.Repository.Link())
+		ctx.JSONRedirect(ctx.Repo.Repository.Link())
 		return
 	}
 	handleActionError(ctx, err)
@@ -22,7 +22,7 @@ func rejectTransfer(ctx *context.Context) {
 	err := repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
 	if err == nil {
 		ctx.Flash.Success(ctx.Tr("repo.settings.transfer.rejected"))
-		ctx.Redirect(ctx.Repo.Repository.Link())
+		ctx.JSONRedirect(ctx.Repo.Repository.Link())
 		return
 	}
 	handleActionError(ctx, err)

services/context/repo.go

@@ -411,8 +411,9 @@ func RedirectToRepo(ctx *Base, redirectRepoID int64) {
 	ctx.Redirect(path.Join(setting.AppSubURL, redirectPath), http.StatusMovedPermanently)
 }
 
-func repoAssignment(ctx *Context, repo *repo_model.Repository) {
+func repoAssignmentLegacy(ctx *Context, data *repoAssignmentPrepareDataStruct) {
 	var err error
+	repo := data.repo
 	if err = repo.LoadOwner(ctx); err != nil {
 		ctx.ServerError("LoadOwner", err)
 		return
@@ -459,13 +460,25 @@ func InitRepoPullRequestCtx(ctx *Context, base, head *repo_model.Repository) {
 	ctx.Data["PullRequestCtx"] = ctx.Repo.PullRequestCtx
 }
 
-// RepoAssignment returns a middleware to handle repository assignment
-func RepoAssignment(ctx *Context) {
+type repoAssignmentPrepareDataStruct struct {
+	ownerName string
+	repoName  string
+	repo      *repo_model.Repository
+}
+
+func repoAssignmentPreCheck(ctx *Context) {
 	if ctx.Data["Repository"] != nil {
 		setting.PanicInDevOrTesting("RepoAssignment should not be executed twice")
 	}
+	if ctx.Repo.GitRepo != nil {
+		setting.PanicInDevOrTesting("RepoAssignment: GitRepo should be nil")
+		_ = ctx.Repo.GitRepo.Close()
+		ctx.Repo.GitRepo = nil
+	}
+}
 
-	var err error
+func repoAssignmentPrepareData(ctx *Context) *repoAssignmentPrepareDataStruct {
+	// HINT: here it doesn't handle ".wiki" extension, it is handled in repoAssignmentAutoRedirectWiki, need to be refactored in the future
 	userName := ctx.PathParam("username")
 	repoName := ctx.PathParam("reponame")
 	repoName = strings.TrimSuffix(repoName, ".git")
@@ -474,7 +487,12 @@ func RepoAssignment(ctx *Context) {
 		repoName = strings.TrimSuffix(repoName, ".rss")
 		repoName = strings.TrimSuffix(repoName, ".atom")
 	}
+	return &repoAssignmentPrepareDataStruct{ownerName: userName, repoName: repoName}
+}
 
+func repoAssignmentPrepareOwner(ctx *Context, data *repoAssignmentPrepareDataStruct) {
+	var err error
+	userName := data.ownerName
 	// Check if the user is the same as the repository owner
 	if ctx.IsSigned && strings.EqualFold(ctx.Doer.LowerName, userName) {
 		ctx.Repo.Owner = ctx.Doer
@@ -504,7 +522,10 @@ func RepoAssignment(ctx *Context) {
 	}
 	ctx.ContextUser = ctx.Repo.Owner
 	ctx.Data["ContextUser"] = ctx.ContextUser
+}
 
+func repoAssignmentAutoRedirectWiki(ctx *Context, data *repoAssignmentPrepareDataStruct) {
+	userName, repoName := data.ownerName, data.repoName
 	// redirect link to wiki
 	if strings.HasSuffix(repoName, ".wiki") {
 		// ctx.Req.URL.Path does not have the preceding appSubURL - any redirect must have this added
@@ -524,7 +545,10 @@ func RepoAssignment(ctx *Context) {
 		ctx.Redirect(path.Join(setting.AppSubURL, redirectPath))
 		return
 	}
+}
 
+func repoAssignmentPrepareRepo(ctx *Context, data *repoAssignmentPrepareDataStruct) {
+	repoName := data.repoName
 	// Get repository.
 	repo, err := repo_model.GetRepositoryByName(ctx, ctx.Repo.Owner.ID, repoName)
 	if err != nil {
@@ -547,12 +571,11 @@ func RepoAssignment(ctx *Context) {
 		return
 	}
 	repo.Owner = ctx.Repo.Owner
+	data.repo = repo
+}
 
-	repoAssignment(ctx, repo)
-	if ctx.Written() {
-		return
-	}
-
+func repoAssignmentPrepareTemplateData(ctx *Context, data *repoAssignmentPrepareDataStruct) {
+	repo := data.repo
 	ctx.Repo.RepoLink = repo.Link()
 	ctx.Data["RepoLink"] = ctx.Repo.RepoLink
 	ctx.Data["FeedURL"] = ctx.Repo.RepoLink
@@ -645,33 +668,35 @@ func RepoAssignment(ctx *Context) {
 			return
 		}
 	}
+}
 
-	isHomeOrSettings := ctx.Link == ctx.Repo.RepoLink ||
-		ctx.Link == ctx.Repo.RepoLink+"/settings" ||
-		strings.HasPrefix(ctx.Link, ctx.Repo.RepoLink+"/settings/") ||
-		ctx.Link == ctx.Repo.RepoLink+"/-/migrate/status"
+func repoAssignmentIsHomeOrSettings(ctx *Context, data *repoAssignmentPrepareDataStruct) bool {
+	repoLink := data.repo.Link()
+	return ctx.Link == repoLink ||
+		strings.HasPrefix(ctx.Link+"/", repoLink+"/settings/") ||
+		ctx.Link == repoLink+"/-/migrate/status"
+}
 
+func repoAssignmentAutoRedirectNotReady(ctx *Context, data *repoAssignmentPrepareDataStruct) {
 	// Disable everything when the repo is being created
 	if ctx.Repo.Repository.IsBeingCreated() || ctx.Repo.Repository.IsBroken() {
-		if !isHomeOrSettings {
+		if !repoAssignmentIsHomeOrSettings(ctx, data) {
 			ctx.Redirect(ctx.Repo.RepoLink)
 		}
 		return
 	}
+}
 
-	if ctx.Repo.GitRepo != nil {
-		setting.PanicInDevOrTesting("RepoAssignment: GitRepo should be nil")
-		_ = ctx.Repo.GitRepo.Close()
-		ctx.Repo.GitRepo = nil
-	}
-
+func repoAssignmentPrepareGitRepo(ctx *Context, data *repoAssignmentPrepareDataStruct) {
+	var err error
+	repo := data.repo
 	ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo)
 	if err != nil {
 		if strings.Contains(err.Error(), "repository does not exist") || strings.Contains(err.Error(), "no such file or directory") {
 			log.Error("Repository %-v has a broken repository on the file system: %s Error: %v", ctx.Repo.Repository, ctx.Repo.Repository.RelativePath(), err)
 			ctx.Repo.Repository.MarkAsBrokenEmpty()
 			// Only allow access to base of repo or settings
-			if !isHomeOrSettings {
+			if !repoAssignmentIsHomeOrSettings(ctx, data) {
 				ctx.Redirect(ctx.Repo.RepoLink)
 			}
 			return
@@ -679,12 +704,12 @@ func RepoAssignment(ctx *Context) {
 		ctx.ServerError("RepoAssignment Invalid repo "+repo.FullName(), err)
 		return
 	}
+}
 
-	// Stop at this point when the repo is empty.
-	if ctx.Repo.Repository.IsEmpty {
+func repoAssignmentPrepareBranches(ctx *Context, data *repoAssignmentPrepareDataStruct) {
+	if data.repo.IsEmpty {
 		return
 	}
-
 	branchOpts := git_model.FindBranchOptions{
 		RepoID:          ctx.Repo.Repository.ID,
 		IsDeletedBranch: optional.Some(false),
@@ -706,7 +731,13 @@ func RepoAssignment(ctx *Context) {
 	}
 
 	ctx.Data["BranchesCount"] = branchesTotal
+}
 
+func repoAssignmentPreparePullRequests(ctx *Context, data *repoAssignmentPrepareDataStruct) {
+	repo := data.repo
+	if repo.IsEmpty {
+		return
+	}
 	// Pull request is allowed if this is a fork repository, and base repository accepts pull requests.
 	if repo.BaseRepo != nil && repo.BaseRepo.AllowsPulls(ctx) {
 		// TODO: this (and below) "BaseRepo" var is not clear and should be removed in the future
@@ -717,7 +748,9 @@ func RepoAssignment(ctx *Context) {
 		ctx.Data["BaseRepo"] = repo
 		InitRepoPullRequestCtx(ctx, repo, repo)
 	}
+}
 
+func repoAssignmentPrepareRepoTransfer(ctx *Context, data *repoAssignmentPrepareDataStruct) {
 	if ctx.Repo.Repository.Status == repo_model.RepositoryPendingTransfer {
 		repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, ctx.Repo.Repository)
 		if err != nil {
@@ -726,16 +759,17 @@ func RepoAssignment(ctx *Context) {
 		}
 
 		if err := repoTransfer.LoadAttributes(ctx); err != nil {
-			ctx.ServerError("LoadRecipient", err)
+			ctx.ServerError("LoadAttributes", err)
 			return
 		}
 
 		ctx.Data["RepoTransfer"] = repoTransfer
-		if ctx.Doer != nil {
-			ctx.Data["CanUserAcceptOrRejectTransfer"] = repoTransfer.CanUserAcceptOrRejectTransfer(ctx, ctx.Doer)
-		}
+		ctx.Data["CanUserAcceptOrRejectTransfer"] = ctx.Doer != nil && repoTransfer.CanUserAcceptOrRejectTransfer(ctx, ctx.Doer)
 	}
+}
 
+func repoAssignmentHandleGoGet(ctx *Context, data *repoAssignmentPrepareDataStruct) {
+	repo := data.repo
 	if ctx.FormString("go-get") == "1" {
 		ctx.Data["GoGetImport"] = ComposeGoGetImport(ctx, repo.Owner.Name, repo.Name)
 		fullURLPrefix := repo.HTMLURL() + "/src/branch/" + util.PathEscapeSegments(ctx.Repo.BranchName)
@@ -744,6 +778,32 @@ func RepoAssignment(ctx *Context) {
 	}
 }
 
+// RepoAssignment returns a middleware to handle repository assignment
+func RepoAssignment(ctx *Context) {
+	repoAssignmentPreCheck(ctx)
+
+	prepareData := repoAssignmentPrepareData(ctx)
+	funcs := []func(ctx *Context, data *repoAssignmentPrepareDataStruct){
+		repoAssignmentPrepareOwner,
+		repoAssignmentAutoRedirectWiki,
+		repoAssignmentPrepareRepo,
+		repoAssignmentLegacy,
+		repoAssignmentPrepareTemplateData,
+		repoAssignmentAutoRedirectNotReady,
+		repoAssignmentPrepareGitRepo,
+		repoAssignmentPrepareRepoTransfer,
+		repoAssignmentPrepareBranches,
+		repoAssignmentPreparePullRequests,
+		repoAssignmentHandleGoGet,
+	}
+	for _, f := range funcs {
+		f(ctx, prepareData)
+		if ctx.Written() {
+			return
+		}
+	}
+}
+
 const headRefName = "HEAD"
 
 func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool) string {

templates/repo/header.tmpl

@@ -39,21 +39,17 @@
 			</div>
 			{{if not (or .IsBeingCreated .IsBroken)}}
 				<div class="flex-text-block tw-flex-wrap">
-					{{if $.RepoTransfer}}
-						<form method="post" action="{{$.RepoLink}}/action/accept_transfer?redirect_to={{$.RepoLink}}">
-							<div class="flex-text-inline" data-tooltip-content="{{if $.CanUserAcceptOrRejectTransfer}}{{ctx.Locale.Tr "repo.transfer.accept_desc" $.RepoTransfer.Recipient.DisplayName}}{{else}}{{ctx.Locale.Tr "repo.transfer.no_permission_to_accept"}}{{end}}">
-								<button type="submit" class="ui compact small basic button {{if $.CanUserAcceptOrRejectTransfer}}primary {{end}} ok small"{{if not $.CanUserAcceptOrRejectTransfer}} disabled{{end}}>
-									{{ctx.Locale.Tr "repo.transfer.accept"}}
-								</button>
-							</div>
-						</form>
-						<form method="post" action="{{$.RepoLink}}/action/reject_transfer?redirect_to={{$.RepoLink}}">
-							<div class="flex-text-inline" data-tooltip-content="{{if $.CanUserAcceptOrRejectTransfer}}{{ctx.Locale.Tr "repo.transfer.reject_desc" $.RepoTransfer.Recipient.DisplayName}}{{else}}{{ctx.Locale.Tr "repo.transfer.no_permission_to_reject"}}{{end}}">
-								<button type="submit" class="ui compact small basic button {{if $.CanUserAcceptOrRejectTransfer}}red {{end}}ok small"{{if not $.CanUserAcceptOrRejectTransfer}} disabled{{end}}>
-									{{ctx.Locale.Tr "repo.transfer.reject"}}
-								</button>
-							</div>
-						</form>
+					{{if $.CanUserAcceptOrRejectTransfer}}
+						<button type="button" class="ui compact small basic primary button link-action" data-url="{{$.RepoLink}}/action/accept_transfer"
+							data-tooltip-content="{{ctx.Locale.Tr "repo.transfer.accept_desc" $.RepoTransfer.Recipient.GetDisplayName}}"
+						>{{ctx.Locale.Tr "repo.transfer.accept"}}</button>
+						<button type="button" class="ui compact small basic red button link-action" data-url="{{$.RepoLink}}/action/reject_transfer"
+							data-tooltip-content="{{ctx.Locale.Tr "repo.transfer.reject_desc" $.RepoTransfer.Recipient.GetDisplayName}}"
+						>{{ctx.Locale.Tr "repo.transfer.reject"}}</button>
+					{{else if $.RepoTransfer}}
+						<div data-tooltip-content="{{ctx.Locale.Tr "repo.transfer.is_transferring_prompt" $.RepoTransfer.Recipient.GetDisplayName}}">
+							<button class="ui compact small basic red button" disabled>{{ctx.Locale.Tr "repo.transfer.is_transferring"}}</button>
+						</div>
 					{{end}}
 					{{if $.EnableFeed}}
 					{{/* An extra div-element is not necessary here, as this button does not secretly contain two buttons. */}}