Merge pull request #374 from nyaruka/better_telegram_keyboards

Smarter organization of quick replies into rows and columns for telegram keyboards

handlers/telegram/keyboard.go

@@ -0,0 +1,32 @@
+package telegram
+
+import "github.com/nyaruka/courier/utils"
+
+// KeyboardButton is button on a keyboard, see https://core.telegram.org/bots/api/#keyboardbutton
+type KeyboardButton struct {
+	Text            string `json:"text"`
+	RequestContact  bool   `json:"request_contact,omitempty"`
+	RequestLocation bool   `json:"request_location,omitempty"`
+}
+
+// ReplyKeyboardMarkup models a keyboard, see https://core.telegram.org/bots/api/#replykeyboardmarkup
+type ReplyKeyboardMarkup struct {
+	Keyboard        [][]KeyboardButton `json:"keyboard"`
+	ResizeKeyboard  bool               `json:"resize_keyboard"`
+	OneTimeKeyboard bool               `json:"one_time_keyboard"`
+}
+
+// NewKeyboardFromReplies creates a keyboard from the given quick replies
+func NewKeyboardFromReplies(replies []string) *ReplyKeyboardMarkup {
+	rows := utils.StringsToRows(replies, 5, 30, 2)
+	keyboard := make([][]KeyboardButton, len(rows))
+
+	for i := range rows {
+		keyboard[i] = make([]KeyboardButton, len(rows[i]))
+		for j := range rows[i] {
+			keyboard[i][j].Text = rows[i][j]
+		}
+	}
+
+	return &ReplyKeyboardMarkup{Keyboard: keyboard, ResizeKeyboard: true, OneTimeKeyboard: true}
+}

handlers/telegram/keyboard_test.go

