Close issue #014

[JonghunYu]

close 014

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Fixed a defect where closing an issue after editing would incorrectly create a new open-issue file instead of properly closing the edited issue.
  • Issues now maintain their original identifiers throughout the edit-and-close workflow.

• Tests

  • Added test coverage for the issue closing workflow following edits.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR fixes a bug where closing an issue after editing its title creates a new open-issue file instead of properly managing the original. The core change modifies SaveIssue in pkg/storage.go to preserve existing issue filenames when an issue already exists in the target directory, rather than always generating a new slug-based filename. The counter is incremented, a new issue document is added describing the bug, and tests are included to verify the fix works correctly.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• pkg/storage.go: The conditional logic for checking existing filenames and reusing paths is straightforward but affects core save behavior—verify the FindIssueFile call and error handling are correct
• pkg/storage_test.go: Contains a duplicate test (TestSaveIssuePreservesExistingFilename appears twice)—one copy should be removed
• cmd/close_test.go: The new test is clear and follows existing patterns
• Counter and issue document: Trivial updates

Poem

🐰 A title may change, but the issue's true face,
Now keeps its original slug in its place,
When closed after edits, no duplicate born—
Just one tidy file, as it was that fine morn!
Hop-tidy-hop! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title 'Close issue #14' directly corresponds to the main objective described in the PR description: closing issue #14, which is documented in the newly added file 014-create-new-issue-when-close.md and addressed through changes to storage.go and related tests.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/014

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

.issues/closed/014-create-new-issue-when-close.md (1)

18-26: Consider adding language specification to the fenced code block.

The fenced code block showing the console output would benefit from a language specification for better rendering and syntax highlighting.

Apply this diff:

-```
+```bash
 gi close 004 -c
 ✓ Closed issue #004
 [feat/balance-check 9901acb] Close issue #004

cmd/close_test.go (1)

182-223: Duplicate test coverage detected.

The new test TestRunCloseAfterEditDoesNotCreateNewIssueFile appears to duplicate functionality already covered by TestRunClosePreservesFilenameWhenTitleModified (lines 225-286). Both tests:

• Create an issue with "Original Title"
• Edit the title to change the slug
• Close the issue
• Verify the filename contains "original-title"
• Verify no duplicate files were created

The primary difference is that the new test uses pkg.SaveIssue to edit (line 197), while the existing test uses os.WriteFile directly (line 250). Consider consolidating these tests or clearly differentiating what unique behavior each one validates.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fbd35d5 and 1747007.

📒 Files selected for processing (5)

• .issues/.counter (1 hunks)
• .issues/closed/014-create-new-issue-when-close.md (1 hunks)
• cmd/close_test.go (1 hunks)
• pkg/storage.go (1 hunks)
• pkg/storage_test.go (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

pkg/storage_test.go (1)

pkg/storage.go (5)

• InitializeRepo (22-95)
• NewIssue (330-341)
• SaveIssue (122-150)
• OpenDir (14-14)
• IssuesDir (13-13)

pkg/storage.go (1)

pkg/parser.go (1)

• GenerateSlug (80-105)

cmd/close_test.go (1)

pkg/storage.go (6)

• LoadIssue (153-173)
• SaveIssue (122-150)
• ListIssues (223-254)
• OpenDir (14-14)
• FindIssueFile (258-272)
• ClosedDir (15-15)

🪛 markdownlint-cli2 (0.18.1)

.issues/closed/014-create-new-issue-when-close.md

18-18: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (3)

.issues/.counter (1)

1-1: LGTM!

Counter increment is correct for the new issue being added.

pkg/storage_test.go (2)

6-6: LGTM!

The strings import is necessary for the filename validation logic in the new test.

---

166-208: LGTM!

The test effectively verifies that SaveIssue preserves the original filename slug when an issue is saved after editing its title. The test logic is clear and comprehensive, checking both that only one file exists and that it contains the original slug.