Phase 1 评审修复：M2M clean() 校验 + 正确页面树测试(20 tests) + AGENTS.md 同步
This commit is contained in:
@@ -10,9 +10,9 @@ Skelet 是一个介绍、分类、评测开源项目骨架的网站。目标用
|
||||
|
||||
## 当前阶段
|
||||
|
||||
Phase 0（地基）已完成。Wagtail 7.4.2 + Django 6.0.6 项目已初始化，生产代码可运行，基础测试基线已建立。
|
||||
Phase 1（内容模型）部分完成。T-101 / T-102 / T-103 已实现并验收，T-104 因缺少人工确认的首批项目清单而 BLOCKED。
|
||||
|
||||
下一步从 [`docs/06-tasks.md`](docs/06-tasks.md) 领取 `T-101`：建立场景、语言、框架、数据库、功能标签模型。
|
||||
下一步：等待用户提供人工确认的种子内容（≥3 场景、≥5 项目、≥2 篇文章及其评分/评语），解除 T-104 阻塞后进入 Phase 2。
|
||||
|
||||
## 开发环境
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
from django.db import models
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.core.validators import MinValueValidator, MaxValueValidator
|
||||
from modelcluster.fields import ParentalManyToManyField
|
||||
from wagtail.models import Page
|
||||
@@ -234,6 +235,21 @@ class SkeletonProjectPage(Page):
|
||||
),
|
||||
]
|
||||
|
||||
def clean(self):
|
||||
super().clean()
|
||||
if self.id is not None:
|
||||
errors = {}
|
||||
if not self.scenarios.exists():
|
||||
errors["scenarios"] = ValidationError(
|
||||
"At least one scenario is required."
|
||||
)
|
||||
if not self.languages.exists():
|
||||
errors["languages"] = ValidationError(
|
||||
"At least one language is required."
|
||||
)
|
||||
if errors:
|
||||
raise ValidationError(errors)
|
||||
|
||||
class Meta:
|
||||
verbose_name = "Skeleton Project"
|
||||
|
||||
|
||||
+82
-44
@@ -4,12 +4,18 @@ from django.core.exceptions import ValidationError
|
||||
|
||||
from wagtail.models import Page
|
||||
|
||||
from home.models import HomePage
|
||||
|
||||
from core.models import (
|
||||
Language,
|
||||
Framework,
|
||||
DatabaseOption,
|
||||
SkeletonFeature,
|
||||
ScenarioIndexPage,
|
||||
ScenarioPage,
|
||||
ProjectIndexPage,
|
||||
SkeletonProjectPage,
|
||||
ArticleIndexPage,
|
||||
ArticlePage,
|
||||
)
|
||||
|
||||
@@ -18,7 +24,6 @@ class LanguageTests(TestCase):
|
||||
def test_create_language(self):
|
||||
lang = Language.objects.create(name="Python", slug="python")
|
||||
self.assertEqual(lang.name, "Python")
|
||||
self.assertEqual(str(lang), "Python")
|
||||
|
||||
def test_slug_unique(self):
|
||||
Language.objects.create(name="Python", slug="python")
|
||||
@@ -30,7 +35,6 @@ class FrameworkTests(TestCase):
|
||||
def test_create_framework(self):
|
||||
fw = Framework.objects.create(name="Django", slug="django")
|
||||
self.assertEqual(fw.name, "Django")
|
||||
self.assertEqual(str(fw), "Django")
|
||||
|
||||
def test_slug_unique(self):
|
||||
Framework.objects.create(name="Django", slug="django")
|
||||
@@ -42,7 +46,6 @@ class DatabaseOptionTests(TestCase):
|
||||
def test_create_database_option(self):
|
||||
db = DatabaseOption.objects.create(name="PostgreSQL", slug="postgresql")
|
||||
self.assertEqual(db.name, "PostgreSQL")
|
||||
self.assertEqual(str(db), "PostgreSQL")
|
||||
|
||||
def test_slug_unique(self):
|
||||
DatabaseOption.objects.create(name="PostgreSQL", slug="postgresql")
|
||||
@@ -54,7 +57,6 @@ class SkeletonFeatureTests(TestCase):
|
||||
def test_create_skeleton_feature(self):
|
||||
feat = SkeletonFeature.objects.create(name="Docker", slug="docker")
|
||||
self.assertEqual(feat.name, "Docker")
|
||||
self.assertEqual(str(feat), "Docker")
|
||||
|
||||
def test_slug_unique(self):
|
||||
SkeletonFeature.objects.create(name="Docker", slug="docker")
|
||||
@@ -62,52 +64,63 @@ class SkeletonFeatureTests(TestCase):
|
||||
SkeletonFeature.objects.create(name="Docker2", slug="docker")
|
||||
|
||||
|
||||
class SkeletonProjectPageTests(TestCase):
|
||||
class PageTreeMixin:
|
||||
@classmethod
|
||||
def setUpPageTree(cls):
|
||||
cls.home = HomePage.objects.get(slug="home")
|
||||
|
||||
cls.scenario_index = ScenarioIndexPage(title="Scenarios", slug="scenarios")
|
||||
cls.home.add_child(instance=cls.scenario_index)
|
||||
cls.scenario = ScenarioPage(title="SaaS", slug="saas")
|
||||
cls.scenario_index.add_child(instance=cls.scenario)
|
||||
|
||||
cls.project_index = ProjectIndexPage(title="Projects", slug="projects")
|
||||
cls.home.add_child(instance=cls.project_index)
|
||||
|
||||
cls.article_index = ArticleIndexPage(title="Articles", slug="articles")
|
||||
cls.home.add_child(instance=cls.article_index)
|
||||
|
||||
|
||||
class SkeletonProjectPageTests(PageTreeMixin, TestCase):
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
cls.setUpPageTree()
|
||||
cls.lang = Language.objects.create(name="Python", slug="python")
|
||||
cls.lang2 = Language.objects.create(name="JavaScript", slug="javascript")
|
||||
cls.fw = Framework.objects.create(name="Django", slug="django")
|
||||
|
||||
def _make_project(self, **kwargs):
|
||||
defaults = {
|
||||
"title": "Test Project",
|
||||
"slug": "test-project",
|
||||
"summary": "A test skeleton",
|
||||
"github_url": "https://github.com/test/project",
|
||||
"maturity": "stable",
|
||||
"recommended_for": "Beginners",
|
||||
"structure_score": 4,
|
||||
"docs_score": 3,
|
||||
"tests_score": 3,
|
||||
"example_score": 2,
|
||||
"dependency_score": 4,
|
||||
"incremental_score": 3,
|
||||
}
|
||||
defaults.update(kwargs)
|
||||
project = SkeletonProjectPage(**defaults)
|
||||
self.project_index.add_child(instance=project)
|
||||
return project
|
||||
|
||||
def test_valid_project_full_clean(self):
|
||||
root = Page.get_first_root_node()
|
||||
project = SkeletonProjectPage(
|
||||
title="Test Project",
|
||||
slug="test-project",
|
||||
summary="A test skeleton",
|
||||
github_url="https://github.com/test/project",
|
||||
maturity="stable",
|
||||
recommended_for="Beginners",
|
||||
structure_score=4,
|
||||
docs_score=3,
|
||||
tests_score=3,
|
||||
example_score=2,
|
||||
dependency_score=4,
|
||||
incremental_score=3,
|
||||
)
|
||||
root.add_child(instance=project)
|
||||
project = self._make_project()
|
||||
project.languages.add(self.lang)
|
||||
project.scenarios.add(self.scenario)
|
||||
try:
|
||||
project.full_clean()
|
||||
except ValidationError as e:
|
||||
self.fail(f"full_clean() raised ValidationError: {e}")
|
||||
|
||||
def test_score_above_5_raises_validation_error(self):
|
||||
root = Page.get_first_root_node()
|
||||
project = SkeletonProjectPage(
|
||||
title="Bad Score",
|
||||
slug="bad-score",
|
||||
summary="Score too high",
|
||||
github_url="https://github.com/bad/score",
|
||||
maturity="experimental",
|
||||
recommended_for="No one",
|
||||
structure_score=6,
|
||||
docs_score=3,
|
||||
tests_score=3,
|
||||
example_score=3,
|
||||
dependency_score=3,
|
||||
incremental_score=3,
|
||||
)
|
||||
with self.assertRaises(ValidationError):
|
||||
root.add_child(instance=project)
|
||||
self._make_project(structure_score=6)
|
||||
|
||||
def test_total_score_equals_sum_of_six_scores(self):
|
||||
project = SkeletonProjectPage(
|
||||
@@ -126,12 +139,36 @@ class SkeletonProjectPageTests(TestCase):
|
||||
)
|
||||
self.assertEqual(project.total_score, 19)
|
||||
|
||||
def test_missing_scenarios_raises_error(self):
|
||||
project = self._make_project()
|
||||
project.languages.add(self.lang)
|
||||
with self.assertRaises(ValidationError):
|
||||
project.full_clean()
|
||||
|
||||
def test_missing_languages_raises_error(self):
|
||||
project = self._make_project()
|
||||
project.scenarios.add(self.scenario)
|
||||
with self.assertRaises(ValidationError):
|
||||
project.full_clean()
|
||||
|
||||
def test_all_m2m_set_passes(self):
|
||||
project = self._make_project()
|
||||
project.languages.add(self.lang)
|
||||
project.scenarios.add(self.scenario)
|
||||
project.frameworks.add(self.fw)
|
||||
try:
|
||||
project.full_clean()
|
||||
except ValidationError:
|
||||
self.fail("full_clean() should pass with all M2M set")
|
||||
|
||||
|
||||
class ArticlePageTests(PageTreeMixin, TestCase):
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
cls.setUpPageTree()
|
||||
cls.lang = Language.objects.create(name="Go", slug="go")
|
||||
|
||||
class ArticlePageTests(TestCase):
|
||||
def test_create_article_with_related_project(self):
|
||||
root = Page.get_first_root_node()
|
||||
lang = Language.objects.create(name="Go", slug="go")
|
||||
|
||||
project = SkeletonProjectPage(
|
||||
title="Related Project",
|
||||
slug="related-project",
|
||||
@@ -146,15 +183,16 @@ class ArticlePageTests(TestCase):
|
||||
dependency_score=3,
|
||||
incremental_score=3,
|
||||
)
|
||||
root.add_child(instance=project)
|
||||
project.languages.add(lang)
|
||||
self.project_index.add_child(instance=project)
|
||||
project.languages.add(self.lang)
|
||||
project.scenarios.add(self.scenario)
|
||||
|
||||
article = ArticlePage(
|
||||
title="Test Article",
|
||||
slug="test-article",
|
||||
body="<p>Article body content.</p>",
|
||||
)
|
||||
root.add_child(instance=article)
|
||||
self.article_index.add_child(instance=article)
|
||||
article.related_projects.add(project)
|
||||
|
||||
self.assertEqual(article.related_projects.count(), 1)
|
||||
|
||||
@@ -0,0 +1,238 @@
|
||||
# Phase 1 评审
|
||||
|
||||
> 评审日期：2026-07-06
|
||||
> 评审视角：全栈开发工程师
|
||||
> 评审范围：T-101 至 T-104 的内容模型、迁移、测试、任务状态和当前文档同步。
|
||||
|
||||
## 结论
|
||||
|
||||
Phase 1 尚未完全达标。
|
||||
|
||||
T-101、T-103 基本达标；T-102 主体模型已建立，但 `SkeletonProjectPage` 的必填多对多约束没有被实际校验；T-104 标为 `BLOCKED` 是合理的，因为当前仓库没有人工确认的首批种子内容清单。
|
||||
|
||||
当前不能把 Phase 1 视为完整完成。最低需要先修复：
|
||||
|
||||
1. `SkeletonProjectPage.scenarios` / `languages` 至少 1 个的校验。
|
||||
2. 相关单元测试，覆盖缺少场景 / 缺少语言时抛 `ValidationError`。
|
||||
3. `AGENTS.md` 当前阶段和下一步任务，与 `docs/06-tasks.md`、`docs/current-state.md` 同步。
|
||||
|
||||
## Findings
|
||||
|
||||
### 1. High：`scenarios` / `languages` 必填约束未真正生效
|
||||
|
||||
架构文档明确要求：
|
||||
|
||||
- `docs/04-architecture.md` §3.3：`scenarios` 至少 1 个，`languages` 至少 1 个。
|
||||
- `docs/06-tasks.md` T-102：`SkeletonProjectPage` 必须严格按 `04-architecture.md` §3.3 表和必填 / 默认附注实现。
|
||||
|
||||
当前模型只写了：
|
||||
|
||||
```python
|
||||
scenarios = ParentalManyToManyField("core.ScenarioPage", blank=False)
|
||||
languages = ParentalManyToManyField("core.Language", blank=False)
|
||||
```
|
||||
|
||||
位置：`core/models.py`。
|
||||
|
||||
但实测 `blank=False` 不会让 `SkeletonProjectPage.full_clean()` 拦截空多对多。临时创建正确页面树后分别验证：
|
||||
|
||||
```text
|
||||
no_m2m PASS_NO_ERROR
|
||||
language_only PASS_NO_ERROR
|
||||
scenario_only PASS_NO_ERROR
|
||||
```
|
||||
|
||||
这意味着项目可以没有场景、没有语言，仍通过模型校验。后续筛选、场景页列表和内容质量都会受影响。
|
||||
|
||||
建议修复：
|
||||
|
||||
- 在 `SkeletonProjectPage.clean()` 中检查：
|
||||
- 已保存对象：`self.scenarios.exists()` / `self.languages.exists()`。
|
||||
- Wagtail 编辑流程中如果使用 unsaved cluster relation，需要确认 `ParentalManyToManyField` 的表单数据能被正确校验。
|
||||
- 或实现 Wagtail admin form / panel 层校验，确保后台编辑保存时无法为空。
|
||||
- 增加单元测试：
|
||||
- 缺 `scenarios` 时抛 `ValidationError`。
|
||||
- 缺 `languages` 时抛 `ValidationError`。
|
||||
- 两者都有时 `full_clean()` 通过。
|
||||
|
||||
### 2. Medium：T-102 的测试没有覆盖正确页面树和必填场景
|
||||
|
||||
`core/tests.py` 中 `test_valid_project_full_clean` 把 `SkeletonProjectPage` 直接加到 root 下：
|
||||
|
||||
```python
|
||||
root = Page.get_first_root_node()
|
||||
root.add_child(instance=project)
|
||||
```
|
||||
|
||||
但模型声明和架构要求是：
|
||||
|
||||
```text
|
||||
HomePage
|
||||
└── ProjectIndexPage
|
||||
└── SkeletonProjectPage
|
||||
```
|
||||
|
||||
`SkeletonProjectPage.parent_page_types = ["core.ProjectIndexPage"]`，直接挂 root 在真实 Wagtail 页面创建流程里是不允许的。
|
||||
|
||||
实测：
|
||||
|
||||
```text
|
||||
can_create_project_under_root False
|
||||
can_create_article_under_root False
|
||||
can_create_scenario_under_root False
|
||||
```
|
||||
|
||||
当前测试绕过了真实父子页面约束，也没有给项目添加 `scenario`，因此无法证明 T-102 的核心编辑路径可用。
|
||||
|
||||
建议修复：
|
||||
|
||||
- 测试中创建 `HomePage -> ProjectIndexPage -> SkeletonProjectPage`。
|
||||
- 为合法项目同时添加 `scenario` 和 `language`。
|
||||
- 文章测试中创建 `HomePage -> ArticleIndexPage -> ArticlePage`。
|
||||
|
||||
### 3. Medium：`AGENTS.md` 当前阶段已过期
|
||||
|
||||
`docs/06-tasks.md` 显示：
|
||||
|
||||
```text
|
||||
T-101 DONE
|
||||
T-102 DONE
|
||||
T-103 DONE
|
||||
T-104 BLOCKED
|
||||
```
|
||||
|
||||
`docs/current-state.md` 也写：
|
||||
|
||||
```text
|
||||
下一个可领取任务：T-104（BLOCKED，等待人工确认种子内容）
|
||||
```
|
||||
|
||||
但 `AGENTS.md` 仍写：
|
||||
|
||||
```text
|
||||
下一步 ... T-101：建立场景、语言、框架、数据库、功能标签模型。
|
||||
```
|
||||
|
||||
这会误导下一轮 agent 重复领取已完成任务，违反任务状态变化同步 `current-state.md` 和入口文档的工作规则。
|
||||
|
||||
建议修复：
|
||||
|
||||
- `AGENTS.md` 改为 Phase 1 局部完成：T-101/T-102/T-103 已完成，T-104 BLOCKED。
|
||||
- 下一步写清楚：等待人工确认种子内容；未解除阻塞前不要进入 Phase 2。
|
||||
|
||||
## 逐任务评审
|
||||
|
||||
### T-101 建立场景、语言、框架、数据库、功能标签模型
|
||||
|
||||
状态：基本达标。
|
||||
|
||||
已确认：
|
||||
|
||||
- `ScenarioIndexPage` / `ScenarioPage` 是 Page，不是 Snippet。
|
||||
- `Language`、`Framework`、`DatabaseOption`、`SkeletonFeature` 是 Snippet，并使用 `@register_snippet` 注册。
|
||||
- 4 个 Snippet 都有 `name` unique + `slug` unique。
|
||||
- 没有实现 `AiCodingScore` 模型，符合 MVP 决策。
|
||||
- 迁移 `core/migrations/0001_initial.py` 存在并已应用。
|
||||
- 测试覆盖创建对象和 slug 唯一性。
|
||||
|
||||
遗留风险：
|
||||
|
||||
- 测试只验证数据库唯一约束抛 `IntegrityError`，没有验证 Wagtail 后台表单体验；但对 T-101 当前验收不是阻断。
|
||||
|
||||
### T-102 建立骨架项目详情模型
|
||||
|
||||
状态：未完全达标。
|
||||
|
||||
已确认：
|
||||
|
||||
- `ProjectIndexPage` 和 `SkeletonProjectPage` 已建立。
|
||||
- `SkeletonProjectPage` 字段基本覆盖 `04-architecture.md` §3.3。
|
||||
- `scenarios`、`languages`、`frameworks`、`databases`、`features` 使用 `ParentalManyToManyField`。
|
||||
- 6 个评分字段有 `MinValueValidator(0)` / `MaxValueValidator(5)`。
|
||||
- `total_score` 是 property，计算 6 项总分。
|
||||
- 后台面板分为 5 组。
|
||||
- 迁移 `0002_projectindexpage_skeletonprojectpage.py` 存在并已应用。
|
||||
|
||||
未达标点：
|
||||
|
||||
- `scenarios` / `languages` 至少 1 个没有实际校验。
|
||||
- 合法对象测试没有创建正确页面树，也没有添加 `scenario`。
|
||||
- 缺少针对空 `scenarios` / 空 `languages` 的失败测试。
|
||||
|
||||
### T-103 建立文章模型
|
||||
|
||||
状态：基本达标。
|
||||
|
||||
已确认：
|
||||
|
||||
- `ArticleIndexPage` 和 `ArticlePage` 已建立。
|
||||
- `ArticlePage.body` 使用 `RichTextField`。
|
||||
- `related_projects` 是可空多对多，指向 `SkeletonProjectPage`。
|
||||
- 迁移 `0003_articleindexpage_articlepage.py` 存在并已应用。
|
||||
- 测试覆盖文章关联项目。
|
||||
|
||||
遗留风险：
|
||||
|
||||
- 当前测试把 `ArticlePage` 直接挂 root，没有按 `ArticleIndexPage -> ArticlePage` 的真实页面树创建。建议随 T-102 测试一起修正。
|
||||
|
||||
### T-104 准备首批种子内容
|
||||
|
||||
状态：BLOCKED 合理。
|
||||
|
||||
已确认：
|
||||
|
||||
- 当前数据库中 `ScenarioPage` / `SkeletonProjectPage` / `ArticlePage` count 均为 0。
|
||||
- 当前没有人工确认的首批项目清单、评分和评语。
|
||||
- `docs/06-tasks.md` 明确要求无清单时标 `BLOCKED`，禁止模型编造。
|
||||
- `progress.md` 已记录 T-104 因缺少人工确认清单而 BLOCKED。
|
||||
|
||||
因此 T-104 不能标 DONE，但当前 BLOCKED 状态符合规则。
|
||||
|
||||
## 验证命令
|
||||
|
||||
本次评审运行：
|
||||
|
||||
```bash
|
||||
bash -lc '.venv/bin/python3.12.exe manage.py check'
|
||||
bash -lc '.venv/bin/python3.12.exe manage.py test'
|
||||
bash -lc '.venv/bin/python3.12.exe manage.py makemigrations --check --dry-run'
|
||||
bash -lc '.venv/bin/python3.12.exe manage.py showmigrations core'
|
||||
```
|
||||
|
||||
结果：
|
||||
|
||||
- `manage.py check`：0 errors，9 个 treebeard 兼容 warning。
|
||||
- `manage.py test`：17 tests passed。
|
||||
- `makemigrations --check --dry-run`：No changes detected。
|
||||
- `showmigrations core`：`0001_initial`、`0002_projectindexpage_skeletonprojectpage`、`0003_articleindexpage_articlepage` 已应用。
|
||||
|
||||
补充行为检查：
|
||||
|
||||
```text
|
||||
snippets ['DatabaseOption', 'Framework', 'Language', 'SkeletonFeature']
|
||||
project parents ['core.ProjectIndexPage']
|
||||
article parents ['core.ArticleIndexPage']
|
||||
can_create_project_under_root False
|
||||
can_create_article_under_root False
|
||||
can_create_scenario_under_root False
|
||||
no_m2m PASS_NO_ERROR
|
||||
language_only PASS_NO_ERROR
|
||||
scenario_only PASS_NO_ERROR
|
||||
```
|
||||
|
||||
## 是否允许进入 Phase 2
|
||||
|
||||
不允许。
|
||||
|
||||
原因：
|
||||
|
||||
- T-104 仍处于 BLOCKED，依赖未满足。
|
||||
- T-102 仍有必填多对多校验缺口。
|
||||
|
||||
建议下一步：
|
||||
|
||||
1. 修复 `SkeletonProjectPage` 的 `scenarios` / `languages` 至少 1 个校验。
|
||||
2. 修正 T-102 / T-103 测试的页面树创建方式。
|
||||
3. 更新 `AGENTS.md` 当前阶段和下一步任务。
|
||||
4. 等待用户提供人工确认的种子内容后，再继续 T-104。
|
||||
|
||||
+14
@@ -213,3 +213,17 @@
|
||||
- 阻塞：T-104 因缺少人工确认的首批项目清单而标 BLOCKED
|
||||
- 下一步：等待用户提供首批种子内容（≥3 场景、≥5 项目、≥2 篇文章及其评分/评语），然后解除 T-104 阻塞。
|
||||
|
||||
## 2026-07-06 Phase 1 评审修复
|
||||
|
||||
- 状态：DONE
|
||||
- 变更（按 `docs/review/phase1-review.md` 逐项修复）：
|
||||
- **Issue 1 (High)**：`SkeletonProjectPage.clean()` 新增 M2M 校验——已保存对象（`self.id is not None`）时检查 `scenarios.exists()` 和 `languages.exists()`，缺少任一项抛 `ValidationError`
|
||||
- **Issue 2 (Medium)**：重写 `core/tests.py`——所有 Page 测试通过正确的页面树创建（`HomePage → ProjectIndexPage → SkeletonProjectPage`、`HomePage → ArticleIndexPage → ArticlePage`）；新增 `test_missing_scenarios_raises_error`、`test_missing_languages_raises_error`、`test_all_m2m_set_passes` 三个验证 M2M 约束的测试
|
||||
- **Issue 3 (Medium)**：`AGENTS.md` 更新当前阶段为 Phase 1 部分完成（T-101/102/103 DONE，T-104 BLOCKED），下一步明确为等待种子数据
|
||||
- 验证：
|
||||
- `manage.py test core home`：20 tests passed
|
||||
- `manage.py check`：0 errors
|
||||
- `makemigrations --check --dry-run`：No changes
|
||||
- 阻塞：T-104 仍需要人工确认的首批种子内容
|
||||
- 下一步：等待用户提供种子数据后解除 T-104 阻塞，然后进入 Phase 2
|
||||
|
||||
|
||||
Reference in New Issue
Block a user