Merge pull request 'Fixed bot verbosity' (#656) from tg-655 into main

Reviewed-on: #656

api/gateway/tgsettle/config.yml

@@ -54,4 +54,6 @@ treasury:
     allowed_chats: []
     users: 
       - telegram_user_id: "8273799472"
-      - ledger_account: "6972c738949b91ea0395e5fb"
+        ledger_account: "6972c738949b91ea0395e5fb"
+      - telegram_user_id: "8273507566"
+        ledger_account: "6995d6c118bca1d8baa5f2be"

api/gateway/tgsettle/internal/service/gateway/service.go

@@ -188,7 +188,30 @@ func (s *Service) startTreasuryModule() {
 		s.logger.Warn("Treasury module disabled: discovery registry is unavailable")
 		return
 	}
-	if len(s.cfg.Treasury.Telegram.Users) == 0 {
+	configuredUsers := s.cfg.Treasury.Telegram.Users
+	if len(configuredUsers) == 0 {
+		return
+	}
+	users := make([]treasurysvc.UserBinding, 0, len(configuredUsers))
+	configuredUserIDs := make([]string, 0, len(configuredUsers))
+	for _, binding := range configuredUsers {
+		userID := strings.TrimSpace(binding.TelegramUserID)
+		accountID := strings.TrimSpace(binding.LedgerAccount)
+		if userID != "" {
+			configuredUserIDs = append(configuredUserIDs, userID)
+		}
+		if userID == "" || accountID == "" {
+			continue
+		}
+		users = append(users, treasurysvc.UserBinding{
+			TelegramUserID: userID,
+			LedgerAccount:  accountID,
+		})
+	}
+	if len(users) == 0 {
+		s.logger.Warn("Treasury module disabled: no valid treasury.telegram.users bindings",
+			zap.Int("configured_bindings", len(configuredUsers)),
+			zap.Strings("configured_user_ids", configuredUserIDs))
 		return
 	}
 
@@ -215,14 +238,6 @@ func (s *Service) startTreasuryModule() {
 		pollInterval = defaultTreasuryPollInterval
 	}
 
-	users := make([]treasurysvc.UserBinding, 0, len(s.cfg.Treasury.Telegram.Users))
-	for _, binding := range s.cfg.Treasury.Telegram.Users {
-		users = append(users, treasurysvc.UserBinding{
-			TelegramUserID: binding.TelegramUserID,
-			LedgerAccount:  binding.LedgerAccount,
-		})
-	}
-
 	module, err := treasurysvc.NewModule(
 		s.logger,
 		s.repo.TreasuryRequests(),

api/gateway/tgsettle/internal/service/treasury/bot/commands.go

@@ -0,0 +1,58 @@
+package bot
+
+import "strings"
+
+type Command string
+
+const (
+	CommandStart    Command = "start"
+	CommandFund     Command = "fund"
+	CommandWithdraw Command = "withdraw"
+	CommandConfirm  Command = "confirm"
+	CommandCancel   Command = "cancel"
+)
+
+var supportedCommands = []Command{
+	CommandStart,
+	CommandFund,
+	CommandWithdraw,
+	CommandConfirm,
+	CommandCancel,
+}
+
+func (c Command) Slash() string {
+	name := strings.TrimSpace(string(c))
+	if name == "" {
+		return ""
+	}
+	return "/" + name
+}
+
+func parseCommand(text string) Command {
+	text = strings.TrimSpace(text)
+	if !strings.HasPrefix(text, "/") {
+		return ""
+	}
+	token := text
+	if idx := strings.IndexAny(token, " \t\n\r"); idx >= 0 {
+		token = token[:idx]
+	}
+	token = strings.TrimPrefix(token, "/")
+	if idx := strings.Index(token, "@"); idx >= 0 {
+		token = token[:idx]
+	}
+	return Command(strings.ToLower(strings.TrimSpace(token)))
+}
+
+func supportedCommandsMessage() string {
+	lines := make([]string, 0, len(supportedCommands)+1)
+	lines = append(lines, "Supported commands:")
+	for _, cmd := range supportedCommands {
+		lines = append(lines, cmd.Slash())
+	}
+	return strings.Join(lines, "\n")
+}
+
+func confirmationCommandsMessage() string {
+	return "Confirm operation?\n\n" + CommandConfirm.Slash() + "\n" + CommandCancel.Slash()
+}

api/gateway/tgsettle/internal/service/treasury/bot/router.go

@@ -15,7 +15,9 @@ import (
 )
 
 const unauthorizedMessage = "Sorry, your Telegram account is not authorized to perform treasury operations."
-const welcomeMessage = "Welcome to tgsettle treasury bot.\n\nUse /fund to credit your account and /withdraw to debit it.\nAfter entering an amount, use /confirm or /cancel."
+const unauthorizedChatMessage = "Sorry, this Telegram chat is not authorized to perform treasury operations."
+
+var welcomeMessage = "Welcome to tgsettle treasury bot.\n\nUse " + CommandFund.Slash() + " to credit your account and " + CommandWithdraw.Slash() + " to debit it.\nAfter entering an amount, use " + CommandConfirm.Slash() + " or " + CommandCancel.Slash() + "."
 
 type SendTextFunc func(ctx context.Context, chatID string, text string) error
 
@@ -119,6 +121,8 @@ func (r *Router) HandleUpdate(ctx context.Context, update *model.TelegramWebhook
 	}
 	if !r.allowAnyChat {
 		if _, ok := r.allowedChats[chatID]; !ok {
+			r.logUnauthorized(update)
+			_ = r.sendText(ctx, chatID, unauthorizedChatMessage)
 			return true
 		}
 	}
@@ -132,19 +136,19 @@ func (r *Router) HandleUpdate(ctx context.Context, update *model.TelegramWebhook
 
 	command := parseCommand(text)
 	switch command {
-	case "start":
+	case CommandStart:
 		_ = r.sendText(ctx, chatID, welcomeMessage)
 		return true
-	case "fund":
+	case CommandFund:
 		r.startAmountDialog(ctx, userID, accountID, chatID, storagemodel.TreasuryOperationFund)
 		return true
-	case "withdraw":
+	case CommandWithdraw:
 		r.startAmountDialog(ctx, userID, accountID, chatID, storagemodel.TreasuryOperationWithdraw)
 		return true
-	case "confirm":
+	case CommandConfirm:
 		r.confirm(ctx, userID, accountID, chatID)
 		return true
-	case "cancel":
+	case CommandCancel:
 		r.cancel(ctx, userID, accountID, chatID)
 		return true
 	}
@@ -156,13 +160,20 @@ func (r *Router) HandleUpdate(ctx context.Context, update *model.TelegramWebhook
 			r.captureAmount(ctx, userID, accountID, chatID, session.OperationType, text)
 			return true
 		case DialogStateWaitingConfirmation:
-			_ = r.sendText(ctx, chatID, "Confirm operation?\n\n/confirm\n/cancel")
+			_ = r.sendText(ctx, chatID, confirmationCommandsMessage())
 			return true
 		}
 	}
 
 	if strings.HasPrefix(text, "/") {
-		_ = r.sendText(ctx, chatID, "Supported commands:\n/start\n/fund\n/withdraw\n/confirm\n/cancel")
+		_ = r.sendText(ctx, chatID, supportedCommandsMessage())
+		return true
+	}
+	if strings.TrimSpace(message.ReplyToMessageID) != "" {
+		return false
+	}
+	if text != "" {
+		_ = r.sendText(ctx, chatID, supportedCommandsMessage())
 		return true
 	}
 	return false
@@ -212,22 +223,22 @@ func (r *Router) captureAmount(ctx context.Context, userID, accountID, chatID st
 		if typed, ok := err.(limitError); ok {
 			switch typed.LimitKind() {
 			case "per_operation":
-				_ = r.sendText(ctx, chatID, "Amount exceeds allowed limit.\n\nMax per operation: "+typed.LimitMax()+"\n\nEnter another amount or /cancel")
+				_ = r.sendText(ctx, chatID, "Amount exceeds allowed limit.\n\nMax per operation: "+typed.LimitMax()+"\n\nEnter another amount or "+CommandCancel.Slash())
 				return
 			case "daily":
-				_ = r.sendText(ctx, chatID, "Daily amount limit exceeded.\n\nMax per day: "+typed.LimitMax()+"\n\nEnter another amount or /cancel")
+				_ = r.sendText(ctx, chatID, "Daily amount limit exceeded.\n\nMax per day: "+typed.LimitMax()+"\n\nEnter another amount or "+CommandCancel.Slash())
 				return
 			}
 		}
 		if errors.Is(err, merrors.ErrInvalidArg) {
-			_ = r.sendText(ctx, chatID, "Invalid amount.\n\nEnter another amount or /cancel")
+			_ = r.sendText(ctx, chatID, "Invalid amount.\n\nEnter another amount or "+CommandCancel.Slash())
 			return
 		}
-		_ = r.sendText(ctx, chatID, "Failed to create treasury request.\n\nEnter another amount or /cancel")
+		_ = r.sendText(ctx, chatID, "Failed to create treasury request.\n\nEnter another amount or "+CommandCancel.Slash())
 		return
 	}
 	if record == nil {
-		_ = r.sendText(ctx, chatID, "Failed to create treasury request.\n\nEnter another amount or /cancel")
+		_ = r.sendText(ctx, chatID, "Failed to create treasury request.\n\nEnter another amount or "+CommandCancel.Slash())
 		return
 	}
 	r.dialogs.Set(userID, DialogSession{
@@ -254,7 +265,7 @@ func (r *Router) confirm(ctx context.Context, userID string, accountID string, c
 	}
 	record, err := r.service.ConfirmRequest(ctx, requestID, userID)
 	if err != nil {
-		_ = r.sendText(ctx, chatID, "Unable to confirm treasury request.\n\nUse /cancel or create a new request with /fund or /withdraw.")
+		_ = r.sendText(ctx, chatID, "Unable to confirm treasury request.\n\nUse "+CommandCancel.Slash()+" or create a new request with "+CommandFund.Slash()+" or "+CommandWithdraw.Slash()+".")
 		return
 	}
 	if r.tracker != nil {
@@ -323,18 +334,18 @@ func (r *Router) logUnauthorized(update *model.TelegramWebhookUpdate) {
 
 func pendingRequestMessage(record *storagemodel.TreasuryRequest) string {
 	if record == nil {
-		return "You already have a pending treasury operation.\n\n/cancel"
+		return "You already have a pending treasury operation.\n\n" + CommandCancel.Slash()
 	}
 	return "You already have a pending treasury operation.\n\n" +
 		"Request ID: " + strings.TrimSpace(record.RequestID) + "\n" +
 		"Status: " + strings.TrimSpace(string(record.Status)) + "\n" +
 		"Amount: " + strings.TrimSpace(record.Amount) + " " + strings.TrimSpace(record.Currency) + "\n\n" +
-		"Wait for execution or cancel it.\n\n/cancel"
+		"Wait for execution or cancel it.\n\n" + CommandCancel.Slash()
 }
 
 func confirmationPrompt(record *storagemodel.TreasuryRequest) string {
 	if record == nil {
-		return "Request created.\n\n/confirm\n/cancel"
+		return "Request created.\n\n" + CommandConfirm.Slash() + "\n" + CommandCancel.Slash()
 	}
 	title := "Funding request created."
 	if record.OperationType == storagemodel.TreasuryOperationWithdraw {
@@ -343,23 +354,7 @@ func confirmationPrompt(record *storagemodel.TreasuryRequest) string {
 	return title + "\n\n" +
 		"Account: " + strings.TrimSpace(record.LedgerAccountID) + "\n" +
 		"Amount: " + strings.TrimSpace(record.Amount) + " " + strings.TrimSpace(record.Currency) + "\n\n" +
-		"Confirm operation?\n\n/confirm\n/cancel"
+		confirmationCommandsMessage()
-}
-
-func parseCommand(text string) string {
-	text = strings.TrimSpace(text)
-	if !strings.HasPrefix(text, "/") {
-		return ""
-	}
-	token := text
-	if idx := strings.IndexAny(token, " \t\n\r"); idx >= 0 {
-		token = token[:idx]
-	}
-	token = strings.TrimPrefix(token, "/")
-	if idx := strings.Index(token, "@"); idx >= 0 {
-		token = token[:idx]
-	}
-	return strings.ToLower(strings.TrimSpace(token))
 }
 
 func formatSeconds(value int64) string {

api/gateway/tgsettle/internal/service/treasury/bot/router_test.go

@@ -67,7 +67,7 @@ func TestRouterUnauthorizedInAllowedChatSendsAccessDenied(t *testing.T) {
 	}
 }
 
-func TestRouterUnknownChatIsIgnored(t *testing.T) {
+func TestRouterUnknownChatGetsDenied(t *testing.T) {
 	var sent []string
 	router := NewRouter(
 		mloggerfactory.NewLogger(false),
@@ -90,8 +90,11 @@ func TestRouterUnknownChatIsIgnored(t *testing.T) {
 	if !handled {
 		t.Fatalf("expected update to be handled")
 	}
-	if len(sent) != 0 {
+	if len(sent) != 1 {
-		t.Fatalf("expected no messages, got %d", len(sent))
+		t.Fatalf("expected one message, got %d", len(sent))
+	}
+	if sent[0] != unauthorizedChatMessage {
+		t.Fatalf("unexpected message: %q", sent[0])
 	}
 }
 
@@ -218,3 +221,34 @@ func TestRouterStartUnauthorizedGetsDenied(t *testing.T) {
 		t.Fatalf("unexpected message: %q", sent[0])
 	}
 }
+
+func TestRouterPlainTextWithoutSession_ShowsSupportedCommands(t *testing.T) {
+	var sent []string
+	router := NewRouter(
+		mloggerfactory.NewLogger(false),
+		fakeService{},
+		func(_ context.Context, _ string, text string) error {
+			sent = append(sent, text)
+			return nil
+		},
+		nil,
+		nil,
+		map[string]string{"123": "acct-1"},
+	)
+	handled := router.HandleUpdate(context.Background(), &model.TelegramWebhookUpdate{
+		Message: &model.TelegramMessage{
+			ChatID:     "777",
+			FromUserID: "123",
+			Text:       "hello",
+		},
+	})
+	if !handled {
+		t.Fatalf("expected update to be handled")
+	}
+	if len(sent) != 1 {
+		t.Fatalf("expected one message, got %d", len(sent))
+	}
+	if sent[0] != supportedCommandsMessage() {
+		t.Fatalf("unexpected message: %q", sent[0])
+	}
+}