fix: use issue.number instead of issue.id in comment command #256

timstoop commented 2025年10月11日 19:04:48 +02:00

Summary

Fixes #224 where berg issue comment returned a 404 error when the issue's internal database ID differed from its visible issue number.

Problem

The issue comment command was using issue.id (internal database ID) instead of issue.number (the visible issue number like #1, #2, etc.) when making the API call. The Forgejo API expects the issue number, not the internal ID.

When these values differ (which happens in repositories with closed/deleted issues), the API returns a 404 error.

Solution

Changed src/actions/issue/comment.rs:93 to use issue.number instead of issue.id, aligning with how other commands like issue edit already work.

Testing

• The code compiles and passes cargo clippy
• Manually verified the fix addresses the root cause
• A test case has been added to main branch that would catch this regression

Related

This fix aligns with the pattern used in other commands:

• issue edit uses issue.number (line 76)
• milestone view uses issue.number (line 106)

Closes #224

🤖 Generated with Claude Code

## Summary Fixes #224 where `berg issue comment` returned a 404 error when the issue's internal database ID differed from its visible issue number. ## Problem The `issue comment` command was using `issue.id` (internal database ID) instead of `issue.number` (the visible issue number like #1, #2, etc.) when making the API call. The Forgejo API expects the issue number, not the internal ID. When these values differ (which happens in repositories with closed/deleted issues), the API returns a 404 error. ## Solution Changed `src/actions/issue/comment.rs:93` to use `issue.number` instead of `issue.id`, aligning with how other commands like `issue edit` already work. ## Testing - The code compiles and passes cargo clippy - Manually verified the fix addresses the root cause - A test case has been added to main branch that would catch this regression ## Related This fix aligns with the pattern used in other commands: - `issue edit` uses `issue.number` (line 76) - `milestone view` uses `issue.number` (line 106) Closes #224 🤖 Generated with [Claude Code](https://claude.com/claude-code)

timstoop commented 2025年10月12日 14:36:12 +02:00

While reviewing the codebase, we found the same bug in berg issue view --comments (both JSON and Pretty output modes). The view command was also using issue.id instead of issue.number when fetching comments.

Since this is the exact same root cause (API expects issue numbers, not internal database IDs), I've added the fix to this PR along with a test case that would have caught both bugs.

Changes in the latest commit:

• Fixed src/actions/issue/view.rs:88 (JSON output mode)
• Fixed src/actions/issue/view.rs:144 (Pretty output mode)
• Added test issueViewCommentsWithDifferentIdAndNumber

While reviewing the codebase, we found the same bug in `berg issue view --comments` (both JSON and Pretty output modes). The view command was also using `issue.id` instead of `issue.number` when fetching comments. Since this is the exact same root cause (API expects issue numbers, not internal database IDs), I've added the fix to this PR along with a test case that would have caught both bugs. Changes in the latest commit: - Fixed `src/actions/issue/view.rs:88` (JSON output mode) - Fixed `src/actions/issue/view.rs:144` (Pretty output mode) - Added test `issueViewCommentsWithDifferentIdAndNumber`

timstoop commented 2025年10月12日 20:33:34 +02:00

Split the fix for view.rs off into its own branch, to keep the reviewing as focussed as possible.

Split the fix for view.rs off into its own branch, to keep the reviewing as focussed as possible.

Aviac commented 2025年10月13日 19:25:18 +02:00

Sorry for how this went here. I just now realize that you also tackled the issue. I was writing a test which could properly reproduce the issue since this was non trivial. The fix itself is now already merged!

Sorry for how this went here. I just now realize that you also tackled the issue. I was writing a test which could properly reproduce the issue since this was non trivial. The fix itself is now already merged!