Skip to content

Commit fbd35d5

Browse files
authored
Fix: Preserve filename when closing/reopening issues (#11)
1 parent b0936e1 commit fbd35d5

8 files changed

Lines changed: 361 additions & 29 deletions

File tree

.issues/.counter

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
13
1+
14
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
id: "013"
3+
assignee: ""
4+
labels:
5+
- bug
6+
- fixed
7+
created: 2025-11-18T10:20:19.765626+09:00
8+
updated: 2025-11-18T10:24:47.5259+09:00
9+
---
10+
11+
# Fix filename preservation bug when closing/opening issues with modified titles
12+
13+
## Problem
14+
15+
When users modified issue titles after creation, the `close` and `open` commands created duplicate files with malformed filenames:
16+
17+
1. **Korean/Special Character Titles**: Generated malformed filenames like `001-.md`
18+
2. **Modified English Titles**: Created new files with updated slugs, leaving originals behind
19+
20+
### Root Cause
21+
22+
Both `close` and `open` commands called `SaveIssue()` before `MoveIssue()`, which:
23+
- Generated new slugs based on current `issue.Title` (which could be modified by users)
24+
- Created new files instead of updating existing ones
25+
- Left original files in place, resulting in duplicates
26+
27+
28+
---
29+
30+
31+
## Solution
32+
33+
1. **Modified `MoveIssue` in `pkg/storage.go`**:
34+
- Update `updated` timestamp after successful file relocation
35+
- Preserve original filename during move
36+
37+
2. **Removed redundant `SaveIssue` calls in `cmd/close.go` and `cmd/open.go`**:
38+
- Eliminated duplicate file creation
39+
- Simplified command logic
40+
41+
3. **Added regression tests**:
42+
- `TestRunClosePreservesFilenameWithKoreanTitle`
43+
- `TestRunClosePreservesFilenameWhenTitleModified`
44+
- `TestRunOpenPreservesFilenameWithKoreanTitle`
45+
- `TestRunOpenPreservesFilenameWhenTitleModified`
46+
- Updated `TestMoveIssue` to verify timestamp updates
47+
48+
## Test Results
49+
50+
- ✅ All existing tests pass
51+
- ✅ New regression tests pass
52+
- ✅ Code coverage maintained: 86.7% (cmd), 76.0% (pkg)
53+
54+
## Files Changed
55+
56+
- `pkg/storage.go`: Add timestamp update in `MoveIssue`
57+
- `cmd/close.go`: Remove `SaveIssue` call
58+
- `cmd/open.go`: Remove `SaveIssue` call
59+
- `pkg/storage_test.go`: Add timestamp verification
60+
- `cmd/close_test.go`: Add filename preservation tests
61+
- `cmd/open_test.go`: Add filename preservation tests

cmd/close.go

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"fmt"
55
"os"
66
"os/exec"
7-
"time"
87

98
"github.com/Allra-Fintech/git-issue/pkg"
109
"github.com/spf13/cobra"
@@ -28,8 +27,8 @@ func init() {
2827
func runClose(cmd *cobra.Command, args []string) error {
2928
issueID := args[0]
3029

31-
// Load the issue
32-
issue, currentDir, err := pkg.LoadIssue(issueID)
30+
// Load the issue to check its status
31+
_, currentDir, err := pkg.LoadIssue(issueID)
3332
if err != nil {
3433
return fmt.Errorf("failed to load issue: %w", err)
3534
}
@@ -39,15 +38,7 @@ func runClose(cmd *cobra.Command, args []string) error {
3938
return fmt.Errorf("issue #%s is already closed", issueID)
4039
}
4140

42-
// Update timestamp
43-
issue.Updated = time.Now()
44-
45-
// Save the issue with updated timestamp
46-
if err := pkg.SaveIssue(issue, pkg.OpenDir); err != nil {
47-
return fmt.Errorf("failed to update issue: %w", err)
48-
}
49-
50-
// Move issue from open to closed
41+
// Move issue from open to closed (timestamp update included)
5142
if err := pkg.MoveIssue(issueID, pkg.OpenDir, pkg.ClosedDir); err != nil {
5243
return fmt.Errorf("failed to move issue: %w", err)
5344
}

cmd/close_test.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cmd
22

33
import (
4+
"os"
45
"strings"
56
"testing"
67

@@ -117,3 +118,126 @@ func TestRunCloseCommitCreatesGitCommit(t *testing.T) {
117118
t.Fatalf("unexpected commit message %q", lastMessage)
118119
}
119120
}
121+
122+
func TestRunClosePreservesFilenameWithKoreanTitle(t *testing.T) {
123+
_, cleanup := setupCommandTestRepo(t)
124+
defer cleanup()
125+
126+
// Create issue with English title
127+
if err := runCreate(nil, []string{"Initial English Title"}); err != nil {
128+
t.Fatalf("runCreate() failed: %v", err)
129+
}
130+
131+
// Get original filename
132+
originalPath, _, err := pkg.FindIssueFile("001")
133+
if err != nil {
134+
t.Fatalf("failed to find issue: %v", err)
135+
}
136+
137+
// Edit issue to have Korean title
138+
issue, _, err := pkg.LoadIssue("001")
139+
if err != nil {
140+
t.Fatalf("failed to load issue: %v", err)
141+
}
142+
issue.Title = "한글 제목으로 변경"
143+
content, err := pkg.SerializeIssue(issue)
144+
if err != nil {
145+
t.Fatalf("failed to serialize issue: %v", err)
146+
}
147+
if err := os.WriteFile(originalPath, []byte(content), 0644); err != nil {
148+
t.Fatalf("failed to write issue: %v", err)
149+
}
150+
151+
// Close the issue
152+
if err := runClose(nil, []string{"001"}); err != nil {
153+
t.Fatalf("runClose() failed: %v", err)
154+
}
155+
156+
// Verify issue is in closed directory with original filename
157+
closedPath, dir, err := pkg.FindIssueFile("001")
158+
if err != nil {
159+
t.Fatalf("failed to find closed issue: %v", err)
160+
}
161+
if dir != pkg.ClosedDir {
162+
t.Fatalf("issue should be in closed dir, got %s", dir)
163+
}
164+
165+
// Verify filename was preserved (contains "initial-english-title")
166+
if !strings.Contains(closedPath, "initial-english-title") {
167+
t.Errorf("filename should be preserved, got %s", closedPath)
168+
}
169+
170+
// Verify no malformed files like "001-.md" were created
171+
openFiles, _ := pkg.ListIssues(pkg.OpenDir)
172+
closedFiles, _ := pkg.ListIssues(pkg.ClosedDir)
173+
174+
if len(openFiles) != 0 {
175+
t.Errorf("open directory should be empty, found %d files", len(openFiles))
176+
}
177+
if len(closedFiles) != 1 {
178+
t.Errorf("closed directory should have exactly 1 file, found %d", len(closedFiles))
179+
}
180+
}
181+
182+
func TestRunClosePreservesFilenameWhenTitleModified(t *testing.T) {
183+
_, cleanup := setupCommandTestRepo(t)
184+
defer cleanup()
185+
186+
// Create issue
187+
if err := runCreate(nil, []string{"Original Title"}); err != nil {
188+
t.Fatalf("runCreate() failed: %v", err)
189+
}
190+
191+
// Get original filename
192+
originalPath, _, err := pkg.FindIssueFile("001")
193+
if err != nil {
194+
t.Fatalf("failed to find issue: %v", err)
195+
}
196+
197+
// Edit issue to have completely different title
198+
issue, _, err := pkg.LoadIssue("001")
199+
if err != nil {
200+
t.Fatalf("failed to load issue: %v", err)
201+
}
202+
issue.Title = "Completely Different Modified Title"
203+
content, err := pkg.SerializeIssue(issue)
204+
if err != nil {
205+
t.Fatalf("failed to serialize issue: %v", err)
206+
}
207+
if err := os.WriteFile(originalPath, []byte(content), 0644); err != nil {
208+
t.Fatalf("failed to write issue: %v", err)
209+
}
210+
211+
// Close the issue
212+
if err := runClose(nil, []string{"001"}); err != nil {
213+
t.Fatalf("runClose() failed: %v", err)
214+
}
215+
216+
// Verify issue is in closed directory with original filename
217+
closedPath, dir, err := pkg.FindIssueFile("001")
218+
if err != nil {
219+
t.Fatalf("failed to find closed issue: %v", err)
220+
}
221+
if dir != pkg.ClosedDir {
222+
t.Fatalf("issue should be in closed dir, got %s", dir)
223+
}
224+
225+
// Verify filename was preserved (contains "original-title", not "completely-different")
226+
if !strings.Contains(closedPath, "original-title") {
227+
t.Errorf("original filename should be preserved, got %s", closedPath)
228+
}
229+
if strings.Contains(closedPath, "completely-different") {
230+
t.Errorf("filename should not change to modified title, got %s", closedPath)
231+
}
232+
233+
// Verify no duplicate files were created
234+
openFiles, _ := pkg.ListIssues(pkg.OpenDir)
235+
closedFiles, _ := pkg.ListIssues(pkg.ClosedDir)
236+
237+
if len(openFiles) != 0 {
238+
t.Errorf("open directory should be empty, found %d files", len(openFiles))
239+
}
240+
if len(closedFiles) != 1 {
241+
t.Errorf("closed directory should have exactly 1 file, found %d", len(closedFiles))
242+
}
243+
}

cmd/open.go

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package cmd
22

33
import (
44
"fmt"
5-
"time"
65

76
"github.com/Allra-Fintech/git-issue/pkg"
87
"github.com/spf13/cobra"
@@ -26,8 +25,8 @@ func init() {
2625
func runOpen(cmd *cobra.Command, args []string) error {
2726
issueID := args[0]
2827

29-
// Load the issue
30-
issue, currentDir, err := pkg.LoadIssue(issueID)
28+
// Load the issue to check its status
29+
_, currentDir, err := pkg.LoadIssue(issueID)
3130
if err != nil {
3231
return fmt.Errorf("failed to load issue: %w", err)
3332
}
@@ -37,15 +36,7 @@ func runOpen(cmd *cobra.Command, args []string) error {
3736
return fmt.Errorf("issue #%s is already open", issueID)
3837
}
3938

40-
// Update timestamp
41-
issue.Updated = time.Now()
42-
43-
// Save the issue with updated timestamp
44-
if err := pkg.SaveIssue(issue, pkg.ClosedDir); err != nil {
45-
return fmt.Errorf("failed to update issue: %w", err)
46-
}
47-
48-
// Move issue from closed to open
39+
// Move issue from closed to open (timestamp update included)
4940
if err := pkg.MoveIssue(issueID, pkg.ClosedDir, pkg.OpenDir); err != nil {
5041
return fmt.Errorf("failed to move issue: %w", err)
5142
}

0 commit comments

Comments
 (0)