feat(i18n): migrate modules/group to ResponseErrorL (Phase 2.1)

modules/group/api_bot_ownership_test.go

@@ -81,6 +81,7 @@ func setupBotOwnershipGroup(t *testing.T) (*Group, http.Handler) {
 		Vercode: fmt.Sprintf("%s@1", util.GenerUUID()),
 	})
 	assert.NoError(t, err)
+	wireI18nRendererForGroupTest(s)
 	return f, s.GetRoute()
 }
 
@@ -124,9 +125,11 @@ func TestGroupMemberAdd_BotOwnedByOther(t *testing.T) {
 	insertBotUser(t, f, "yutestspacebot1_bot", "YuTestSpaceBot1", "yts1_bot_sn", "user_a")
 
 	w := postAddMembers(t, h, "g_bot_own", []string{"yutestspacebot1_bot"})
-	assert.Equal(t, http.StatusForbidden, w.Code, "user-c 邀请别人的 bot 应返回 403, got body=%s", w.Body.String())
-	assert.True(t, strings.Contains(w.Body.String(), "no permission to invite this bot"),
-		"响应体应包含明确的权限错误信息, got=%s", w.Body.String())
+	// D14: wire status is fixed at 400 during the compat window; the 403
+	// semantics now live in error.http_status / error.code.
+	assert.Equal(t, http.StatusBadRequest, w.Code, "user-c 邀请别人的 bot 应返回 400 信封, got body=%s", w.Body.String())
+	assert.True(t, strings.Contains(w.Body.String(), "err.server.group.bot_ownership_denied"),
+		"响应体应包含 bot 归属错误码, got=%s", w.Body.String())
 
 	exist, err := f.db.ExistMember("yutestspacebot1_bot", "g_bot_own")
 	assert.NoError(t, err)
@@ -144,7 +147,8 @@ func TestGroupMemberAdd_BotThirdPartyNoCreator(t *testing.T) {
 	insertBotUser(t, f, "ppt_bot", "PPT Bot", "ppt_bot_sn", "")
 
 	w := postAddMembers(t, h, "g_bot_own", []string{"ppt_bot"})
-	assert.Equal(t, http.StatusForbidden, w.Code, "第三方 bot 应返回 403, got body=%s", w.Body.String())
+	assert.Equal(t, http.StatusBadRequest, w.Code, "第三方 bot 应返回 400 信封, got body=%s", w.Body.String())
+	assert.Contains(t, w.Body.String(), "err.server.group.bot_ownership_denied", "got=%s", w.Body.String())
 
 	exist, err := f.db.ExistMember("ppt_bot", "g_bot_own")
 	assert.NoError(t, err)
@@ -164,7 +168,8 @@ func TestGroupMemberAdd_BotOwnershipBlocksMixedBatch(t *testing.T) {
 	insertBotUser(t, f, "spacebottest1_bot", "SpaceBotTest1", "sbt1_bot_sn", "user_b")
 
 	w := postAddMembers(t, h, "g_bot_own", []string{"human_friend", "spacebottest1_bot"})
-	assert.Equal(t, http.StatusForbidden, w.Code, "mixed batch with foreign bot 应返回 403, got body=%s", w.Body.String())
+	assert.Equal(t, http.StatusBadRequest, w.Code, "mixed batch with foreign bot 应返回 400 信封, got body=%s", w.Body.String())
+	assert.Contains(t, w.Body.String(), "err.server.group.bot_ownership_denied", "got=%s", w.Body.String())
 
 	// 混合批次被拒时，所有成员都不应被写入（避免半提交）
 	exist, err := f.db.ExistMember("spacebottest1_bot", "g_bot_own")

modules/group/api_i18n.go

@@ -0,0 +1,95 @@
+package group
+
+import (
+	"errors"
+
+	"github.com/Mininglamp-OSS/octo-lib/pkg/wkhttp"
+	"github.com/Mininglamp-OSS/octo-server/pkg/errcode"
+	"github.com/Mininglamp-OSS/octo-server/pkg/httperr"
+	"github.com/Mininglamp-OSS/octo-server/pkg/i18n"
+	"github.com/Mininglamp-OSS/octo-server/pkg/i18n/codes"
+)
+
+// respond helpers for modules/group. Most migrated sites call
+// httperr.ResponseErrorL(c, errcode.ErrGroupXxx, nil, nil) directly; the
+// helpers below exist only for the high-frequency shapes that either carry a
+// Detail field (so the SafeDetailKeys contract stays in one place) or resolve a
+// shared err.shared.* code at init.
+//
+// Internal=true codes (ErrGroupQueryFailed / ErrGroupStoreFailed /
+// ErrGroupNotifyFailed) are intentionally NOT wrapped: each call site keeps its
+// existing g.Error(..., zap.Error(err)) log so ops can debug from logs, and the
+// wire response carries no message.
+
+// respondGroupRequestInvalid covers the common "X 不能为空" / "数据格式有误" /
+// BindJSON-failure shape — one code, one optional field detail. An empty field
+// is omitted so the renderer does not surface a noisy empty key to clients.
+func respondGroupRequestInvalid(c *wkhttp.Context, field string) {
+	details := i18n.Details{}
+	if field != "" {
+		details["field"] = field
+	}
+	httperr.ResponseErrorL(c, errcode.ErrGroupRequestInvalid, nil, details)
+}
+
+// respondGroupMdContentTooLarge surfaces the GROUP.md size cap so the client can
+// render a localized hint without hard-coding the limit.
+func respondGroupMdContentTooLarge(c *wkhttp.Context, maxSize int) {
+	httperr.ResponseErrorL(c, errcode.ErrGroupMdContentTooLarge, nil, i18n.Details{
+		"field":    "content",
+		"max_size": maxSize,
+	})
+}
+
+// errSharedAuthRequired / errSharedForbidden cache the shared auth codes so the
+// per-handler login / permission guards do not pay a registry lookup on every
+// miss. Looked up at package init; a missing registration panics loudly rather
+// than silently rendering an empty envelope at request time.
+var (
+	errSharedAuthRequired = mustLookupSharedCode("err.shared.auth.required")
+	errSharedForbidden    = mustLookupSharedCode("err.shared.auth.forbidden")
+)
+
+func mustLookupSharedCode(id string) codes.Code {
+	c, ok := codes.Lookup(id)
+	if !ok {
+		panic("modules/group: shared code not registered: " + id)
+	}
+	return c
+}
+
+// respondGroupNotLoggedIn renders the shared 401 envelope for the public routes
+// (group scan-join / invite authorize) whose legacy "请先登录" guard runs before
+// AuthMiddleware would reject the request.
+func respondGroupNotLoggedIn(c *wkhttp.Context) {
+	httperr.ResponseErrorL(c, errSharedAuthRequired, nil, nil)
+}
+
+// respondGroupForbidden renders the shared 403 envelope for the generic
+// "用户无权执行此操作" / "操作用户权限不够" guards that carry no role-specific
+// hint. Role-specific gates use the dedicated err.server.group.creator_only /
+// manager_only / creator_or_manager_only codes instead.
+func respondGroupForbidden(c *wkhttp.Context) {
+	httperr.ResponseErrorL(c, errSharedForbidden, nil, nil)
+}
+
+// errGroupInfoQueryFailed / errGroupInfoNotFound are getGroupInfo's sentinel
+// returns. Call sites map them to the right envelope via errors.Is
+// (respondGroupInfoError) instead of leaking the underlying Chinese string
+// behind a fixed HTTP 400.
+var (
+	errGroupInfoQueryFailed = errors.New("query group failed")
+	errGroupInfoNotFound    = errors.New("group not found or disbanded")
+)
+
+// respondGroupInfoError maps getGroupInfo's sentinel error to the localized
+// envelope: a missing / disbanded group is 404, any other (query) failure is
+// 500 (Internal). getGroupInfo already logged the underlying DB error, so the
+// query branch does not log again.
+func respondGroupInfoError(c *wkhttp.Context, err error) {
+	if errors.Is(err, errGroupInfoNotFound) {
+		httperr.ResponseErrorL(c, errcode.ErrGroupNotFound, nil, nil)
+		return
+	}
+	httperr.ResponseErrorL(c, errcode.ErrGroupQueryFailed, nil, nil)
+}

modules/group/api_i18n_regression_test.go

@@ -0,0 +1,199 @@
+package group
+
+import (
+	"bytes"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/Mininglamp-OSS/octo-lib/pkg/util"
+	"github.com/Mininglamp-OSS/octo-lib/pkg/wkhttp"
+	"github.com/Mininglamp-OSS/octo-lib/testutil"
+	"github.com/Mininglamp-OSS/octo-server/modules/user"
+	"github.com/stretchr/testify/assert"
+)
+
+// putGroupSetting issues a PUT /setting with the given JSON body using the test
+// caller's token.
+func putGroupSetting(t *testing.T, handler http.Handler, groupNo, body string) *httptest.ResponseRecorder {
+	t.Helper()
+	w := httptest.NewRecorder()
+	req, err := http.NewRequest("PUT", "/v1/groups/"+groupNo+"/setting", bytes.NewReader([]byte(body)))
+	assert.NoError(t, err)
+	req.Header.Set("token", testutil.Token)
+	handler.ServeHTTP(w, req)
+	return w
+}
+
+// TestGroupExit_NotFoundGroup pins the fix for the review finding that
+// groupExit returned 500 (query_failed) for a missing / disbanded group
+// because it ignored getGroupInfo's not-found sentinel. The exit of a
+// non-existent group is a user-facing 404, not an internal error.
+func TestGroupExit_NotFoundGroup(t *testing.T) {
+	s, ctx := testutil.NewTestServer()
+	wireI18nRendererForGroupTest(s)
+	_ = New(ctx)
+
+	err := testutil.CleanAllTables(ctx)
+	assert.NoError(t, err)
+
+	w := httptest.NewRecorder()
+	req, err := http.NewRequest("POST", "/v1/groups/does-not-exist/exit", nil)
+	assert.NoError(t, err)
+	req.Header.Set("token", testutil.Token)
+	s.GetRoute().ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusBadRequest, w.Code, "wire status 固定 400, body=%s", w.Body.String())
+	assert.Contains(t, w.Body.String(), "err.server.group.not_found",
+		"退不存在的群应是 404 业务错误而非内部错误, body=%s", w.Body.String())
+}
+
+// TestGroupMemberInviteSure_ExpiredCode pins the fix for the review finding
+// that an expired / missing auth_code (Redis returns "") fell through to a
+// JSON-decode failure mapped to store_failed (500). An expired authorization
+// code is a normal user-facing state and must surface as auth_code_invalid.
+func TestGroupMemberInviteSure_ExpiredCode(t *testing.T) {
+	s, ctx := testutil.NewTestServer()
+	wireI18nRendererForGroupTest(s)
+	_ = New(ctx)
+
+	w := httptest.NewRecorder()
+	req, err := http.NewRequest("POST", "/v1/group/invite/sure?auth_code=expired-"+util.GenerUUID(), nil)
+	assert.NoError(t, err)
+	s.GetRoute().ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusBadRequest, w.Code, "wire status 固定 400, body=%s", w.Body.String())
+	assert.Contains(t, w.Body.String(), "err.server.group.auth_code_invalid",
+		"过期/无效 auth_code 应是用户态错误而非内部错误, body=%s", w.Body.String())
+}
+
+// TestGroupMemberAdd_BlankMembersIsRequestInvalid pins the fix for the review
+// finding that members consisting solely of blank strings pass Check() but
+// AddGroupMembers returns "no valid members after deduplication" — a 400
+// validation error, not the store_failed (500) it was being mapped to.
+func TestGroupMemberAdd_BlankMembersIsRequestInvalid(t *testing.T) {
+	f, h := setupBotOwnershipGroup(t)
+	_ = f
+
+	w := postAddMembers(t, h, "g_bot_own", []string{"   "})
+	assert.Equal(t, http.StatusBadRequest, w.Code, "wire status 固定 400, body=%s", w.Body.String())
+	assert.Contains(t, w.Body.String(), "err.server.group.request_invalid",
+		"全空白成员应是 400 校验错误而非内部错误, body=%s", w.Body.String())
+}
+
+// TestManagerMemberRemove_NotInGroupIsNotFound pins the fix for the review
+// finding that the management (CheckLoginRole==nil) delete path skips the
+// per-member pre-check, so removing UIDs that are not in the group made
+// RemoveGroupMembers return "none of the members are in this group" — a 404
+// business error, not the store_failed (500) it was being mapped to.
+func TestManagerMemberRemove_NotInGroupIsNotFound(t *testing.T) {
+	s, ctx := testutil.NewTestServer()
+	wireI18nRendererForGroupTest(s)
+	f := New(ctx)
+
+	err := testutil.CleanAllTables(ctx)
+	assert.NoError(t, err)
+
+	// Promote the test caller to SuperAdmin so memberRemove takes the
+	// management path that skips the normal-member pre-check.
+	cfg := ctx.GetConfig()
+	assert.NoError(t, ctx.Cache().Set(
+		cfg.Cache.TokenCachePrefix+testutil.Token,
+		testutil.UID+"@test@"+string(wkhttp.SuperAdmin),
+	))
+
+	groupNo := "g-ghost-rm"
+	err = f.userDB.Insert(&user.Model{UID: testutil.UID, Name: "admin", ShortNo: "ghost_admin"})
+	assert.NoError(t, err)
+	err = f.db.Insert(&Model{GroupNo: groupNo, Name: "ghost rm", Creator: testutil.UID, Status: GroupStatusNormal, Version: 1})
+	assert.NoError(t, err)
+
+	body := util.ToJson(map[string]any{"members": []string{"ghost-not-in-group"}})
+	w := httptest.NewRecorder()
+	req, err := http.NewRequest("DELETE", "/v1/groups/"+groupNo+"/members", bytes.NewReader([]byte(body)))
+	assert.NoError(t, err)
+	req.Header.Set("token", testutil.Token)
+	s.GetRoute().ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusBadRequest, w.Code, "wire status 固定 400, body=%s", w.Body.String())
+	assert.Contains(t, w.Body.String(), "err.server.group.member_not_in_group",
+		"删除非群成员应是 404 业务错误而非内部错误, body=%s", w.Body.String())
+}
+
+// TestGroupSettingUpdate_InvalidValueTypeIsRequestInvalid pins the fix for the
+// review finding that a wrong-typed setting value (e.g. a string for the
+// numeric "mute" toggle) returned by settingActionMap was collapsed into
+// store_failed (500). A malformed value is a 400 client error.
+func TestGroupSettingUpdate_InvalidValueTypeIsRequestInvalid(t *testing.T) {
+	_, h := setupBotOwnershipGroup(t)
+
+	// "mute" expects a number; sending a string trips safeIntFromFloat64.
+	w := putGroupSetting(t, h, "g_bot_own", `{"mute":"not-a-number"}`)
+	assert.Equal(t, http.StatusBadRequest, w.Code, "wire status 固定 400, body=%s", w.Body.String())
+	assert.Contains(t, w.Body.String(), "err.server.group.request_invalid",
+		"错类型的设置值应是 400 校验错误而非内部错误, body=%s", w.Body.String())
+}
+
+// TestGroupSettingUpdate_AllowExternalRangeIsRequestInvalid pins the fix for the
+// review finding that an out-of-range allow_external value returned by the
+// group-attr action was collapsed into store_failed (500). The test caller is
+// the creator, so checkPermissions passes and the range check is what rejects.
+func TestGroupSettingUpdate_AllowExternalRangeIsRequestInvalid(t *testing.T) {
+	_, h := setupBotOwnershipGroup(t)
+
+	w := putGroupSetting(t, h, "g_bot_own", `{"allow_external":2}`)
+	assert.Equal(t, http.StatusBadRequest, w.Code, "wire status 固定 400, body=%s", w.Body.String())
+	assert.Contains(t, w.Body.String(), "err.server.group.request_invalid",
+		"allow_external 越界应是 400 校验错误而非内部错误, body=%s", w.Body.String())
+}
+
+// TestGroupSettingUpdate_NonManagerForbidden pins the fix for the review finding
+// that a non-manager/creator toggling a group-level attribute had
+// checkPermissions's "没有权限！" collapsed into store_failed (500). Updating a
+// group attribute without permission is a 403, not an internal error.
+func TestGroupSettingUpdate_NonManagerForbidden(t *testing.T) {
+	s, ctx := testutil.NewTestServer()
+	wireI18nRendererForGroupTest(s)
+	f := New(ctx)
+
+	err := testutil.CleanAllTables(ctx)
+	assert.NoError(t, err)
+
+	// Group is owned by someone else; the test caller (testutil.UID) is neither
+	// creator nor manager, so checkPermissions returns errGroupUpdateForbidden.
+	groupNo := "g-perm-deny"
+	err = f.db.Insert(&Model{GroupNo: groupNo, Name: "perm deny", Creator: "other-owner", Status: GroupStatusNormal, Version: 1})
+	assert.NoError(t, err)
+
+	w := putGroupSetting(t, s.GetRoute(), groupNo, `{"forbidden":1}`)
+	assert.Equal(t, http.StatusBadRequest, w.Code, "wire status 固定 400, body=%s", w.Body.String())
+	assert.Contains(t, w.Body.String(), "err.server.group.creator_or_manager_only",
+		"非管理员改群属性应是 403 而非内部错误, body=%s", w.Body.String())
+}
+
+// TestGroupAvatarUpload_MissingFileIsRequestInvalid pins the fix for the review
+// finding that avatarUpload mapped a missing multipart "file" field
+// (http.ErrMissingFile) to store_failed (500). Forgetting to attach the file is
+// a 400 client mistake, mirroring the sibling ParseMultipartForm branch.
+func TestGroupAvatarUpload_MissingFileIsRequestInvalid(t *testing.T) {
+	_, handler := setupBotOwnershipGroup(t)
+
+	// Build a valid multipart body that carries a field but no "file", so
+	// ParseMultipartForm succeeds and FormFile("file") returns ErrMissingFile.
+	var buf bytes.Buffer
+	mw := multipart.NewWriter(&buf)
+	assert.NoError(t, mw.WriteField("other", "x"))
+	assert.NoError(t, mw.Close())
+
+	w := httptest.NewRecorder()
+	req, err := http.NewRequest("POST", "/v1/groups/g_bot_own/avatar", &buf)
+	assert.NoError(t, err)
+	req.Header.Set("token", testutil.Token)
+	req.Header.Set("Content-Type", mw.FormDataContentType())
+	handler.ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusBadRequest, w.Code, "wire status 固定 400, body=%s", w.Body.String())
+	assert.Contains(t, w.Body.String(), "err.server.group.request_invalid",
+		"缺少上传文件应是 400 校验错误而非内部错误, body=%s", w.Body.String())
+}