feat: add test-cases media upload-url CLI command (API parity)#87
Conversation
WalkthroughThis PR adds a media upload URL feature to the CLI: new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant TestCaseCommands
participant TestCasesClient
participant API
CLI->>TestCaseCommands: MediaUploadUrl(MediaUploadUrlArgs)
TestCaseCommands->>TestCasesClient: media_upload_url(test_case_id, filename, mime_type)
TestCasesClient->>API: POST media:upload-url (MediaUploadUrlRequest)
API-->>TestCasesClient: MediaUploadUrlResponse
TestCasesClient-->>TestCaseCommands: MediaUploadUrlResponse
TestCaseCommands-->>CLI: emit_one_with_actions(response)
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/client/mod.rs (1)
498-512: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent method signature vs.
create/updatepattern.
create()andupdate()take a pre-built request struct (req: models::CreateTestCaseRequest/models::UpdateTestCaseRequest), butmedia_upload_urlinstead takes raw scalar params (filename,mime_type) and builds the request struct internally. This diverges from the established pattern and makes the request struct's existence redundant for callers.♻️ Proposed refactor for consistency
pub async fn media_upload_url( &self, test_case_id: &str, - filename: &str, - mime_type: &str, + req: models::MediaUploadUrlRequest, ) -> Result<models::MediaUploadUrlResponse, ApiError> { let url = self .0 .url(&format!("/v1/test-cases/{test_case_id}/media:upload-url")); - let req = models::MediaUploadUrlRequest { - filename: filename.to_string(), - mime_type: mime_type.to_string(), - }; self.0.post(url, &req).await }🤖 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 `@src/client/mod.rs` around lines 498 - 512, `media_upload_url` is inconsistent with the `create` and `update` API patterns because it accepts raw fields and constructs `models::MediaUploadUrlRequest` internally. Refactor `media_upload_url` to take a pre-built request struct argument, matching the signatures of `create` and `update`, and keep the URL/post logic in `Client::media_upload_url` unchanged aside from using the provided request value.src/commands/test_cases.rs (1)
303-309: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNo follow-up next actions offered for this command.
Every other mutating command (
Create,Update,Delete) emits contextualnext_actions(e.g.,get/context), butMediaUploadUrlpasses an emptyvec![]. Consider adding at leastnext_actions::context("test-cases")(and possiblynext_actions::get("test-cases", &args.test_case_id)) for consistency with the rest of the agent-facing UX.♻️ Proposed fix
emit_one_with_actions(ctx, "test-cases", operation, &result, vec![]); + emit_one_with_actions( + ctx, + "test-cases", + operation, + &result, + vec![ + next_actions::get("test-cases", &args.test_case_id).primary(), + next_actions::context("test-cases"), + ], + );🤖 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 `@src/commands/test_cases.rs` around lines 303 - 309, The MediaUploadUrl branch in test_cases.rs emits no follow-up actions, unlike the other mutating commands. Update the TestCaseCommands::MediaUploadUrl handling to pass contextual next_actions through emit_one_with_actions, using the same pattern as Create/Update/Delete in this module; include at least next_actions::context("test-cases") and consider next_actions::get("test-cases", &args.test_case_id) for consistency. Keep the change localized to the MediaUploadUrl match arm and preserve the existing client call and result emission flow.
🤖 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 `@src/client/mod.rs`:
- Around line 498-512: `media_upload_url` is inconsistent with the `create` and
`update` API patterns because it accepts raw fields and constructs
`models::MediaUploadUrlRequest` internally. Refactor `media_upload_url` to take
a pre-built request struct argument, matching the signatures of `create` and
`update`, and keep the URL/post logic in `Client::media_upload_url` unchanged
aside from using the provided request value.
In `@src/commands/test_cases.rs`:
- Around line 303-309: The MediaUploadUrl branch in test_cases.rs emits no
follow-up actions, unlike the other mutating commands. Update the
TestCaseCommands::MediaUploadUrl handling to pass contextual next_actions
through emit_one_with_actions, using the same pattern as Create/Update/Delete in
this module; include at least next_actions::context("test-cases") and consider
next_actions::get("test-cases", &args.test_case_id) for consistency. Keep the
change localized to the MediaUploadUrl match arm and preserve the existing
client call and result emission flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d05d207-606a-4ba6-8ab9-f34b40556fea
📒 Files selected for processing (4)
.gitignoresrc/client/mod.rssrc/client/models/test_case.rssrc/commands/test_cases.rs
|
🟢 |
borgesius
left a comment
borgesius
left a comment
There was a problem hiding this comment.
Looks reasonable. The media upload-url request/response fields match the backend schema and the command is a small additive wrapper around the new endpoint.
2 participants
Summary
Adds
media-upload-urlsubcommand tocoval test-cases.What's New
coval test-cases media-upload-url <id> --filename <name> [--mime-type <type>]Generates a presigned S3 upload URL for attaching media to a test case.
Matches
POST /v1/test-cases/{id}/media:upload-url.Verification
cargo build✓ -cargo test90/90 ✓Greptile Summary
Adds a
media-upload-urlsubcommand tocoval test-cases, providing API parity withPOST /v1/test-cases/{id}/media:upload-url. The command generates a presigned S3 upload URL for attaching media to a test case.MediaUploadUrlArgsstruct withtest_case_id(positional),--filename(required), and--mime-type(optional, defaults toapplication/octet-stream), plus doc comments on all fields for--helpoutput consistency.MediaUploadUrlRequest/MediaUploadUrlResponsemodel structs and a correspondingmedia_upload_urlmethod onTestCasesClient..worktrees/added to.gitignoreto prevent accidental worktree commits.Confidence Score: 5/5
Safe to merge — the change is a self-contained new subcommand with no modifications to existing logic.
The new command wires directly to a single new API endpoint with no shared state or side-effects on existing commands. Previous review feedback (unsafe default MIME type, missing help text, accidental worktree commit) has all been addressed in this revision.
No files require special attention.
Reviews (3): Last reviewed commit: "fix: address review comments" | Re-trigger Greptile