Skip to content
Merged

lint #82

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cmd/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,8 +693,9 @@ func runBotCoordinators(
lastHealthCheck: time.Now(),
}

// Start initial coordinators
cm.startCoordinators(ctx)
// Discover GitHub installations and start coordinators immediately at startup
slog.Info("discovering GitHub installations at startup")
cm.handleRefreshInstallations(ctx)

// Refresh installations every 5 minutes
installationTicker := time.NewTicker(5 * time.Minute)
Expand Down
121 changes: 80 additions & 41 deletions pkg/home/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

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

// Header
Expand All @@ -30,51 +30,53 @@ func BuildBlocks(dashboard *Dashboard) []slack.Block {
slack.NewTextBlockObject("plain_text", "🔄 Refresh Dashboard", true, false),
).WithStyle("primary"),
),
slack.NewDividerBlock(),
)

// PR sections
blocks = append(blocks, BuildPRSections(dashboard.IncomingPRs, dashboard.OutgoingPRs)...)

// Organizations section
blocks = append(blocks, slack.NewDividerBlock())

var orgLines []string
for _, org := range dashboard.WorkspaceOrgs {
// URL-escape org name to prevent injection
esc := url.PathEscape(org)
orgLine := fmt.Sprintf("• <%s|%s> [<%s|config>, <%s|dashboard>]",
fmt.Sprintf("https://github.com/%s", esc),
org,
fmt.Sprintf("https://github.com/%s/.github/blob/main/.codeGROOVE/slack.yaml", esc),
fmt.Sprintf("https://%s.ready-to-review.dev", esc),
)
orgLines = append(orgLines, orgLine)
}

