bagisto-code-review

Compare original and translation side by side

🇺🇸

Original

English

🇨🇳

Translation

Chinese

Code Review

代码审查

What to look for in a Bagisto change, ordered so the findings that matter arrive first. Pint and the test suites already decide the mechanical questions — spend the review on what they cannot see.

在审查Bagisto代码变更时需要关注的要点，按重要性排序呈现。Pint和测试套件已经可以处理机械性问题——请将审查精力放在它们无法检测的内容上。

What the tools already cover

工具已覆盖的检查项

Do not spend review effort on these; run them instead.

Checked by	Covers
`vendor/bin/pint --test`	Formatting, spacing, import order, trailing commas
`vendor/bin/pest`	Behaviour the suite asserts
`php artisan bagisto:translations:check`	A key missing from any of the 22 locales
Playwright	The browser layer

If a review comment could have been an exit code, the fix is to run the tool, not to write the comment.

无需在这些内容上花费审查精力，直接运行对应工具即可。

检查工具	覆盖范围
`vendor/bin/pint --test`	代码格式、空格、导入顺序、尾随逗号
`vendor/bin/pest`	测试套件所断言的行为逻辑
`php artisan bagisto:translations:check`	检查22种语言环境中是否缺失翻译键
Playwright	浏览器层功能

如果某个审查意见可以通过工具的退出码来判断，那么正确做法是运行工具，而非撰写评论。

Blocking

阻塞性问题

A change should not merge with any of these outstanding.

Correctness

A query inside a loop, or an N+1 from a missing eager load. The cost is invisible on seeded data and appears on a real catalogue.
An unbounded
```
->get()
```
on a table that grows — products, orders, customers. Paginate or chunk.
A repository method that touches seller- or customer-owned rows without scoping to the owner.
A raw fragment (
```
whereRaw
```
,
```
selectRaw
```
,
```
DB::raw
```
,
```
orderByRaw
```
) built by interpolating a request value.

Security

Owned by the bagisto-coding-standards
skill — load it when the diff touches authorization, rendered output, input, uploads, raw SQL, secrets or payments, and work its checklist rather than a remembered list. The two that account for most real findings:

A storefront query selecting by an id from the request without scoping to the authenticated customer.
A DataGrid closure interpolating a value into an HTML attribute without
```
e()
```
— tags are stripped for you, quotes are not.

Architecture

```
DB::
```
or model queries outside a repository. The one sanctioned exception is a DataGrid's
```
prepareQueryBuilder()
```
.
A new model without its Contract, Model, Proxy and Repository.
A package registered in
```
bootstrap/providers.php
```
but not
```
config/concord.php
```
, or the reverse.
Core files edited to serve an extension. Marketplace and B2B Suite both forbid core edits outright — the change belongs in a bound subclass, a
```
view_render_event
```
listener, or a Concord model swap.
A user-facing string not passed through
```
trans()
```
.

存在以下任何未解决问题的变更都不应合并。

正确性

循环内执行查询，或因缺少预加载导致的N+1查询问题。这类问题在测试数据中不易察觉，但会在真实商品目录中暴露性能问题。
对持续增长的表（如商品、订单、客户表）执行无限制的
```
->get()
```
操作。应使用分页或分块处理。
仓储方法在未按所有者范围过滤的情况下，修改属于卖家或客户的数据行。
通过拼接请求值构建原生SQL片段（
```
whereRaw
```
、
```
selectRaw
```
、
```
DB::raw
```
、
```
orderByRaw
```
）。

安全性

此部分由**

bagisto-coding-standards

**技能负责——当代码变更涉及授权、渲染输出、输入、上传、原生SQL、密钥或支付相关内容时，请加载该技能并按照其检查清单进行审查，而非依赖记忆。以下两类问题是实际审查中最常见的：

前台查询直接使用请求中的ID进行筛选，未按已认证客户的范围过滤。
DataGrid闭包将值直接插入HTML属性时未使用
```
e()
```
函数——标签会被自动剥离，但引号不会。

架构

