Skip to content

Commit 81c13a6

Browse files
tstrombergtstromberg
authored
Merge pull request #82 from codeGROOVE-dev/reliable
lint
2 parents 59f45cb + dd0180e commit 81c13a6

File tree

4 files changed

+226
-55
lines changed

4 files changed

+226
-55
lines changed

‎cmd/server/main.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -693,8 +693,9 @@ func runBotCoordinators(
693693
lastHealthCheck: time.Now(),
694694
}
695695

696-
// Start initial coordinators
697-
cm.startCoordinators(ctx)
696+
// Discover GitHub installations and start coordinators immediately at startup
697+
slog.Info("discovering GitHub installations at startup")
698+
cm.handleRefreshInstallations(ctx)
698699

699700
// Refresh installations every 5 minutes
700701
installationTicker := time.NewTicker(5 * time.Minute)

‎pkg/home/ui.go‎

Lines changed: 80 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313

1414
// BuildBlocks creates Slack Block Kit UI for the home dashboard.
1515
// Design matches dashboard at https://ready-to-review.dev - modern minimal with indigo accents.
16-
func BuildBlocks(dashboard *Dashboard) []slack.Block {
16+
func BuildBlocks(dashboard *Dashboard, userTZ string) []slack.Block {
1717
var blocks []slack.Block
1818

1919
// Header
@@ -30,51 +30,53 @@ func BuildBlocks(dashboard *Dashboard) []slack.Block {
3030
slack.NewTextBlockObject("plain_text", "🔄 Refresh Dashboard", true, false),
3131
).WithStyle("primary"),
3232
),
33-
slack.NewDividerBlock(),
3433
)
3534

