feat: add bl team add / update / delete / icon commands#105
Merged
Conversation
📝 WalkthroughWalkthroughAdds team management: new BacklogApi trait methods and BacklogClient team implementations; four CLI subcommands (team add/update/delete/icon) with argument types, wiring, output handling and tests; shared formatting helper moved; docs, i18n, testing notes, and a spellcheck token added. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI (bl team add/update/delete)"
participant Client as "BacklogClient"
participant Server as "Backlog API (HTTP)"
participant Printer as "Stdout"
CLI->>Client: call create/update/delete_team(params)
Client->>Server: HTTP POST/PATCH/DELETE /api/v2/teams...
Server-->>Client: JSON Team response
Client-->>CLI: returns Team
alt json flag
CLI->>Printer: print pretty JSON
else text flag
CLI->>Printer: print format_team_row(Team)
end
sequenceDiagram
participant CLI as "CLI (bl team icon)"
participant Client as "BacklogClient"
participant Server as "Backlog API (HTTP)"
participant FS as "Filesystem"
participant Printer as "Stdout"
CLI->>Client: call download_team_icon(team_id)
Client->>Server: HTTP GET /api/v2/teams/{id}/icon
Server-->>Client: bytes + filename (headers)
Client-->>CLI: returns (bytes, filename)
CLI->>FS: write bytes to path (derived from output or filename)
FS-->>CLI: success
CLI->>Printer: print saved path and byte count
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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✨ Simplify code
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.
Pull request overview
Adds the remaining mutating bl team subcommands (add/update/delete) plus a team icon download command, wiring them through the CLI layer, API client layer, and user docs to complete the team-management feature set (Issue #56).
Changes:
- Added
bl team add,bl team update,bl team delete, andbl team iconCLI commands and clap wiring. - Implemented corresponding
BacklogClient+BacklogApimethods for/teamscreate/update/delete and/teams/{id}/icondownload. - Updated EN/JA command documentation and the command coverage tables.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/api/team.rs |
Adds create/update/delete/download icon endpoints for teams, plus httpmock tests. |
src/api/mod.rs |
Extends BacklogApi trait and dispatch impl with team mutation + icon methods. |
src/cmd/team/add.rs |
New command implementation + unit tests for team creation. |
src/cmd/team/update.rs |
New command implementation + unit tests + try_new validation. |
src/cmd/team/delete.rs |
New command implementation + unit tests for team deletion. |
src/cmd/team/icon.rs |
New command implementation + unit tests for icon download/save behavior. |
src/cmd/team/mod.rs |
Exposes the new team subcommands/Args. |
src/main.rs |
Adds clap variants and runtime dispatch for the new bl team subcommands. |
website/docs/commands.md |
Documents new commands + marks endpoints implemented in coverage table (EN). |
website/i18n/ja/.../commands.md |
Documents new commands + marks endpoints implemented in coverage table (JA). |
docs/TESTING.md |
Adds testing guidance for ANSI formatting and conditional Debug derivations. |
.cspell/dicts/project.txt |
Adds PRRT to the project dictionary. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…to delete/update output Addresses review comments: deduplication of format_team_row across add/list/delete/update, consistent member count in text output, and accurate Debug requirement explanation in TESTING.md
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/cmd/team/delete.rs (1)
1-105: Please keep team subcommands consolidated in a single team command file.This repo’s established pattern is one consolidated resource file for subcommands (with
*_withvariants), so introducing separatedelete.rsincreases fragmentation versus existing convention.Based on learnings: "In the backlog-cli repo 23prime/backlog-cli, sub-resource command files under src/cmd// ... should consolidate all subcommands ... into a single file per resource. Do not split each subcommand into its own file."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/team/delete.rs` around lines 1 - 105, This change adds a standalone src/cmd/team/delete.rs file which fragments the team subcommands; consolidate its contents into the existing team command file by moving TeamDeleteArgs, the delete and delete_with functions (preserve signatures and use of BacklogClient::from_config and BacklogApi::delete_team), and the unit tests into the main team module (keeping references to format_team_row and serde_json usage unchanged), then remove this delete.rs file and update module visibility/imports as needed so tests and callers still compile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cmd/team/update.rs`:
- Around line 22-24: The try_new validation currently allows members =
Some(vec![]) to pass when name is absent; update Args::try_new (the function
validating name and members) to treat an empty members vector as absent and
reject the call when there is no name and members is either None or Some(empty).
Concretely, modify the conditional that checks name.is_none() &&
members.is_none() to also consider members.as_ref().map(|v|
v.is_empty()).unwrap_or(true) (or equivalent) so that try_new bails with the
same error ("at least one of --name or --member must be provided") when name is
None and members is empty or missing.
---
Nitpick comments:
In `@src/cmd/team/delete.rs`:
- Around line 1-105: This change adds a standalone src/cmd/team/delete.rs file
which fragments the team subcommands; consolidate its contents into the existing
team command file by moving TeamDeleteArgs, the delete and delete_with functions
(preserve signatures and use of BacklogClient::from_config and
BacklogApi::delete_team), and the unit tests into the main team module (keeping
references to format_team_row and serde_json usage unchanged), then remove this
delete.rs file and update module visibility/imports as needed so tests and
callers still compile.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56d0090e-ddfe-416f-8e4d-bd1245381aa6
📒 Files selected for processing (6)
docs/TESTING.mdsrc/cmd/team/add.rssrc/cmd/team/delete.rssrc/cmd/team/list.rssrc/cmd/team/mod.rssrc/cmd/team/update.rs
✅ Files skipped from review due to trivial changes (1)
- docs/TESTING.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/cmd/team/mod.rs
- src/cmd/team/add.rs
…s absent Addresses review comment: treat Some(vec![]) as equivalent to None for members validation
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/cmd/team/update.rs (2)
70-77: Assert outgoingteam_idand form params in tests.Current
MockApiignores request inputs, so regressions in"members[]"expansion or ID forwarding won’t be caught. Add assertions in the mock and expected params in tests.Suggested test hardening
struct MockApi { team: Option<Team>, + expected_team_id: u64, + expected_params: Vec<(String, String)>, } impl crate::api::BacklogApi for MockApi { - fn update_team(&self, _team_id: u64, _params: &[(String, String)]) -> anyhow::Result<Team> { + fn update_team(&self, team_id: u64, params: &[(String, String)]) -> anyhow::Result<Team> { + assert_eq!(team_id, self.expected_team_id); + assert_eq!(params, self.expected_params.as_slice()); self.team.clone().ok_or_else(|| anyhow!("update failed")) } } #[test] fn update_with_text_output_succeeds() { let api = MockApi { team: Some(sample_team()), + expected_team_id: 1, + expected_params: vec![("name".to_string(), "dev-team".to_string())], }; assert!(update_with(&args(false), &api).is_ok()); }Also applies to: 109-130
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/team/update.rs` around lines 70 - 77, MockApi currently ignores the inputs so tests won't catch regressions; change MockApi (struct and impl) so it stores expected_team_id: u64 and expected_params: Option<Vec<(String,String)>> (or similar) and update update_team(&self, team_id, params) to assert team_id == self.expected_team_id and that params contains the expected key/value pairs (including exact "members[]" expansions and ID string values), returning the cloned Team only if assertions pass (or Err otherwise); update the tests that construct MockApi to set expected_team_id and expected_params to match the test's expected "members[]" entries and other form fields so the mock verifies ID forwarding and param expansion.
1-5: Consider aligning team command layout with the repo’s resource-file convention.This PR places subcommands in separate files (
add.rs,update.rs,delete.rs,icon.rs), but the established repo pattern is to keep subcommands together under one resource file for consistency and discoverability. Based on learnings: “In the backlog-cli repo 23prime/backlog-cli, sub-resource command files under src/cmd// should consolidate all subcommands (add, update, delete, reorder, etc.) and their *_with variants into a single file per resource.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/team/update.rs` around lines 1 - 5, The team subcommands are split across separate files (add.rs, update.rs, delete.rs, icon.rs) which deviates from the repo convention of grouping all subcommands for a resource into a single resource file; consolidate those separate subcommand modules into one file (e.g., src/cmd/team.rs) so add, update, delete, icon and any *_with variants live together, move the relevant functions (e.g., the handlers and helper functions currently in update.rs) into that single file, update the module exports and use sites to import from the consolidated module (ensure symbols like format_team_row, BacklogApi/BacklogClient usages are preserved), and remove the now-unnecessary separate subcommand files or re-export them from the single resource file to maintain discoverability and consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cmd/team/update.rs`:
- Around line 70-77: MockApi currently ignores the inputs so tests won't catch
regressions; change MockApi (struct and impl) so it stores expected_team_id: u64
and expected_params: Option<Vec<(String,String)>> (or similar) and update
update_team(&self, team_id, params) to assert team_id == self.expected_team_id
and that params contains the expected key/value pairs (including exact
"members[]" expansions and ID string values), returning the cloned Team only if
assertions pass (or Err otherwise); update the tests that construct MockApi to
set expected_team_id and expected_params to match the test's expected
"members[]" entries and other form fields so the mock verifies ID forwarding and
param expansion.
- Around line 1-5: The team subcommands are split across separate files (add.rs,
update.rs, delete.rs, icon.rs) which deviates from the repo convention of
grouping all subcommands for a resource into a single resource file; consolidate
those separate subcommand modules into one file (e.g., src/cmd/team.rs) so add,
update, delete, icon and any *_with variants live together, move the relevant
functions (e.g., the handlers and helper functions currently in update.rs) into
that single file, update the module exports and use sites to import from the
consolidated module (ensure symbols like format_team_row,
BacklogApi/BacklogClient usages are preserved), and remove the now-unnecessary
separate subcommand files or re-export them from the single resource file to
maintain discoverability and consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ebb409c4-d7f3-42ed-9204-c41ed75ed222
📒 Files selected for processing (1)
src/cmd/team/update.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
mainwebsite/docs/,website/i18n/ja/,README.md)Summary
Add mutating
bl teamcommands and icon download:bl team add --name <name> [--member <id>...]—POST /api/v2/teamsbl team update <id> --name <name> | --member <id>...—PATCH /api/v2/teams/{teamId}bl team delete <id>—DELETE /api/v2/teams/{teamId}bl team icon <id> [--output <path>]—GET /api/v2/teams/{teamId}/iconReason for change
Implements the remaining team management commands planned in Issue #56.
Changes
src/api/team.rs: Addedcreate_team,update_team,delete_team,download_team_iconsrc/api/mod.rs: Added trait methods andBacklogClientdispatchsrc/cmd/team/add.rs,update.rs,delete.rs,icon.rs: New command files with testssrc/cmd/team/mod.rs: Re-exports for new commandssrc/main.rs: Clap wiring forAdd,Update,Delete,Iconvariants inTeamCommandsNotes
POST/PATCH/DELETE commands were not tested against the real API to avoid affecting live data.
Closes #56