Simplify DM handling

Author: Thomas Stromberg

pkg/bot/bot.go

@@ -25,7 +25,7 @@ func TestMain(m *testing.M) {
 
 	// Cleanup (before os.Exit to avoid exitAfterDefer lint error)
 	mockServer.Close()
-	_ = os.Unsetenv("TURN_TEST_BACKEND") // Best effort cleanup
+	_ = os.Unsetenv("TURN_TEST_BACKEND") //nolint:errcheck // Best effort cleanup
 
 	os.Exit(code)
 }

pkg/bot/bot_test.go

@@ -9,17 +9,23 @@ import (
 
 	"github.com/codeGROOVE-dev/slacker/pkg/bot/cache"
 	"github.com/codeGROOVE-dev/slacker/pkg/github"
+	slackapi "github.com/codeGROOVE-dev/slacker/pkg/slack"
 	"github.com/codeGROOVE-dev/slacker/pkg/state"
 	"github.com/slack-go/slack"
 )
 
 // mockStateStore implements StateStore interface from bot package.
+//
+//nolint:govet // fieldalignment optimization would reduce test readability
 type mockStateStore struct {
 	markProcessedErr  error
 	saveThreadErr     error
+	saveDMMessageErr  error
 	threads           map[string]cache.ThreadInfo
 	dmTimes           map[string]time.Time
 	dmUsers           map[string][]string
+	dmMessages        map[string]state.DMInfo
+	pendingDMs        []*state.PendingDM
 	processedEvents   map[string]bool
 	lastNotifications map[string]time.Time
 	mu                sync.Mutex
@@ -85,6 +91,48 @@ func (m *mockStateStore) ListDMUsers(ctx context.Context, prURL string) []string
 	return []string{}
 }
 