36-
// PR sections
37-
blocks = append(blocks, BuildPRSections(dashboard.IncomingPRs, dashboard.OutgoingPRs)...)
38-
39-
// Organizations section
40-
blocks = append(blocks, slack.NewDividerBlock())
41-
42-
var orgLines []string
43-
for _, org := range dashboard.WorkspaceOrgs {
44-
// URL-escape org name to prevent injection
45-
esc := url.PathEscape(org)
46-
orgLine := fmt.Sprintf("• <%s|%s> [<%s|config>, <%s|dashboard>]",
47-
fmt.Sprintf("https://github.com/%s", esc),
48-
org,
49-
fmt.Sprintf("https://github.com/%s/.github/blob/main/.codeGROOVE/slack.yaml", esc),
50-
fmt.Sprintf("https://%s.ready-to-review.dev", esc),
51-
)
52-
orgLines = append(orgLines, orgLine)
53-
}
54-
35+
// Updated timestamp - right after refresh button
36+
now := formatTimestamp(time.Now(), userTZ)
5537
blocks = append(blocks,
56-
slack.NewSectionBlock(
38+
slack.NewContextBlock("",
5739
slack.NewTextBlockObject("mrkdwn",
58-
"*Organizations*\n"+strings.Join(orgLines, "\n"),
40+
fmt.Sprintf("Updated %s", now),
5941
false,
6042
false,
6143
),
62-
nil,
63-
nil,
6444
),
45+
slack.NewDividerBlock(),
6546
)
6647

67-
// Updated timestamp
68-
now := time.Now().Format("Jan 2, 3:04pm MST")
69-
blocks = append(blocks,
70-
slack.NewContextBlock("",
71-
slack.NewTextBlockObject("mrkdwn",
72-
fmt.Sprintf("Updated: %s", now),
73-
false,
74-
false,
48+
// PR sections
49+
blocks = append(blocks, BuildPRSections(dashboard.IncomingPRs, dashboard.OutgoingPRs)...)
50+
51+
// Organizations section - only show if there are orgs configured
52+
if len(dashboard.WorkspaceOrgs) > 0 {
53+
blocks = append(blocks, slack.NewDividerBlock())
54+
55+
var orgLines []string
56+
for _, org := range dashboard.WorkspaceOrgs {
57+
// URL-escape org name to prevent injection
58+
esc := url.PathEscape(org)
59+
orgLine := fmt.Sprintf("• <%s|%s> [<%s|config>, <%s|dashboard>]",
60+
fmt.Sprintf("https://github.com/%s", esc),
61+
org,
62+
fmt.Sprintf("https://github.com/%s/.github/blob/main/.codeGROOVE/slack.yaml", esc),
63+
fmt.Sprintf("https://%s.ready-to-review.dev", esc),
64+
)
65+
orgLines = append(orgLines, orgLine)
66+
}
67+
68+
blocks = append(blocks,
69+
slack.NewSectionBlock(
70+
slack.NewTextBlockObject("mrkdwn",
71+
"*Organizations*\n"+strings.Join(orgLines, "\n"),
72+
false,
73+
false,
74+
),
75+
nil,
76+
nil,
7577
),
76-
),
77-
)
78+
)
79+
}
7880

7981
return blocks
8082
}
@@ -223,12 +225,13 @@ func BuildPRSections(incoming, outgoing []PR) []slack.Block {
223225
}
224226

225227
// BuildBlocksWithDebug creates Slack Block Kit UI with debug information about user mapping.
226-
func BuildBlocksWithDebug(dashboard *Dashboard, mapping *usermapping.ReverseMapping) []slack.Block {
228+
func BuildBlocksWithDebug(dashboard *Dashboard, userTZ string, mapping *usermapping.ReverseMapping) []slack.Block {
227229
// Build standard blocks first
228-
blocks := BuildBlocks(dashboard)
230+
blocks := BuildBlocks(dashboard, userTZ)
229231

230-
// Add debug section if mapping info is available
231-
if mapping != nil {
232+
// Add debug section based on mapping status
233+
switch {
234+
case mapping != nil:
232235
blocks = append(blocks,
233236
slack.NewDividerBlock(),
234237
slack.NewSectionBlock(
@@ -260,8 +263,27 @@ func BuildBlocksWithDebug(dashboard *Dashboard, mapping *usermapping.ReverseMapp
260263
),
261264
)
262265
}
263-
} else {
264-
// No mapping found - show error message
266+
case len(dashboard.WorkspaceOrgs) == 0:
267+
// No organizations configured for this workspace (likely startup/race condition)
268+
blocks = append(blocks,
269+
slack.NewDividerBlock(),
270+
slack.NewSectionBlock(
271+
slack.NewTextBlockObject("mrkdwn",
272+
"⏳ *Setting up workspace...*\n"+
273+
"No organizations configured yet. This usually resolves automatically.\n\n"+
274+
"If this persists:\n"+
275+
"1. Ensure your GitHub App is installed for your organization\n"+
276+
"2. Check that `.codeGROOVE/slack.yaml` exists in your org's `.github` repo\n"+
277+
"3. Verify the `slack:` field matches this workspace's domain",
278+
false,
279+
false,
280+
),
281+
nil,
282+
nil,
283+
),
284+
)
285+
default:
286+
// User mapping failed
265287
blocks = append(blocks,
266288
slack.NewDividerBlock(),
267289
slack.NewSectionBlock(
@@ -279,3 +301,20 @@ func BuildBlocksWithDebug(dashboard *Dashboard, mapping *usermapping.ReverseMapp
279301

280302
return blocks
281303
}
304+
305+
// formatTimestamp formats a timestamp in the user's timezone without the colon after "Updated".
306+
// Example: "Nov 6, 12:48am America/Los_Angeles".
307+
func formatTimestamp(t time.Time, tzName string) string {
308+
// Load the timezone
309+
loc, err := time.LoadLocation(tzName)
310+
if err != nil {
311+
// Fallback to UTC if timezone is invalid
312+
loc = time.UTC
313+
}
314+
315+
// Convert to user's timezone
316+
t = t.In(loc)
317+
318+
// Format as "Jan 2, 3:04pm MST"
319+
return t.Format("Jan 2, 3:04pm MST")
320+
}

‎pkg/home/ui_test.go‎

Lines changed: 112 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66
"time"
77

8+
"github.com/codeGROOVE-dev/slacker/pkg/usermapping"
89
"github.com/slack-go/slack"
910
)
1011

@@ -82,13 +83,13 @@ func TestBuildBlocks(t *testing.T) {
8283
t.Error("expected dashboard link in Organizations section")
8384
}
8485

85-
// Should have Updated timestamp
86+
// Should have Updated timestamp (no colon after "Updated")
8687
foundTimestamp := false
8788
for _, block := range blocks {
8889
if cb, ok := block.(*slack.ContextBlock); ok {
8990
for _, elem := range cb.ContextElements.Elements {
9091
if txt, ok := elem.(*slack.TextBlockObject); ok {
91-
if strings.Contains(txt.Text, "Updated:") {
92+
if strings.Contains(txt.Text, "Updated ") {
9293
foundTimestamp = true
9394
}
9495
}
@@ -234,7 +235,7 @@ func TestBuildBlocks(t *testing.T) {
234235

235236
for _, tt := range tests {
236237
t.Run(tt.name, func(t *testing.T) {
237-
blocks := BuildBlocks(tt.dashboard)
238+
blocks := BuildBlocks(tt.dashboard, "America/Los_Angeles")
238239
tt.validate(t, blocks)
239240
})
240241
}
@@ -248,7 +249,7 @@ func TestBuildBlocks_RefreshButton(t *testing.T) {
248249
OutgoingPRs: []PR{},
249250
}
250251

251-
blocks := BuildBlocks(dashboard)
252+
blocks := BuildBlocks(dashboard, "America/Los_Angeles")
252253

253254
// Should have action block with refresh button
254255
foundRefresh := false
@@ -285,7 +286,7 @@ func TestBuildBlocks_DividersBetweenSections(t *testing.T) {
285286
},
286287
}
287288

288-
blocks := BuildBlocks(dashboard)
289+
blocks := BuildBlocks(dashboard, "America/Los_Angeles")
289290

290291
// Count dividers
291292
dividerCount := 0
@@ -408,3 +409,109 @@ func TestBuildPRSections_SortOrder(t *testing.T) {
408409
t.Error("outgoing blocked PRs should use :large_green_circle:")
409410
}
410411
}
412+
413+
// TestBuildBlocksWithDebug_NoOrgsConfigured verifies improved handling for startup race conditions.
414+
func TestBuildBlocksWithDebug_NoOrgsConfigured(t *testing.T) {
415+
tests := []struct {
416+
name string
417+
dashboard *Dashboard
418+
mapping *usermapping.ReverseMapping
419+
expectText string
420+
notExpect string
421+
description string
422+
}{
423+
{
424+
name: "no orgs configured - startup race condition",
425+
dashboard: &Dashboard{
426+
WorkspaceOrgs: []string{}, // Empty - no orgs found yet
427+
IncomingPRs: []PR{},
428+
OutgoingPRs: []PR{},
429+
},
430+
mapping: nil,
431+
expectText: "Setting up workspace",
432+
notExpect: "Could not map Slack user to GitHub",
433+
description: "should show startup message, not user mapping error",
434+
},
435+
{
436+
name: "has orgs but user mapping failed",
437+
dashboard: &Dashboard{
438+
WorkspaceOrgs: []string{"test-org", "another-org"},
439+
IncomingPRs: []PR{},
440+
OutgoingPRs: []PR{},
441+
},
442+
mapping: nil,
443+
expectText: "Could not map Slack user to GitHub",
444+
notExpect: "Setting up workspace",
445+
description: "should show user mapping error, not startup message",
446+
},
447+
{
448+
name: "successful mapping with good confidence",
449+
dashboard: &Dashboard{
450+
WorkspaceOrgs: []string{"test-org"},
451+
IncomingPRs: []PR{},
452+
OutgoingPRs: []PR{},
453+
},
454+
mapping: &usermapping.ReverseMapping{
455+
GitHubUsername: "testuser",
456+
SlackEmail: "test@example.com",
457+
MatchMethod: "email_match",
458+
Confidence: 95,
459+
},
460+
expectText: "Debug Info",
461+
notExpect: "Low confidence mapping",
462+
description: "should show debug info without low confidence warning",
463+
},
464+
{
465+
name: "successful mapping with low confidence",
466+
dashboard: &Dashboard{
467+
WorkspaceOrgs: []string{"test-org"},
468+
IncomingPRs: []PR{},
469+
OutgoingPRs: []PR{},
470+
},
471+
mapping: &usermapping.ReverseMapping{
472+
GitHubUsername: "testuser",
473+
SlackEmail: "test@example.com",
474+
MatchMethod: "name_similarity",
475+
Confidence: 65,
476+
},
477+
expectText: "Low confidence mapping",
478+
notExpect: "",
479+
description: "should show low confidence warning with suggestion",
480+
},
481+
}
482+
483+
for _, tt := range tests {
484+
t.Run(tt.name, func(t *testing.T) {
485+
blocks := BuildBlocksWithDebug(tt.dashboard, "America/Los_Angeles", tt.mapping)
486+
487+
// Convert all blocks to text for searching
488+
var allText string
489+
for _, block := range blocks {
490+
if sb, ok := block.(*slack.SectionBlock); ok {
491+
if sb.Text != nil {
492+
allText += sb.Text.Text + "\n"
493+
}
494+
}
495+
if cb, ok := block.(*slack.ContextBlock); ok {
496+
for _, elem := range cb.ContextElements.Elements {
497+
if txt, ok := elem.(*slack.TextBlockObject); ok {
498+
allText += txt.Text + "\n"
499+
}
500+
}
501+
}
502+
}
503+
504+
// Check expected text is present
505+
if tt.expectText != "" && !strings.Contains(allText, tt.expectText) {
506+
t.Errorf("%s: expected to find %q in output, but didn't.\nGot: %s",
507+
tt.description, tt.expectText, allText)
508+
}
509+
510+
// Check unwanted text is not present
511+
if tt.notExpect != "" && strings.Contains(allText, tt.notExpect) {
512+
t.Errorf("%s: should not contain %q, but found it.\nGot: %s",
513+
tt.description, tt.notExpect, allText)
514+
}
515+
})
516+
}
517+
}

0 commit comments

Comments
 (0)