fix(api): support --body alias for raw requests#504
fix(api): support --body alias for raw requests#504hiSandog wants to merge 1 commit intolarksuite:mainfrom
Conversation
|
|
github-actions
bot
added
the
size/L
📝 WalkthroughWalkthroughA new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/api/api.go (1)
162-169:⚠️ Potential issue | 🟡 Minor
--bodyusers can still get--data-only validation messages.After alias resolution, this flow can be reached via
--body, but Line 168 still says--data, andParseOptionalBodyerrors (invoked at Line 163 and Line 189) also report--data. Please emit--data/--body(or pass a flag label) so guidance stays accurate for users and automation.Suggested direction
- return client.RawApiRequest{}, nil, output.ErrValidation("--data must be a JSON object when used with --file") + return client.RawApiRequest{}, nil, output.ErrValidation("--data/--body must be a JSON object when used with --file")And consider adding a
ParseOptionalBodyWithFlag(flagName, ...)variant so invalid-JSON/resolve-input errors can mention--data/--bodyin this command path.As per coding guidelines: "Error messages must be structured, actionable, and specific since they will be parsed by AI agents to determine next actions."
Also applies to: 189-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/api.go` around lines 162 - 169, The validation/error text wrongly references only "--data" even when the input came via the "--body" alias; update the code paths that call ParseOptionalBody (the dataFlag handling using variable dataFlag and the later call that also invokes ParseOptionalBody) to surface the correct flag name by either (a) passing a flag label into a new ParseOptionalBodyWithFlag(flagName, method, flagValue, stdin) and use that in error returns, or (b) build the error strings dynamically from dataFlag (or an explicit flagName variable) so the output.ErrValidation and any ParseOptionalBody error messages mention "--data/--body" (or the actual flag used) instead of always "--data"; ensure you update both call sites that currently call ParseOptionalBody to use the new variant or dynamic flag-aware error text and change the ErrValidation message reference (the return that currently uses output.ErrValidation("--data must be a JSON object when used with --file")) to include the correct flag label.
🧹 Nitpick comments (2)
cmd/api/api_test.go (2)
117-120: Add explicit config-dir isolation in these new tests.Please set
LARKSUITE_CLI_CONFIG_DIRviat.Setenvin each new test before creating the factory, to keep test config state isolated.Suggested patch
func TestBuildAPIRequest_BodyAlias(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, }) @@ func TestBuildAPIRequest_DataAndBodyConflict(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, })As per coding guidelines: "
**/*_test.go: Isolate config state in Go tests by usingt.Setenv(\"LARKSUITE_CLI_CONFIG_DIR\", t.TempDir())."Also applies to: 143-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/api_test.go` around lines 117 - 120, The new tests (e.g., TestBuildAPIRequest_BodyAlias) do not isolate config state; before calling cmdutil.TestFactory(t, ...) set the LARKSUITE_CLI_CONFIG_DIR environment to a temp dir using t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) so each test gets its own config directory; apply the same change to the other affected test(s) around the TestBuildAPIRequest_* functions (the places that call cmdutil.TestFactory) so factory creation happens after t.Setenv is called.
143-161: Consider adding a non-conflict parity test (--data==--body).You currently assert the differing-values conflict; a same-value case would lock the intended “allowed” behavior and prevent regressions in
resolveBodyFlag.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/api_test.go` around lines 143 - 161, Add a new unit test (e.g., TestBuildAPIRequest_DataAndBodyParity) that mirrors TestBuildAPIRequest_DataAndBodyConflict but sets APIOptions.Data and APIOptions.Body to the identical JSON string and asserts buildAPIRequest returns no error and the resolved request body equals that string; locate the test helper/TestFactory usage and call buildAPIRequest (same symbol) and also validate resolveBodyFlag behavior indirectly by checking the request body or returned payload to ensure parity is allowed and preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@cmd/api/api.go`:
- Around line 162-169: The validation/error text wrongly references only
"--data" even when the input came via the "--body" alias; update the code paths
that call ParseOptionalBody (the dataFlag handling using variable dataFlag and
the later call that also invokes ParseOptionalBody) to surface the correct flag
name by either (a) passing a flag label into a new
ParseOptionalBodyWithFlag(flagName, method, flagValue, stdin) and use that in
error returns, or (b) build the error strings dynamically from dataFlag (or an
explicit flagName variable) so the output.ErrValidation and any
ParseOptionalBody error messages mention "--data/--body" (or the actual flag
used) instead of always "--data"; ensure you update both call sites that
currently call ParseOptionalBody to use the new variant or dynamic flag-aware
error text and change the ErrValidation message reference (the return that
currently uses output.ErrValidation("--data must be a JSON object when used with
--file")) to include the correct flag label.
---
Nitpick comments:
In `@cmd/api/api_test.go`:
- Around line 117-120: The new tests (e.g., TestBuildAPIRequest_BodyAlias) do
not isolate config state; before calling cmdutil.TestFactory(t, ...) set the
LARKSUITE_CLI_CONFIG_DIR environment to a temp dir using
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) so each test gets its own
config directory; apply the same change to the other affected test(s) around the
TestBuildAPIRequest_* functions (the places that call cmdutil.TestFactory) so
factory creation happens after t.Setenv is called.
- Around line 143-161: Add a new unit test (e.g.,
TestBuildAPIRequest_DataAndBodyParity) that mirrors
TestBuildAPIRequest_DataAndBodyConflict but sets APIOptions.Data and
APIOptions.Body to the identical JSON string and asserts buildAPIRequest returns
no error and the resolved request body equals that string; locate the test
helper/TestFactory usage and call buildAPIRequest (same symbol) and also
validate resolveBodyFlag behavior indirectly by checking the request body or
returned payload to ensure parity is allowed and preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5f6c5c2-1a30-4691-8f5c-49d83aa64be9
📒 Files selected for processing (2)
cmd/api/api.gocmd/api/api_test.go
2 participants
Summary
Validation
Context
README examples currently show --body, while the command only accepted --data. An open docs PR already fixes the examples; this change makes the CLI itself tolerant of the documented flag without overlapping that PR.
Summary by CodeRabbit
New Features
--bodyflag as an alternative option to--datafor specifying request body content in API requests.--dataand--bodyflags when set to different values; using both simultaneously will return an error indicating the conflict.