+// DMMessage returns DM message info for a user and PR.
+func (m *mockStateStore) DMMessage(ctx context.Context, userID, prURL string) (state.DMInfo, bool) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	key := userID + ":" + prURL
+	if m.dmMessages != nil {
+		if info, ok := m.dmMessages[key]; ok {
+			return info, true
+		}
+	}
+	return state.DMInfo{}, false
+}
+
+// SaveDMMessage saves DM message info for a user and PR.
+func (m *mockStateStore) SaveDMMessage(ctx context.Context, userID, prURL string, info state.DMInfo) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.saveDMMessageErr != nil {
+		return m.saveDMMessageErr
+	}
+	key := userID + ":" + prURL
+	if m.dmMessages == nil {
+		m.dmMessages = make(map[string]state.DMInfo)
+	}
+	m.dmMessages[key] = info
+	// Also track this user for ListDMUsers
+	if m.dmUsers == nil {
+		m.dmUsers = make(map[string][]string)
+	}
+	found := false
+	for _, u := range m.dmUsers[prURL] {
+		if u == userID {
+			found = true
+			break
+		}
+	}
+	if !found {
+		m.dmUsers[prURL] = append(m.dmUsers[prURL], userID)
+	}
+	return nil
+}
+
 func (m *mockStateStore) WasProcessed(ctx context.Context, eventKey string) bool {
 	m.mu.Lock()
 	defer m.mu.Unlock()
@@ -129,16 +177,35 @@ func (m *mockStateStore) RecordNotification(ctx context.Context, prURL string, n
 }
 
 // QueuePendingDM implements notify.Store interface for DM queue management.
-func (*mockStateStore) QueuePendingDM(_ *state.PendingDM) error {
-	return nil // No-op for tests
+func (m *mockStateStore) QueuePendingDM(ctx context.Context, dm *state.PendingDM) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	m.pendingDMs = append(m.pendingDMs, dm)
+	return nil
 }
 
-func (*mockStateStore) PendingDMs(_ time.Time) ([]state.PendingDM, error) {
-	return nil, nil // Return empty list for tests
+func (m *mockStateStore) PendingDMs(ctx context.Context, before time.Time) ([]state.PendingDM, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	var result []state.PendingDM
+	for _, dm := range m.pendingDMs {
+		if dm.SendAfter.Before(before) {
+			result = append(result, *dm)
+		}
+	}
+	return result, nil
 }
 
-func (*mockStateStore) RemovePendingDM(_ string) error {
-	return nil // No-op for tests
+func (m *mockStateStore) RemovePendingDM(ctx context.Context, id string) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	for i, dm := range m.pendingDMs {
+		if dm.ID == id {
+			m.pendingDMs = append(m.pendingDMs[:i], m.pendingDMs[i+1:]...)
+			break
+		}
+	}
+	return nil
 }
 
 func (*mockStateStore) Close() error {
@@ -149,26 +216,30 @@ func (*mockStateStore) Close() error {
 //
 //nolint:govet // fieldalignment optimization would reduce test readability
 type mockSlackClient struct {
-	mu                  sync.Mutex
-	postThreadFunc      func(ctx context.Context, channelID, text string, attachments []slack.Attachment) (string, error)
-	updateMessageFunc   func(ctx context.Context, channelID, timestamp, text string) error
-	updateDMMessageFunc func(ctx context.Context, userID, timestamp, text string) error
-	channelHistoryFunc  func(ctx context.Context, channelID string, oldest, latest string, limit int) (*slack.GetConversationHistoryResponse, error)
-	resolveChannelFunc  func(ctx context.Context, channelName string) string
-	botInChannelFunc    func(ctx context.Context, channelID string) bool
-	botInfoFunc         func(ctx context.Context) (*slack.AuthTestResponse, error)
-	workspaceInfoFunc   func(ctx context.Context) (*slack.TeamInfo, error)
-	publishHomeFunc     func(ctx context.Context, userID string, blocks []slack.Block) error
-	apiFunc             func() *slack.Client
+	mu                    sync.Mutex
+	postThreadFunc        func(ctx context.Context, channelID, text string, attachments []slack.Attachment) (string, error)
+	updateMessageFunc     func(ctx context.Context, channelID, timestamp, text string) error
+	updateDMMessageFunc   func(ctx context.Context, userID, timestamp, text string) error
+	sendDirectMessageFunc func(ctx context.Context, userID, text string) (dmChannelID, messageTS string, err error)
+	isUserInChannelFunc   func(ctx context.Context, channelID, userID string) bool
+	findDMMessagesFunc    func(ctx context.Context, userID, prURL string, since time.Time) ([]slackapi.DMLocation, error)
+	channelHistoryFunc    func(ctx context.Context, channelID string, oldest, latest string, limit int) (*slack.GetConversationHistoryResponse, error)
+	resolveChannelFunc    func(ctx context.Context, channelName string) string
+	botInChannelFunc      func(ctx context.Context, channelID string) bool
+	botInfoFunc           func(ctx context.Context) (*slack.AuthTestResponse, error)
+	workspaceInfoFunc     func(ctx context.Context) (*slack.TeamInfo, error)
+	publishHomeFunc       func(ctx context.Context, userID string, blocks []slack.Block) error
+	apiFunc               func() *slack.Client
 
 	// For direct workspace info control
 	workspaceInfo    *slack.TeamInfo
 	workspaceInfoErr bool
 
 	// Tracking for test assertions
-	postedMessages   []mockPostedMessage
-	updatedMessages  []mockUpdatedMessage
-	updatedDMMessage []mockUpdatedDMMessage
+	postedMessages     []mockPostedMessage
+	updatedMessages    []mockUpdatedMessage
+	updatedDMMessage   []mockUpdatedDMMessage
+	sentDirectMessages []mockSentDirectMessage
 }
 
 type mockPostedMessage struct {
@@ -189,6 +260,11 @@ type mockUpdatedDMMessage struct {
 	Text   string
 }
 
+type mockSentDirectMessage struct {
+	UserID string
+	Text   string
+}
+
 func (m *mockSlackClient) PostThread(ctx context.Context, channelID, text string, attachments []slack.Attachment) (string, error) {
 	m.mu.Lock()
 	m.postedMessages = append(m.postedMessages, mockPostedMessage{
@@ -287,6 +363,37 @@ func (m *mockSlackClient) API() *slack.Client {
 	return nil
 }
 
+// SendDirectMessage sends a DM to a user.
+func (m *mockSlackClient) SendDirectMessage(ctx context.Context, userID, text string) (dmChannelID, messageTS string, err error) {
+	m.mu.Lock()
+	m.sentDirectMessages = append(m.sentDirectMessages, mockSentDirectMessage{
+		UserID: userID,
+		Text:   text,
+	})
+	m.mu.Unlock()
+	if m.sendDirectMessageFunc != nil {
+		return m.sendDirectMessageFunc(ctx, userID, text)
+	}
+	return "D" + userID, "1234567890.123456", nil
+}
+
+// IsUserInChannel checks if a user is in a channel.
+func (m *mockSlackClient) IsUserInChannel(ctx context.Context, channelID, userID string) bool {
+	if m.isUserInChannelFunc != nil {
+		return m.isUserInChannelFunc(ctx, channelID, userID)
+	}
+	return false
+}
+
+// FindDMMessagesInHistory searches DM history for messages containing a PR URL.
+func (m *mockSlackClient) FindDMMessagesInHistory(ctx context.Context, userID, prURL string, since time.Time) ([]slackapi.DMLocation, error) {
+	if m.findDMMessagesFunc != nil {
+		return m.findDMMessagesFunc(ctx, userID, prURL, since)
+	}
+	// Default: return empty (no DMs found in history)
+	return nil, nil
+}
+
 // mockUserMapper is a simple mock for user mapping in tests.
 type mockUserMapper struct {
 	slackHandleFunc func(ctx context.Context, githubUser, org, domain string) (string, error)