fix: 已停用角色导致用户无法保存编辑

2026-09-22 16:41:19 +08:00 · 2026-09-22 16:41:19 +08:00 · aa017a32b4
commit aa017a32b4
parent 688dffebaa
5 changed files with 273 additions and 20 deletions
--- a/frontend/src/views/Users.test.js
+++ b/frontend/src/views/Users.test.js
@ -1,5 +1,6 @@
 import { flushPromises, mount } from '@vue/test-utils'
 import ElementPlus, { ElMessage } from 'element-plus'
+import { defineComponent, ref } from 'vue'
 import { beforeEach, describe, expect, test, vi } from 'vitest'
 import Users from './Users.vue'
 import { organizationsApi } from '../api/organizations'
@ -99,6 +100,25 @@ describe('用户管理角色', () => {
    expect(wrapper.text()).toContain('采购人员')
    wrapper.unmount()
  })
+
+  // 列表里同样要标出来：停用角色的 tag 和启用的一模一样，管理员会以为这个用户还在
+  // 通过它拿权限，实际上 GenerateToken 已经把它过滤掉了。
+  test('列表里已停用的角色也带后缀，启用的不带', async () => {
+    usersStore.users = [{
+      ...usersStore.users[0],
+      roles: [
+        { id: 2, code: 'warehouse_admin', name: '仓库管理员', enabled: true, permissions: [] },
+        { id: 3, code: 'purchaser', name: '采购人员', enabled: false, permissions: [] }
+      ]
+    }]
+    const wrapper = mountUsers()
+    await flushPromises()
+
+    expect(wrapper.text()).toContain('采购人员（已停用）')
+    expect(wrapper.text()).toContain('仓库管理员')
+    expect(wrapper.text()).not.toContain('仓库管理员（已停用）')
+    wrapper.unmount()
+  })
 })

 describe('新增用户后列表与搜索框保持一致', () => {
@ -155,3 +175,113 @@ describe('新增用户后列表与搜索框保持一致', () => {
    wrapper.unmount()
  })
 })