在仓储类之外使用
```
DB::
```
或模型查询。唯一允许的例外是DataGrid的
```
prepareQueryBuilder()
```
方法。
新增模型但未同时创建对应的Contract、Model、Proxy和仓储类。
包在
```
bootstrap/providers.php
```
中注册，但未在
```
config/concord.php
```
中注册，或反之。
为实现扩展而修改核心文件。Marketplace和B2B Suite完全禁止修改核心文件——此类变更应放在绑定的子类、
```
view_render_event
```
监听器或Concord模型替换中实现。
用户可见的字符串未通过
```
trans()
```
函数处理。

Worth raising, not blocking

值得提出但不阻塞合并的问题

Duplication on the third occurrence. Twice is a coincidence; three times is a helper.
A method whose body needs a comment to follow. The codebase forbids comments inside method bodies, so this is a signal to extract a named method, not to add prose.
A docblock or member order violation in a file the change touches. A pre-existing violation in a touched file is the author's to fix.
A test that asserts a count or a list position. Both drift as the shared database grows; assert on the named record instead.
An event fired on the single-record path but not the mass-action path, or the reverse.

第三次出现重复代码。两次重复是巧合，三次重复则应提取为辅助方法。
方法体需要注释才能理解。代码库禁止在方法体内添加注释，因此这是一个信号，提示应提取一个命名方法，而非添加注释。
变更涉及的文件存在文档块或成员顺序违规。变更涉及文件中已存在的违规问题应由作者修复。
测试用例断言数量或列表位置。随着共享数据库的增长，这两类断言都会失效；应改为断言指定的记录。
单条记录路径触发了事件，但批量操作路径未触发，或反之。

How to review

审查方法

Read the tests first. They state what the author believes the change does. A change with no test for the bug it fixes is the first question.
Ask what breaks it. For each claim, look for the input that falsifies it — an empty collection, a second locale, a guest, a channel that is not the default.
Check the reverse. A regression test that passes with the fix reverted guards nothing. Where the fix is subtle, ask the author to show it failing.
Follow one path end to end — request, form request, controller, repository, model, view — rather than reading the diff hunk by hunk. Most real defects sit in the seam between two files that each look fine.
Confirm the gates ran, and which were skipped.

先阅读测试用例。测试用例说明了作者认为该变更实现的功能。如果修复某个bug的变更没有对应的测试用例，这应是第一个需要询问的问题。
思考什么会导致它失效。对于每个功能声明，寻找能使其不成立的输入——比如空集合、第二种语言环境、访客、非默认渠道。
检查反向情况。如果撤销修复后回归测试仍然通过，说明该测试没有起到防护作用。当修复逻辑较为微妙时，请要求作者展示未修复时的失败场景。
从头到尾跟踪完整流程——请求、表单请求、控制器、仓储、模型、视图——而非逐块阅读差异。大多数实际缺陷都存在于两个看似正常的文件之间的衔接处。
确认检查流程已执行，以及哪些步骤被跳过。

Writing the finding

撰写审查意见

State the defect, then the input that triggers it, then the fix. A finding without a concrete failure is a preference, and should be marked as one.

CategoryRepository::saveMediaAltText()
writes to
translateOrNew($locale)
where
$locale
can be the literal
all
— the category create form posts
locale=all
. On MySQL that silently creates a row with an empty
name
and
slug
; on PostgreSQL it violates NOT NULL. Expand the sentinel to every locale before writing.

Separate what you verified from what you suspect. "This is an N+1" and "this might be an N+1, I did not check the relation" are different claims, and conflating them costs the author more time than saying so.

REQUIRED SUB-SKILL: Use bagisto-change-verification before calling any change done.

先说明缺陷，再说明触发缺陷的输入，最后给出修复方案。没有具体失败场景的意见属于个人偏好，应标记为偏好。

CategoryRepository::saveMediaAltText()
方法使用
translateOrNew($locale)
写入数据，其中
$locale
可能是字面量
all
——商品分类创建表单会提交
locale=all
。在MySQL中，这会静默创建一条
name
和
slug
为空的数据行；在PostgreSQL中，这会违反NOT NULL约束。应先将
sentinel
扩展到所有语言环境，再执行写入操作。

区分你已验证的内容和你怀疑的内容。“这是一个N+1问题”和“这可能是一个N+1问题，我未检查关联关系”是不同的表述，混淆两者会浪费作者的时间。