From 53584c462fe51f9d1c61dbdccb2b342d44fd37ed Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Sun, 9 Nov 2025 14:49:50 -0500 Subject: [PATCH] Improve dashboard styling --- pkg/home/ui.go | 223 +++++++++++++++++++++++++++----------------- pkg/home/ui_test.go | 24 ++--- 2 files changed, 150 insertions(+), 97 deletions(-) diff --git a/pkg/home/ui.go b/pkg/home/ui.go index 14b498c..947dce7 100644 --- a/pkg/home/ui.go +++ b/pkg/home/ui.go @@ -27,7 +27,7 @@ func BuildBlocks(dashboard *Dashboard, userTZ string) []slack.Block { slack.NewButtonBlockElement( "refresh_dashboard", "refresh", - slack.NewTextBlockObject("plain_text", "🔄 Refresh Dashboard", true, false), + slack.NewTextBlockObject("plain_text", "🔄 Refresh", true, false), ).WithStyle("primary"), ), ) @@ -52,15 +52,19 @@ func BuildBlocks(dashboard *Dashboard, userTZ string) []slack.Block { if len(dashboard.WorkspaceOrgs) > 0 { blocks = append(blocks, slack.NewDividerBlock()) + // Sort orgs alphabetically + sortedOrgs := make([]string, len(dashboard.WorkspaceOrgs)) + copy(sortedOrgs, dashboard.WorkspaceOrgs) + sort.Strings(sortedOrgs) + var orgLines []string - for _, org := range dashboard.WorkspaceOrgs { + for _, org := range sortedOrgs { // 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), + orgLine := fmt.Sprintf("• %s [<%s|dashboard> | <%s|config>]", org, - fmt.Sprintf("https://github.com/%s/.github/blob/main/.codeGROOVE/slack.yaml", esc), fmt.Sprintf("https://%s.ready-to-review.dev", esc), + fmt.Sprintf("https://github.com/%s/.codeGROOVE/blob/main/slack.yaml", esc), ) orgLines = append(orgLines, orgLine) } @@ -81,6 +85,49 @@ func BuildBlocks(dashboard *Dashboard, userTZ string) []slack.Block { return blocks } +// formatPRLine formats a single PR line with indicator, repo, number, title, and optional action. +func formatPRLine(pr PR, isIncoming bool) string { + // Extract repo name + repo := pr.Repository + if idx := strings.LastIndex(repo, "/"); idx >= 0 { + repo = repo[idx+1:] + } + + // Determine indicator + var indicator string + if isIncoming { + switch { + case pr.NeedsReview, pr.IsBlocked: + indicator = ":red_circle:" + case pr.ActionKind != "": + indicator = ":speech_balloon:" + default: + indicator = ":white_small_square:" + } + } else { + switch { + case pr.NeedsReview, pr.IsBlocked: + indicator = ":large_green_circle:" + case pr.ActionKind != "": + indicator = ":speech_balloon:" + default: + indicator = ":white_small_square:" + } + } + + // Build line + line := fmt.Sprintf("%s <%s|%s#%d> • %s", indicator, pr.URL, repo, pr.Number, pr.Title) + if pr.ActionKind != "" { + action := strings.ReplaceAll(pr.ActionKind, "_", " ") + // Bold the action if this PR is blocked on the user + if pr.IsBlocked || (isIncoming && pr.NeedsReview) { + action = "*" + action + "*" + } + line = fmt.Sprintf("%s — %s", line, action) + } + return line +} + // BuildPRSections creates Block Kit blocks for PR sections (incoming/outgoing). // Format inspired by goose - simple, minimal, action-focused. // This is the core formatting used by both daily reports and the dashboard. @@ -108,32 +155,7 @@ func BuildPRSections(incoming, outgoing []PR) []slack.Block { if prs[i].IsBlocked || prs[i].NeedsReview { n++ } - - // Format PR line - extract repo name - repo := prs[i].Repository - if idx := strings.LastIndex(repo, "/"); idx >= 0 { - repo = repo[idx+1:] - } - - // Determine indicator - var indicator string - switch { - case prs[i].NeedsReview: - indicator = ":red_circle:" - case prs[i].IsBlocked: - indicator = ":red_circle:" - case prs[i].ActionKind != "": - indicator = ":speech_balloon:" - default: - indicator = ":white_small_square:" - } - - // Build line - line := fmt.Sprintf("%s <%s|%s#%d> • %s", indicator, prs[i].URL, repo, prs[i].Number, prs[i].Title) - if prs[i].ActionKind != "" { - line = fmt.Sprintf("%s — %s", line, strings.ReplaceAll(prs[i].ActionKind, "_", " ")) - } - lines = append(lines, line) + lines = append(lines, formatPRLine(prs[i], true)) } // Build header @@ -154,6 +176,15 @@ func BuildPRSections(incoming, outgoing []PR) []slack.Block { ) } + // Spacer between Incoming and Outgoing sections + if len(incoming) > 0 && len(outgoing) > 0 { + blocks = append(blocks, + slack.NewContextBlock("", + slack.NewTextBlockObject("plain_text", " ", false, false), + ), + ) + } + // Outgoing PRs section if len(outgoing) > 0 { // Sort: blocked first, then by most recent within each group @@ -175,32 +206,7 @@ func BuildPRSections(incoming, outgoing []PR) []slack.Block { if prs[i].IsBlocked { n++ } - - // Format PR line - extract repo name - repo := prs[i].Repository - if idx := strings.LastIndex(repo, "/"); idx >= 0 { - repo = repo[idx+1:] - } - - // Determine indicator - var indicator string - switch { - case prs[i].NeedsReview: - indicator = ":large_green_circle:" - case prs[i].IsBlocked: - indicator = ":large_green_circle:" - case prs[i].ActionKind != "": - indicator = ":speech_balloon:" - default: - indicator = ":white_small_square:" - } - - // Build line - line := fmt.Sprintf("%s <%s|%s#%d> • %s", indicator, prs[i].URL, repo, prs[i].Number, prs[i].Title) - if prs[i].ActionKind != "" { - line = fmt.Sprintf("%s — %s", line, strings.ReplaceAll(prs[i].ActionKind, "_", " ")) - } - lines = append(lines, line) + lines = append(lines, formatPRLine(prs[i], false)) } // Build header @@ -224,23 +230,69 @@ func BuildPRSections(incoming, outgoing []PR) []slack.Block { return blocks } -// BuildBlocksWithDebug creates Slack Block Kit UI with debug information about user mapping. +// BuildBlocksWithDebug creates Slack Block Kit UI with GitHub username integrated into the header. func BuildBlocksWithDebug(dashboard *Dashboard, userTZ string, mapping *usermapping.ReverseMapping) []slack.Block { - // Build standard blocks first - blocks := BuildBlocks(dashboard, userTZ) + var blocks []slack.Block + + // Header with GitHub username if available + headerText := "🚀 Ready to Review" + if mapping != nil { + headerText = fmt.Sprintf("🚀 Ready to Review — @%s", mapping.GitHubUsername) + } + + blocks = append(blocks, + slack.NewHeaderBlock( + slack.NewTextBlockObject("plain_text", headerText, true, false), + ), + // Refresh button + slack.NewActionBlock( + "refresh_actions", + slack.NewButtonBlockElement( + "refresh_dashboard", + "refresh", + slack.NewTextBlockObject("plain_text", "🔄 Refresh", true, false), + ).WithStyle("primary"), + ), + slack.NewDividerBlock(), + ) + + // PR sections + blocks = append(blocks, BuildPRSections(dashboard.IncomingPRs, dashboard.OutgoingPRs)...) + + // Add spacing after PR sections if we have content + if len(dashboard.IncomingPRs) > 0 || len(dashboard.OutgoingPRs) > 0 { + blocks = append(blocks, + slack.NewContextBlock("", + slack.NewTextBlockObject("plain_text", " ", false, false), + ), + ) + } + + // Organizations section - only show if there are orgs configured + if len(dashboard.WorkspaceOrgs) > 0 { + blocks = append(blocks, slack.NewDividerBlock()) + + // Sort orgs alphabetically + sortedOrgs := make([]string, len(dashboard.WorkspaceOrgs)) + copy(sortedOrgs, dashboard.WorkspaceOrgs) + sort.Strings(sortedOrgs) + + var orgLines []string + for _, org := range sortedOrgs { + // URL-escape org name to prevent injection + esc := url.PathEscape(org) + orgLine := fmt.Sprintf("• %s [<%s|dashboard> | <%s|config>]", + org, + fmt.Sprintf("https://%s.ready-to-review.dev", esc), + fmt.Sprintf("https://github.com/%s/.codeGROOVE/blob/main/slack.yaml", esc), + ) + orgLines = append(orgLines, orgLine) + } - // Add debug section based on mapping status - switch { - case mapping != nil: blocks = append(blocks, - slack.NewDividerBlock(), slack.NewSectionBlock( slack.NewTextBlockObject("mrkdwn", - fmt.Sprintf("🔍 *Debug Info*\n"+ - "GitHub: `@%s` • Mapped via: `%s` • Confidence: `%d%%`", - mapping.GitHubUsername, - mapping.MatchMethod, - mapping.Confidence), + "*Organizations*\n"+strings.Join(orgLines, "\n"), false, false, ), @@ -248,22 +300,10 @@ func BuildBlocksWithDebug(dashboard *Dashboard, userTZ string, mapping *usermapp nil, ), ) + } - // Add mapping guidance if confidence is low - if mapping.Confidence < 80 { - blocks = append(blocks, - slack.NewContextBlock("", - slack.NewTextBlockObject("mrkdwn", - fmt.Sprintf("⚠️ Low confidence mapping. Add manual override to `slack.yaml`:\n```yaml\nusers:\n %s: %s\n```", - mapping.GitHubUsername, - mapping.SlackEmail), - false, - false, - ), - ), - ) - } - case len(dashboard.WorkspaceOrgs) == 0: + // Add helpful messages for error cases + if mapping == nil && len(dashboard.WorkspaceOrgs) == 0 { // No organizations configured for this workspace (likely startup/race condition) blocks = append(blocks, slack.NewDividerBlock(), @@ -282,7 +322,7 @@ func BuildBlocksWithDebug(dashboard *Dashboard, userTZ string, mapping *usermapp nil, ), ) - default: + } else if mapping == nil { // User mapping failed blocks = append(blocks, slack.NewDividerBlock(), @@ -299,6 +339,19 @@ func BuildBlocksWithDebug(dashboard *Dashboard, userTZ string, mapping *usermapp ) } + // Updated timestamp at the bottom + now := formatTimestamp(time.Now(), userTZ) + blocks = append(blocks, + slack.NewDividerBlock(), + slack.NewContextBlock("", + slack.NewTextBlockObject("mrkdwn", + fmt.Sprintf("Updated %s", now), + false, + false, + ), + ), + ) + return blocks } diff --git a/pkg/home/ui_test.go b/pkg/home/ui_test.go index 6ea1594..c5cd7eb 100644 --- a/pkg/home/ui_test.go +++ b/pkg/home/ui_test.go @@ -325,9 +325,9 @@ func TestBuildPRSections_SortOrder(t *testing.T) { blocks := BuildPRSections(incoming, outgoing) - // Should have 2 blocks (incoming and outgoing sections) - if len(blocks) != 2 { - t.Fatalf("expected 2 blocks, got %d", len(blocks)) + // Should have 3 blocks (incoming section, spacer, outgoing section) + if len(blocks) != 3 { + t.Fatalf("expected 3 blocks (incoming, spacer, outgoing), got %d", len(blocks)) } // Check incoming section order @@ -366,10 +366,10 @@ func TestBuildPRSections_SortOrder(t *testing.T) { t.Error("newer non-blocked PR#3 should appear before older non-blocked PR#1") } - // Check outgoing section order - outgoingBlock, ok := blocks[1].(*slack.SectionBlock) + // Check outgoing section order (index 2, after spacer at index 1) + outgoingBlock, ok := blocks[2].(*slack.SectionBlock) if !ok { - t.Fatal("expected second block to be SectionBlock") + t.Fatal("expected third block to be SectionBlock") } outgoingText := outgoingBlock.Text.Text @@ -457,9 +457,9 @@ func TestBuildBlocksWithDebug_NoOrgsConfigured(t *testing.T) { MatchMethod: "email_match", Confidence: 95, }, - expectText: "Debug Info", - notExpect: "Low confidence mapping", - description: "should show debug info without low confidence warning", + expectText: "Updated", + notExpect: "Could not map Slack user to GitHub", + description: "should show timestamp without error messages", }, { name: "successful mapping with low confidence", @@ -474,9 +474,9 @@ func TestBuildBlocksWithDebug_NoOrgsConfigured(t *testing.T) { MatchMethod: "name_similarity", Confidence: 65, }, - expectText: "Low confidence mapping", - notExpect: "", - description: "should show low confidence warning with suggestion", + expectText: "Updated", + notExpect: "Could not map Slack user to GitHub", + description: "should show timestamp without error messages regardless of confidence", }, }