Fix #456: Add inline editing for provider priority, weight and cost multiplier#486
Conversation
…r fields (ding113#456) - Add InlineEditPopover component with Popover + Input + save/cancel buttons - Integrate inline editing in ProviderRichListItem for admin users - Add i18n strings for 5 locales (en/ja/ru/zh-CN/zh-TW) - Allow weight minimum value to be 0 (PROVIDER_LIMITS.WEIGHT.MIN) - Add keyboard focus styles and upper bound validation - Add unit tests for schema validation rules
Weight=0 previously meant "skip this provider in weighted selection", which was confusing. Now enforce 1-100 range; disable the provider via is_enabled flag instead. - Update PROVIDER_LIMITS.WEIGHT.MIN to 1 - Add Chinese error messages to Zod schemas - Update all 5 locale error messages - Adjust unit tests accordingly
Summary of ChangesHello @YangQing-Lin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求为供应商列表页面引入了行内编辑功能，允许管理员直接修改供应商的 priority（优先级）、weight（权重）和 costMultiplier（成本倍数）这三个调度相关字段，而无需打开完整的编辑对话框。这一改进旨在简化供应商参数的管理流程，提高管理员操作效率和用户体验。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
bot
added
enhancement
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR adds inline editing functionality for provider priority, weight, and cost multiplier fields. The implementation includes a new reusable InlineEditPopover component, proper validation, and comprehensive i18n support. The code quality is high with good error handling, appropriate loading states, and keyboard shortcuts.
PR Size: M
- Lines changed: 489 (479 additions, 10 deletions)
- Files changed: 9
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Detailed Analysis
Strengths:
- Excellent error handling: All three save handlers (
handleSavePriority,handleSaveWeight,handleSaveCostMultiplier) properly catch errors, log them, and provide user feedback via toast notifications - Proper validation: Client-side validators use the same constants (
PROVIDER_LIMITS.WEIGHT.MIN/MAX) as the server schema, ensuring consistency - Good UX: Includes keyboard shortcuts (Enter to save, Escape to cancel), loading states with
Loader2spinner, and appropriate button disabling - Comprehensive testing: New
schemas.test.tsvalidates the alignment between schema rules for priority/weight/costMultiplier - Good type safety: Props are properly typed with clear interfaces
- Proper i18n: All user-facing strings are translated across 5 locales
- Defensive programming: The component properly validates input before enabling save, and handles edge cases like empty input
- No silent failures: All errors are surfaced to users via toast notifications
Design Decision Note:
The change of WEIGHT.MIN from 0 to 1 is well-documented in the PR description and makes semantic sense - weight=0 for "disabled" conflicts with is_enabled=false. The inline edit popover enforces this 1-100 range consistently.
Code Quality Observations:
- The
InlineEditPopovercomponent follows React best practices with proper cleanup inuseEffect - Event propagation is correctly stopped with
stopPropagationhandlers - The component is well-designed for reusability with configurable
type(integer/number) andsuffixprops
Minor Notes (No Action Required):
- The three save handler functions in
provider-rich-list-item.tsxare repetitive but appropriately so - extracting them would add unnecessary abstraction for just three instances console.errorusage in catch blocks is consistent with the existing codebase pattern for client-side error logging
Automated review by Claude AI
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
本次 PR 实现了供应商调度参数的行内编辑功能，极大地提升了管理员的操作效率，做得非常棒！新的 InlineEditPopover 组件设计得很好，可复用性强，并且考虑了加载状态、禁用态和键盘快捷键等细节。
在代码审查中，我发现 provider-rich-list-item.tsx 文件中的三个保存处理函数（handleSavePriority, handleSaveWeight, handleSaveCostMultiplier）存在较多重复代码，并且在错误处理中存在一个小 bug（错误地使用了删除失败的提示文案）。我提供了一个重构建议，通过创建一个通用的处理函数来解决这两个问题，以提高代码的可维护性。
除此之外，其他部分的实现，包括 i18n、Zod schema 的更新和单元测试的添加，都完成得很好。请查看具体的审查意见。
| const handleSavePriority = async (value: number) => { | ||
| try { | ||
| const res = await editProvider(provider.id, { priority: value }); | ||
| if (res.ok) { | ||
| toast.success(tInline("saveSuccess")); | ||
| queryClient.invalidateQueries({ queryKey: ["providers"] }); | ||
| router.refresh(); | ||
| return true; | ||
| } | ||
| toast.error(tInline("saveFailed"), { description: res.error || tList("unknownError") }); | ||
| return false; | ||
| } catch (error) { | ||
| console.error("更新 priority 失败:", error); | ||
| toast.error(tInline("saveFailed"), { description: tList("deleteError") }); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| const handleSaveWeight = async (value: number) => { | ||
| try { | ||
| const res = await editProvider(provider.id, { weight: value }); | ||
| if (res.ok) { | ||
| toast.success(tInline("saveSuccess")); | ||
| queryClient.invalidateQueries({ queryKey: ["providers"] }); | ||
| router.refresh(); | ||
| return true; | ||
| } | ||
| toast.error(tInline("saveFailed"), { description: res.error || tList("unknownError") }); | ||
| return false; | ||
| } catch (error) { | ||
| console.error("更新 weight 失败:", error); | ||
| toast.error(tInline("saveFailed"), { description: tList("deleteError") }); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| const handleSaveCostMultiplier = async (value: number) => { | ||
| try { | ||
| const res = await editProvider(provider.id, { cost_multiplier: value }); | ||
| if (res.ok) { | ||
| toast.success(tInline("saveSuccess")); | ||
| queryClient.invalidateQueries({ queryKey: ["providers"] }); | ||
| router.refresh(); | ||
| return true; | ||
| } | ||
| toast.error(tInline("saveFailed"), { description: res.error || tList("unknownError") }); | ||
| return false; | ||
| } catch (error) { | ||
| console.error("更新 costMultiplier 失败:", error); | ||
| toast.error(tInline("saveFailed"), { description: tList("deleteError") }); | ||
| return false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
这三个保存处理函数 handleSavePriority、handleSaveWeight 和 handleSaveCostMultiplier 的逻辑几乎完全相同，导致了代码重复。
此外，在 catch 块中，错误提示信息使用了 tList("deleteError")，这可能会误导用户。这里应该使用一个更通用的保存失败提示，例如 tList("unknownError")。
为了解决这两个问题并提高代码的可维护性，建议将它们重构为一个通用的工厂函数。这样不仅可以消除重复代码，还能统一错误处理逻辑。
请参考以下建议进行修改：
const createSaveHandler = (fieldName: "priority" | "weight" | "cost_multiplier") => {
return async (value: number) => {
try {
const res = await editProvider(provider.id, { [fieldName]: value });
if (res.ok) {
toast.success(tInline("saveSuccess"));
queryClient.invalidateQueries({ queryKey: ["providers"] });
router.refresh();
return true;
}
toast.error(tInline("saveFailed"), { description: res.error || tList("unknownError") });
return false;
} catch (error) {
console.error(`更新 ${fieldName} 失败:`, error);
toast.error(tInline("saveFailed"), { description: tList("unknownError") });
return false;
}
};
};
const handleSavePriority = createSaveHandler("priority");
const handleSaveWeight = createSaveHandler("weight");
const handleSaveCostMultiplier = createSaveHandler("cost_multiplier");
- Consolidate handleSavePriority, handleSaveWeight, handleSaveCostMultiplier into a single createSaveHandler factory function - Fix incorrect error message key in catch block (deleteError → unknownError)
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
2 participants
PR: 供应商优先级/权重/成本倍数行内编辑
分支:
feature/inline-provider-editingIssue: #456
里程碑: v0.4.1
概述
在供应商列表页添加行内编辑功能，允许管理员直接点击数值快速修改 priority、weight、costMultiplier 三个调度相关字段，无需打开完整的编辑对话框。
变更内容
新增组件
src/app/[locale]/settings/providers/_components/inline-edit-popover.tsx修改文件
provider-rich-list-item.tsxprovider.constants.tsschemas.tsschemas.test.tsmessages/*/settings.json设计决策
1. Weight 最小值改为 1
is_enabled=false语义重叠，容易混淆is_enabled显式表达2. CostMultiplier 支持行内编辑
3. 仅管理员可编辑
canEdit权限判断控制测试计划
schemas.test.ts验证规则覆盖bun run test全部通过 (170 tests)截图
提交记录
07dd22f
feat(providers): add inline editing for priority/weight/costMultiplier fields (#456)9b31e7a
fix(providers): change weight minimum from 0 to 1 for clarity风险点
queryClient.invalidateQueries刷新缓存md:断点以上显示行内编辑，移动端仍用完整表单Greptile Summary
This PR successfully implements inline editing for provider priority, weight, and cost multiplier fields, addressing issue #456. The implementation includes a reusable
InlineEditPopovercomponent with validation, keyboard shortcuts (Enter/Escape), and proper loading states.Key changes:
InlineEditPopovercomponent with real-time validation and error displayProviderRichListItemfor admin users only (viacanEditcheck)is_enabled=falseDesign decisions:
canEditpermission checkcostMultiplierin addition to priority/weightConfidence Score: 5/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant Admin as Admin User participant UI as Provider List UI participant Popover as InlineEditPopover participant Handler as Save Handler participant API as editProvider API participant Cache as QueryClient participant Router as Next Router Admin->>UI: Click on priority/weight/cost value UI->>Popover: Open popover (pass value, validator, onSave) Popover->>Popover: Focus input & select text Admin->>Popover: Edit value Popover->>Popover: Validate input (real-time) alt Invalid input Popover-->>Admin: Show validation error message end Admin->>Popover: Press Enter or click Save Popover->>Popover: Check canSave (valid & not saving) Popover->>Handler: onSave(parsedValue) Handler->>API: editProvider(id, {field: value}) API-->>Handler: {ok: true} or {ok: false, error} alt Save successful Handler->>Cache: invalidateQueries(['providers']) Handler->>Router: router.refresh() Handler-->>Popover: return true Popover->>Popover: Close popover Popover-->>Admin: Show success toast else Save failed Handler-->>Popover: return false Popover-->>Admin: Show error toast & keep popover open end