Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds user-management and user-interaction features: new BacklogApi methods and types (users, recently viewed projects/wikis, stars), BacklogClient implementations, CLI commands (add/update/delete, recently-viewed, star list/count) with argument validation, wiring in main, tests, and documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI (main.rs)
participant Cmd as Command Module
participant Config as Config Loader
participant Client as BacklogClient
participant API as BacklogApi
User->>CLI: run `bl user add ...` (or update/delete/recently-viewed/star)
CLI->>Cmd: construct argument struct (new / try_new)
Cmd->>Config: load credentials & endpoint
Config-->>Cmd: return Config
Cmd->>Client: create BacklogClient(config)
Cmd->>API: call BacklogApi::<method>(params) via BacklogClient
Client->>API: forward to concrete client method
API-->>Client: return Result<T> (User / list / count)
Client-->>Cmd: return result
Cmd->>Cmd: format output (JSON or text)
Cmd-->>User: print output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 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 unit tests (beta)
✨ Simplify code
📝 Coding Plan
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 new bl user subcommands to manage users (add/update/delete) and to retrieve user-related lists (recently viewed projects/wikis, stars list/count), wiring them through the API layer and documenting the new commands.
Changes:
- Extend CLI (
src/main.rs) with newuseranduser starsubcommands plus aRoleTypevalue enum. - Add new command implementations under
src/cmd/user/*for user CRUD and the new GET endpoints. - Extend
BacklogClient/BacklogApiwith new user endpoints and update the command reference docs (EN/JA).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/commands.md | Adds EN docs for new bl user commands and updates endpoint coverage table statuses. |
| website/i18n/ja/docusaurus-plugin-content-docs/current/commands.md | Adds JA docs for new bl user commands and updates endpoint coverage table statuses. |
| src/main.rs | Adds RoleType ValueEnum and new clap subcommands/routes for user CRUD, recently-viewed, and stars. |
| src/cmd/user/mod.rs | Registers and re-exports the new user command modules. |
| src/cmd/user/add.rs | Implements bl user add command and unit tests with a mock API. |
| src/cmd/user/update.rs | Implements bl user update command and unit tests with a mock API. |
| src/cmd/user/delete.rs | Implements bl user delete command and unit tests with a mock API. |
| src/cmd/user/recently_viewed_projects.rs | Implements bl user recently-viewed-projects and unit tests. |
| src/cmd/user/recently_viewed_wikis.rs | Implements bl user recently-viewed-wikis and unit tests. |
| src/cmd/user/star/mod.rs | Introduces user star module wiring. |
| src/cmd/user/star/list.rs | Implements bl user star list and unit tests, including argument validation. |
| src/cmd/user/star/count.rs | Implements bl user star count and unit tests. |
| src/api/user.rs | Adds new API structs + BacklogClient methods for the new endpoints, with httpmock tests. |
| src/api/mod.rs | Extends BacklogApi trait and forwards new methods in the BacklogClient impl. |
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/cmd/user/add.rs (1)
76-80: Tests should assert the exact request payload sent toadd_user.Right now the mock ignores
_params, so key-name regressions (e.g.,mailAddress,roleType) won’t fail tests.🧪 Suggested payload assertion pattern
struct MockApi { user: Option<User>, + seen_params: std::cell::RefCell<Vec<(String, String)>>, } impl crate::api::BacklogApi for MockApi { - fn add_user(&self, _params: &[(String, String)]) -> anyhow::Result<User> { + fn add_user(&self, params: &[(String, String)]) -> anyhow::Result<User> { + *self.seen_params.borrow_mut() = params.to_vec(); self.user.clone().ok_or_else(|| anyhow!("add failed")) } } #[test] fn add_with_text_output_succeeds() { let api = MockApi { user: Some(sample_user()), + seen_params: std::cell::RefCell::new(Vec::new()), }; assert!(add_with(&args(false), &api).is_ok()); + let sent = api.seen_params.borrow(); + assert!(sent.iter().any(|(k, v)| k == "userId" && v == "john")); + assert!(sent.iter().any(|(k, v)| k == "mailAddress" && v == "john@example.com")); + assert!(sent.iter().any(|(k, v)| k == "roleType" && v == "2")); }Also applies to: 106-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/user/add.rs` around lines 76 - 80, The MockApi's BacklogApi::add_user implementation ignores the incoming _params so tests won't catch payload key/name regressions; update MockApi (the add_user method) to validate the exact request payload by matching the passed params (e.g., convert &[(String,String)] to a map or ordered vec) against the expected keys and values (including exact key names like "mailAddress" and "roleType"), return the stored user on match and return an error or panic with a clear message on mismatch; apply the same change pattern to the other mock methods referenced (lines 106-127) so tests assert precise payload structure rather than ignoring params.src/cmd/user/delete.rs (1)
50-53: Strengthen tests by asserting the forwardeduser_id.Current
MockApi::delete_userignores its input, so a regression in ID wiring would not be caught.🔍 Suggested test hardening
struct MockApi { user: Option<User>, + called_user_id: std::cell::Cell<Option<u64>>, } impl crate::api::BacklogApi for MockApi { - fn delete_user(&self, _user_id: u64) -> anyhow::Result<User> { + fn delete_user(&self, user_id: u64) -> anyhow::Result<User> { + self.called_user_id.set(Some(user_id)); self.user.clone().ok_or_else(|| anyhow!("delete failed")) } } #[test] fn delete_with_text_output_succeeds() { let api = MockApi { user: Some(sample_user()), + called_user_id: std::cell::Cell::new(None), }; assert!(delete_with(&args(false), &api).is_ok()); + assert_eq!(api.called_user_id.get(), Some(1)); }Also applies to: 73-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/user/delete.rs` around lines 50 - 53, MockApi::delete_user currently ignores its _user_id parameter so tests can't catch bad ID wiring; update MockApi::delete_user (and other mocked methods noted around the 73-94 region) to validate the incoming user_id against the expected value (e.g., stored expected_user_id or record the last_received_user_id) and return an error if it differs, or assert/compare in the mock before returning the cloned user—this ensures tests fail when the forwarded user_id is incorrect.src/cmd/user/star/count.rs (1)
59-67: Add tests that assertsince/untilquery forwarding.Because
_paramsis ignored in the mock, regressions in query key/value construction won’t be detected.Also applies to: 73-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/user/star/count.rs` around lines 59 - 67, The MockApi's count_user_stars currently ignores the _params so tests won't catch regressions; update MockApi::count_user_stars to capture and validate the incoming params (e.g., check for "since" and/or "until" keys or store the params into a public field like last_params on MockApi) and return the existing StarCount result only after verifying params or exposing them for assertions; apply the same change to the other mocked methods in this impl (the similar block covering lines 73-94) so tests can assert that query key/value forwarding works.
🤖 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/user/add.rs`:
- Around line 76-80: The MockApi's BacklogApi::add_user implementation ignores
the incoming _params so tests won't catch payload key/name regressions; update
MockApi (the add_user method) to validate the exact request payload by matching
the passed params (e.g., convert &[(String,String)] to a map or ordered vec)
against the expected keys and values (including exact key names like
"mailAddress" and "roleType"), return the stored user on match and return an
error or panic with a clear message on mismatch; apply the same change pattern
to the other mock methods referenced (lines 106-127) so tests assert precise
payload structure rather than ignoring params.
In `@src/cmd/user/delete.rs`:
- Around line 50-53: MockApi::delete_user currently ignores its _user_id
parameter so tests can't catch bad ID wiring; update MockApi::delete_user (and
other mocked methods noted around the 73-94 region) to validate the incoming
user_id against the expected value (e.g., stored expected_user_id or record the
last_received_user_id) and return an error if it differs, or assert/compare in
the mock before returning the cloned user—this ensures tests fail when the
forwarded user_id is incorrect.
In `@src/cmd/user/star/count.rs`:
- Around line 59-67: The MockApi's count_user_stars currently ignores the
_params so tests won't catch regressions; update MockApi::count_user_stars to
capture and validate the incoming params (e.g., check for "since" and/or "until"
keys or store the params into a public field like last_params on MockApi) and
return the existing StarCount result only after verifying params or exposing
them for assertions; apply the same change to the other mocked methods in this
impl (the similar block covering lines 73-94) so tests can assert that query
key/value forwarding works.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97bd9717-eb00-445a-9f80-9a5e9ed5d8c0
📒 Files selected for processing (14)
src/api/mod.rssrc/api/user.rssrc/cmd/user/add.rssrc/cmd/user/delete.rssrc/cmd/user/mod.rssrc/cmd/user/recently_viewed_projects.rssrc/cmd/user/recently_viewed_wikis.rssrc/cmd/user/star/count.rssrc/cmd/user/star/list.rssrc/cmd/user/star/mod.rssrc/cmd/user/update.rssrc/main.rswebsite/docs/commands.mdwebsite/i18n/ja/docusaurus-plugin-content-docs/current/commands.md
Addresses review comment: flags not shown in docs, hard to discover
Addresses review comment: empty PATCH should be rejected locally
There was a problem hiding this comment.
🧹 Nitpick comments (2)
website/i18n/ja/docusaurus-plugin-content-docs/current/commands.md (1)
1166-1167: Optional cleanup: avoid duplicated star mapping rows in JA coverage tables.As with the English doc, listing the same endpoints in both Stars and Users sections may cause future status drift. A canonical section + cross-reference would reduce maintenance overhead.
Also applies to: 1207-1208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/i18n/ja/docusaurus-plugin-content-docs/current/commands.md` around lines 1166 - 1167, Remove the duplicated star endpoint rows from the Users section in the Japanese commands doc (the table rows containing `bl user star list <id> | GET /api/v2/users/{userId}/stars` and `bl user star count <id> | GET /api/v2/users/{userId}/stars/count`) and instead add a single canonical source of truth in the Stars section with a brief cross-reference note in the Users section that points readers to the Stars section; update any nearby duplicate rows at the other occurrence (the ones around the `1207-1208` equivalent) to follow the same pattern so the JA table mirrors the EN structure and avoids future drift.website/docs/commands.md (1)
1162-1163: Consider using a single source-of-truth for user star endpoint coverage.
bl user star list/countare documented in both Stars and Users coverage tables. This is fine now, but it can drift during future updates. Consider keeping one canonical row set and linking from the other section.Also applies to: 1203-1204
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/commands.md` around lines 1162 - 1163, Choose one canonical table for the user-star endpoints (either the Stars section or the Users section) and remove the duplicate rows for `bl user star list <id>` and `bl user star count <id>` from the other section; in that other section replace the removed rows with a link to the canonical table (use the endpoint paths `GET /api/v2/users/{userId}/stars` and `GET /api/v2/users/{userId}/stars/count` as anchor targets or headings) so coverage is maintained without duplication — update the duplicate occurrences (including the other instance referenced around the same area) and ensure the canonical table has an anchor/id so the link resolves.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@website/docs/commands.md`:
- Around line 1162-1163: Choose one canonical table for the user-star endpoints
(either the Stars section or the Users section) and remove the duplicate rows
for `bl user star list <id>` and `bl user star count <id>` from the other
section; in that other section replace the removed rows with a link to the
canonical table (use the endpoint paths `GET /api/v2/users/{userId}/stars` and
`GET /api/v2/users/{userId}/stars/count` as anchor targets or headings) so
coverage is maintained without duplication — update the duplicate occurrences
(including the other instance referenced around the same area) and ensure the
canonical table has an anchor/id so the link resolves.
In `@website/i18n/ja/docusaurus-plugin-content-docs/current/commands.md`:
- Around line 1166-1167: Remove the duplicated star endpoint rows from the Users
section in the Japanese commands doc (the table rows containing `bl user star
list <id> | GET /api/v2/users/{userId}/stars` and `bl user star count <id> | GET
/api/v2/users/{userId}/stars/count`) and instead add a single canonical source
of truth in the Stars section with a brief cross-reference note in the Users
section that points readers to the Stars section; update any nearby duplicate
rows at the other occurrence (the ones around the `1207-1208` equivalent) to
follow the same pattern so the JA table mirrors the EN structure and avoids
future drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 74cf43b3-7145-4dbf-88ba-c5edf9a22300
📒 Files selected for processing (2)
website/docs/commands.mdwebsite/i18n/ja/docusaurus-plugin-content-docs/current/commands.md
Addresses review comment: contradictory query params should be validated
Checklist
mainwebsite/docs/,website/i18n/ja/,README.md)Summary
bl user add— POST /api/v2/users (requires Space Admin)bl user update <id>— PATCH /api/v2/users/{userId} (requires Space Admin)bl user delete <id>— DELETE /api/v2/users/{userId} (requires Space Admin)bl user recently-viewed-projects— GET /api/v2/users/myself/recentlyViewedProjectsbl user recently-viewed-wikis— GET /api/v2/users/myself/recentlyViewedWikisbl user star list <id>— GET /api/v2/users/{userId}/starsbl user star count <id>— GET /api/v2/users/{userId}/stars/countRoleTypeValueEnum (Administrator/Normal/Reporter/Viewer/GuestReporter/GuestViewer)Reason for change
Implements Issue #53.
Changes
src/api/user.rs— new structs (RecentlyViewedProject,RecentlyViewedWiki,Star,StarCount) + 7 newBacklogClientmethods + testssrc/api/mod.rs— trait methods + impls for all 7 methodssrc/cmd/user/add.rs,update.rs,delete.rs— new command filessrc/cmd/user/recently_viewed_projects.rs,recently_viewed_wikis.rs— new command filessrc/cmd/user/star/list.rs,star/count.rs,star/mod.rs— new command filessrc/cmd/user/mod.rs— updated re-exportssrc/main.rs—RoleTypeValueEnum +UserCommandsextended +UserStarCommandsaddedwebsite/docs/commands.md+website/i18n/ja/.../commands.md— docs added, coverage table updatedNotes
POST/PATCH/DELETE commands were not tested against the real API (would affect real data).
GET commands verified against real API.
Closes #53