@@ -0,0 +1,62 @@
+package telegram_test
+
+import (
+	"testing"
+
+	"github.com/nyaruka/courier/handlers/telegram"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestKeyboardFromReplies(t *testing.T) {
+	tcs := []struct {
+		replies  []string
+		expected *telegram.ReplyKeyboardMarkup
+	}{
+		{
+
+			[]string{"OK"},
+			&telegram.ReplyKeyboardMarkup{
+				[][]telegram.KeyboardButton{
+					{{Text: "OK"}},
+				},
+				true, true,
+			},
+		},
+		{
+			[]string{"Yes", "No", "Maybe"},
+			&telegram.ReplyKeyboardMarkup{
+				[][]telegram.KeyboardButton{
+					{{Text: "Yes"}, {Text: "No"}, {Text: "Maybe"}},
+				},
+				true, true,
+			},
+		},
+		{
+			[]string{"Vanilla", "Chocolate", "Mint", "Lemon Sorbet", "Papaya", "Strawberry"},
+			&telegram.ReplyKeyboardMarkup{
+				[][]telegram.KeyboardButton{
+					{{Text: "Vanilla"}, {Text: "Chocolate"}},
+					{{Text: "Mint"}, {Text: "Lemon Sorbet"}},
+					{{Text: "Papaya"}, {Text: "Strawberry"}},
+				},
+				true, true,
+			},
+		},
+		{
+			[]string{"A", "B", "C", "D", "Chicken", "Fish", "Peanut Butter Pickle"},
+			&telegram.ReplyKeyboardMarkup{
+				[][]telegram.KeyboardButton{
+					{{Text: "A"}, {Text: "B"}, {Text: "C"}, {Text: "D"}},
+					{{Text: "Chicken"}, {Text: "Fish"}},
+					{{Text: "Peanut Butter Pickle"}},
+				},
+				true, true,
+			},
+		},
+	}
+
+	for _, tc := range tcs {
+		kb := telegram.NewKeyboardFromReplies(tc.replies)
+		assert.Equal(t, tc.expected, kb, "keyboard mismatch for replies %v", tc.replies)
+	}
+}

handlers/telegram/telegram.go

@@ -2,7 +2,6 @@ package telegram
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -15,6 +14,7 @@ import (
 	"github.com/nyaruka/courier"
 	"github.com/nyaruka/courier/handlers"
 	"github.com/nyaruka/courier/utils"
+	"github.com/nyaruka/gocommon/jsonx"
 	"github.com/nyaruka/gocommon/urns"
 )
 
@@ -131,12 +131,12 @@ func (h *handler) receiveMessage(ctx context.Context, channel courier.Channel, w
 	return handlers.WriteMsgsAndResponse(ctx, h, []courier.Msg{msg}, w, r)
 }
 
-func (h *handler) sendMsgPart(msg courier.Msg, token string, path string, form url.Values, replies string) (string, *courier.ChannelLog, error) {
-	// either include or remove our keyboard depending on whether we have quick replies
-	if replies == "" {
+func (h *handler) sendMsgPart(msg courier.Msg, token string, path string, form url.Values, keyboard *ReplyKeyboardMarkup) (string, *courier.ChannelLog, error) {
+	// either include or remove our keyboard
+	if keyboard == nil {
 		form.Add("reply_markup", `{"remove_keyboard":true}`)
 	} else {
-		form.Add("reply_markup", replies)
+		form.Add("reply_markup", string(jsonx.MustMarshal(keyboard)))
 	}
 
 	sendURL := fmt.Sprintf("%s/bot%s/%s", apiURL, token, path)
@@ -188,20 +188,9 @@ func (h *handler) SendMsg(ctx context.Context, msg courier.Msg) (courier.MsgStat
 
 	// figure out whether we have a keyboard to send as well
 	qrs := msg.QuickReplies()
-	replies := ""
-
+	var keyboard *ReplyKeyboardMarkup
 	if len(qrs) > 0 {
-		keys := make([]moKey, len(qrs))
-		for i, qr := range qrs {
-			keys[i].Text = qr
-		}
-
-		tk := moKeyboard{true, true, [][]moKey{keys}}
-		replyBytes, err := json.Marshal(tk)
-		if err != nil {
-			return nil, err
-		}
-		replies = string(replyBytes)
+		keyboard = NewKeyboardFromReplies(qrs)
 	}
 
 	// if we have text, send that if we aren't sending it as a caption
@@ -211,13 +200,13 @@ func (h *handler) SendMsg(ctx context.Context, msg courier.Msg) (courier.MsgStat
 			"text":    []string{msg.Text()},
 		}
 
-		externalID, log, err := h.sendMsgPart(msg, authToken, "sendMessage", form, replies)
+		externalID, log, err := h.sendMsgPart(msg, authToken, "sendMessage", form, keyboard)
 		status.SetExternalID(externalID)
 		hasError = err != nil
 		status.AddLog(log)
 
-		// clear our replies, they've been sent
-		replies = ""
+		// clear our keyboard which has now been sent
+		keyboard = nil
 	}
 
 	// send each attachment
@@ -230,7 +219,7 @@ func (h *handler) SendMsg(ctx context.Context, msg courier.Msg) (courier.MsgStat
 				"photo":   []string{mediaURL},
 				"caption": []string{caption},
 			}
-			externalID, log, err := h.sendMsgPart(msg, authToken, "sendPhoto", form, replies)
+			externalID, log, err := h.sendMsgPart(msg, authToken, "sendPhoto", form, keyboard)
 			status.SetExternalID(externalID)
 			hasError = err != nil
 			status.AddLog(log)
@@ -241,7 +230,7 @@ func (h *handler) SendMsg(ctx context.Context, msg courier.Msg) (courier.MsgStat
 				"video":   []string{mediaURL},
 				"caption": []string{caption},
 			}
-			externalID, log, err := h.sendMsgPart(msg, authToken, "sendVideo", form, replies)
+			externalID, log, err := h.sendMsgPart(msg, authToken, "sendVideo", form, keyboard)
 			status.SetExternalID(externalID)
 			hasError = err != nil
 			status.AddLog(log)
@@ -252,7 +241,7 @@ func (h *handler) SendMsg(ctx context.Context, msg courier.Msg) (courier.MsgStat
 				"audio":   []string{mediaURL},
 				"caption": []string{caption},
 			}
-			externalID, log, err := h.sendMsgPart(msg, authToken, "sendAudio", form, replies)
+			externalID, log, err := h.sendMsgPart(msg, authToken, "sendAudio", form, keyboard)
 			status.SetExternalID(externalID)
 			hasError = err != nil
 			status.AddLog(log)
@@ -263,7 +252,7 @@ func (h *handler) SendMsg(ctx context.Context, msg courier.Msg) (courier.MsgStat
 				"document": []string{mediaURL},
 				"caption":  []string{caption},
 			}
-			externalID, log, err := h.sendMsgPart(msg, authToken, "sendDocument", form, replies)
+			externalID, log, err := h.sendMsgPart(msg, authToken, "sendDocument", form, keyboard)
 			status.SetExternalID(externalID)
 			hasError = err != nil
 			status.AddLog(log)
@@ -275,8 +264,8 @@ func (h *handler) SendMsg(ctx context.Context, msg courier.Msg) (courier.MsgStat
 
 		}
 
-		// clear our replies, we only send it on the first message
-		replies = ""
+		// clear our keyboard, we only send it on the first message
+		keyboard = nil
 	}
 
 	if !hasError {
@@ -332,16 +321,6 @@ func (h *handler) resolveFileID(ctx context.Context, channel courier.Channel, fi
 	return fmt.Sprintf("%s/file/bot%s/%s", apiURL, authToken, filePath), nil
 }
 
-type moKeyboard struct {
-	ResizeKeyboard  bool      `json:"resize_keyboard"`
-	OneTimeKeyboard bool      `json:"one_time_keyboard"`
-	Keyboard        [][]moKey `json:"keyboard"`
-}
-
-type moKey struct {
-	Text string `json:"text"`
-}
-
 type moFile struct {
 	FileID   string `json:"file_id"    validate:"required"`
 	FileSize int    `json:"file_size"`

handlers/telegram/telegram_test.go

@@ -581,7 +581,7 @@ var defaultSendTestCases = []ChannelSendTestCase{
 		PostParams: map[string]string{
 			"text":         "Are you happy?",
 			"chat_id":      "12345",
-			"reply_markup": `{"resize_keyboard":true,"one_time_keyboard":true,"keyboard":[[{"text":"Yes"},{"text":"No"}]]}`,
+			"reply_markup": `{"keyboard":[[{"text":"Yes"},{"text":"No"}]],"resize_keyboard":true,"one_time_keyboard":true}`,
 		},
 		SendPrep: setSendURL},
 	{Label: "Unicode Send",

utils/misc.go

@@ -109,3 +109,43 @@ func BasePathForURL(rawURL string) (string, error) {
 	}
 	return path.Base(parsedURL.Path), nil
 }
+
+// StringsToRows takes a slice of strings and re-organizes it into rows and columns
+func StringsToRows(strs []string, maxRows, maxRowRunes, paddingRunes int) [][]string {
+	// calculate rune length if it's all one row
+	totalRunes := 0
+	for i := range strs {
+		totalRunes += utf8.RuneCountInString(strs[i]) + paddingRunes*2
+	}
+
+	if totalRunes <= maxRowRunes {
+		// if all strings fit on a single row, do that
+		return [][]string{strs}
+	} else if len(strs) <= maxRows {
+		// if each string can be a row, do that
+		rows := make([][]string, len(strs))
+		for i := range strs {
+			rows[i] = []string{strs[i]}
+		}
+		return rows
+	}
+
+	rows := [][]string{{}}
+	curRow := 0
+	rowRunes := 0
+
+	for _, str := range strs {
+		strRunes := utf8.RuneCountInString(str) + paddingRunes*2
+
+		// take a new row if we can't fit this string and the current row isn't empty and we haven't hit the row limit
+		if rowRunes+strRunes > maxRowRunes && len(rows[curRow]) > 0 && len(rows) < maxRows {
+			rows = append(rows, []string{})
+			curRow += 1
+			rowRunes = 0
+		}
+
+		rows[curRow] = append(rows[curRow], str)
+		rowRunes += strRunes
+	}
+	return rows
+}