diff --git a/pkg/home/ui.go b/pkg/home/ui.go index 268b58a..453f2b3 100644 --- a/pkg/home/ui.go +++ b/pkg/home/ui.go @@ -13,34 +13,49 @@ 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, primaryOrg string) []slack.Block { +func BuildBlocks(dashboard *Dashboard) []slack.Block { var blocks []slack.Block - // Header - gradient-inspired title + // Header blocks = append(blocks, slack.NewHeaderBlock( slack.NewTextBlockObject("plain_text", "🚀 Ready to Review", true, false), ), + // Refresh button + slack.NewActionBlock( + "refresh_actions", + slack.NewButtonBlockElement( + "refresh_dashboard", + "refresh", + slack.NewTextBlockObject("plain_text", "🔄 Refresh Dashboard", true, false), + ).WithStyle("primary"), + ), + slack.NewDividerBlock(), ) - c := dashboard.Counts() + // PR sections + blocks = append(blocks, BuildPRSections(dashboard.IncomingPRs, dashboard.OutgoingPRs)...) + + // Organizations section + blocks = append(blocks, slack.NewDividerBlock()) - // Status overview - quick summary - emoji := "✨" - status := "All clear" - if c.IncomingBlocked > 0 || c.OutgoingBlocked > 0 { - emoji = "⚡" - status = "Action needed" + 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", - fmt.Sprintf("%s *%s* • %d incoming • %d outgoing", - emoji, - status, - c.IncomingTotal, - c.OutgoingTotal), + "*Organizations*\n"+strings.Join(orgLines, "\n"), false, false, ), @@ -49,50 +64,12 @@ func BuildBlocks(dashboard *Dashboard, primaryOrg string) []slack.Block { ), ) - // Organization monitoring + last updated - links := make([]string, 0, len(dashboard.WorkspaceOrgs)) - for _, org := range dashboard.WorkspaceOrgs { - // URL-escape org name to prevent injection - esc := url.PathEscape(org) - links = append(links, fmt.Sprintf("<%s|%s>", - fmt.Sprintf("https://github.com/%s/.codeGROOVE/blob/main/slack.yaml", esc), - org)) - } + // Updated timestamp now := time.Now().Format("Jan 2, 3:04pm MST") - context := fmt.Sprintf("Monitoring: %s • Updated: %s", - strings.Join(links, ", "), - now) - - blocks = append(blocks, - slack.NewContextBlock("", - slack.NewTextBlockObject("mrkdwn", context, false, false), - ), - // Refresh button - slack.NewActionBlock( - "refresh_actions", - slack.NewButtonBlockElement( - "refresh_dashboard", - "refresh", - slack.NewTextBlockObject("plain_text", "🔄 Refresh Dashboard", true, false), - ).WithStyle("primary"), - ), - slack.NewDividerBlock(), - ) - - // Use the same clean report format for PR sections - blocks = append(blocks, BuildPRSections(dashboard.IncomingPRs, dashboard.OutgoingPRs)...) - - // Footer - full dashboard link - // URL-escape org name to prevent injection - esc := url.PathEscape(primaryOrg) blocks = append(blocks, - slack.NewDividerBlock(), slack.NewContextBlock("", slack.NewTextBlockObject("mrkdwn", - fmt.Sprintf("📊 <%s|View full dashboard at %s.ready-to-review.dev>", - fmt.Sprintf("https://%s.ready-to-review.dev", esc), - primaryOrg, - ), + fmt.Sprintf("Updated: %s", now), false, false, ), @@ -246,9 +223,9 @@ func BuildPRSections(incoming, outgoing []PR) []slack.Block { } // BuildBlocksWithDebug creates Slack Block Kit UI with debug information about user mapping. -func BuildBlocksWithDebug(dashboard *Dashboard, primaryOrg string, mapping *usermapping.ReverseMapping) []slack.Block { +func BuildBlocksWithDebug(dashboard *Dashboard, mapping *usermapping.ReverseMapping) []slack.Block { // Build standard blocks first - blocks := BuildBlocks(dashboard, primaryOrg) + blocks := BuildBlocks(dashboard) // Add debug section if mapping info is available if mapping != nil { diff --git a/pkg/home/ui_test.go b/pkg/home/ui_test.go index 3f15db7..c445a95 100644 --- a/pkg/home/ui_test.go +++ b/pkg/home/ui_test.go @@ -13,10 +13,9 @@ import ( //nolint:gocognit,maintidx // Comprehensive test with many test cases - complexity acceptable func TestBuildBlocks(t *testing.T) { tests := []struct { - name string - dashboard *Dashboard - primaryOrg string - validate func(t *testing.T, blocks []slack.Block) + name string + dashboard *Dashboard + validate func(t *testing.T, blocks []slack.Block) }{ { name: "empty dashboard", @@ -25,7 +24,6 @@ func TestBuildBlocks(t *testing.T) { IncomingPRs: []PR{}, OutgoingPRs: []PR{}, }, - primaryOrg: "test-org", validate: func(t *testing.T, blocks []slack.Block) { t.Helper() if len(blocks) == 0 { @@ -45,49 +43,60 @@ func TestBuildBlocks(t *testing.T) { t.Error("expected header block with 'Ready to Review'") } - // Should have "All clear" status - foundStatus := false + // Verify we don't have any PR section blocks (empty dashboard) + hasPRSections := false for _, block := range blocks { if sb, ok := block.(*slack.SectionBlock); ok { - if sb.Text != nil && strings.Contains(sb.Text.Text, "All clear") { - foundStatus = true + if sb.Text != nil && (strings.Contains(sb.Text.Text, "Incoming") || strings.Contains(sb.Text.Text, "Outgoing")) { + hasPRSections = true } } } - if !foundStatus { - t.Error("expected 'All clear' status for empty dashboard") + if hasPRSections { + t.Error("expected no PR sections for empty dashboard") } - // With new format, empty dashboards don't show "No incoming PRs" message - // They just show header/status/refresh with no PR sections - // Verify we don't have any PR section blocks - hasPRSections := false + // Should have Organizations section + foundOrgs := false for _, block := range blocks { if sb, ok := block.(*slack.SectionBlock); ok { - if sb.Text != nil && (strings.Contains(sb.Text.Text, "Incoming") || strings.Contains(sb.Text.Text, "Outgoing")) { - hasPRSections = true + if sb.Text != nil && strings.Contains(sb.Text.Text, "Organizations") { + foundOrgs = true } } } - if hasPRSections { - t.Error("expected no PR sections for empty dashboard") + if !foundOrgs { + t.Error("expected Organizations section") } - // Should have dashboard link + // Should have dashboard link in Organizations section foundLink := false + for _, block := range blocks { + if sb, ok := block.(*slack.SectionBlock); ok { + if sb.Text != nil && strings.Contains(sb.Text.Text, "ready-to-review.dev") { + foundLink = true + } + } + } + if !foundLink { + t.Error("expected dashboard link in Organizations section") + } + + // Should have Updated timestamp + 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, "ready-to-review.dev") { - foundLink = true + if strings.Contains(txt.Text, "Updated:") { + foundTimestamp = true } } } } } - if !foundLink { - t.Error("expected dashboard link in footer") + if !foundTimestamp { + t.Error("expected Updated timestamp") } }, }, @@ -110,23 +119,9 @@ func TestBuildBlocks(t *testing.T) { }, OutgoingPRs: []PR{}, }, - primaryOrg: "test-org", validate: func(t *testing.T, blocks []slack.Block) { t.Helper() - // Should have "Action needed" status - foundActionNeeded := false - for _, block := range blocks { - if sb, ok := block.(*slack.SectionBlock); ok { - if sb.Text != nil && strings.Contains(sb.Text.Text, "Action needed") { - foundActionNeeded = true - } - } - } - if !foundActionNeeded { - t.Error("expected 'Action needed' status with blocked PRs") - } - - // Should show "1 blocked on you" in section header (new format) + // Should show "1 blocked on you" in section header foundBlocked := false for _, block := range blocks { if sb, ok := block.(*slack.SectionBlock); ok { @@ -139,7 +134,7 @@ func TestBuildBlocks(t *testing.T) { t.Error("expected 'blocked on you' message in header") } - // Should have PR with large red square (incoming blocked indicator) + // Should have PR with red circle (incoming blocked indicator) foundBlockedPR := false for _, block := range blocks { if sb, ok := block.(*slack.SectionBlock); ok { @@ -171,7 +166,6 @@ func TestBuildBlocks(t *testing.T) { }, }, }, - primaryOrg: "test-org", validate: func(t *testing.T, blocks []slack.Block) { t.Helper() // Should show outgoing PR section with "blocked on you" (new format) @@ -212,30 +206,27 @@ func TestBuildBlocks(t *testing.T) { IncomingPRs: []PR{}, OutgoingPRs: []PR{}, }, - primaryOrg: "org1", validate: func(t *testing.T, blocks []slack.Block) { t.Helper() - // Should list all orgs in monitoring section + // Should list all orgs in Organizations section foundOrgs := 0 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, "org1") { - foundOrgs++ - } - if strings.Contains(txt.Text, "org2") { - foundOrgs++ - } - if strings.Contains(txt.Text, "org3") { - foundOrgs++ - } + if sb, ok := block.(*slack.SectionBlock); ok { + if sb.Text != nil && strings.Contains(sb.Text.Text, "Organizations") { + if strings.Contains(sb.Text.Text, "org1") { + foundOrgs++ + } + if strings.Contains(sb.Text.Text, "org2") { + foundOrgs++ + } + if strings.Contains(sb.Text.Text, "org3") { + foundOrgs++ } } } } if foundOrgs < 3 { - t.Errorf("expected all 3 orgs in monitoring section, found %d", foundOrgs) + t.Errorf("expected all 3 orgs in Organizations section, found %d", foundOrgs) } }, }, @@ -243,7 +234,7 @@ func TestBuildBlocks(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - blocks := BuildBlocks(tt.dashboard, tt.primaryOrg) + blocks := BuildBlocks(tt.dashboard) tt.validate(t, blocks) }) } @@ -257,7 +248,7 @@ func TestBuildBlocks_RefreshButton(t *testing.T) { OutgoingPRs: []PR{}, } - blocks := BuildBlocks(dashboard, "test-org") + blocks := BuildBlocks(dashboard) // Should have action block with refresh button foundRefresh := false @@ -294,7 +285,7 @@ func TestBuildBlocks_DividersBetweenSections(t *testing.T) { }, } - blocks := BuildBlocks(dashboard, "test-org") + blocks := BuildBlocks(dashboard) // Count dividers dividerCount := 0 diff --git a/pkg/slack/home_handler.go b/pkg/slack/home_handler.go index d07c095..a656ffd 100644 --- a/pkg/slack/home_handler.go +++ b/pkg/slack/home_handler.go @@ -139,8 +139,8 @@ func (h *HomeHandler) tryHandleAppHomeOpened(ctx context.Context, teamID, slackU // Add workspace orgs to dashboard for UI display dashboard.WorkspaceOrgs = workspaceOrgs - // Build Block Kit UI - use first org as primary, include debug info - blocks := home.BuildBlocksWithDebug(dashboard, workspaceOrgs[0], mapping) + // Build Block Kit UI with debug info + blocks := home.BuildBlocksWithDebug(dashboard, mapping) // Publish to Slack if err := slackClient.PublishHomeView(ctx, slackUserID, blocks); err != nil { @@ -187,7 +187,7 @@ func (*HomeHandler) publishPlaceholderHome(ctx context.Context, slackClient *Cli IncomingPRs: nil, OutgoingPRs: nil, WorkspaceOrgs: []string{"your-org"}, - }, "your-org", mapping) + }, mapping) return slackClient.PublishHomeView(ctx, slackUserID, blocks) } diff --git a/pkg/slack/slack.go b/pkg/slack/slack.go index 61ca8f4..1b30ce6 100644 --- a/pkg/slack/slack.go +++ b/pkg/slack/slack.go @@ -1040,6 +1040,9 @@ func (c *Client) handleBlockAction(ctx context.Context, interaction *slack.Inter } // handleRefreshDashboard handles the refresh dashboard button action. +// It runs the dashboard fetch asynchronously to avoid Slack's 3-second interaction timeout. +// +//nolint:unparam // ctx intentionally unused - we use context.Background() in goroutine func (c *Client) handleRefreshDashboard(ctx context.Context, interaction *slack.InteractionCallback) { c.homeViewHandlerMu.RLock() handler := c.homeViewHandler @@ -1051,26 +1054,33 @@ func (c *Client) handleRefreshDashboard(ctx context.Context, interaction *slack. return } - handlerCtx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - - slog.Info("refreshing dashboard via button click", + slog.Info("refreshing dashboard via button click (async)", "team_id", interaction.Team.ID, "user_id", interaction.User.ID, "trigger", "refresh_button") - if err := handler(handlerCtx, interaction.Team.ID, interaction.User.ID); err != nil { - slog.Error("failed to refresh dashboard", - "team_id", interaction.Team.ID, - "user_id", interaction.User.ID, - "trigger", "refresh_button", - "error", err) - } else { - slog.Info("successfully refreshed dashboard", - "team_id", interaction.Team.ID, - "user_id", interaction.User.ID, - "trigger", "refresh_button") - } + // Run dashboard fetch asynchronously to avoid blocking the HTTP response. + // Slack requires interactions to be acknowledged within 3 seconds. + // We use context.Background() because the HTTP request context will be cancelled + // after we return, but we want the dashboard fetch to continue. + //nolint:contextcheck // Intentionally using Background() - HTTP context cancelled after return + go func() { + handlerCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + if err := handler(handlerCtx, interaction.Team.ID, interaction.User.ID); err != nil { + slog.Error("failed to refresh dashboard", + "team_id", interaction.Team.ID, + "user_id", interaction.User.ID, + "trigger", "refresh_button", + "error", err) + } else { + slog.Info("successfully refreshed dashboard", + "team_id", interaction.Team.ID, + "user_id", interaction.User.ID, + "trigger", "refresh_button") + } + }() } // SlashCommandHandler handles Slack slash commands.