+
+// 已停用角色在下拉里怎么呈现，直接决定"编辑用户"这条路走不走得通：保存是全量替换
+// 角色，前端若摘不掉那个停用角色、后端又要求角色全部启用，这个用户就永远存不回去。
+// 现在后端已放行"本来就持有的停用角色"，前端这两条是配套：既让人摘得掉，也不让人
+// 新分配一个停用角色。
+describe('已停用角色的选择状态', () => {
+  const disabledRole = { id: 3, code: 'purchaser', name: '采购人员', enabled: false, permissions: [] }
+  const unassignedDisabledRole = { id: 4, code: 'obsolete_role', name: '作废角色', enabled: false, permissions: [] }
+
+  beforeEach(() => {
+    // 用户 7 正持有已停用的角色 3；角色 4 同样停用但该用户没有
+    usersStore.users = [{
+      id: 7,
+      username: 'zhangsan',
+      description: '测试用户',
+      created_at: '2026-09-16T09:00:00+08:00',
+      roles: [disabledRole]
+    }]
+    usersStore.updateUser.mockReset()
+    usersStore.updateUser.mockResolvedValue({ data: {} })
+    organizationsApi.listRoles.mockResolvedValue({
+      data: [
+        { id: 2, code: 'warehouse_admin', name: '仓库管理员', enabled: true, permissions: [] },
+        disabledRole,
+        unassignedDisabledRole
+      ]
+    })
+  })
+
+  async function openEdit(wrapper) {
+    const state = wrapper.vm.$.setupState
+    state.showEditModal(usersStore.users[0])
+    await flushPromises()
+    return state
+  }
+
+  function findOption(wrapper, value) {
+    return wrapper.findAllComponents(RoleOptionStub).find(option => option.props('value') === value)
+  }
+
+  test('用户已持有的停用角色不禁用，否则它摘不掉', async () => {
+    const wrapper = mountUsers()
+    await flushPromises()
+    const state = await openEdit(wrapper)
+
+    expect(state.form.role_ids).toEqual([3])
+    // disabled 跟着 form.role_ids 走：已选中就不禁用，tag 上才有关闭按钮
+    expect(findOption(wrapper, 3).props('disabled')).toBe(false)
+    wrapper.unmount()
+  })
+
+  test('未持有的停用角色仍然禁用，不能通过编辑新分配', async () => {
+    const wrapper = mountUsers()
+    await flushPromises()
+    await openEdit(wrapper)
+
+    expect(findOption(wrapper, 4).props('disabled')).toBe(true)
+    wrapper.unmount()
+  })
+
+  test('停用的角色在下拉里标注出来', async () => {
+    const wrapper = mountUsers()
+    await flushPromises()
+    await openEdit(wrapper)
+
+    expect(findOption(wrapper, 3).props('label')).toBe('采购人员（已停用）')
+    expect(findOption(wrapper, 2).props('label')).toBe('仓库管理员')
+    wrapper.unmount()
+  })
+})
+
+// 上面几条用的是 stub，只能证明 :disabled 这个 prop 算对了；而"管理员究竟能不能把那个
+// 停用角色摘掉"取决于 element-plus 内部——multiple 模式下 tag 的关闭按钮绑在 option 的
+// isDisabled 上（select2.mjs 的 closable: !selectDisabled && !item.isDisabled）。这里是
+// 用真实组件把这条依赖验一遍，否则 Users.vue 的修复只是纸面上成立。
+describe('element-plus 多选下停用角色 tag 的可关闭性', () => {
+  const roles = [
+    { id: 2, name: 'warehouse_admin', enabled: true },
+    { id: 3, name: 'purchaser', enabled: false }
+  ]
+
+  // 只铺一个最小场景：已选中那个停用的角色 3
+  function buildSelect(disabledExpression) {
+    return defineComponent({
+      template: `
+        <el-select v-model="ids" multiple>
+          <el-option v-for="role in roles" :key="role.id" :label="role.name" :value="role.id"
+            :disabled="${disabledExpression}" />
+        </el-select>
+      `,
+      setup() {
+        return { ids: ref([3]), roles }
+      }
+    })
+  }
+
+  async function closeIconCount(disabledExpression) {
+    const wrapper = mount(buildSelect(disabledExpression), { global: { plugins: [ElementPlus] } })
+    await flushPromises()
+    return wrapper.findAll('.el-tag__close').length
+  }
+
+  test('Users.vue 的写法：已选中的停用角色 tag 保留关闭按钮', async () => {
+    expect(await closeIconCount('!role.enabled && !ids.includes(role.id)')).toBe(1)
+  })
+
+  test('静态 disabled 的写法：同一个 tag 没有关闭按钮（原死锁现场）', async () => {
+    expect(await closeIconCount('!role.enabled')).toBe(0)
+  })
+})
--- a/frontend/src/views/Users.vue
+++ b/frontend/src/views/Users.vue
@ -32,7 +32,7 @@
            :type="role.code === 'super_admin' ? 'danger' : 'info'"
            class="role-tag"
          >
-            {{ role.name }}
+            {{ roleLabel(role) }}
          </el-tag>
          <span v-if="!row.roles || row.roles.length === 0">-</span>
        </template>
@ -60,13 +60,18 @@
          <el-input v-model="form.password" type="password" />
        </el-form-item>
        <el-form-item label="角色" required>
+          <!-- disabled 依赖 form.role_ids 而不是写成静态的 !role.enabled：停用的角色只对
+               "尚未选中"的项禁用，已选中的必须留住关闭按钮。element-plus 把 tag 的
+               closable 绑在 option 的 isDisabled 上，所以这个表达式会跟着重算。若改回
+               静态判断，用户身上已有的停用角色就摘不掉，而保存又是全量替换角色，
+               这个用户将永远存不回去（后端会拒绝）。 -->
          <el-select v-model="form.role_ids" multiple style="width: 100%" placeholder="请选择至少一个角色">
            <el-option
              v-for="role in roles"
              :key="role.id"
-              :label="role.name"
+              :label="roleLabel(role)"
              :value="role.id"
-              :disabled="!role.enabled"
+              :disabled="!role.enabled && !form.role_ids.includes(role.id)"
            />
          </el-select>
        </el-form-item>
@ -126,6 +131,13 @@ async function loadRoles() {
  roles.value = response.data || []
 }

