171 lines
9.2 KiB
Markdown
171 lines
9.2 KiB
Markdown
# Phase 2 前台 MVP 评审
|
||
|
||
评审日期：2026-07-06
|
||
评审角色：全栈开发工程师
|
||
评审对象：DeepSeek 完成的 Phase 2（T-201 至 T-206）
|
||
|
||
## 结论
|
||
|
||
Phase 2 **暂不达标**。
|
||
|
||
基础页面、首页、项目详情、文章详情等主要页面已经可访问，自动化测试也通过；但筛选搜索的组合参数存在 500 错误，且场景页没有实现任务详单要求的分页。因此不能把 Phase 2 视为完整验收通过。
|
||
|
||
## 阻断问题
|
||
|
||
### P0：项目列表 `min_score + q` 组合筛选会 500
|
||
|
||
- 影响任务：T-204
|
||
- 位置：`core/models.py:151` 至 `core/models.py:162`
|
||
- 现象：`min_score` 合法时，代码把 `projects` 从 QuerySet 转成 list；后续如果同时存在 `q`，继续调用 `projects.filter(...)`，触发 `AttributeError: 'list' object has no attribute 'filter'`。
|
||
- 复现：
|
||
|
||
```bash
|
||
.venv/bin/python3.12.exe manage.py shell -c "from django.test import Client; c=Client(raise_request_exception=False); print(c.get('/projects/?min_score=18&q=Django').status_code)"
|
||
```
|
||
|
||
实际结果：`500`
|
||
|
||
任务契约要求 `language` / `framework` / `database` / `min_score` / `q` / `page` 多参数同时出现时按 AND 组合，见 `docs/04-architecture.md:157` 至 `docs/04-architecture.md:161`。当前实现只覆盖了单项筛选测试，没有覆盖组合筛选，见 `core/tests.py:119` 至 `core/tests.py:136`。
|
||
|
||
建议修复方向：
|
||
|
||
- 保持筛选链路的数据类型一致。
|
||
- 可以先用 ORM 对 `q`、language、framework、database 过滤，再对 `min_score` 做 Python 层过滤；也可以用 annotation 计算总分后在 ORM 层过滤。
|
||
- 增加组合筛选测试，例如 `/projects/?language=python&framework=django&min_score=18&q=Django`。
|
||
|
||
### P1：场景页未实现分页
|
||
|
||
- 影响任务：T-203
|
||
- 位置：`core/models.py:32` 至 `core/models.py:40`，`core/templates/core/scenario_page.html:7` 至 `core/templates/core/scenario_page.html:27`
|
||
- 现象：场景页直接把全部匹配项目放进 `context["projects"]`，模板也没有分页控件。
|
||
- 任务详单要求：场景页只列 live 项目，分页每页 20，空场景渲染空状态。
|
||
|
||
当前只满足 live 项目和空状态；分页缺失。已有测试只验证有项目和空状态，未验证超过 20 条时分页行为，见 `core/tests.py:40` 至 `core/tests.py:75`。
|
||
|
||
建议修复方向：
|
||
|
||
- 在 `ScenarioPage.get_context()` 中使用 `Paginator(projects, 20)`。
|
||
- 支持非法 `page` 回退到第一页，行为和项目列表页一致。
|
||
- 在 `scenario_page.html` 复用项目列表页的分页 UI。
|
||
- 增加 21 个项目的场景页分页测试。
|
||
|
||
## 非阻断问题
|
||
|
||
### P2：`docs/current-state.md` 测试数量已过期
|
||
|
||
- 位置：`docs/current-state.md:19`
|
||
- 现象：文档写的是 `home/tests.py` 5 个、`core/tests.py` 17 个；实际 `manage.py test` 输出为 31 个测试。
|
||
- 影响：不影响运行，但会误导后续 agent 判断当前状态。
|
||
|
||
建议修复：更新当前快照里的测试数量。
|
||
|
||
### P2：文章列表实现偏薄，但可接受
|
||
|
||
- 影响任务：T-206
|
||
- 位置：`core/templates/core/article_index_page.html:7` 至 `core/templates/core/article_index_page.html:13`
|
||
- 现状：文章列表通过 `page.get_children.live` 直接渲染标题链接，没有自定义 `get_context()`，也没有分页。
|
||
- 评估：T-206 只要求文章可访问、文章详情可链接项目详情；当前能满足最低验收。后续内容量增长时建议补排序、摘要和分页。
|
||
|
||
## 逐任务验收
|
||
|
||
| 任务 | 结论 | 说明 |
|
||
| --- | --- | --- |
|
||
| T-201 基础页面框架 | 达标 | `base.html` 有 viewport meta，导航、页脚、基础 CSS 已实现，容器使用 `max-width` 和响应式宽度。 |
|
||
| T-202 首页 | 达标 | 首页展示场景入口、推荐骨架、最新文章，内容来自数据库上下文。 |
|
||
| T-203 场景页和项目列表页 | 未达标 | 项目列表页有分页和空状态；场景页有项目和空状态，但缺少每页 20 的分页。 |
|
||
| T-204 筛选与基础搜索 | 未达标 | 单项筛选可用，但 `min_score` 与 `q` 组合请求返回 500，不符合 AND 组合契约。 |
|
||
| T-205 骨架详情页 | 达标 | 展示详情字段、评分、推荐理由，`is_sponsored=True` 时渲染 Sponsored 标识。 |
|
||
| T-206 文章列表和详情 | 达标 | `/articles/` 和文章详情可访问，文章详情可链接关联项目；实现较薄但满足最低验收。 |
|
||
|
||
## 验证记录
|
||
|
||
已执行：
|
||
|
||
```bash
|
||
.venv/bin/python3.12.exe manage.py check
|
||
.venv/bin/python3.12.exe manage.py makemigrations --check --dry-run
|
||
.venv/bin/python3.12.exe manage.py test
|
||
.venv/bin/python3.12.exe manage.py shell -c "from core.models import ScenarioPage,SkeletonProjectPage,ArticlePage; print(ScenarioPage.objects.live().count(), SkeletonProjectPage.objects.live().count(), ArticlePage.objects.live().count())"
|
||
.venv/bin/python3.12.exe manage.py shell -c "from django.test import Client; c=Client(raise_request_exception=False); paths=['/','/projects/','/articles/','/projects/?language=python','/projects/?min_score=18','/projects/?q=Django','/projects/?min_score=18&q=Django']; [print(p, c.get(p).status_code) for p in paths]"
|
||
```
|
||
|
||
结果摘要：
|
||
|
||
- `manage.py check`：0 error，9 个 treebeard/Wagtail 兼容 warning。
|
||
- `makemigrations --check --dry-run`：No changes detected。
|
||
- `manage.py test`：31 tests passed。
|
||
- seed 数据：3 个 live 场景、5 个 live 项目、2 篇 live 文章。
|
||
- URL 补测：
|
||
- `/`：200
|
||
- `/projects/`：200
|
||
- `/articles/`：200
|
||
- `/projects/?language=python`：200
|
||
- `/projects/?min_score=18`：200
|
||
- `/projects/?q=Django`：200
|
||
- `/projects/?min_score=18&q=Django`：500
|
||
|
||
## 修复验收建议
|
||
|
||
修复后至少补充并通过以下测试：
|
||
|
||
1. 项目列表组合筛选：`language + framework + database + min_score + q` 同时存在时返回 200，并只展示满足全部条件的项目。
|
||
2. 项目列表组合筛选无结果时返回 200，并展示空状态。
|
||
3. 场景页 21 个 live 项目时第一页展示 20 个，第二页展示剩余项目。
|
||
4. 场景页非法 `page` 参数返回第一页，不报错。
|
||
|
||
修复完成后重新运行：
|
||
|
||
```bash
|
||
.venv/bin/python3.12.exe manage.py check
|
||
.venv/bin/python3.12.exe manage.py makemigrations --check --dry-run
|
||
.venv/bin/python3.12.exe manage.py test
|
||
```
|
||
|
||
## 2026-07-06 修复复查
|
||
|
||
复查对象：提交 `714d218 Phase 2 评审修复: P0 min_score+&q=500 (ORM→list 顺序) + P1 场景分页(20/page) + P2 文档同步 + 36 tests`
|
||
|
||
复查结论：原 Phase 2 阻断问题 **已达标**，Phase 2 当前可视为通过复查。
|
||
|
||
### 复查结果
|
||
|
||
| 原问题 | 状态 | 证据 |
|
||
| --- | --- | --- |
|
||
| P0：项目列表 `min_score + q` 组合筛选会 500 | 已达标 | `ProjectIndexPage.get_context()` 先执行 `q` ORM 过滤，再做 `min_score` Python 过滤；`/projects/?min_score=18&q=Django` 复测返回 200。 |
|
||
| P1：场景页未实现分页 | 已达标 | `ScenarioPage.get_context()` 已使用 `Paginator(projects, 20)`；`scenario_page.html` 已渲染分页控件；新增 21 条项目分页测试。 |
|
||
| P2：`docs/current-state.md` 测试数量已过期 | 已达标 | 当前快照已更新为 31 个测试基线；修复后实际测试为 36 个。 |
|
||
| P2：文章列表实现偏薄 | 可接受 | 该项原本非阻断，当前未要求扩展；仍满足 T-206 最低验收。 |
|
||
|
||
### 代码核对
|
||
|
||
- `core/models.py:32` 至 `core/models.py:46`：场景页项目列表已分页，非法页码和越界页码回退第一页。
|
||
- `core/models.py:151` 至 `core/models.py:168`：项目列表筛选顺序已调整，`q` 不再在 list 上调用 `.filter()`。
|
||
- `core/templates/core/scenario_page.html:26` 至 `core/templates/core/scenario_page.html:36`：场景页已展示分页导航。
|
||
- `core/tests.py:78` 至 `core/tests.py:116`：新增场景页分页测试。
|
||
- `core/tests.py:178` 至 `core/tests.py:190`：新增组合筛选与空状态测试。
|
||
|
||
### 复查验证命令
|
||
|
||
已执行：
|
||
|
||
```bash
|
||
.venv/bin/python3.12.exe manage.py check
|
||
.venv/bin/python3.12.exe manage.py makemigrations --check --dry-run
|
||
.venv/bin/python3.12.exe manage.py test
|
||
.venv/bin/python3.12.exe manage.py shell -c "from django.test import Client; from core.models import ScenarioPage,SkeletonProjectPage,ArticlePage; c=Client(raise_request_exception=False); scenario=ScenarioPage.objects.live().first(); paths=['/projects/?min_score=18&q=Django','/projects/?language=python&framework=django&min_score=18&q=Django','/projects/?language=go&framework=django&min_score=18', scenario.url, scenario.url+'?page=2', scenario.url+'?page=abc']; print('counts', ScenarioPage.objects.live().count(), SkeletonProjectPage.objects.live().count(), ArticlePage.objects.live().count()); [print(p, c.get(p).status_code) for p in paths]"
|
||
```
|
||
|
||
结果摘要：
|
||
|
||
- `manage.py check`：0 error，9 个 treebeard/Wagtail 兼容 warning。
|
||
- `makemigrations --check --dry-run`：No changes detected。
|
||
- `manage.py test`：36 tests passed。
|
||
- seed 数据：3 个 live 场景、5 个 live 项目、2 篇 live 文章。
|
||
- URL 复测：
|
||
- `/projects/?min_score=18&q=Django`：200
|
||
- `/projects/?language=python&framework=django&min_score=18&q=Django`：200
|
||
- `/projects/?language=go&framework=django&min_score=18`：200
|
||
- `/scenarios/modern-desktop-app-templates/`：200
|
||
- `/scenarios/modern-desktop-app-templates/?page=2`：200
|
||
- `/scenarios/modern-desktop-app-templates/?page=abc`：200
|