// Updated timestamp - right after refresh button
now := formatTimestamp(time.Now(), userTZ)
blocks = append(blocks,
slack.NewSectionBlock(
slack.NewContextBlock("",
slack.NewTextBlockObject("mrkdwn",
"*Organizations*\n"+strings.Join(orgLines, "\n"),
fmt.Sprintf("Updated %s", now),
false,
false,
),
nil,
nil,
),
slack.NewDividerBlock(),
)

// Updated timestamp
now := time.Now().Format("Jan 2, 3:04pm MST")
blocks = append(blocks,
slack.NewContextBlock("",
slack.NewTextBlockObject("mrkdwn",
fmt.Sprintf("Updated: %s", now),
false,
false,
// PR sections
blocks = append(blocks, BuildPRSections(dashboard.IncomingPRs, dashboard.OutgoingPRs)...)

// Organizations section - only show if there are orgs configured
if len(dashboard.WorkspaceOrgs) > 0 {
blocks = append(blocks, slack.NewDividerBlock())

var orgLines []string
for _, org := range dashboard.WorkspaceOrgs {
// URL-escape org name to prevent injection
esc := url.PathEscape(org)
orgLine := fmt.Sprintf("• <%s|%s> [<%s|config>, <%s|dashboard>]",
fmt.Sprintf("https://github.com/%s", esc),
org,
fmt.Sprintf("https://github.com/%s/.github/blob/main/.codeGROOVE/slack.yaml", esc),
fmt.Sprintf("https://%s.ready-to-review.dev", esc),
)
orgLines = append(orgLines, orgLine)
}

blocks = append(blocks,
slack.NewSectionBlock(
slack.NewTextBlockObject("mrkdwn",
"*Organizations*\n"+strings.Join(orgLines, "\n"),
false,
false,
),
nil,
nil,
),
),
)
)
}

return blocks
}
Expand Down Expand Up @@ -223,12 +225,13 @@ func BuildPRSections(incoming, outgoing []PR) []slack.Block {
}

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

// Add debug section if mapping info is available
if mapping != nil {
// Add debug section based on mapping status
switch {
case mapping != nil:
blocks = append(blocks,
slack.NewDividerBlock(),
slack.NewSectionBlock(
Expand Down Expand Up @@ -260,8 +263,27 @@ func BuildBlocksWithDebug(dashboard *Dashboard, mapping *usermapping.ReverseMapp
),
)
}
} else {
// No mapping found - show error message
case len(dashboard.WorkspaceOrgs) == 0:
// No organizations configured for this workspace (likely startup/race condition)
blocks = append(blocks,
slack.NewDividerBlock(),
slack.NewSectionBlock(
slack.NewTextBlockObject("mrkdwn",
"⏳ *Setting up workspace...*\n"+
"No organizations configured yet. This usually resolves automatically.\n\n"+
"If this persists:\n"+
"1. Ensure your GitHub App is installed for your organization\n"+
"2. Check that `.codeGROOVE/slack.yaml` exists in your org's `.github` repo\n"+
"3. Verify the `slack:` field matches this workspace's domain",
false,
false,
),
nil,
nil,
),
)
default:
// User mapping failed
blocks = append(blocks,
slack.NewDividerBlock(),
slack.NewSectionBlock(
Expand All @@ -279,3 +301,20 @@ func BuildBlocksWithDebug(dashboard *Dashboard, mapping *usermapping.ReverseMapp

return blocks
}

// formatTimestamp formats a timestamp in the user's timezone without the colon after "Updated".
// Example: "Nov 6, 12:48am America/Los_Angeles".
func formatTimestamp(t time.Time, tzName string) string {
// Load the timezone
loc, err := time.LoadLocation(tzName)
if err != nil {
// Fallback to UTC if timezone is invalid
loc = time.UTC
}

// Convert to user's timezone
t = t.In(loc)

// Format as "Jan 2, 3:04pm MST"
return t.Format("Jan 2, 3:04pm MST")
}
117 changes: 112 additions & 5 deletions pkg/home/ui_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"
"time"

"github.com/codeGROOVE-dev/slacker/pkg/usermapping"
"github.com/slack-go/slack"
)

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

// Should have Updated timestamp
// Should have Updated timestamp (no colon after "Updated")
foundTimestamp := false
for _, block := range blocks {
if cb, ok := block.(*slack.ContextBlock); ok {
for _, elem := range cb.ContextElements.Elements {
if txt, ok := elem.(*slack.TextBlockObject); ok {
if strings.Contains(txt.Text, "Updated:") {
if strings.Contains(txt.Text, "Updated ") {
foundTimestamp = true
}
}
Expand Down Expand Up @@ -234,7 +235,7 @@ func TestBuildBlocks(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
blocks := BuildBlocks(tt.dashboard)
blocks := BuildBlocks(tt.dashboard, "America/Los_Angeles")
tt.validate(t, blocks)
})
}
Expand All @@ -248,7 +249,7 @@ func TestBuildBlocks_RefreshButton(t *testing.T) {
OutgoingPRs: []PR{},
}

blocks := BuildBlocks(dashboard)
blocks := BuildBlocks(dashboard, "America/Los_Angeles")

// Should have action block with refresh button
foundRefresh := false
Expand Down Expand Up @@ -285,7 +286,7 @@ func TestBuildBlocks_DividersBetweenSections(t *testing.T) {
},
}

blocks := BuildBlocks(dashboard)
blocks := BuildBlocks(dashboard, "America/Los_Angeles")

// Count dividers
dividerCount := 0
Expand Down Expand Up @@ -408,3 +409,109 @@ func TestBuildPRSections_SortOrder(t *testing.T) {
t.Error("outgoing blocked PRs should use :large_green_circle:")
}
}

// TestBuildBlocksWithDebug_NoOrgsConfigured verifies improved handling for startup race conditions.
func TestBuildBlocksWithDebug_NoOrgsConfigured(t *testing.T) {
tests := []struct {
name string
dashboard *Dashboard
mapping *usermapping.ReverseMapping
expectText string
notExpect string
description string
}{
{
name: "no orgs configured - startup race condition",
dashboard: &Dashboard{
WorkspaceOrgs: []string{}, // Empty - no orgs found yet
IncomingPRs: []PR{},
OutgoingPRs: []PR{},
},
mapping: nil,
expectText: "Setting up workspace",
notExpect: "Could not map Slack user to GitHub",
description: "should show startup message, not user mapping error",
},
{
name: "has orgs but user mapping failed",
dashboard: &Dashboard{
WorkspaceOrgs: []string{"test-org", "another-org"},
IncomingPRs: []PR{},
OutgoingPRs: []PR{},
},
mapping: nil,
expectText: "Could not map Slack user to GitHub",
notExpect: "Setting up workspace",
description: "should show user mapping error, not startup message",
},
{
name: "successful mapping with good confidence",
dashboard: &Dashboard{
WorkspaceOrgs: []string{"test-org"},
IncomingPRs: []PR{},
OutgoingPRs: []PR{},
},
mapping: &usermapping.ReverseMapping{
GitHubUsername: "testuser",
SlackEmail: "test@example.com",
MatchMethod: "email_match",
Confidence: 95,
},
expectText: "Debug Info",
notExpect: "Low confidence mapping",
description: "should show debug info without low confidence warning",
},
{
name: "successful mapping with low confidence",
dashboard: &Dashboard{
WorkspaceOrgs: []string{"test-org"},
IncomingPRs: []PR{},
OutgoingPRs: []PR{},
},
mapping: &usermapping.ReverseMapping{
GitHubUsername: "testuser",
SlackEmail: "test@example.com",
MatchMethod: "name_similarity",
Confidence: 65,
},
expectText: "Low confidence mapping",
notExpect: "",
description: "should show low confidence warning with suggestion",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
blocks := BuildBlocksWithDebug(tt.dashboard, "America/Los_Angeles", tt.mapping)

// Convert all blocks to text for searching
var allText string
for _, block := range blocks {
if sb, ok := block.(*slack.SectionBlock); ok {
if sb.Text != nil {
allText += sb.Text.Text + "\n"
}
}
if cb, ok := block.(*slack.ContextBlock); ok {
for _, elem := range cb.ContextElements.Elements {
if txt, ok := elem.(*slack.TextBlockObject); ok {
allText += txt.Text + "\n"
}
}
}
}

// Check expected text is present
if tt.expectText != "" && !strings.Contains(allText, tt.expectText) {
t.Errorf("%s: expected to find %q in output, but didn't.\nGot: %s",
tt.description, tt.expectText, allText)
}

// Check unwanted text is not present
if tt.notExpect != "" && strings.Contains(allText, tt.notExpect) {
t.Errorf("%s: should not contain %q, but found it.\nGot: %s",
tt.description, tt.notExpect, allText)
}
})
}
}
Loading
Loading