---
req_id: REQ-MOD-002
date: 2026-05-06
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/entity/ModuleEntity.java:56 — `iParentId` 改为 `FieldStrategy.IGNORED` 是 entity 全局行为变更。本期 update 走 load-then-modify 全量回填路径所以安全；但未来若有 partial updateById 路径会把 iParentId 写成 NULL。建议在字段注释加 "调用方必须 selectById 后再 updateById"，或将 NULL 写入语义收敛到 update 方法本地。
- backend/src/test/java/com/xly/erp/module/mod/controller/ModuleControllerIT.java:207 — spec § 验收 #2「正向 — 设置父模块到合法 sibling」只覆盖了 setParentToNull，没有覆盖"把 iParentId 改到另一个未删除模块"的正向写入路径。建议追加一个 IT 用例。
- backend/src/test/java/com/xly/erp/module/mod/service/ModuleServiceImplTest.java:630 — `update_full_returnsVOWithUpdatedFields` 的 dto.iParentId 与 target.iParentId 都是 null，断言只覆盖了 null→null。建议把 dto.setIParentId(非 null) 并 mock 合法父返回，覆盖 walk-up depth=1 直接退出循环的非环场景。
- backend/src/main/java/com/xly/erp/module/mod/controller/ModuleController.java:24 — 本 REQ 顺手把 REQ-MOD-001 那行 `// REQ-USR-004 完成后追加 @PreAuthorize(...)` 类外注释改成了 Javadoc，触及既有代码（非严格 surgical）。可读性改善但 commit message 未披露；下次类似情况建议拆 refactor commit 或在 body 注脚说明。
- backend/src/test/java/com/xly/erp/module/mod/service/ModuleServiceImplTest.java:597 — `when(moduleMapper.selectById(999999)).thenReturn(null)` 冗余（Mockito 默认就是 null），可删除。

## 反例 / 测试覆盖缺口

1. spec § 验收 #2「正向设置父模块到 sibling」在 IT 与单元两层都缺成功路径用例；只覆盖了"清空父"和各种 parent 校验失败用例，未直接验证 iParentId 从 null/某值改到另一个有效模块后 DB 实际写入了新父 id 的主线路径。
2. spec § 验收 #6「目标已软删除」仅在单元层覆盖（`update_targetSoftDeleted_throws40421`），IT 缺一个"先 update bDeleted=1 再 PUT 返回 40421"用例。
3. spec § 验收 #8（枚举非法 `sDisplayType="X"`）/ #9（`sModuleType=51 字符`）只在 `ModuleUpdateDTOValidationTest` 单元层验证，未在 IT 走一遍 PUT 端到端断言 40010。
4. 环路检查的"4-5 层深度边界"和"父链中存在已软删除节点导致提前 break 的非环场景"未单独覆盖；本期数据量低可接受。
5. `FieldStrategy.IGNORED` 是 entity 全局变更，对未来其他 service 走 partial `updateById` 路径会埋雷；建议要么文档化要么把 NULL 写入收敛到本地策略。
6. 未发现硬编码凭据、未跨模块改动、未引入技术栈外组件、commit 全部带 `REQ-MOD-002` 标签、响应不回显堆栈——全部合规。错误码（40010/40411/40421/40921）与 docs/05 / spec 一致。环路检查实现 walk-up 沿父链最多 5 层 + 已删节点 break，逻辑正确。