+// 列表的 tag 和编辑下拉都用它。已停用的角色照旧显示（用户可能正持有它，编辑时得
+// 看得见、摘得掉），但必须把"已停用"标出来：停用意味着不再授予权限，只显示角色名
+// 会让管理员以为这个角色还在生效。
+function roleLabel(role) {
+  return role.enabled ? role.name : `${role.name}（已停用）`
+}
+
 function handleSearch() {
  usersStore.fetchUsers(1, searchKeyword.value)
 }
--- a/internal/handler/api.go
+++ b/internal/handler/api.go
@ -1076,8 +1076,8 @@ func (h *Handler) CreateUser(c *gin.Context) {

 	user, err := h.authSvc.CreateUser(req.Username, req.Password, req.Description, req.RoleIDs)
 	if err != nil {
-		if errors.Is(err, repository.ErrInvalidRoles) {
-			response.BadRequest(c, err.Error())
+		if message := roleErrorText(err); message != "" {
+			response.BadRequest(c, message)
 			return
 		}
 		response.InternalError(c, err.Error())
@ -1107,8 +1107,8 @@ func (h *Handler) UpdateUser(c *gin.Context) {

 	err = h.authSvc.UpdateUser(id, req.Description, req.RoleIDs, req.Password)
 	if err != nil {
-		if errors.Is(err, repository.ErrInvalidRoles) || errors.Is(err, repository.ErrLastSuperAdmin) {
-			response.BadRequest(c, err.Error())
+		if message := roleErrorText(err); message != "" {
+			response.BadRequest(c, message)
 			return
 		}
 		response.InternalError(c, err.Error())
@ -1133,8 +1133,8 @@ func (h *Handler) DeleteUser(c *gin.Context) {
 	}

 	if err := h.authSvc.DeleteUser(id); err != nil {
-		if errors.Is(err, repository.ErrLastSuperAdmin) {
-			response.BadRequest(c, err.Error())
+		if message := roleErrorText(err); message != "" {
+			response.BadRequest(c, message)
 			return
 		}
 		response.InternalError(c, err.Error())
@ -1144,6 +1144,21 @@ func (h *Handler) DeleteUser(c *gin.Context) {
 	response.SuccessWithMessage(c, "user deleted successfully", nil)
 }

+// roleErrorText 把角色相关的仓储层错误翻成面向用户的中文，返回空字符串表示这个错误
+// 与角色无关。翻译放在 handler 而非仓储层：仓储层的错误常量是给开发者看的标识，不该
+// 承担界面文案。前端的 utils/errorText.js 只放行带中文的消息，英文会被换成笼统的
+// "请求参数有误"，用户看不出问题出在角色上。
+func roleErrorText(err error) string {
+	switch {
+	case errors.Is(err, repository.ErrInvalidRoles):
+		return "所选角色无效或已被停用，请重新选择"
+	case errors.Is(err, repository.ErrLastSuperAdmin):
+		return "不能移除最后一个超级管理员"
+	default:
+		return ""
+	}
+}
+
 // Application handlers

 func (h *Handler) CreateApplication(c *gin.Context) {
--- a/internal/repository/user.go
+++ b/internal/repository/user.go
@ -132,7 +132,7 @@ func (r *UserRepository) update(user *model.User, roleIDs []int64, updatePasswor
 	defer func() { _ = tx.Rollback() }()

 	roleIDs = uniqueRoleIDs(roleIDs)
-	if err := validateRolesTx(tx, roleIDs); err != nil {
+	if err := validateRolesTx(tx, user.ID, roleIDs); err != nil {
 		return err
 	}
 	if err := protectLastSuperAdminTx(tx, user.ID, roleIDs); err != nil {
@ -208,23 +208,37 @@ func loadRolesForUsers(queryer interface {

 func assignRolesTx(tx *sql.Tx, userID int64, roleIDs []int64) error {
 	roleIDs = uniqueRoleIDs(roleIDs)
-	if err := validateRolesTx(tx, roleIDs); err != nil {
+	if err := validateRolesTx(tx, userID, roleIDs); err != nil {
 		return err
 	}
 	return insertUserRolesTx(tx, userID, roleIDs)
 }

-func validateRolesTx(tx *sql.Tx, roleIDs []int64) error {
+// validateRolesTx 校验 roleIDs 是否都是可分配给该用户的角色。
+// 可分配 = 角色处于启用状态，或它本来就是该用户当前持有的角色。
+//   - 新建用户时 user_roles 里还没有该用户的记录，EXISTS 恒为假，等价于原来的
+//     "只允许启用中的角色"。
+//   - 编辑用户时前端会把现有角色原样回传，其中可能含已被停用的。停用只是不再授予
+//     权限（GenerateToken 会过滤掉停用角色），没必要把已有分配一并抹掉；若在这里
+//     拒绝，管理员就再也保存不了这个用户：下拉里的停用角色是禁用项，已选中的 tag
+//     也没有关闭按钮。
+//
+// 必须在 DELETE FROM user_roles 之前调用，否则 EXISTS 取不到旧分配。
+func validateRolesTx(tx *sql.Tx, userID int64, roleIDs []int64) error {
 	if len(roleIDs) == 0 {
 		return ErrInvalidRoles
 	}
 	placeholders := strings.TrimSuffix(strings.Repeat("?, ", len(roleIDs)), ", ")
-	args := make([]interface{}, len(roleIDs))
-	for index, id := range roleIDs {
-		args[index] = id
+	args := make([]interface{}, 0, len(roleIDs)+1)
+	for _, id := range roleIDs {
+		args = append(args, id)
 	}
+	args = append(args, userID)
 	var count int
-	query := fmt.Sprintf(`SELECT COUNT(*) FROM roles WHERE enabled = TRUE AND id IN (%s)`, placeholders)
+	query := fmt.Sprintf(`SELECT COUNT(*) FROM roles r
+		WHERE r.id IN (%s)
+		AND (r.enabled = TRUE OR EXISTS (
+			SELECT 1 FROM user_roles ur WHERE ur.role_id = r.id AND ur.user_id = ?))`, placeholders)
 	if err := tx.QueryRow(query, args...).Scan(&count); err != nil {
 		return err
 	}
--- a/internal/repository/user_test.go
+++ b/internal/repository/user_test.go
@ -87,8 +87,11 @@ func TestUpdateRejectsRemovingRoleFromLastSuperAdmin(t *testing.T) {
 	database.DB = db

 	mock.ExpectBegin()
-	mock.ExpectQuery(regexp.QuoteMeta(`SELECT COUNT(*) FROM roles WHERE enabled = TRUE AND id IN (?)`)).
-		WithArgs(int64(2)).
+	mock.ExpectQuery(regexp.QuoteMeta(`SELECT COUNT(*) FROM roles r
+		WHERE r.id IN (?)
+		AND (r.enabled = TRUE OR EXISTS (
+			SELECT 1 FROM user_roles ur WHERE ur.role_id = r.id AND ur.user_id = ?))`)).
+		WithArgs(int64(2), int64(7)).
 		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
 	mock.ExpectQuery(regexp.QuoteMeta(`SELECT id FROM roles WHERE code = 'super_admin' FOR UPDATE`)).
 		WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(1))
@ -110,6 +113,80 @@ func TestUpdateRejectsRemovingRoleFromLastSuperAdmin(t *testing.T) {
 	}
 }

+// 编辑用户时把已被停用的角色原样回传必须能保存成功。停用只是不再授予权限，不要求
+// 管理员顺手把它从用户身上摘掉；旧实现在这里返回 ErrInvalidRoles，而前端下拉里的
+// 停用角色是禁用项、已选中的 tag 也关不掉，结果这个用户再也编辑不了。
+func TestUpdateKeepsDisabledRoleAlreadyAssignedToUser(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("open sql mock: %v", err)
+	}
+	t.Cleanup(func() { database.DB = nil; db.Close() })
+	database.DB = db
+
+	mock.ExpectBegin()
+	// count 命中 1：角色 3 虽然已停用，但该用户当前正持有它，EXISTS 放行。
+	mock.ExpectQuery(regexp.QuoteMeta(`SELECT COUNT(*) FROM roles r
+		WHERE r.id IN (?)
+		AND (r.enabled = TRUE OR EXISTS (
+			SELECT 1 FROM user_roles ur WHERE ur.role_id = r.id AND ur.user_id = ?))`)).
+		WithArgs(int64(3), int64(7)).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
+	mock.ExpectQuery(regexp.QuoteMeta(`SELECT id FROM roles WHERE code = 'super_admin' FOR UPDATE`)).
+		WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(1))
+	mock.ExpectQuery(regexp.QuoteMeta(`SELECT COUNT(*) FROM user_roles WHERE user_id = ? AND role_id = ?`)).
+		WithArgs(int64(7), int64(1)).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
+	mock.ExpectExec(regexp.QuoteMeta(`UPDATE users SET description = ? WHERE id = ?`)).
+		WithArgs("改个描述", int64(7)).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectExec(regexp.QuoteMeta(`DELETE FROM user_roles WHERE user_id = ?`)).
+		WithArgs(int64(7)).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectExec(regexp.QuoteMeta(`INSERT INTO user_roles (user_id, role_id) VALUES (?, ?)`)).
+		WithArgs(int64(7), int64(3)).
+		WillReturnResult(sqlmock.NewResult(0, 1))
+	mock.ExpectCommit()
+
+	user := &model.User{ID: 7, Description: "改个描述"}
+	if err := NewUserRepository().Update(user, []int64{3}); err != nil {
+		t.Fatalf("update user: %v", err)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// 反面：把一个该用户还没持有的停用角色塞进来仍要拒绝。EXISTS 为假，count 对不上，
+// 这样"不能通过接口新分配停用角色"这条约束就没有被上面那个放行口子带松。
+func TestUpdateRejectsNewlyAssignedDisabledRole(t *testing.T) {
+	db, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("open sql mock: %v", err)
+	}
+	t.Cleanup(func() { database.DB = nil; db.Close() })
+	database.DB = db
+
+	mock.ExpectBegin()
+	// count 为 0：角色 4 已停用，且不在该用户当前持有的角色里。
+	mock.ExpectQuery(regexp.QuoteMeta(`SELECT COUNT(*) FROM roles r
+		WHERE r.id IN (?)
+		AND (r.enabled = TRUE OR EXISTS (
+			SELECT 1 FROM user_roles ur WHERE ur.role_id = r.id AND ur.user_id = ?))`)).
+		WithArgs(int64(4), int64(7)).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
+	mock.ExpectRollback()
+
+	user := &model.User{ID: 7, Description: "改个描述"}
+	err = NewUserRepository().Update(user, []int64{4})
+	if !errors.Is(err, ErrInvalidRoles) {
+		t.Fatalf("update error = %v, want ErrInvalidRoles", err)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func TestCreateUserAssignsMultipleEnabledRolesInTransaction(t *testing.T) {
 	db, mock, err := sqlmock.New()
 	if err != nil {
@ -122,8 +199,13 @@ func TestCreateUserAssignsMultipleEnabledRolesInTransaction(t *testing.T) {
 	mock.ExpectExec(regexp.QuoteMeta(`INSERT INTO users (username, password, description) VALUES (?, ?, ?)`)).
 		WithArgs("zhangsan", "hash", "测试用户").
 		WillReturnResult(sqlmock.NewResult(7, 1))
-	mock.ExpectQuery(regexp.QuoteMeta(`SELECT COUNT(*) FROM roles WHERE enabled = TRUE AND id IN (?, ?)`)).
-		WithArgs(int64(2), int64(3)).
+	// 新建的用户在 user_roles 里还没有记录，EXISTS 恒为假，所以这里等价于
+	// "只允许启用中的角色"。
+	mock.ExpectQuery(regexp.QuoteMeta(`SELECT COUNT(*) FROM roles r
+		WHERE r.id IN (?, ?)
+		AND (r.enabled = TRUE OR EXISTS (
+			SELECT 1 FROM user_roles ur WHERE ur.role_id = r.id AND ur.user_id = ?))`)).
+		WithArgs(int64(2), int64(3), int64(7)).
 		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(2))
 	mock.ExpectExec(regexp.QuoteMeta(`INSERT INTO user_roles (user_id, role_id) VALUES (?, ?), (?, ?)`)).
 		WithArgs(int64(7), int64(2), int64(7), int64(3)).