feat: model display order - #5090
Open
l1i1 wants to merge 5 commits into
Open
Conversation
Contributor
WalkthroughAdds display ordering and pinning across backend model schema, a new order-only UpdateModelMeta branch, pricing ordering, frontend forms and interactive table controls, default sort changes, and translation keys; listing responses and pricing now use the new metadata ordering. ChangesModel Ordering & Pinning
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
web/default/src/i18n/locales/en.json (1)
1097-1097: ⚡ Quick winUse hierarchical i18n keys for the newly added entries.
These new keys are sentence-style (
"Default Order","Pin model", etc.) instead of hierarchical keys, which breaks the i18n key naming rule and makes namespace management harder over time. Please convert them to semantically grouped keys (for example undermodels.ordering.*) and update usages accordingly.As per coding guidelines: "Use hierarchical and semantically clear translation key names such as
dashboard.overview.titleand maintain naming consistency".Also applies to: 1227-1228, 1659-1659, 2897-2899, 4124-4124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/i18n/locales/en.json` at line 1097, The new flat sentence-style i18n keys like "Default Order" and "Pin model" violate the project's hierarchical naming rules; rename them into semantically grouped keys (for example move "Default Order" to models.ordering.defaultOrder and "Pin model" to models.pin.label or similar under a consistent namespace like models.* or models.ordering.*), update the en.json entries accordingly, and update all code usages where the string keys are referenced to use the new hierarchical keys (search for exact keys "Default Order", "Pin model" and the other occurrences at the specified ranges and replace with the new names). Ensure naming follows existing patterns (e.g., models.ordering.*) and keep translations unchanged when migrating values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/model_meta.go`:
- Around line 148-154: The update call using
model.DB.Model(&model.Model{}).Where("id = ?", req.Id).Updates(updates) can
succeed with zero rows affected and you still call model.RefreshPricing() and
common.ApiSuccess, causing a false-positive; change this to capture the GORM
result (e.g., res := model.DB.Model(&model.Model{}).Where("id = ?",
req.Id).Updates(updates)), check res.Error first and return common.ApiError on
error, then if res.RowsAffected == 0 return a not-found error via
common.ApiError (or a 404 response) instead of calling model.RefreshPricing()
and common.ApiSuccess, otherwise proceed to call model.RefreshPricing() and
common.ApiSuccess with req.Id.
In `@model/pricing.go`:
- Around line 350-359: The current sort.Slice comparator for pricingMap
(comparing Pinned, DisplayOrder, Id) can leave ties when Pinned, DisplayOrder
and Id are equal, causing nondeterministic ordering; update the sort to be
deterministic by either switching to sort.SliceStable or extending the
comparator in sort.Slice to include a final deterministic tie-breaker (e.g.,
compare ModelName or another stable string/unique field) after Id so equal-key
entries always sort the same; reference the existing sort.Slice call and the
pricingMap elements' fields Pinned, DisplayOrder, Id and ModelName (or another
unique field) when making the change.
In `@web/classic/src/components/table/models/modals/EditModelModal.jsx`:
- Around line 542-547: The numeric input for the display_order field
(Form.InputNumber with field='display_order') currently permits negative values;
add min={0} to the Form.InputNumber to prevent negative entry and also
normalize/coerce the value to an integer >= 0 right before submit (in the form
submission handler that maps/creates the payload for saving, e.g., the submit
mapping or onFinish handler) to ensure negative or fractional values cannot be
written to the backend. Ensure you update both occurrences mentioned (the modal
at the shown diff and the other occurrence around lines 205-206) and reference
the 'display_order' field when normalizing.
In `@web/classic/src/components/table/models/ModelsColumnDefs.jsx`:
- Around line 354-357: The InputNumber is using defaultValue, which makes it
uncontrolled and stale; change it to a controlled input by replacing
defaultValue with value (bind it to the current row field, e.g.
value={row.someOrderField ?? 0}) and wire its onChange to the table/row update
handler (call the existing cell update function or dispatch
setRowData/updateRecord from ModelsColumnDefs) so edits update the underlying
data model; remove defaultValue and ensure the value prop reflects the latest
row data.
In `@web/default/src/features/models/components/drawers/model-mutate-drawer.tsx`:
- Line 92: The drawer's Zod schema currently declares display_order as
z.number(), allowing negatives and non-integers; update the schema in
model-mutate-drawer.tsx to require an integer >= 0 (e.g., use Zod integer +
nonnegative/min) for the display_order field, and ensure any form-level
validation/error messages and default values that reference display_order are
consistent with the new constraints so the form cannot submit invalid orders.
In `@web/default/src/features/models/lib/model-form.ts`:
- Line 43: The Zod schema for the model form allows negative/decimal
display_order values; update the schema entry for display_order (the property
named display_order in the Zod schema in model-form.ts) to enforce integers and
non-negativity by chaining the appropriate Zod validators (use .int() and
.min(0)) before .default(0) so invalid ordering values are rejected at schema
validation time.
---
Nitpick comments:
In `@web/default/src/i18n/locales/en.json`:
- Line 1097: The new flat sentence-style i18n keys like "Default Order" and "Pin
model" violate the project's hierarchical naming rules; rename them into
semantically grouped keys (for example move "Default Order" to
models.ordering.defaultOrder and "Pin model" to models.pin.label or similar
under a consistent namespace like models.* or models.ordering.*), update the
en.json entries accordingly, and update all code usages where the string keys
are referenced to use the new hierarchical keys (search for exact keys "Default
Order", "Pin model" and the other occurrences at the specified ranges and
replace with the new names). Ensure naming follows existing patterns (e.g.,
models.ordering.*) and keep translations unchanged when migrating values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59eb9528-adbd-4d22-a258-1b1256d233cb
📒 Files selected for processing (17)
controller/model_meta.gocontroller/user.gomodel/model_meta.gomodel/pricing.goweb/classic/src/components/table/models/ModelsColumnDefs.jsxweb/classic/src/components/table/models/modals/EditModelModal.jsxweb/default/src/features/models/api.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.tsxweb/default/src/features/models/components/models-columns.tsxweb/default/src/features/models/lib/model-actions.tsweb/default/src/features/models/lib/model-form.tsweb/default/src/features/models/types.tsweb/default/src/features/pricing/constants.tsweb/default/src/features/pricing/hooks/use-filters.tsweb/default/src/features/pricing/lib/filters.tsweb/default/src/features/pricing/types.tsweb/default/src/i18n/locales/en.json
Comment on lines
+354
to
+357
| <InputNumber | ||
| size='small' | ||
| defaultValue={value ?? 0} | ||
| step={1} |
Contributor
There was a problem hiding this comment.
InputNumber should be controlled to avoid stale table values.
defaultValue is only applied on first mount; after refresh or external updates, the cell can display outdated order. Bind value to current row data instead.
Suggested patch
- <InputNumber
+ <InputNumber
size='small'
- defaultValue={value ?? 0}
+ value={value ?? 0}
step={1}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/classic/src/components/table/models/ModelsColumnDefs.jsx` around lines
354 - 357, The InputNumber is using defaultValue, which makes it uncontrolled
and stale; change it to a controlled input by replacing defaultValue with value
(bind it to the current row field, e.g. value={row.someOrderField ?? 0}) and
wire its onChange to the table/row update handler (call the existing cell update
function or dispatch setRowData/updateRecord from ModelsColumnDefs) so edits
update the underlying data model; remove defaultValue and ensure the value prop
reflects the latest row data.
…ut validation, i18n hierarchy
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/model_meta.go`:
- Around line 148-154: The handler UpdateModelMeta should not assume
res.RowsAffected == 0 means "模型不存在"; instead, after calling
model.DB.Model(&model.Model{}).Where("id = ?", req.Id).Updates(updates), if
res.Error == nil and res.RowsAffected == 0 then explicitly verify existence
(e.g., query model.DB.First/Select EXISTS for model.Model with id == req.Id) and
only call common.ApiErrorMsg(c, "模型不存在") if that existence check fails;
otherwise treat it as a no-op success (return 200) or respond accordingly.
Ensure you reference UpdateModelMeta, the res from
model.DB.Model(&model.Model{}).Where(...).Updates(updates), and
common.ApiErrorMsg in the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3ddff59-6d81-4518-9ca4-338388fb47c0
📒 Files selected for processing (10)
controller/model_meta.gomodel/pricing.goweb/classic/src/components/table/models/ModelsColumnDefs.jsxweb/classic/src/components/table/models/modals/EditModelModal.jsxweb/default/src/features/models/components/drawers/model-mutate-drawer.tsxweb/default/src/features/models/components/models-columns.tsxweb/default/src/features/models/lib/model-actions.tsweb/default/src/features/models/lib/model-form.tsweb/default/src/features/pricing/constants.tsweb/default/src/i18n/locales/en.json
✅ Files skipped from review due to trivial changes (1)
- web/default/src/i18n/locales/en.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/model_meta.go`:
- Around line 155-157: The code currently conflates DB query errors with a
not-found result when calling model.DB.Model(&model.Model{}).Where("id = ?",
req.Id).Count(&count).Error and then returns common.ApiErrorMsg(c, "模型不存在");
change this to first check if err != nil and handle it as a DB/internal error
(e.g., call common.ApiErrorMsg with an internal error message or the err
details) and only if err == nil and count == 0 return the not-found message
"模型不存在"; update the error-handling branches around Count(&count).Error, keeping
the same req.Id/Model references and using common.ApiErrorMsg for each distinct
case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 23b70c09-c907-4ab4-aaf2-98216a4f7e60
📒 Files selected for processing (1)
controller/model_meta.go
Comment on lines
+155
to
+157
| if err := model.DB.Model(&model.Model{}).Where("id = ?", req.Id).Count(&count).Error; err != nil || count == 0 { | ||
| common.ApiErrorMsg(c, "模型不存在") | ||
| return |
Contributor
There was a problem hiding this comment.
Do not map DB count errors to “模型不存在”.
Line 155 currently treats query errors and true not-found as the same result. That can hide real DB failures and return a false 404-style message.
Suggested fix
- if err := model.DB.Model(&model.Model{}).Where("id = ?", req.Id).Count(&count).Error; err != nil || count == 0 {
- common.ApiErrorMsg(c, "模型不存在")
- return
- }
+ if err := model.DB.Model(&model.Model{}).Where("id = ?", req.Id).Count(&count).Error; err != nil {
+ common.ApiError(c, err)
+ return
+ }
+ if count == 0 {
+ common.ApiErrorMsg(c, "模型不存在")
+ return
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/model_meta.go` around lines 155 - 157, The code currently
conflates DB query errors with a not-found result when calling
model.DB.Model(&model.Model{}).Where("id = ?", req.Id).Count(&count).Error and
then returns common.ApiErrorMsg(c, "模型不存在"); change this to first check if err
!= nil and handle it as a DB/internal error (e.g., call common.ApiErrorMsg with
an internal error message or the err details) and only if err == nil and count
== 0 return the not-found message "模型不存在"; update the error-handling branches
around Count(&count).Error, keeping the same req.Id/Model references and using
common.ApiErrorMsg for each distinct case.
Author
|
来个佬Review一下呗 |
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 变更描述 / Description
为模型元数据增加展示排序能力，管理员可以在模型管理中设置模型是否置顶以及展示顺序。
后端为模型元数据新增
pinned和display_order字段，并统一按pinned DESC, display_order ASC, id DESC排序。模型广场、用户模型列表和管理端模型列表都会基于同一套模型元数据排序，因此配置后可以稳定影响前台展示顺序。前端在新版和经典版模型管理页面都增加了置顶与展示顺序配置入口，并提供仅更新排序字段的接口路径，避免行内修改排序时影响模型的其他元数据。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
Summary by CodeRabbit
New Features
Documentation / Localization