Skip to content

Commit 45b3894

Browse files
fix(calendar): improve code quality from review
- Patch only attendees array when declining, not full event object - Add defensive guard in expandRepeatSchedule against infinite loop - Add JSON output test for propose-time command Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent f137a2e commit 45b3894

3 files changed

Lines changed: 97 additions & 1 deletion

File tree

internal/cmd/calendar_propose_time.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"runtime"
1010
"strings"
1111

12+
"google.golang.org/api/calendar/v3"
13+
1214
"github.com/steipete/gogcli/internal/outfmt"
1315
"github.com/steipete/gogcli/internal/ui"
1416
)
@@ -90,7 +92,12 @@ func (c *CalendarProposeTimeCmd) Run(ctx context.Context, flags *RootFlags) erro
9092
event.Attendees[*selfIdx].Comment = strings.TrimSpace(c.Comment)
9193
}
9294

93-
if _, err := svc.Events.Patch(calendarID, eventID, event).Do(); err != nil {
95+
// Create a minimal patch with only attendees to avoid side effects
96+
patchEvent := &calendar.Event{
97+
Attendees: event.Attendees,
98+
}
99+
100+
if _, err := svc.Events.Patch(calendarID, eventID, patchEvent).Do(); err != nil {
94101
return fmt.Errorf("failed to decline event: %w", err)
95102
}
96103
}

internal/cmd/calendar_propose_time_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"google.golang.org/api/calendar/v3"
1515
"google.golang.org/api/option"
1616

17+
"github.com/steipete/gogcli/internal/outfmt"
1718
"github.com/steipete/gogcli/internal/ui"
1819
)
1920

@@ -139,6 +140,89 @@ func TestCalendarProposeTimeCmd_Text(t *testing.T) {
139140
}
140141
}
141142

143+
func TestCalendarProposeTimeCmd_JSON(t *testing.T) {
144+
origNew := newCalendarService
145+
origOpen := openProposeTimeBrowser
146+
t.Cleanup(func() {
147+
newCalendarService = origNew
148+
openProposeTimeBrowser = origOpen
149+
})
150+
openProposeTimeBrowser = func(url string) error { return nil }
151+
152+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
153+
path := strings.TrimPrefix(r.URL.Path, "/calendar/v3")
154+
if strings.Contains(path, "/calendars/cal1/events/evt1") && r.Method == http.MethodGet {
155+
w.Header().Set("Content-Type", "application/json")
156+
_ = json.NewEncoder(w).Encode(map[string]any{
157+
"id": "evt1",
158+
"summary": "Team Meeting",
159+
"start": map[string]string{"dateTime": "2026-01-16T19:30:00-08:00"},
160+
"end": map[string]string{"dateTime": "2026-01-16T20:30:00-08:00"},
161+
"attendees": []map[string]any{
162+
{"email": "a@b.com", "self": true},
163+
{"email": "organizer@b.com", "organizer": true},
164+
},
165+
})
166+
return
167+
}
168+
http.NotFound(w, r)
169+
}))
170+
defer srv.Close()
171+
172+
svc, err := calendar.NewService(context.Background(),
173+
option.WithoutAuthentication(),
174+
option.WithHTTPClient(srv.Client()),
175+
option.WithEndpoint(srv.URL+"/"),
176+
)
177+
if err != nil {
178+
t.Fatalf("NewService: %v", err)
179+
}
180+
newCalendarService = func(context.Context, string) (*calendar.Service, error) { return svc, nil }
181+
182+
flags := &RootFlags{Account: "a@b.com", JSON: true}
183+
out := captureStdout(t, func() {
184+
u, uiErr := ui.New(ui.Options{Stdout: os.Stdout, Stderr: io.Discard, Color: "never"})
185+
if uiErr != nil {
186+
t.Fatalf("ui.New: %v", uiErr)
187+
}
188+
ctx := ui.WithUI(context.Background(), u)
189+
ctx = outfmt.WithMode(ctx, outfmt.Mode{JSON: true})
190+
191+
cmd := &CalendarProposeTimeCmd{}
192+
if err := runKong(t, cmd, []string{"cal1", "evt1"}, ctx, flags); err != nil {
193+
t.Fatalf("propose-time JSON: %v", err)
194+
}
195+
})
196+
197+
// Parse and verify JSON structure
198+
var result map[string]any
199+
if err := json.Unmarshal([]byte(out), &result); err != nil {
200+
t.Fatalf("failed to parse JSON output: %v\noutput: %s", err, out)
201+
}
202+
203+
// Verify required fields
204+
requiredFields := []string{"event_id", "calendar_id", "summary", "propose_url", "api_limitation", "issue_tracker_url", "upvote_action", "current_start", "current_end"}
205+
for _, field := range requiredFields {
206+
if _, ok := result[field]; !ok {
207+
t.Errorf("JSON missing required field %q", field)
208+
}
209+
}
210+
211+
if result["event_id"] != "evt1" {
212+
t.Errorf("event_id = %v, want evt1", result["event_id"])
213+
}
214+
if result["calendar_id"] != "cal1" {
215+
t.Errorf("calendar_id = %v, want cal1", result["calendar_id"])
216+
}
217+
if result["summary"] != "Team Meeting" {
218+
t.Errorf("summary = %v, want Team Meeting", result["summary"])
219+
}
220+
proposeURL, ok := result["propose_url"].(string)
221+
if !ok || !strings.Contains(proposeURL, "proposetime/") {
222+
t.Errorf("propose_url invalid: %v", result["propose_url"])
223+
}
224+
}
225+
142226
func TestCalendarProposeTimeCmd_WithDecline(t *testing.T) {
143227
origNew := newCalendarService
144228
origOpen := openProposeTimeBrowser

internal/cmd/tasks_repeat.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ func expandRepeatSchedule(start time.Time, unit repeatUnit, count int, until *ti
6565
if count < 0 {
6666
count = 0
6767
}
68+
// Defensive guard: if neither count nor until is set, return single occurrence
69+
// to prevent infinite loop (caller should validate, but be safe)
70+
if count == 0 && until == nil {
71+
return []time.Time{start}
72+
}
6873
out := []time.Time{}
6974
for i := 0; ; i++ {
7075
t := addRepeat(start, unit, i)

0 commit comments

Comments
 (0)