Skip to content

Commit c46b66d

Browse files
tstrombergtstromberg
committed
auto-refresh
1 parent 7e05582 commit c46b66d

File tree

4 files changed

+264
-6
lines changed

4 files changed

+264
-6
lines changed

‎cmd/goose/click_test.go‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
package main
2+
3+
import (
4+
"context"
5+
"sync"
6+
"testing"
7+
"time"
8+
)
9+
10+
// TestMenuClickRateLimit tests that menu clicks are properly rate limited.
11+
func TestMenuClickRateLimit(t *testing.T) {
12+
ctx := context.Background()
13+
14+
// Create app with initial state
15+
app := &App{
16+
mu: sync.RWMutex{},
17+
stateManager: NewPRStateManager(time.Now().Add(-35 * time.Second)),
18+
hiddenOrgs: make(map[string]bool),
19+
seenOrgs: make(map[string]bool),
20+
lastSearchAttempt: time.Now().Add(-15 * time.Second), // 15 seconds ago
21+
}
22+
23+
// Simulate the click handler logic (without the actual UI interaction)
24+
testClick := func() (shouldRefresh bool, timeSinceLastSearch time.Duration) {
25+
app.mu.RLock()
26+
timeSince := time.Since(app.lastSearchAttempt)
27+
app.mu.RUnlock()
28+
29+
if timeSince >= minUpdateInterval {
30+
// Would trigger refresh
31+
app.mu.Lock()
32+
app.lastSearchAttempt = time.Now()
33+
app.mu.Unlock()
34+
return true, timeSince
35+
}
36+
return false, timeSince
37+
}
38+
39+
// Test 1: First click should allow refresh (last search was 15s ago)
40+
shouldRefresh, timeSince := testClick()
41+
if !shouldRefresh {
42+
t.Errorf("First click should allow refresh, last search was %v ago", timeSince)
43+
}
44+
45+
// Test 2: Immediate second click should be rate limited
46+
shouldRefresh2, timeSince2 := testClick()
47+
if shouldRefresh2 {
48+
t.Errorf("Second click should be rate limited, last search was %v ago", timeSince2)
49+
}
50+
51+
// Test 3: After waiting 10+ seconds, should allow refresh again
52+
app.mu.Lock()
53+
app.lastSearchAttempt = time.Now().Add(-11 * time.Second)
54+
app.mu.Unlock()
55+
56+
shouldRefresh3, timeSince3 := testClick()
57+
if !shouldRefresh3 {
58+
t.Errorf("Click after 11 seconds should allow refresh, last search was %v ago", timeSince3)
59+
}
60+
61+
_ = ctx // Keep context for potential future use
62+
}
63+
64+
// TestScheduledUpdateRateLimit tests that scheduled updates respect rate limiting.
65+
func TestScheduledUpdateRateLimit(t *testing.T) {
66+
app := &App{
67+
mu: sync.RWMutex{},
68+
stateManager: NewPRStateManager(time.Now().Add(-35 * time.Second)),
69+
hiddenOrgs: make(map[string]bool),
70+
seenOrgs: make(map[string]bool),
71+
lastSearchAttempt: time.Now().Add(-5 * time.Second), // 5 seconds ago
72+
}
73+
74+
// Simulate the scheduled update logic
75+
testScheduledUpdate := func() (shouldUpdate bool, timeSinceLastSearch time.Duration) {
76+
app.mu.RLock()
77+
timeSince := time.Since(app.lastSearchAttempt)
78+
app.mu.RUnlock()
79+
80+
return timeSince >= minUpdateInterval, timeSince
81+
}
82+
83+
// Test 1: Scheduled update should be skipped (last search was only 5s ago)
84+
shouldUpdate, timeSince := testScheduledUpdate()
85+
if shouldUpdate {
86+
t.Errorf("Scheduled update should be skipped, last search was %v ago", timeSince)
87+
}
88+
89+
// Test 2: After waiting 10+ seconds, scheduled update should proceed
90+
app.mu.Lock()
91+
app.lastSearchAttempt = time.Now().Add(-12 * time.Second)
92+
app.mu.Unlock()
93+
94+
shouldUpdate2, timeSince2 := testScheduledUpdate()
95+
if !shouldUpdate2 {
96+
t.Errorf("Scheduled update after 12 seconds should proceed, last search was %v ago", timeSince2)
97+
}
98+
}

