Skip to content

chore(presets): remove the no-op OrderableField.DBColumn - #1108

Merged
molon merged 1 commit into
mainfrom
chore/remove-deprecated-listing-apis
Sep 8, 2026
Merged

chore(presets): remove the no-op OrderableField.DBColumn#1108
molon merged 1 commit into
mainfrom
chore/remove-deprecated-listing-apis

Conversation

@molon

@molon molon commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Draft —— 请勿在下游迁移完成前合并。
依赖:theplant/kakuyasu-services#2732

改动

移除 OrderableField.DBColumn,并清掉 example/docs/ 里已无意义的字面量。

纯删除,8 行,不改动任何逻辑分支。

为什么这个可以删

0b86c728(init relay)起,orderableFieldMap[v.FieldName] = v.DBColumn 被改成 = true,值不再被读取。此后排序一律走 FieldName → GORM schema(FieldsByNamefield.DBName)解析。1626c4e2 才补标 // Deprecated

也就是说这个字段填对、填错、留空,运行时行为完全一致。它不提供任何兼容性,只提供「它好像有用」的错觉。

删掉之后,原本会静默生成错误 SQL 的情况,变成 gormrelay 显式报 missing field %q in schema for OrderBy——把一类静默错误变成了编译期/运行期的显式错误

为什么不一并删 DefaultOrderBys

初版 PR 里我把 DefaultOrderBys 一起删了,这是错的,已撤回。

两者性质完全不同:

DBColumn DefaultOrderBys
兼容层 无——值根本不被读取 getOrderBy 里经 relay.OrderByFromOrderBys 真实参与运算
老代码行为 与预期不符(静默失效) 与重构前完全一致
删除的收益 消除一个陷阱 无收益
删除的代价 破坏所有老调用方

68689e44 当初特意为 DefaultOrderBys 写了 fallback 分支,目的就是让下游按自己的节奏迁移。几个月后就把它删掉,等于取消了当初承诺的兼容期。它的维护成本也就是一个结构体字段加一个分支,没有删除的理由。

DefaultOrderBys 保持原样(下游迁移仍推荐做,见 theplant/kakuyasu-services#2734,但那是下游自愿升级,不是被迫)。

兼容性影响

编译期破坏、运行时行为不变。编译器会精确指出每一处,改法唯一:

{FieldName: "ProductCode", DBColumn: "product_code"}  ->  {FieldName: "ProductCode"}

前提FieldName 必须是模型里真实存在的 Go 字段名。

版本号

建议在 v3 内以 minor 发布(v3.3.0),不升 v4:

  • 下游目前全部通过 pseudo-version 钉 commit SHA(v3.2.1-0.2026xxxx-<sha>),不会被动升级,v4 的隔离价值几乎为零
  • 而升 v4 需要所有下游改 module import 路径,成本远高于收益
  • 本次属于「编译期可见、行为无变化」,release note 写清迁移方式即可

未包含

  • ListingBuilder.CellWrapperFuncDeprecated: Use WrapCell instead)—— 仓库内 redirection/redirection.go:65 仍在调用,且同样有可用的兼容路径
  • seo.AutoMigrateDeprecated: use Migrate instead)—— 它与 Migrate 并不等价,除建表外还会经 insertIfNotExists 写入初始数据。照注释盲目替换会丢数据,该注释本身可能有问题,属独立议题

验证

  • go build ./... 通过
  • go vet ./presets/... ./example/... ./docs/... 通过
  • go test ./presets/... 通过(presetspresets/integration 均 ok)

DBColumn has had no effect since 0b86c72 ("init relay"), which changed
orderableFieldMap to a map[string]bool and stopped reading the value;
1626c4e later marked it // Deprecated. Ordering resolves FieldName
against the GORM schema (FieldsByName -> field.DBName), so the field was
pure decoration: setting it right, wrong, or not at all behaved the same.

Unlike DefaultOrderBys — which is deprecated but backed by a working
compatibility shim and is deliberately left in place — DBColumn offers
callers no compatibility to preserve, only the false impression that it
does something. Removing it turns a silent no-op into a compile error.

Also drops the now-meaningless DBColumn entries from the example and docs
listings.
@molon
molon force-pushed the chore/remove-deprecated-listing-apis branch from 8ad6e6b to 57ac701 Compare September 3, 2026 02:24
@molon molon changed the title chore(presets): remove the deprecated OrderableField.DBColumn and DefaultOrderBys chore(presets): remove the no-op OrderableField.DBColumn Sep 3, 2026
@deepsource-io

deepsource-io Bot commented Sep 3, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in eec11ce...57ac701 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
Go Sep 3, 2026 2:24a.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
example/admin/input_demo_config.go 34.48% <ø> (-0.96%) ⬇️
example/admin/user_config.go 71.38% <ø> (-0.17%) ⬇️
presets/listing_builder.go 84.39% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@molon
molon requested a review from zhangshanwen September 8, 2026 03:15
@molon
molon marked this pull request as ready for review September 8, 2026 03:15
Copilot AI lite review requested due to automatic review settings September 8, 2026 03:15
@molon
molon merged commit 9a65991 into main Sep 8, 2026
11 checks passed
@molon
molon deleted the chore/remove-deprecated-listing-apis branch September 8, 2026 03:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Removing an exported field is an API-breaking change that needs an explicit versioning/release decision (and the PR is also marked Draft / blocked by downstream migration).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR removes the deprecated, no-op OrderableField.DBColumn field from presets and updates in-repo examples/docs to stop setting it, aligning configuration with the current ordering implementation (FieldName → GORM schema DBName).

Changes:

  • Remove DBColumn from presets.OrderableField.
  • Update example/ configs to omit DBColumn literals.
  • Update docs/ example code to omit DBColumn literals.
File summaries
File Description
presets/listing_builder.go Removes OrderableField.DBColumn from the public API surface.
example/admin/user_config.go Drops DBColumn usage in listing orderable fields example.
example/admin/input_demo_config.go Drops DBColumn usage in listing orderable fields example.
docs/docsrc/examples/examples_presets/listing.go Drops DBColumn usage in documentation example.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 37 to 39
type OrderableField struct {
FieldName string
DBColumn string // Deprecated
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants