2026-04-30-REQ-USR-002.md 3.64 KB

req_id: REQ-USR-002 date: 2026-04-30 round: 1

reviewer: superpower-code-reviewer

Review: REQ-USR-002 — round 1

结论

approve

Must-fix

(无)

Nice-to-have

  • backend/src/main/java/com/xly/erp/module/usr/service/impl/UserServiceImpl.java:134-140 — 与 USR-001 review #3 同源:permissionCategoryIds=[1,1,2] 全有效时 SQL IN 隐式去重 → countActiveByIds=2 ≠ ids.size=3 → 误抛 40023 假阴性。建议 ids = ids.stream().distinct().toList() 4 行修复,或 module-report 阶段统一处理两处。
  • backend/src/test/java/com/xly/erp/module/usr/controller/UserControllerIT.java:200-229 — putValidBody_with_jwt_returns200_andUpdates 已断言 sPasswordHash / sCreatedBy 保留,漏 tCreateDate 与 sBrandsId 锁。spec § 验收 工程 #2 显列 'sPasswordHash / sCreatedBy / tCreateDate 在 PUT 前后字面相同'。建议 SELECT 加这两列断言保留原值。
  • backend/src/test/java/com/xly/erp/module/usr/controller/UserControllerIT.java:200-229 — 类似 USR-001 review #4:响应 data 缺 sPasswordHash 不变量没断言。建议加 assertThat(jb.get("data").has("sPasswordHash")).isFalse()
  • backend/src/test/java/com/xly/erp/module/usr/service/UserServiceImplTest.java:305-317 — updateWithEmptyPermissionIds_clearsExisting 只覆盖 null 路径,未覆盖 [] 空 list 路径。spec § 输入末行明言 'null / 空 list → 清空' 是同等语义。建议补等价 case 或 parameterize。
  • backend/src/main/java/com/xly/erp/module/usr/dto/UpdateUserDTO.java — 类头缺 JavaDoc 说明 sPasswordHash 故意剔除(spec 显列 'API 契约声明该字段不可改')。两行注释帮助未来读者理解 DTO 与 CreateUserDTO 字段差异。
  • backend/src/main/java/com/xly/erp/module/usr/service/impl/UserServiceImpl.java:120 — update 方法体缺 // REQ-USR-002: 用户修改 行内锚点(与 USR-001 review #7 同源)。建议 module-report 阶段统一补齐 USR/MOD 所有 REQ 行内锚点。
  • backend/src/test/java/com/xly/erp/module/usr/controller/UserControllerIT.java:342-371 — putWithoutJwt_permitAllStub_* / putTamperedJwt_* 沿袭 stub 路径仍缺 // REQ-MOD-001 stub: see USR-004 follow-up 锚点;与上条统一 module-report 处理。

反例 / 测试覆盖缺口

Spec 验收清单(service 10 + mapperIT 1 + controllerIT 10)100% 落地,mvn 110/110 全绿。

  • 重建权限组策略(先 deleteByUserId 后 batch INSERT)严格匹配 spec § 业务规则 #6,@Transactional(rollbackFor = Exception.class) 包整流程。
  • 流程顺序(selectById 存在性 → 枚举 → staff → permission → updateById catch DupKey → deleteByUserId → INSERT × N)严格匹配 spec § 业务规则 1–7。
  • updateById 抛 DupKey 后 deleteByUserId 不被调用由 verify(userPermissionMapper, never()).deleteByUserId(any()) 单测显式锁定,事务回滚由 @Transactional 保证。
  • 不可改字段(sPasswordHash / sCreatedBy / tCreateDate / sBrandsId / sSubsidiaryId)依赖 MyBatis-Plus 默认 NOT_NULL 跳过 null 字段策略;service 单测用 ArgumentCaptor 断言这些字段在 entity 上为 null。
  • permissionCategoryIds null / 空 list 通过 ids != null && !ids.isEmpty() 短路同时跳过校验与 INSERT 但仍执行 deleteByUserId,正确实现「清空」语义。
  • LocalDateTime.now() 已 hoist 出 INSERT 循环(修正 USR-001 review nice-to-have #2)。

非阻塞缺口:permissionCategoryIds 重复 id 假阴性、sPasswordHash 响应不变量锁、tCreateDate/sBrandsId 保留断言、[] 空 list 等价路径覆盖、行内 REQ 锚点 + stub 锚点。前 4 条可在本 REQ 后续修补,后 2 条按既定路径留 module-report 统一处理。