proxy,slack: Reply in threads

mock_internal_test.go

@@ -8,13 +8,15 @@ import (
 type testProxy struct {
 	C            chan bot.Message
 	SendFunc     func(bot.Message) error
+	ReplyFunc    func(bot.Message) error
 	ReactFunc    func(bot.Message) error
 	SetTopicFunc func(room, topic string) error
 }
 
 func newTestProxy() *testProxy {
 	return &testProxy{
 		SendFunc:     func(bot.Message) error { return nil },
+		ReplyFunc:    func(bot.Message) error { return nil },
 		ReactFunc:    func(bot.Message) error { return nil },
 		SetTopicFunc: func(string, string) error { return nil },
 	}
@@ -23,6 +25,7 @@ func newTestProxy() *testProxy {
 func (p *testProxy) Connect() chan bot.Message         { return p.C }
 func (p *testProxy) Disconnect()                       {}
 func (p *testProxy) Send(m bot.Message) error          { return p.SendFunc(m) }
+func (p *testProxy) Reply(m bot.Message) error         { return p.ReplyFunc(m) }
 func (p *testProxy) React(m bot.Message) error         { return p.ReactFunc(m) }
 func (p *testProxy) SetTopic(room, topic string) error { return p.SetTopicFunc(room, topic) }
 

proxy.go

@@ -41,6 +41,23 @@ func (p *proxy) onConnect(ev *slack.ConnectedEvent) {
 	}
 }
 
+func (p *proxy) Reply(m bot.Message) error {
+	// Force special messages to use "Send" and skip if there is no
+	// message it's responding to.
+	if m.Params != nil || m.Envelope == nil {
+		return p.Send(m)
+	}
+
+	msg := m.Envelope.(slack.Message)
+	threadTs := msg.ThreadTimestamp
+	if threadTs == "" {
+		threadTs = msg.Timestamp
+	}
+
+	p.RTM.SendMessage(p.RTM.NewOutgoingMessage(m.Text, m.Room, slack.RTMsgOptionTS(threadTs)))
+	return nil
+}
+
 func (p *proxy) Send(m bot.Message) error {
 	if m.Params == nil {
 		p.RTM.SendMessage(p.RTM.NewOutgoingMessage(m.Text, m.Room))

slack.go

@@ -21,6 +21,7 @@ type Adapter struct {
 		Connect() chan bot.Message
 		Disconnect()
 		Send(bot.Message) error
+		Reply(bot.Message) error
 		React(bot.Message) error
 		SetTopic(room, topic string) error
 	}
@@ -120,12 +121,12 @@ func (a *Adapter) Reply(m bot.Message) error {
 		return errors.New("No room provided")
 	}
 
-	// No need to @ the user if it's a DM
-	if m.Room[0] != 'D' {
-		m.Text = "<@" + m.User + "> " + m.Text
+	// No need to thread the response in a DM.
+	if m.Room[0] == 'D' {
+		return a.proxy.Send(m)
 	}
 
-	return a.proxy.Send(m)
+	return a.proxy.Reply(m)
 }
 
 // Topic uses the web API to change the topic. It prefers

slack_test.go

@@ -163,25 +163,30 @@ func TestReply(t *testing.T) {
 	envelope.User = user
 
 	cases := []struct {
-		In  bot.Message
-		Out bot.Message
-		Err bool
+		Name string
+		In   bot.Message
+		Out  bot.Message
+		Err  bool
 	}{
 		{
-			In:  bot.Message{Room: "D4321", Text: "foo", Envelope: envelope},
-			Out: bot.Message{User: user, Room: "D4321", Text: "foo", Envelope: envelope},
+			Name: "When DMing",
+			In:   bot.Message{Room: "D4321", Text: "foo", Envelope: envelope},
+			Out:  bot.Message{User: user, Room: "D4321", Text: "foo", Envelope: envelope},
 		},
 		{
-			In:  bot.Message{Room: "general", Text: "foo", Envelope: envelope},
-			Out: bot.Message{User: user, Room: "C1234", Text: "<@U1234> foo", Envelope: envelope},
+			Name: "When replying in a channel",
+			In:   bot.Message{Room: "general", Text: "foo", Envelope: envelope},
+			Out:  bot.Message{User: user, Room: "C1234", Text: "foo", Envelope: envelope},
 		},
 		{
-			In:  bot.Message{User: "Jane", Room: "general", Text: "foo"},
-			Err: true,
+			Name: "Given an invalid user",
+			In:   bot.Message{User: "Jane", Room: "general", Text: "foo"},
+			Err:  true,
 		},
 		{
-			In:  bot.Message{User: user, Text: "foo"},
-			Err: true,
+			Name: "Given an invalid room",
+			In:   bot.Message{User: user, Text: "foo"},
+			Err:  true,
 		},
 	}
 	store := newTestStore()
@@ -191,16 +196,21 @@ func TestReply(t *testing.T) {
 	store.User.Name = "Jean"
 
 	for _, c := range cases {
-		proxy, run := setUpProxySend(t, c.Out)
-		adapter := Adapter{Store: store, proxy: proxy, BotID: user}
-		err := adapter.Reply(c.In)
-		if c.Err {
-			assert.NotNil(t, err)
-			assert.False(t, *run)
-		} else {
-			assert.Nil(t, err)
-			assert.True(t, *run)
-		}
+		t.Run(c.Name, func(t *testing.T) {
+			proxy, run := setUpProxyReply(t, c.Out)
+			if c.Name == "When DMing" {
+				proxy, run = setUpProxySend(t, c.Out)
+			}
+			adapter := Adapter{Store: store, proxy: proxy, BotID: user}
+			err := adapter.Reply(c.In)
+			if c.Err {
+				assert.NotNil(t, err)
+				assert.False(t, *run)
+			} else {
+				assert.Nil(t, err)
+				assert.True(t, *run)
+			}
+		})
 	}
 }
 
@@ -263,3 +273,15 @@ func setUpProxySend(t *testing.T, out bot.Message) (*testProxy, *bool) {
 
 	return proxy, &run
 }
+
+func setUpProxyReply(t *testing.T, out bot.Message) (*testProxy, *bool) {
+	var run bool
+	proxy := newTestProxy()
+	proxy.ReplyFunc = func(m bot.Message) error {
+		assert.Equal(t, out, m)
+		run = true
+		return nil
+	}
+
+	return proxy, &run
+}