fix(api-client): wrap provider group-count and model-suggestion wrappers in toActionResult#1235
Conversation
…in toActionResult
The getProviderGroupsWithCount and getModelSuggestionsByProviderGroup
wrappers returned raw arrays, but dashboard consumers check res.ok,
causing a misleading error toast because ok was undefined.
Wrap them in toActionResult to return { ok, data } and add contract
tests for success and error paths.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthrough两个提供商 API 查询方法 ( Changes提供商 API ActionResult 包装
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 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 docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request updates getProviderGroupsWithCount and getModelSuggestionsByProviderGroup in the API client to wrap their results in an ActionResult using toActionResult. This ensures compatibility with consumers expecting an { ok, data } structure rather than raw arrays. Additionally, comprehensive unit tests have been added to verify the wrapping behavior for successful and failed requests. I have no feedback to provide on these changes.
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: S
- Lines changed: 67
- Files changed: 2
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
Automated review by Codex AI
There was a problem hiding this comment.
- Identified PR
#1235(fix(api-client): wrap provider group-count and model-suggestion wrappers in toActionResult) and reviewed the full diff forsrc/lib/api-client/v1/actions/providers.tsandtests/unit/api/v1/api-client-actions.test.ts. - Applied the size label:
size/S(67 lines changed across 2 files). - Completed a diff-scoped 6-perspective review (comments, tests, error-handling, types, general correctness/security, simplification); no issues met the >=80 confidence reporting threshold, so no inline review comments were posted.
- Posted the required PR review summary via
gh pr review. - Note: I could not run repo quality checks here because
bunxis not available in this runner (bunx: command not found).
provider-form-{total-limit-ui,endpoint-pool} mocked getModelSuggestionsByProviderGroup with a raw [] (old contract). After wrapping the real api-client fn in toActionResult, update both mocks to { ok: true, data: [] } so the test doubles match production and would catch a regression in the model-suggestion path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🧪 测试结果
总体结果: ✅ 所有测试通过 |
… focused (#1237) Fixes the create-user dialog where the "provider group" dropdown would not open to select existing groups. TagInput now auto-opens its suggestions the first time they load asynchronously while the input is focused (fire-once, disabled-guarded), so focusing the field before the data arrives no longer leaves the dropdown stuck closed. The data-fetch half of #1212 was fixed separately in #1235; this completes the interaction-layer fix. Includes regression tests (incl. the dialog/portal path) and corrects the test mock path. Fixes #1212 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Related Issues/PRs:
toActionResultwrapper missing insrc/lib/api-client/v1/actions/providers.ts), same fix approachtoActionResult/dashboardCompatOptionspattern introduced in the dashboard API compatibility fix问题
在用户管理页面展开「用户编辑面板」(创建/编辑用户对话框) 时,100% 必现前端报错
获取供应商分组统计失败: undefined,但所有后端接口均返回 200,无任何接口错误。复现
实测确认 (本地登录后端 dev server 浏览器复现):
GET /api/v1/providers/groups?include=count→ HTTP 200,响应体为裸数组[](不含ok字段)。获取供应商分组统计失败:(第二个参数为undefined)。根因
src/lib/api-client/v1/actions/providers.ts中的 api-client 包装器getProviderGroupsWithCount与同类的getModelSuggestionsByProviderGroup直接返回了原始 HTTP body,未包裹toActionResult。而后端 handler 经
actionJson()解包后返回的是裸数据(数组/对象,没有ok)。但 UI 消费方仍按旧的ActionResult { ok, data, error }形态消费:res.ok永远是undefined,即使 HTTP 200 也会进入错误分支,故每次面板挂载必现该 toast。getModelSuggestionsByProviderGroup是同类问题(use-model-suggestions.ts检查res.ok && res.data),只是表现为模型自动补全静默失效、不弹 toast。属于与 #1158 (testProviderUnified) 完全相同的契约错配类别。修复
将两个包装器用
toActionResult封装,与本文件其余函数及 #1158 的约定一致。React Query 直接消费的getProvidersHealthStatus仍保持裸返回不变。测试 (TDD)
在
tests/unit/api/v1/api-client-actions.test.ts(该文件用vi.importActual加载真实包装器并 mock@/lib/api-client/v1/client,绕过全局替换)新增 3 个契约测试:成功封装、失败映射 (ApiError→{ ok:false })、model-suggestions 封装。修复前 red,修复后 green。浏览器实测:修复后展开对话框无任何控制台报错,分组字段正常挂载。
本地验证 (全绿)
bun run build✅bun run typecheck✅bun run lint✅bun run test✅ 688 文件 / 6181 用例通过,0 失败🤖 Generated with Claude Code
Greptile Summary
This PR fixes a contract mismatch in the v1 API client:
getProviderGroupsWithCountandgetModelSuggestionsByProviderGroupwere returning raw HTTP response bodies instead of theActionResult<T>shape expected by their UI consumers, causing guaranteed false-error toasts whenever the provider-group edit panel was opened.providers.ts: Both affected wrappers are now wrapped withtoActionResult, matching every other mutating/querying function in the file (exceptgetProvidersHealthStatus, which is intentionally bare for React Query direct consumption).api-client-actions.test.ts: Three contract tests are added usingvi.importActualto exercise the real wrappers against a mocked HTTP client — covering success and failure paths forgetProviderGroupsWithCount, and the success path forgetModelSuggestionsByProviderGroup.provider-form-*.test.tsx: Test mocks forgetModelSuggestionsByProviderGroupare updated from the old bare-array shape to the correct{ ok: true, data: [] }shape, keeping existing tests consistent with the new contract.Confidence Score: 5/5
Safe to merge — the fix is a minimal, targeted wrapping of two functions to match the contract already enforced everywhere else in the same file.
The change touches only two wrapper functions, adds three regression tests that were red before and green after, and updates two test mocks to stay in sync. The intentional exception (getProvidersHealthStatus kept bare for React Query) is preserved and explicitly documented in both the source and test comments. No new logic paths, no schema changes, no side effects introduced.
No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant UI as provider-group-select.tsx participant AC as api-client/providers.ts participant BE as GET /api/v1/providers/groups?include=count Note over UI,BE: Before fix UI->>AC: getProviderGroupsWithCount() AC->>BE: apiGet(...) BE-->>AC: 200 [] (bare array) AC-->>UI: [] (bare array, no ok/data) UI->>UI: "res.ok === undefined → error branch" UI->>UI: toast.error(获取供应商分组统计失败: undefined) Note over UI,BE: After fix UI->>AC: getProviderGroupsWithCount() AC->>BE: apiGet(...) BE-->>AC: 200 [] (bare array) AC->>AC: toActionResult(promise) AC-->>UI: "{ ok: true, data: [] }" UI->>UI: "res.ok === true → success branch"Reviews (2): Last reviewed commit: "test: align model-suggestion mocks with ..." | Re-trigger Greptile