diff --git a/pkg/bot/bot.go b/pkg/bot/bot.go index 5a56e49..ce0eb6f 100644 --- a/pkg/bot/bot.go +++ b/pkg/bot/bot.go @@ -853,6 +853,78 @@ func (*Coordinator) extractBlockedUsersFromTurnclient(checkResult *turn.CheckRes return blockedUsers } +// shouldPostThread determines if a PR thread should be posted based on configured threshold. +// Returns (shouldPost bool, reason string). +func (c *Coordinator) shouldPostThread( + checkResult *turn.CheckResponse, + when string, +) (bool, string) { + if checkResult == nil { + return false, "no_check_result" + } + + pr := checkResult.PullRequest + analysis := checkResult.Analysis + + // Terminal states ALWAYS post (ensure visibility) + if pr.Merged { + return true, "pr_merged" + } + if pr.State == "closed" { + return true, "pr_closed" + } + + switch when { + case "immediate": + return true, "immediate_mode" + + case "assigned": + // Post when PR has assignees + if len(pr.Assignees) > 0 { + return true, fmt.Sprintf("has_%d_assignees", len(pr.Assignees)) + } + return false, "no_assignees" + + case "blocked": + // Post when someone needs to take action + blockedUsers := c.extractBlockedUsersFromTurnclient(checkResult) + if len(blockedUsers) > 0 { + return true, fmt.Sprintf("blocked_on_%d_users", len(blockedUsers)) + } + return false, "not_blocked_yet" + + case "passing": + // Post when tests pass - use WorkflowState as primary signal + switch analysis.WorkflowState { + case string(turn.StateAssignedWaitingForReview), + string(turn.StateReviewedNeedsRefinement), + string(turn.StateRefinedWaitingForApproval), + string(turn.StateApprovedWaitingForMerge): + return true, fmt.Sprintf("workflow_state_%s", analysis.WorkflowState) + + case string(turn.StateNewlyPublished), + string(turn.StateInDraft), + string(turn.StatePublishedWaitingForTests), + string(turn.StateTestedWaitingForAssignment): + return false, fmt.Sprintf("waiting_for_%s", analysis.WorkflowState) + + default: + // Fallback: check test status directly + if analysis.Checks.Failing > 0 { + return false, "tests_failing" + } + if analysis.Checks.Pending > 0 || analysis.Checks.Waiting > 0 { + return false, "tests_pending" + } + return true, "tests_passed_fallback" + } + + default: + slog.Warn("invalid when value, defaulting to immediate", "when", when) + return true, "invalid_config_default_immediate" + } +} + // formatNextActions formats NextAction map into a compact string like "fix tests: user1, user2; review: user3". // It groups users by action kind and formats each action as "action_name: user1, user2". // Multiple actions are separated by semicolons. @@ -1082,7 +1154,8 @@ func (c *Coordinator) processPRForChannel( oldState = threadInfo.LastState } - // Find or create thread + // Check if thread already exists + cacheKey := fmt.Sprintf("%s/%s#%d:%s", owner, repo, prNumber, channelID) pullRequestStruct := struct { CreatedAt time.Time `json:"created_at"` User struct { @@ -1098,6 +1171,36 @@ func (c *Coordinator) processPRForChannel( Number: event.PullRequest.Number, CreatedAt: event.PullRequest.CreatedAt, } + + _, _, threadExists := c.findPRThread(ctx, cacheKey, channelID, owner, repo, prNumber, prState, pullRequestStruct) + + // If thread doesn't exist, check if we should create it based on "when" threshold + if !threadExists { + when := c.configManager.When(owner, channelName) + + if when != "immediate" { + shouldPost, reason := c.shouldPostThread(checkResult, when) + + if !shouldPost { + slog.Debug("not creating thread - threshold not met", + "workspace", c.workspaceName, + logFieldPR, fmt.Sprintf(prFormatString, owner, repo, prNumber), + "channel", channelDisplay, + "when", when, + "reason", reason) + return nil // Don't create thread yet - next event will check again + } + + slog.Info("creating thread - threshold met", + "workspace", c.workspaceName, + logFieldPR, fmt.Sprintf(prFormatString, owner, repo, prNumber), + "channel", channelDisplay, + "when", when, + "reason", reason) + } + } + + // Find or create thread threadTS, wasNewlyCreated, currentText, err := c.findOrCreatePRThread(ctx, threadCreationParams{ ChannelID: channelID, ChannelName: channelName, diff --git a/pkg/bot/bot_test.go b/pkg/bot/bot_test.go index 6c27df2..c05723d 100644 --- a/pkg/bot/bot_test.go +++ b/pkg/bot/bot_test.go @@ -6,7 +6,9 @@ import ( "os" "testing" + "github.com/codeGROOVE-dev/prx/pkg/prx" "github.com/codeGROOVE-dev/slacker/pkg/bot/cache" + "github.com/codeGROOVE-dev/turnclient/pkg/turn" "github.com/slack-go/slack" ) @@ -309,3 +311,234 @@ func TestThreadCache_Set(t *testing.T) { t.Errorf("expected channel ID %s, got %s", threadInfo.ChannelID, retrieved.ChannelID) } } + +func TestShouldPostThread(t *testing.T) { + coord := &Coordinator{} + + tests := []struct { + name string + when string + checkResult *turn.CheckResponse + wantPost bool + wantReasonPart string // Part of the reason string to check for + }{ + { + name: "immediate mode always posts", + when: "immediate", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + }, + }, + wantPost: true, + wantReasonPart: "immediate_mode", + }, + { + name: "merged PR always posts regardless of when", + when: "passing", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "closed", + Merged: true, + }, + }, + wantPost: true, + wantReasonPart: "pr_merged", + }, + { + name: "closed PR always posts regardless of when", + when: "passing", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "closed", + Merged: false, + }, + }, + wantPost: true, + wantReasonPart: "pr_closed", + }, + { + name: "assigned: posts when has assignees", + when: "assigned", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + Assignees: []string{"user1", "user2"}, + }, + }, + wantPost: true, + wantReasonPart: "has_2_assignees", + }, + { + name: "assigned: does not post when no assignees", + when: "assigned", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + Assignees: []string{}, + }, + }, + wantPost: false, + wantReasonPart: "no_assignees", + }, + { + name: "blocked: posts when users are blocked", + when: "blocked", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + }, + Analysis: turn.Analysis{ + NextAction: map[string]turn.Action{ + "user1": {Kind: "review"}, + "user2": {Kind: "approve"}, + }, + }, + }, + wantPost: true, + wantReasonPart: "blocked_on_2_users", + }, + { + name: "blocked: does not post when no users blocked", + when: "blocked", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + }, + Analysis: turn.Analysis{ + NextAction: map[string]turn.Action{}, + }, + }, + wantPost: false, + wantReasonPart: "not_blocked_yet", + }, + { + name: "blocked: ignores _system sentinel", + when: "blocked", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + }, + Analysis: turn.Analysis{ + NextAction: map[string]turn.Action{ + "_system": {Kind: "processing"}, + }, + }, + }, + wantPost: false, + wantReasonPart: "not_blocked_yet", + }, + { + name: "passing: posts when in review state", + when: "passing", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + }, + Analysis: turn.Analysis{ + WorkflowState: string(turn.StateAssignedWaitingForReview), + }, + }, + wantPost: true, + wantReasonPart: "workflow_state", + }, + { + name: "passing: does not post when tests pending", + when: "passing", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + }, + Analysis: turn.Analysis{ + WorkflowState: string(turn.StatePublishedWaitingForTests), + }, + }, + wantPost: false, + wantReasonPart: "waiting_for", + }, + { + name: "passing: uses fallback when workflow state unknown and tests passing", + when: "passing", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + }, + Analysis: turn.Analysis{ + WorkflowState: "unknown_state", + Checks: turn.Checks{ + Passing: 5, + Failing: 0, + Pending: 0, + Waiting: 0, + }, + }, + }, + wantPost: true, + wantReasonPart: "tests_passed_fallback", + }, + { + name: "passing: uses fallback when tests failing", + when: "passing", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + }, + Analysis: turn.Analysis{ + WorkflowState: "unknown_state", + Checks: turn.Checks{ + Passing: 3, + Failing: 2, + }, + }, + }, + wantPost: false, + wantReasonPart: "tests_failing", + }, + { + name: "nil check result returns false", + when: "passing", + checkResult: nil, + wantPost: false, + wantReasonPart: "no_check_result", + }, + { + name: "invalid when value defaults to immediate", + when: "invalid_value", + checkResult: &turn.CheckResponse{ + PullRequest: prx.PullRequest{ + State: "open", + }, + }, + wantPost: true, + wantReasonPart: "invalid_config", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotPost, gotReason := coord.shouldPostThread(tt.checkResult, tt.when) + + if gotPost != tt.wantPost { + t.Errorf("shouldPostThread() gotPost = %v, wantPost %v", gotPost, tt.wantPost) + } + + if tt.wantReasonPart != "" && !contains(gotReason, tt.wantReasonPart) { + t.Errorf("shouldPostThread() reason = %q, want to contain %q", gotReason, tt.wantReasonPart) + } + }) + } +} + +// Helper function to check if a string contains a substring +func contains(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && (s[:len(substr)] == substr || s[len(s)-len(substr):] == substr || containsMiddle(s, substr))) +} + +func containsMiddle(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} diff --git a/pkg/bot/integration_test.go b/pkg/bot/integration_test.go index a291dfd..38e7af4 100644 --- a/pkg/bot/integration_test.go +++ b/pkg/bot/integration_test.go @@ -386,6 +386,10 @@ func (m *mockConfigManager) ReminderDMDelay(org, channel string) int { return m.dmDelay } +func (m *mockConfigManager) When(org, channel string) string { + return "immediate" // Default for tests +} + func (m *mockConfigManager) ChannelsForRepo(org, repo string) []string { if m.channelsFunc != nil { return m.channelsFunc(org, repo) diff --git a/pkg/bot/interfaces.go b/pkg/bot/interfaces.go index 9dc8ce6..76932c4 100644 --- a/pkg/bot/interfaces.go +++ b/pkg/bot/interfaces.go @@ -54,6 +54,7 @@ type ConfigManager interface { WorkspaceName(org string) string ChannelsForRepo(org, repo string) []string ReminderDMDelay(org, channelName string) int + When(org, channelName string) string SetGitHubClient(org string, client any) SetWorkspaceName(workspaceName string) } diff --git a/pkg/config/config.go b/pkg/config/config.go index 0261522..70581e5 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -42,6 +42,7 @@ type ServerConfig struct { type RepoConfig struct { Channels map[string]struct { ReminderDMDelay *int `yaml:"reminder_dm_delay"` + When *string `yaml:"when"` // Optional: when to post threads ("immediate", "assigned", "blocked", "passing") Repos []string `yaml:"repos"` // Optional: override global delay for this channel (0 = disabled) Mute bool `yaml:"mute"` @@ -51,6 +52,7 @@ type RepoConfig struct { TeamID string `yaml:"team_id"` EmailDomain string `yaml:"email_domain"` ReminderDMDelay int `yaml:"reminder_dm_delay"` + When string `yaml:"when"` // When to post threads: "immediate" (default), "assigned", "blocked", "passing" DisableDailyReport bool `yaml:"disable_daily_report"` // Default false (reports enabled) } `yaml:"global"` // Minutes to wait before sending DM if user tagged in channel (0 = disabled) } @@ -166,6 +168,7 @@ func createDefaultConfig() *RepoConfig { return &RepoConfig{ Channels: make(map[string]struct { ReminderDMDelay *int `yaml:"reminder_dm_delay"` + When *string `yaml:"when"` Repos []string `yaml:"repos"` Mute bool `yaml:"mute"` }), @@ -173,12 +176,14 @@ func createDefaultConfig() *RepoConfig { TeamID string `yaml:"team_id"` EmailDomain string `yaml:"email_domain"` ReminderDMDelay int `yaml:"reminder_dm_delay"` + When string `yaml:"when"` DisableDailyReport bool `yaml:"disable_daily_report"` }{ TeamID: "", EmailDomain: "", ReminderDMDelay: defaultReminderDMDelayMinutes, - DisableDailyReport: false, // Default: daily reports enabled + When: "immediate", // Default: post threads immediately + DisableDailyReport: false, // Default: daily reports enabled }, } } @@ -611,6 +616,32 @@ func (m *Manager) ReminderDMDelay(org, channel string) int { return defaultReminderDMDelayMinutes } +// When returns the posting threshold for a channel. +// Returns "immediate" (default), "assigned", "blocked", or "passing". +func (m *Manager) When(org, channel string) string { + m.mu.RLock() + defer m.mu.RUnlock() + + config, exists := m.configs[org] + if !exists { + return "immediate" // Default + } + + // Check for channel-specific override + if channelConfig, ok := config.Channels[channel]; ok { + if channelConfig.When != nil { + return *channelConfig.When + } + } + + // Return global setting (or default if not set) + if config.Global.When != "" { + return config.Global.When + } + + return "immediate" // Default +} + // ReloadConfig reloads the configuration for an org (e.g., when .codeGROOVE repo is updated). func (m *Manager) ReloadConfig(ctx context.Context, org string) error { slog.Info("reloading config", "org", org) diff --git a/pkg/config/config.test b/pkg/config/config.test new file mode 100755 index 0000000..6c7671e Binary files /dev/null and b/pkg/config/config.test differ diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index d7ec1a9..8914fe2 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -183,6 +183,7 @@ func TestConfigCache_GetSet(t *testing.T) { TeamID string `yaml:"team_id"` EmailDomain string `yaml:"email_domain"` ReminderDMDelay int `yaml:"reminder_dm_delay"` + When string `yaml:"when"` DisableDailyReport bool `yaml:"disable_daily_report"` }{ TeamID: "T123", @@ -386,6 +387,7 @@ func TestManager_ConfigWithManualSetup(t *testing.T) { testConfig := &RepoConfig{ Channels: map[string]struct { ReminderDMDelay *int `yaml:"reminder_dm_delay"` + When *string `yaml:"when"` Repos []string `yaml:"repos"` Mute bool `yaml:"mute"` }{ @@ -402,6 +404,7 @@ func TestManager_ConfigWithManualSetup(t *testing.T) { TeamID string `yaml:"team_id"` EmailDomain string `yaml:"email_domain"` ReminderDMDelay int `yaml:"reminder_dm_delay"` + When string `yaml:"when"` DisableDailyReport bool `yaml:"disable_daily_report"` }{ TeamID: "T123456", @@ -459,6 +462,7 @@ func TestManager_ReminderDMDelayWithChannelOverride(t *testing.T) { testConfig := &RepoConfig{ Channels: map[string]struct { ReminderDMDelay *int `yaml:"reminder_dm_delay"` + When *string `yaml:"when"` Repos []string `yaml:"repos"` Mute bool `yaml:"mute"` }{ @@ -471,6 +475,7 @@ func TestManager_ReminderDMDelayWithChannelOverride(t *testing.T) { TeamID string `yaml:"team_id"` EmailDomain string `yaml:"email_domain"` ReminderDMDelay int `yaml:"reminder_dm_delay"` + When string `yaml:"when"` DisableDailyReport bool `yaml:"disable_daily_report"` }{ ReminderDMDelay: 60, // Global default @@ -500,6 +505,7 @@ func TestManager_ChannelsForRepoWithWildcard(t *testing.T) { testConfig := &RepoConfig{ Channels: map[string]struct { ReminderDMDelay *int `yaml:"reminder_dm_delay"` + When *string `yaml:"when"` Repos []string `yaml:"repos"` Mute bool `yaml:"mute"` }{ @@ -561,6 +567,7 @@ func TestManager_ChannelsForRepoWithMuting(t *testing.T) { testConfig := &RepoConfig{ Channels: map[string]struct { ReminderDMDelay *int `yaml:"reminder_dm_delay"` + When *string `yaml:"when"` Repos []string `yaml:"repos"` Mute bool `yaml:"mute"` }{ @@ -727,6 +734,7 @@ func TestManager_IsChannelMutedCaseInsensitive(t *testing.T) { testConfig := &RepoConfig{ Channels: map[string]struct { ReminderDMDelay *int `yaml:"reminder_dm_delay"` + When *string `yaml:"when"` Repos []string `yaml:"repos"` Mute bool `yaml:"mute"` }{ @@ -759,6 +767,7 @@ func TestManager_ReminderDMDelayZeroGlobal(t *testing.T) { testConfig := &RepoConfig{ Channels: map[string]struct { ReminderDMDelay *int `yaml:"reminder_dm_delay"` + When *string `yaml:"when"` Repos []string `yaml:"repos"` Mute bool `yaml:"mute"` }{}, @@ -766,6 +775,7 @@ func TestManager_ReminderDMDelayZeroGlobal(t *testing.T) { TeamID string `yaml:"team_id"` EmailDomain string `yaml:"email_domain"` ReminderDMDelay int `yaml:"reminder_dm_delay"` + When string `yaml:"when"` DisableDailyReport bool `yaml:"disable_daily_report"` }{ ReminderDMDelay: 0, // Explicitly disabled @@ -790,6 +800,7 @@ func TestManager_ReminderDMDelayChannelZero(t *testing.T) { testConfig := &RepoConfig{ Channels: map[string]struct { ReminderDMDelay *int `yaml:"reminder_dm_delay"` + When *string `yaml:"when"` Repos []string `yaml:"repos"` Mute bool `yaml:"mute"` }{ @@ -801,6 +812,7 @@ func TestManager_ReminderDMDelayChannelZero(t *testing.T) { TeamID string `yaml:"team_id"` EmailDomain string `yaml:"email_domain"` ReminderDMDelay int `yaml:"reminder_dm_delay"` + When string `yaml:"when"` DisableDailyReport bool `yaml:"disable_daily_report"` }{ ReminderDMDelay: 60, diff --git a/pkg/slack/home_handler.go b/pkg/slack/home_handler.go index 66d7d70..da6934b 100644 --- a/pkg/slack/home_handler.go +++ b/pkg/slack/home_handler.go @@ -88,23 +88,62 @@ func (h *HomeHandler) tryHandleAppHomeOpened(ctx context.Context, teamID, slackU return h.publishPlaceholderHome(ctx, slackClient, slackUserID, []string{}, nil) } - // Get config for first org to extract domain and user overrides - cfg, exists := h.configManager.Config(workspaceOrgs[0]) - if !exists { - return fmt.Errorf("no config for org: %s", workspaceOrgs[0]) + // Collect config overrides from all workspace orgs + allOverrides := make(map[string]string) + for _, org := range workspaceOrgs { + cfg, exists := h.configManager.Config(org) + if exists && len(cfg.Users) > 0 { + for ghUser, email := range cfg.Users { + allOverrides[ghUser] = email + } + } } - - // Update reverse mapping overrides from config - if len(cfg.Users) > 0 { - h.reverseMapping.SetOverrides(cfg.Users) + if len(allOverrides) > 0 { + h.reverseMapping.SetOverrides(allOverrides) + slog.Info("loaded user mapping overrides from all orgs", + "workspace_orgs", workspaceOrgs, + "total_overrides", len(allOverrides)) } - // Map Slack user to GitHub username - mapping, err := h.reverseMapping.LookupGitHub(ctx, slackClient.API(), slackUserID, workspaceOrgs[0], cfg.Global.EmailDomain) - if err != nil { - slog.Warn("failed to map Slack user to GitHub", + // Try to map Slack user to GitHub username using all workspace orgs + var mapping *usermapping.ReverseMapping + var lastErr error + for _, org := range workspaceOrgs { + cfg, exists := h.configManager.Config(org) + if !exists { + slog.Warn("no config found for org, skipping", + "org", org, + "slack_user_id", slackUserID) + continue + } + + slog.Debug("attempting user mapping with org", + "org", org, + "slack_user_id", slackUserID, + "email_domain", cfg.Global.EmailDomain) + + m, err := h.reverseMapping.LookupGitHub(ctx, slackClient.API(), slackUserID, org, cfg.Global.EmailDomain) + if err == nil && m != nil { + slog.Info("successfully mapped user via org", + "org", org, + "slack_user_id", slackUserID, + "github_username", m.GitHubUsername, + "confidence", m.Confidence) + mapping = m + break + } + lastErr = err + slog.Debug("mapping attempt failed for org", + "org", org, "slack_user_id", slackUserID, "error", err) + } + + if mapping == nil { + slog.Warn("failed to map Slack user to GitHub in any workspace org", + "slack_user_id", slackUserID, + "workspace_orgs", workspaceOrgs, + "last_error", lastErr) return h.publishPlaceholderHome(ctx, slackClient, slackUserID, workspaceOrgs, nil) } diff --git a/pkg/slack/home_handler_multiorg_test.go b/pkg/slack/home_handler_multiorg_test.go new file mode 100644 index 0000000..a88329b --- /dev/null +++ b/pkg/slack/home_handler_multiorg_test.go @@ -0,0 +1,242 @@ +package slack + +import ( + "testing" + + "github.com/codeGROOVE-dev/slacker/pkg/config" +) + +// TestWorkspaceOrgs_MultipleOrgs verifies that workspaceOrgs correctly identifies all orgs for a workspace. +func TestWorkspaceOrgs_MultipleOrgs(t *testing.T) { + t.Parallel() + + // Create mock config manager with multiple orgs for same workspace + teamID := "T123" + configs := map[string]*config.RepoConfig{ + "org1": { + Users: map[string]string{"gh-user1": "user1@example.com"}, + }, + "org2": { + Users: map[string]string{"gh-user2": "user2@example.com"}, + }, + "org3": { + Users: map[string]string{"gh-user3": "user3@example.com"}, + }, + "org4": { // Different teamID - should be excluded + Users: map[string]string{"gh-user4": "user4@example.com"}, + }, + } + + // Set TeamIDs + configs["org1"].Global.TeamID = teamID + configs["org2"].Global.TeamID = teamID + configs["org3"].Global.TeamID = teamID + configs["org4"].Global.TeamID = "T999" // Different workspace + + // Create a mock implementation that simulates the workspaceOrgs logic + allOrgs := []string{"org1", "org2", "org3", "org4"} + var workspaceOrgs []string + + for _, org := range allOrgs { + cfg, exists := configs[org] + if !exists { + continue + } + if cfg.Global.TeamID == teamID { + workspaceOrgs = append(workspaceOrgs, org) + } + } + + // Verify all matching orgs were found + if len(workspaceOrgs) != 3 { + t.Errorf("expected 3 workspace orgs, got %d", len(workspaceOrgs)) + } + + expectedOrgs := map[string]bool{ + "org1": true, + "org2": true, + "org3": true, + } + + for _, org := range workspaceOrgs { + if !expectedOrgs[org] { + t.Errorf("unexpected org in workspace: %s", org) + } + delete(expectedOrgs, org) + } + + if len(expectedOrgs) > 0 { + t.Errorf("missing expected orgs: %v", expectedOrgs) + } +} + +// TestCollectOverrides_MultipleOrgs verifies that user overrides are collected from all workspace orgs. +func TestCollectOverrides_MultipleOrgs(t *testing.T) { + t.Parallel() + + workspaceOrgs := []string{"org1", "org2", "org3"} + + // Simulate configs for multiple orgs + configs := map[string]*config.RepoConfig{ + "org1": { + Users: map[string]string{ + "github-user-1": "user1@example.com", + "github-user-2": "user2@example.com", + }, + }, + "org2": { + Users: map[string]string{ + "github-user-3": "user3@example.com", + }, + }, + "org3": { + Users: nil, // No overrides + }, + } + + // Simulate the override collection logic from tryHandleAppHomeOpened + allOverrides := make(map[string]string) + for _, org := range workspaceOrgs { + cfg, exists := configs[org] + if exists && len(cfg.Users) > 0 { + for ghUser, email := range cfg.Users { + allOverrides[ghUser] = email + } + } + } + + // Verify all overrides were collected + expected := map[string]string{ + "github-user-1": "user1@example.com", + "github-user-2": "user2@example.com", + "github-user-3": "user3@example.com", + } + + if len(allOverrides) != len(expected) { + t.Errorf("expected %d overrides, got %d", len(expected), len(allOverrides)) + } + + for ghUser, expectedEmail := range expected { + if actualEmail, ok := allOverrides[ghUser]; !ok { + t.Errorf("missing override for %s", ghUser) + } else if actualEmail != expectedEmail { + t.Errorf("wrong email for %s: expected %s, got %s", ghUser, expectedEmail, actualEmail) + } + } +} + +// TestUserMappingFallback_MultipleOrgs verifies that user mapping attempts all orgs until one succeeds. +func TestUserMappingFallback_MultipleOrgs(t *testing.T) { + t.Parallel() + + workspaceOrgs := []string{"org1", "org2", "org3"} + + // Simulate configs for multiple orgs + configs := map[string]*config.RepoConfig{ + "org1": {Users: nil}, + "org2": {Users: nil}, + "org3": {Users: nil}, + } + configs["org1"].Global.EmailDomain = "example.com" + configs["org2"].Global.EmailDomain = "example.org" + configs["org3"].Global.EmailDomain = "example.net" + + // Simulate the user mapping loop logic + type attempt struct { + org string + domain string + } + var attempts []attempt + + // Simulate mapping attempts (fail on first two, succeed on third) + var foundMapping bool + for _, org := range workspaceOrgs { + cfg, exists := configs[org] + if !exists { + continue + } + + attempts = append(attempts, attempt{ + org: org, + domain: cfg.Global.EmailDomain, + }) + + // Simulate: fail on org1 and org2, succeed on org3 + if org == "org3" { + foundMapping = true + break + } + } + + // Verify all three orgs were attempted (loop broke on success) + if len(attempts) != 3 { + t.Errorf("expected 3 attempts, got %d", len(attempts)) + } + + // Verify the attempts were in order + expectedAttempts := []attempt{ + {"org1", "example.com"}, + {"org2", "example.org"}, + {"org3", "example.net"}, + } + + for i, expected := range expectedAttempts { + if i >= len(attempts) { + t.Errorf("missing attempt %d", i) + continue + } + actual := attempts[i] + if actual.org != expected.org || actual.domain != expected.domain { + t.Errorf("attempt %d: expected (%s, %s), got (%s, %s)", + i, expected.org, expected.domain, actual.org, actual.domain) + } + } + + if !foundMapping { + t.Error("mapping should have been found") + } +} + +// TestUserMappingEarlyExit_FirstOrgSucceeds verifies that when first org succeeds, subsequent orgs are not attempted. +func TestUserMappingEarlyExit_FirstOrgSucceeds(t *testing.T) { + t.Parallel() + + workspaceOrgs := []string{"org1", "org2", "org3"} + + // Simulate configs + configs := map[string]*config.RepoConfig{ + "org1": {Users: nil}, + "org2": {Users: nil}, + "org3": {Users: nil}, + } + configs["org1"].Global.EmailDomain = "example.com" + configs["org2"].Global.EmailDomain = "example.org" + configs["org3"].Global.EmailDomain = "example.net" + + // Simulate mapping attempts (succeed on first) + var attempts int + var foundMapping bool + for _, org := range workspaceOrgs { + _, exists := configs[org] + if !exists { + continue + } + + attempts++ + + // Simulate: succeed on first org + if org == "org1" { + foundMapping = true + break + } + } + + // Verify only one attempt was made + if attempts != 1 { + t.Errorf("expected 1 attempt, got %d (should break early on success)", attempts) + } + + if !foundMapping { + t.Error("mapping should have been found") + } +} diff --git a/pkg/slack/mock_builders_test.go b/pkg/slack/mock_builders_test.go index 97e3540..00b5e43 100644 --- a/pkg/slack/mock_builders_test.go +++ b/pkg/slack/mock_builders_test.go @@ -226,6 +226,29 @@ func (b *MockAPIBuilder) WithGetConversationsError(err error) *MockAPIBuilder { return b } +// WithPublishViewSuccess configures the mock to successfully publish views. +func (b *MockAPIBuilder) WithPublishViewSuccess() *MockAPIBuilder { + b.mock.publishViewFunc = func(ctx context.Context, request slack.PublishViewContextRequest) (*slack.ViewResponse, error) { + return &slack.ViewResponse{}, nil + } + return b +} + +// WithPublishViewError configures the mock to fail when publishing views. +func (b *MockAPIBuilder) WithPublishViewError(err error) *MockAPIBuilder { + b.mock.publishViewFunc = func(ctx context.Context, request slack.PublishViewContextRequest) (*slack.ViewResponse, error) { + return nil, err + } + return b +} + +// WithUserTimezone configures the user timezone returned by the mock. +func (b *MockAPIBuilder) WithUserTimezone(timezone string) *MockAPIBuilder { + // UserTimezone is a Client method, not an API method + // We'll need to handle this at the Client level in tests + return b +} + // Build returns the configured mockAPI. func (b *MockAPIBuilder) Build() *mockAPI { return b.mock