‎cmd/goose/github.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,11 @@ type prResult struct {
223223

224224
// fetchPRsInternal fetches PRs and Turn data synchronously for simplicity.
225225
func (app *App) fetchPRsInternal(ctx context.Context) (incoming []PR, outgoing []PR, _ error) {
226+
// Update search attempt time for rate limiting
227+
app.mu.Lock()
228+
app.lastSearchAttempt = time.Now()
229+
app.mu.Unlock()
230+
226231
// Check if we have a client
227232
if app.client == nil {
228233
return nil, nil, fmt.Errorf("no GitHub client available: %s", app.authError)

‎cmd/goose/main.go‎

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"log"
1212
"os"
1313
"path/filepath"
14+
"slices"
1415
"strings"
1516
"sync"
1617
"time"
@@ -64,6 +65,8 @@ type PR struct {
6465
// App holds the application state.
6566
type App struct {
6667
lastSuccessfulFetch time.Time
68+
lastSearchAttempt time.Time // For rate limiting forced refreshes
69+
lastMenuTitles []string // For change detection to prevent unnecessary redraws
6770
startTime time.Time
6871
client *github.Client
6972
turnClient *turn.Client
@@ -251,6 +254,22 @@ func (app *App) onReady(ctx context.Context) {
251254
// Set up click handlers first (needed for both success and error states)
252255
systray.SetOnClick(func(menu systray.IMenu) {
253256
log.Println("Icon clicked")
257+
258+
// Check if we can perform a forced refresh (rate limited to every 10 seconds)
259+
app.mu.RLock()
260+
timeSinceLastSearch := time.Since(app.lastSearchAttempt)
261+
app.mu.RUnlock()
262+
263+
if timeSinceLastSearch >= minUpdateInterval {
264+
log.Printf("[CLICK] Forcing search refresh (last search %v ago)", timeSinceLastSearch)
265+
go func() {
266+
app.updatePRs(ctx)
267+
}()
268+
} else {
269+
remainingTime := minUpdateInterval - timeSinceLastSearch
270+
log.Printf("[CLICK] Rate limited - search performed %v ago, %v remaining", timeSinceLastSearch, remainingTime)
271+
}
272+
254273
if menu != nil {
255274
if err := menu.ShowMenu(); err != nil {
256275
log.Printf("Failed to show menu: %v", err)
@@ -325,8 +344,19 @@ func (app *App) updateLoop(ctx context.Context) {
325344
for {
326345
select {
327346
case <-ticker.C:
328-
log.Println("Running scheduled PR update")
329-
app.updatePRs(ctx)
347+
// Check if we should skip this scheduled update due to recent forced refresh
348+
app.mu.RLock()
349+
timeSinceLastSearch := time.Since(app.lastSearchAttempt)
350+
app.mu.RUnlock()
351+
352+
if timeSinceLastSearch >= minUpdateInterval {
353+
log.Println("Running scheduled PR update")
354+
app.updatePRs(ctx)
355+
} else {
356+
remainingTime := minUpdateInterval - timeSinceLastSearch
357+
log.Printf("Skipping scheduled update - recent search %v ago, %v remaining until next allowed",
358+
timeSinceLastSearch, remainingTime)
359+
}
330360
case <-ctx.Done():
331361
log.Println("Update loop stopping due to context cancellation")
332362
return
@@ -444,13 +474,33 @@ func (app *App) updatePRs(ctx context.Context) {
444474
log.Print("[DEBUG] Completed PR state updates and notifications")
445475
}
446476

447-
// updateMenu rebuilds the menu every time - simple and reliable.
477+
// updateMenu rebuilds the menu only if there are changes to improve UX.
448478
func (app *App) updateMenu(ctx context.Context) {
449-
// Always rebuild - it's just a small menu, performance is not an issue
450-
log.Println("[MENU] Rebuilding menu")
479+
// Generate current menu titles
480+
currentTitles := app.generateMenuTitles()
481+
482+
// Compare with last titles to see if rebuild is needed
483+
app.mu.RLock()
484+
lastTitles := app.lastMenuTitles
485+
app.mu.RUnlock()
486+
487+
// Check if titles have changed
488+
if slices.Equal(currentTitles, lastTitles) {
489+
log.Printf("[MENU] No changes detected, skipping rebuild (%d items unchanged)", len(currentTitles))
490+
return
491+
}
492+
493+
// Titles have changed, rebuild menu
494+
log.Printf("[MENU] Changes detected, rebuilding menu (%d→%d items)", len(lastTitles), len(currentTitles))
451495
app.rebuildMenu(ctx)
496+
497+
// Store new titles
498+
app.mu.Lock()
499+
app.lastMenuTitles = currentTitles
500+
app.mu.Unlock()
452501
}
453502

503+
454504
// updatePRsWithWait fetches PRs and waits for Turn data before building initial menu.
455505
func (app *App) updatePRsWithWait(ctx context.Context) {
456506
incoming, outgoing, err := app.fetchPRsInternal(ctx)

‎cmd/goose/ui.go‎

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,111 @@ func (app *App) addPRSection(ctx context.Context, prs []PR, sectionTitle string,
278278
}
279279
}
280280

281+
// generateMenuTitles generates the list of menu item titles that would be shown
282+
// without actually building the UI. Used for change detection.
283+
func (app *App) generateMenuTitles() []string {
284+
var titles []string
285+
286+
// Check for auth error first
287+
if app.authError != "" {
288+
titles = append(titles, "⚠️ Authentication Error")
289+
titles = append(titles, app.authError)
290+
titles = append(titles, "To fix this issue:")
291+
titles = append(titles, "1. Install GitHub CLI: brew install gh")
292+
titles = append(titles, "2. Run: gh auth login")
293+
titles = append(titles, "3. Or set GITHUB_TOKEN environment variable")
294+
titles = append(titles, "Quit")
295+
return titles
296+
}
297+
298+
app.mu.RLock()
299+
incoming := make([]PR, len(app.incoming))
300+
copy(incoming, app.incoming)
301+
outgoing := make([]PR, len(app.outgoing))
302+
copy(outgoing, app.outgoing)
303+
hiddenOrgs := make(map[string]bool)
304+
for org, hidden := range app.hiddenOrgs {
305+
hiddenOrgs[org] = hidden
306+
}
307+
hideStale := app.hideStaleIncoming
308+
app.mu.RUnlock()
309+
310+
// Add common menu items
311+
titles = append(titles, "Web Dashboard")
312+
313+
// Generate PR section titles
314+
if len(incoming) == 0 && len(outgoing) == 0 {
315+
titles = append(titles, "No pull requests")
316+
} else {
317+
// Add incoming PR titles
318+
if len(incoming) > 0 {
319+
titles = append(titles, "📥 Incoming PRs")
320+
titles = append(titles, app.generatePRSectionTitles(incoming, "Incoming", hiddenOrgs, hideStale)...)
321+
}
322+
323+
// Add outgoing PR titles
324+
if len(outgoing) > 0 {
325+
titles = append(titles, "📤 Outgoing PRs")
326+
titles = append(titles, app.generatePRSectionTitles(outgoing, "Outgoing", hiddenOrgs, hideStale)...)
327+
}
328+
}
329+
330+
// Add settings menu items
331+
titles = append(titles, "⚙️ Settings")
332+
titles = append(titles, "Hide Stale Incoming PRs")
333+
titles = append(titles, "Honks enabled")
334+
titles = append(titles, "Auto-open in Browser")
335+
titles = append(titles, "Hidden Organizations")
336+
titles = append(titles, "Quit")
337+
338+
return titles
339+
}
340+
341+
// generatePRSectionTitles generates the titles for a specific PR section
342+
func (app *App) generatePRSectionTitles(prs []PR, sectionTitle string, hiddenOrgs map[string]bool, hideStale bool) []string {
343+
var titles []string
344+
345+
// Sort PRs by UpdatedAt (most recent first)
346+
sortedPRs := make([]PR, len(prs))
347+
copy(sortedPRs, prs)
348+
sort.Slice(sortedPRs, func(i, j int) bool {
349+
return sortedPRs[i].UpdatedAt.After(sortedPRs[j].UpdatedAt)
350+
})
351+
352+
for prIndex := range sortedPRs {
353+
// Apply filters (same logic as in addPRSection)
354+
org := extractOrgFromRepo(sortedPRs[prIndex].Repository)
355+
if org != "" && hiddenOrgs[org] {
356+
continue
357+
}
358+
359+
if hideStale && sortedPRs[prIndex].UpdatedAt.Before(time.Now().Add(-stalePRThreshold)) {
360+
continue
361+
}
362+
363+
title := fmt.Sprintf("%s #%d", sortedPRs[prIndex].Repository, sortedPRs[prIndex].Number)
364+
365+
// Add bullet point or emoji for blocked PRs (same logic as in addPRSection)
366+
if sortedPRs[prIndex].NeedsReview || sortedPRs[prIndex].IsBlocked {
367+
prState, hasState := app.stateManager.GetPRState(sortedPRs[prIndex].URL)
368+
369+
if hasState && !prState.FirstBlockedAt.IsZero() && time.Since(prState.FirstBlockedAt) < blockedPRIconDuration {
370+
if sectionTitle == "Outgoing" {
371+
title = fmt.Sprintf("🎉 %s", title)
372+
} else {
373+
title = fmt.Sprintf("🪿 %s", title)
374+
}
375+
} else {
376+
title = fmt.Sprintf("• %s", title)
377+
}
378+
}
379+
380+
titles = append(titles, title)
381+
}
382+
383+
return titles
384+
}
385+
281386
// rebuildMenu completely rebuilds the menu from scratch.
282387
func (app *App) rebuildMenu(ctx context.Context) {
283388
// Rebuild entire menu
@@ -478,7 +583,7 @@ func (app *App) addStaticMenuItems(ctx context.Context) {
478583

479584
// Audio cues
480585
// Add 'Audio cues' option
481-
audioItem := systray.AddMenuItem("Audio cues", "Play sounds for notifications")
586+
audioItem := systray.AddMenuItem("Honks enabled", "Play sounds for notifications")
482587
app.mu.RLock()
483588
if app.enableAudioCues {
484589
audioItem.Check()

0 commit comments

Comments
 (0)