feat: add client-side page_size validation for service commands#1222
feat: add client-side page_size validation for service commands#1222xukuncx wants to merge 2 commits into
Conversation
Validate query parameter values against min/max constraints declared in the API schema metadata before sending requests to the server. This provides clear error messages instead of the generic "field validation failed" response from the backend. The validation applies to all numeric query parameters that have min or max constraints in their schema definition, catching out-of-range values early with actionable hints (including the schema command path for reference). sprint: S2
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds numeric min/max enforcement for query parameters during service request construction via a new validateQueryParamRange helper; integrates checks into buildServiceRequest and adds unit and integration tests exercising boundary and type cases, including page_size enforcement in dry-run mode. ChangesQuery Parameter Range Validation
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
github-actions
Bot
added
the
size/L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/service/service.go (1)
437-444: 💤 Low value
validateQueryParamRange(name, value, nil, …)is always a no-op here.This second loop only handles params not already mapped to a declared query param, so
paramSpecis alwaysniland the helper returns immediately at Line 549-551. The call neither validates nor documents a future constraint source. Consider dropping it to avoid implying validation that can never run.♻️ Optional simplification
for name, value := range params { if _, ok := queryParams[name]; !ok { - if err := validateQueryParamRange(name, value, nil, schemaPath); err != nil { - return client.RawApiRequest{}, nil, err - } queryParams[name] = value } }🤖 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 `@cmd/service/service.go` around lines 437 - 444, The second loop over params is calling validateQueryParamRange(name, value, nil, schemaPath) but since this loop only handles names not in queryParams the helper always receives paramSpec == nil and returns immediately; remove the no-op call to validateQueryParamRange from that loop and just set queryParams[name] = value (or, if you intended to validate against a fallback/global constraint, replace the nil paramSpec with the appropriate spec lookup before calling validateQueryParamRange). Ensure references to queryParams, params, validateQueryParamRange, and schemaPath are updated accordingly.
🤖 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.
Nitpick comments:
In `@cmd/service/service.go`:
- Around line 437-444: The second loop over params is calling
validateQueryParamRange(name, value, nil, schemaPath) but since this loop only
handles names not in queryParams the helper always receives paramSpec == nil and
returns immediately; remove the no-op call to validateQueryParamRange from that
loop and just set queryParams[name] = value (or, if you intended to validate
against a fallback/global constraint, replace the nil paramSpec with the
appropriate spec lookup before calling validateQueryParamRange). Ensure
references to queryParams, params, validateQueryParamRange, and schemaPath are
updated accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c6783135-69c7-4b7f-a92b-1bd694beb5fd
📒 Files selected for processing (2)
cmd/service/service.gocmd/service/service_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@0ebb9de4a1d5a345b04247dfdfd27a99ea8cc29b🧩 Skill updatenpx skills add xukuncx/cli#feat/1694214 -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1222 +/- ##
=======================================
Coverage 69.19% 69.20%
=======================================
Files 634 634
Lines 59482 59531 +49
=======================================
+ Hits 41161 41200 +39
- Misses 15007 15018 +11
+ Partials 3314 3313 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Change-Type: ci-fix
|
🤖 AI Review | [P3 可维护性] PR 描述残留恢复运行过程信息 PR 描述包含 修复建议: 将描述改为自包含的 如有疑问或认为判断不准确，欢迎直接回复讨论。 |
|
🤖 AI Review | CR 汇总 | 可合入（2 个建议项） 增量审查：基于之前 1 条 CodeRabbit 审查意见，本次新增 1 条评论。
其中 |
1 participant
Generated by the harness-coding skill (recovery run — original attempt crashed before MR open).
Commits on branch (ahead of main)
This resume run
This MR was created autonomously. Quality gates were enforced by the repo's own pre-commit hooks.
Summary by CodeRabbit
New Features
page_sizeis enforced against configured maximums (dry-run rejects too-large values; boundary values allowed).Tests