Update Block a user form (#37359)

Use the new "form-fetch-action" for better user experience, and use
JSONError to show error messages.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/user/block.go

@@ -90,7 +90,7 @@ func GetBlocking(ctx context.Context, blockerID, blockeeID int64) (*Blocking, er
 		return nil, err
 	}
 	if len(blocks) == 0 {
-		return nil, nil //nolint:nilnil // return nil to indicate that the object does not exist
+		return nil, util.NewNotExistErrorf("blocking record doesn't exist")
 	}
 	return blocks[0], nil
 }

routers/api/v1/shared/block.go

@@ -9,6 +9,7 @@ import (
 
 	user_model "code.gitea.io/gitea/models/user"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/routers/api/v1/utils"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/convert"
@@ -48,17 +49,14 @@ func CheckUserBlock(ctx *context.APIContext, blocker *user_model.User) {
 		return
 	}
 
-	status := http.StatusNotFound
+	_, err = user_model.GetBlocking(ctx, blocker.ID, blockee.ID)
-	blocking, err := user_model.GetBlocking(ctx, blocker.ID, blockee.ID)
+	if errors.Is(err, util.ErrNotExist) {
-	if err != nil {
+		ctx.Status(http.StatusNotFound)
+	} else if err == nil {
+		ctx.Status(http.StatusNoContent)
+	} else {
 		ctx.APIErrorInternal(err)
-		return
 	}
-	if blocking != nil {
-		status = http.StatusNoContent
-	}
-
-	ctx.Status(status)
 }
 
 func BlockUser(ctx *context.APIContext, blocker *user_model.User) {

routers/web/org/block.go

@@ -34,15 +34,5 @@ func BlockedUsers(ctx *context.Context) {
 }
 
 func BlockedUsersPost(ctx *context.Context) {
-	if _, err := shared_user.RenderUserOrgHeader(ctx); err != nil {
+	shared_user.BlockedUsersPost(ctx, ctx.ContextUser, ctx.ContextUser.OrganisationLink()+"/settings/blocked_users")
-		ctx.ServerError("RenderUserOrgHeader", err)
-		return
-	}
-
-	shared_user.BlockedUsersPost(ctx, ctx.ContextUser)
-	if ctx.Written() {
-		return
-	}
-
-	ctx.Redirect(ctx.ContextUser.OrganisationLink() + "/settings/blocked_users")
 }

routers/web/shared/user/block.go

@@ -7,6 +7,8 @@ import (
 	"errors"
 
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/forms"
@@ -28,49 +30,51 @@ func BlockedUsers(ctx *context.Context, blocker *user_model.User) {
 	ctx.Data["UserBlocks"] = blocks
 }
 
-func BlockedUsersPost(ctx *context.Context, blocker *user_model.User) {
+func blockedUsersPost(ctx *context.Context, form *forms.BlockUserForm, blocker *user_model.User) error {
-	form := web.GetForm(ctx).(*forms.BlockUserForm)
-	if ctx.HasError() {
-		ctx.ServerError("FormValidation", nil)
-		return
-	}
-
 	blockee, err := user_model.GetUserByName(ctx, form.Blockee)
 	if err != nil {
-		ctx.ServerError("GetUserByName", nil)
+		return err
-		return
 	}
 
 	switch form.Action {
 	case "block":
-		if err := user_service.BlockUser(ctx, ctx.Doer, blocker, blockee, form.Note); err != nil {
+		err = user_service.BlockUser(ctx, ctx.Doer, blocker, blockee, form.Note)
-			if errors.Is(err, user_model.ErrCanNotBlock) || errors.Is(err, user_model.ErrBlockOrganization) {
+		if errors.Is(err, util.ErrInvalidArgument) {
-				ctx.Flash.Error(ctx.Tr("user.block.block.failure", err.Error()))
+			return util.ErrorWrapTranslatable(err, "user.block.block.failure", err.Error())
-			} else {
-				ctx.ServerError("BlockUser", err)
-				return
-			}
 		}
+		return err
 	case "unblock":
-		if err := user_service.UnblockUser(ctx, ctx.Doer, blocker, blockee); err != nil {
+		err = user_service.UnblockUser(ctx, ctx.Doer, blocker, blockee)
-			if errors.Is(err, user_model.ErrCanNotUnblock) || errors.Is(err, user_model.ErrBlockOrganization) {
+		if errors.Is(err, util.ErrInvalidArgument) {
-				ctx.Flash.Error(ctx.Tr("user.block.unblock.failure", err.Error()))
+			return util.ErrorWrapTranslatable(err, "user.block.unblock.failure", err.Error())
-			} else {
-				ctx.ServerError("UnblockUser", err)
-				return
-			}
 		}
+		return err
 	case "note":
 		block, err := user_model.GetBlocking(ctx, blocker.ID, blockee.ID)
 		if err != nil {
-			ctx.ServerError("GetBlocking", err)
+			return err
-			return
-		}
-		if block != nil {
-			if err := user_model.UpdateBlockingNote(ctx, block.ID, form.Note); err != nil {
-				ctx.ServerError("UpdateBlockingNote", err)
-				return
-			}
 		}
+		return user_model.UpdateBlockingNote(ctx, block.ID, form.Note)
+	}
+	setting.PanicInDevOrTesting("Unknown action: %q", form.Action)
+	return errors.New("unknown action")
+}
+
+func BlockedUsersPost(ctx *context.Context, blocker *user_model.User, redirect string) {
+	if ctx.HasError() {
+		ctx.JSONError(ctx.GetErrMsg())
+		return
+	}
+
+	form := web.GetForm(ctx).(*forms.BlockUserForm)
+	err := blockedUsersPost(ctx, form, blocker)
+	if err == nil {
+		ctx.JSONRedirect(redirect)
+	} else if errTr := util.ErrorAsTranslatable(err); errTr != nil {
+		ctx.JSONError(errTr.Translate(ctx.Locale))
+	} else if errors.Is(err, util.ErrNotExist) {
+		ctx.JSONError(ctx.Locale.Tr("error.not_found"))
+	} else {
+		ctx.ServerError("BlockedUsersPost", err)
 	}
 }

routers/web/shared/user/header.go

@@ -4,6 +4,7 @@
 package user
 
 import (
+	"errors"
 	"net/url"
 
 	"code.gitea.io/gitea/models/db"
@@ -83,10 +84,9 @@ func prepareContextForProfileBigAvatar(ctx *context.Context) {
 	}
 
 	if ctx.Doer != nil {
-		if block, err := user_model.GetBlocking(ctx, ctx.Doer.ID, ctx.ContextUser.ID); err != nil {
+		ctx.Data["UserBlocking"], err = user_model.GetBlocking(ctx, ctx.Doer.ID, ctx.ContextUser.ID)
+		if err != nil && !errors.Is(err, util.ErrNotExist) {
 			ctx.ServerError("GetBlocking", err)
-		} else {
-			ctx.Data["UserBlocking"] = block
 		}
 	}
 }

routers/web/user/setting/block.go

@@ -29,10 +29,5 @@ func BlockedUsers(ctx *context.Context) {
 }
 
 func BlockedUsersPost(ctx *context.Context) {
-	shared_user.BlockedUsersPost(ctx, ctx.Doer)
+	shared_user.BlockedUsersPost(ctx, ctx.Doer, setting.AppSubURL+"/user/settings/blocked_users")
-	if ctx.Written() {
-		return
-	}
-
-	ctx.Redirect(setting.AppSubURL + "/user/settings/blocked_users")
 }

templates/shared/user/blocked_users.tmpl

@@ -5,8 +5,8 @@
 	<p>{{ctx.Locale.Tr "user.block.info"}} <a target="_blank" href="https://docs.gitea.com/usage/access-control/blocking-user">{{ctx.Locale.Tr "user.block.info.docs"}}</a></p>
 </div>
 <div class="ui segment">
-	<form class="ui form ignore-dirty" action="{{$.Link}}" method="post">
+	<form class="ui form form-fetch-action ignore-dirty" action="{{$.Link}}" method="post">
-		<input type="hidden" name="action" value="block" />
+		<input type="hidden" name="action" value="block">
 		<div id="search-user-box" class="field ui fluid search input">
 			<input class="prompt tw-mr-2" name="blockee" placeholder="{{ctx.Locale.Tr "search.user_kind"}}" autocomplete="off" required>
 			<button class="ui red button">{{ctx.Locale.Tr "user.block.block"}}</button>
@@ -40,9 +40,9 @@
 				</div>
 				<div class="flex-item-trailing">
 					<button class="ui compact mini button show-modal" data-modal="#block-user-note-modal" data-modal-modal-blockee="{{.Blockee.Name}}" data-modal-modal-note="{{.Note}}">{{ctx.Locale.Tr "user.block.note.edit"}}</button>
-					<form action="{{$.Link}}" method="post">
+					<form class="form-fetch-action" action="{{$.Link}}" method="post">
-						<input type="hidden" name="action" value="unblock" />
+						<input type="hidden" name="action" value="unblock">
-						<input type="hidden" name="blockee" value="{{.Blockee.Name}}" />
+						<input type="hidden" name="blockee" value="{{.Blockee.Name}}">
 						<button class="ui compact mini button">{{ctx.Locale.Tr "user.block.unblock"}}</button>
 					</form>
 				</div>
@@ -55,12 +55,12 @@
 <div class="ui small modal" id="block-user-note-modal">
 	<div class="header">{{ctx.Locale.Tr "user.block.note.edit"}}</div>
 	<div class="content">
-		<form class="ui form" action="{{$.Link}}" method="post">
+		<form class="ui form form-fetch-action" action="{{$.Link}}" method="post">
-			<input type="hidden" name="action" value="note" />
+			<input type="hidden" name="action" value="note">
-			<input type="hidden" name="blockee" class="modal-blockee" />
+			<input type="hidden" name="blockee" class="modal-blockee">
 			<div class="field">
 				<label>{{ctx.Locale.Tr "user.block.note.title"}}</label>
-				<input name="note" class="modal-note" />
+				<input name="note" class="modal-note">
 				<p class="help">{{ctx.Locale.Tr "user.block.note.info"}}</p>
 			</div>
 			<div class="actions">