docs(mod): review approved + check off REQ-MOD-002

zichun
1 parent e1342b63
Showing 2 changed files with 35 additions and 1 deletions
docs/08-模块任务管理.md
docs/superpowers/reviews/2026-04-29-REQ-MOD-002.md
@@ -61,7 +61,7 @@
   - MR: —
   - 功能:
     - [x] REQ-MOD-001 模块新增
-    - [ ] REQ-MOD-002 模块修改
+    - [x] REQ-MOD-002 模块修改
     - [ ] REQ-MOD-003 模块删除
     - [ ] REQ-MOD-004 模块查询
+---
+req_id: REQ-MOD-002
+date: 2026-04-29
+round: 1
+reviewer: superpower-code-reviewer
+---
+
+# Review: REQ-MOD-002 — round 1
+
+## 结论
+approve
+
+## Must-fix
+（无）
+
+## Nice-to-have
+
+- backend/src/main/java/com/xly/erp/module/mod/service/impl/ModuleServiceImpl.java:111 — 环检测 `depth >= MAX_PARENT_DEPTH` 守卫位于循环体中段，最大执行 51 次 mapper 调用而非 50（`cur.equals(id)` 判断先于守卫）。建议把守卫挪到 for 条件 `depth < MAX_PARENT_DEPTH` 或先判 depth 再判 equals。无功能影响，仅语义更精确。
+- backend/src/test/java/com/xly/erp/module/mod/controller/ModuleControllerIT.java:263 — `putWithoutJwt_permitAllStub_returns200_andDoesNotChangeCreatedBy` 与 MOD-001 同名 POST 用例同样硬绑 stub 行为；建议加 javadoc 锚点 `// REQ-MOD-001 stub: see USR-004 follow-up`，便于 USR-004 上线时一次性 grep 替换为 401 期望。
+- backend/src/test/java/com/xly/erp/module/mod/controller/ModuleControllerIT.java:188 — happy-path IT 仅断言 `sProcedureName` / `sCreatedBy` 保留，未端到端断言 `tCreateDate` / `sBrandsId` / `sSubsidiaryId`。Service 单测已 captureArgument 验证这些字段在 entity 上为 null（依赖 NOT_NULL 跳过），如要 IT 双重保险可加列断言。
+- backend/src/test/java/com/xly/erp/module/mod/service/ModuleServiceImplTest.java:215 — update 用例直接 stub interface default 方法 `existsActiveById` / `selectParentIdById`，与 MOD-001 create 用例 stub 抽象方法 `findActiveFlagById` 风格不一致。功能上 Mockito 支持；建议统一风格——推荐 MOD-001 改为 stub 默认方法（更直观）。
+- backend/src/main/java/com/xly/erp/module/mod/service/impl/ModuleServiceImpl.java:92 — `moduleMapper.updateById(entity)` 返回值（受影响行数）被丢弃。`@Transactional` + 先做 selectById 校验后并发 DELETE 概率极低，但若命中可能 0 行受影响而方法仍返回成功。可断言 affected==1 否则抛 BizException(40400)。
+- backend/src/main/java/com/xly/erp/module/mod/dto/UpdateModuleDTO.java:32 — `iParentId` / `iSortOrder` 缺 `@PositiveOrZero` / `@Min` 约束。spec 未要求；负数等异常值需要在 service 层走父链失败才抛 40021。
+
+## 反例 / 测试覆盖缺口
+
+Spec 验收清单（单元 7 + 集成 7 + 工程 3）端到端实现并 41/41 全绿；分层 / 命名 / 统一响应 / 异常处理 / 事务边界 / 安全 stub 均符合 docs/04 规范。
+
+非阻塞缺口：
+1. `tCreateDate` / `sBrandsId` / `sSubsidiaryId` 在 IT 端未断言保持原值（service 单测已断言 entity 字段 null + NOT_NULL 策略保证不覆盖）。
+2. 父模块指向 `bDeleted=1` 旧记录是否回 40021 未单独验证（与 MOD-001 review gap #2 同源）。
+3. `updateById` 返回 0 行（并发删除）的场景未覆盖。
+4. DTO `@Size` 长度溢出（如 sModuleNameZh > 100）路径未单独 IT。
+5. 环检测 `MAX_PARENT_DEPTH` 超深路径仅有间接覆盖（脏数据 50 层链未构造测试）。