fix(tgbot): send fresh screens for typed commands (#6999)

Typed commands edited the remembered menu message, which a user who cleared the chat can no longer see, and then deleted the command, so /start left the chat empty again. Typed commands (/start, /status, /usage, /clearall) now send a new screen and then remove the old menu, or retire its buttons when it is past Telegram's delete window; button taps still edit in place. Closes #6997.
This commit is contained in:
Hamed
2026-11-03 18:22:20 +03:30
committed by GitHub
parent 728c2fb94f
commit e093854f5b
9 changed files with 207 additions and 77 deletions
@@ -0,0 +1,142 @@
package tgbot
import (
"reflect"
"testing"
"github.com/mymmrac/telego"
)
func TestTypedCommandsSendFreshScreensEvenWhenOldEditsSucceed(t *testing.T) {
for _, tc := range []struct {
name, command string
admin bool
}{
{"admin start", "/start", true},
{"client start", "/start", false},
{"admin status", "/status", true},
{"admin client lookup", "/usage admin@e", true},
{"admin usage", "/usage", true},
{"client usage", "/usage", false},
{"client usage with email", "/usage other@e", false},
{"admin clear confirmation", "/clearall", true},
} {
t.Run(tc.name, func(t *testing.T) {
_, calls := newScreenTgbot(t, false)
tb := initReportDB(t)
setTestAdmins(t, 1)
seedClientForTgUser(t, 1, "admin@e")
seedClientForTgUser(t, 2, "client@e")
tb.resolveScreenArt()
userID := int64(2)
if tc.admin {
userID = 1
}
const chatID int64 = 500
tb.renderScreen(chatID, tb.newScreen("main", "previous menu", tb.homeRows()...))
attempts := 1
if tc.command == "/start" {
attempts = 2
}
for attempt := range attempts {
previous, ok := tb.screens().get(chatID)
if !ok {
t.Fatal("previous menu missing")
}
before := len(calls())
message := &telego.Message{
MessageID: 9000 + attempt,
Chat: telego.Chat{ID: chatID, Type: "private"},
From: &telego.User{ID: userID}, Text: tc.command,
}
tb.answerCommand(message, chatID, tc.admin)
tb.deleteIncoming(message)
got := calls()[before:]
if countMethod(got, "sendPhoto") != 1 || countMethod(got, "editMessageMedia")+countMethod(got, "editMessageCaption")+countMethod(got, "editMessageText") != 0 {
t.Fatalf("command must send a visible new menu even when editing succeeds: %v", methods(got))
}
current, ok := tb.screens().get(chatID)
if !ok || current.msgID == previous.msgID || current.text == "previous menu" {
t.Fatalf("new menu not tracked: previous=%+v current=%+v", previous, current)
}
deletions := []int{}
for _, call := range got {
if call.Method == "deleteMessage" {
deletions = append(deletions, int(call.Payload["message_id"].(float64)))
}
}
if len(deletions) != 2 || deletions[0] != previous.msgID || deletions[1] != message.MessageID || got[0].Method != "sendPhoto" {
t.Fatalf("new menu must be sent before retiring the old menu and command: methods=%v deletions=%v", methods(got), deletions)
}
before = len(calls())
tb.answerCallback(&telego.CallbackQuery{
ID: "return-home", From: *message.From, Data: "home",
Message: &telego.Message{MessageID: current.msgID, Chat: message.Chat},
}, tc.admin)
got = calls()[before:]
if countMethod(got, "sendPhoto")+countMethod(got, "sendMessage") != 0 || countMethod(got, "editMessageMedia")+countMethod(got, "editMessageCaption") != 1 {
t.Fatalf("callback must still edit the visible menu: %v", methods(got))
}
}
})
}
}
func TestReplacedCommandMenuLosesKeyboardWhenDeletionFails(t *testing.T) {
for _, cleanupFails := range []bool{false, true} {
t.Run(map[bool]string{false: "keyboard retired", true: "cleanup also unavailable"}[cleanupFails], func(t *testing.T) {
replies := map[string]any{
"deleteMessage": map[string]any{"ok": false, "error_code": 400, "description": "Bad Request: message can't be deleted"},
"editMessageReplyMarkup": map[string]any{"ok": true, "result": map[string]any{"message_id": 333, "date": 0, "chat": map[string]any{"id": 500, "type": "private"}}},
}
if cleanupFails {
replies["editMessageReplyMarkup"] = map[string]any{"ok": false, "error_code": 400, "description": "Bad Request: message to edit not found"}
}
server, calls := recordingServer(t, false, replies)
swapTestBot(t, server.URL)
t.Cleanup(server.Close)
tb := initReportDB(t)
setTestAdmins(t, 1)
tb.resolveScreenArt()
tb.renderScreen(500, tb.newScreen("main", "old menu", tb.homeRows()...))
previous, _ := tb.screens().get(500)
before := len(calls())
tb.answerCommand(&telego.Message{MessageID: 9999, Chat: telego.Chat{ID: 500, Type: "private"}, From: &telego.User{ID: 1}, Text: "/start"}, 500, true)
got := calls()[before:]
if !reflect.DeepEqual(methods(got), []string{"sendPhoto", "deleteMessage", "editMessageReplyMarkup"}) {
t.Fatalf("retire undeletable menu after new send: %v", methods(got))
}
markup, _ := got[2].Payload["reply_markup"].(map[string]any)
if got[2].Payload["chat_id"] != float64(500) || got[2].Payload["message_id"] != float64(previous.msgID) || !reflect.DeepEqual(markup["inline_keyboard"], []any{}) {
t.Fatalf("only old menu's keyboard must be removed: %+v", got[2].Payload)
}
current, ok := tb.screens().get(500)
if !ok || current.msgID == previous.msgID || current.msgID == 0 {
t.Fatalf("cleanup failure discarded the new menu: %+v", current)
}
})
}
}
func TestFailedCommandScreenSendKeepsPreviousMenu(t *testing.T) {
photo := map[string]any{"file_id": "avatar", "file_unique_id": "u", "width": 1, "height": 1}
server, count := staleButtonServer(t, map[string]any{
"getMe": map[string]any{"ok": true, "result": map[string]any{"id": 42, "is_bot": true, "first_name": "bot", "username": "bot"}},
"getUserProfilePhotos": map[string]any{"ok": true, "result": map[string]any{"total_count": 1, "photos": [][]any{{photo}}}},
"sendPhoto": map[string]any{"ok": false, "error_code": 503, "description": "Service Unavailable"},
"editMessageMedia": map[string]any{"ok": true, "result": map[string]any{"message_id": 333, "date": 0, "chat": map[string]any{"id": 500, "type": "private"}}},
"editMessageReplyMarkup": map[string]any{"ok": true, "result": map[string]any{"message_id": 333, "date": 0, "chat": map[string]any{"id": 500, "type": "private"}}},
})
swapTestBot(t, server.URL)
t.Cleanup(server.Close)
tb := initReportDB(t)
setTestAdmins(t, 1)
tb.resolveScreenArt()
previous := &screen{msgID: 333, kind: "main", hasPhoto: true, photo: "avatar", text: "previous menu"}
tb.screens().put(500, previous)
tb.answerCommand(&telego.Message{MessageID: 444, Chat: telego.Chat{ID: 500, Type: "private"}, From: &telego.User{ID: 1}, Text: "/start"}, 500, true)
current, ok := tb.screens().get(500)
if count("sendPhoto") != 1 || count("editMessageMedia") != 0 || count("deleteMessage") != 0 || count("editMessageReplyMarkup") != 0 || !ok || current != previous {
t.Fatalf("failed send must preserve the previous menu: sends=%d edits=%d deletes=%d keyboardEdits=%d current=%+v", count("sendPhoto"), count("editMessageMedia"), count("deleteMessage"), count("editMessageReplyMarkup"), current)
}
}
+27 -14
View File
@@ -18,8 +18,15 @@ import (
// error, so a screen pages its body instead of risking a lost render.
const botCaptionLimit = 1024
// screen is the bot's only chat surface: one photo message whose caption and
// keyboard are edited on every action, so a chat never grows a second menu.
type screenDelivery uint8
const (
screenEditOrReplace screenDelivery = iota
screenSendNew
)
// screen is the bot's chat surface: callbacks edit its caption and keyboard,
// while typed commands replace it with a newly sent menu.
type screen struct {
msgID int
kind string
@@ -132,9 +139,9 @@ func (t *Tgbot) newScreen(kind, body string, rows ...[]telego.InlineKeyboardButt
return &screen{kind: kind, header: header, body: body, pages: pages, rows: rows, pageMax: len(pages)}
}
// renderScreen is the single entry point for drawing a screen: EDIT first, and
// replace (send then delete) only when the edit is genuinely impossible.
func (t *Tgbot) renderScreen(chatID int64, sc *screen) {
// renderScreen edits the current menu unless a command requests a new message.
// A typed command must not depend on the old message still being visible.
func (t *Tgbot) renderScreen(chatID int64, sc *screen, delivery ...screenDelivery) {
if sc == nil || liveBot() == nil {
return
}
@@ -152,7 +159,8 @@ func (t *Tgbot) renderScreen(chatID int64, sc *screen) {
}
sc.markup = tu.InlineKeyboard(rows...)
if prev, ok := t.screens().get(chatID); ok && prev.msgID != 0 {
sendNew := len(delivery) > 0 && delivery[0] == screenSendNew
if prev, ok := t.screens().get(chatID); !sendNew && ok && prev.msgID != 0 {
if t.editScreen(chatID, prev, sc) {
sc.msgID = prev.msgID
t.screens().put(chatID, sc)
@@ -244,8 +252,10 @@ func (t *Tgbot) replaceScreen(chatID int64, sc *screen) int {
logger.Warning("Failed to send a screen:", err)
return 0
}
if hadPrev && prev.msgID != 0 {
t.deleteOwnMessage(chatID, prev.msgID)
if hadPrev && prev.msgID != 0 && !t.deleteOwnMessage(chatID, prev.msgID) {
t.editMessageCallbackTgBot(chatID, prev.msgID, &telego.InlineKeyboardMarkup{
InlineKeyboard: [][]telego.InlineKeyboardButton{},
})
}
sc.msgID = sent.MessageID
// Telegram hands back the file_id of the uploaded tile: remembering it keeps
@@ -414,17 +424,20 @@ func (t *Tgbot) deleteIncomingID(chatID int64, messageID int) {
t.deleteOwnMessage(chatID, messageID)
}
// deleteOwnMessage removes one of our messages without surfacing a failure: the
// user may have deleted it, and Telegram refuses past 48 hours.
func (t *Tgbot) deleteOwnMessage(chatID int64, msgID int) {
if msgID == 0 || liveBot() == nil {
return
// deleteOwnMessage reports whether a message was removed. Failures are expected
// when it is already gone or outside Telegram's deletion window.
func (t *Tgbot) deleteOwnMessage(chatID int64, msgID int) bool {
b := liveBot()
if msgID == 0 || b == nil {
return false
}
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
defer cancel()
if err := liveBot().DeleteMessage(ctx, &telego.DeleteMessageParams{ChatID: tu.ID(chatID), MessageID: msgID}); err != nil {
if err := b.DeleteMessage(ctx, &telego.DeleteMessageParams{ChatID: tu.ID(chatID), MessageID: msgID}); err != nil {
logger.Debug("Cannot delete a bot message:", err)
return false
}
return true
}
// hideButton is the keyboard of a non-screen message: one tap clears it.
+2 -2
View File
@@ -112,14 +112,14 @@ func (t *Tgbot) restartXrayFromScreen(query *telego.CallbackQuery) {
// searchClientScreen renders the client card as a screen; every per-email
// callback that used to post a message lands here.
func (t *Tgbot) searchClientScreen(chatID int64, email string) {
func (t *Tgbot) searchClientScreen(chatID int64, email string, delivery ...screenDelivery) {
traffic, err := t.inboundService.GetClientTrafficByEmail(email)
if err != nil || traffic == nil {
t.sendNotice(chatID, t.I18nBot("tgbot.noResult"))
return
}
body := t.clientInfoMsg(traffic, true, true, true, true, true, true)
t.renderScreen(chatID, t.newScreen("client", body, t.clientRows(email)...))
t.renderScreen(chatID, t.newScreen("client", body, t.clientRows(email)...), delivery...)
}
// clientRows is the client card's keyboard, in one place so every entry point
+7 -7
View File
@@ -22,18 +22,18 @@ import (
// screenHome is the admin's landing screen: the numbers you open the bot for,
// then the menu, with the bot's avatar until the panel can host its own artwork.
func (t *Tgbot) screenHome(chatID, userID int64) {
func (t *Tgbot) screenHome(chatID, userID int64, delivery ...screenDelivery) {
if level := t.levelOf(userID); level != levelAdmin {
t.screenClientHome(chatID, userID)
t.screenClientHome(chatID, userID, delivery...)
return
}
body := t.homeSummary()
t.renderScreen(chatID, t.newScreen("main", body, t.homeRows()...))
t.renderScreen(chatID, t.newScreen("main", body, t.homeRows()...), delivery...)
}
// screenClientHome is a client's own entry point: its usage, then links and QR.
// /start once gave clients the ADMIN menu (panel ops, every client's email).
func (t *Tgbot) screenClientHome(chatID, userID int64) {
func (t *Tgbot) screenClientHome(chatID, userID int64, delivery ...screenDelivery) {
traffics, err := t.inboundService.GetClientTrafficTgBot(userID)
if err != nil || len(traffics) == 0 {
t.sendNotice(chatID, t.I18nBot("tgbot.answers.askToAddUserId", "TgUserID=="+strconv.FormatInt(userID, 10)))
@@ -59,7 +59,7 @@ func (t *Tgbot) screenClientHome(chatID, userID int64) {
))
}
}
t.renderScreen(chatID, t.rootScreen("main", body.String(), rows...))
t.renderScreen(chatID, t.rootScreen("main", body.String(), rows...), delivery...)
}
// homeSummary is the panel overview line used by the menu and the report, so
@@ -91,13 +91,13 @@ func (t *Tgbot) homeSummary() string {
}
// screenServer renders the server-usage screen.
func (t *Tgbot) screenServer(chatID int64) {
func (t *Tgbot) screenServer(chatID int64, delivery ...screenDelivery) {
body := strings.TrimRight(t.prepareServerUsageInfo(), "\r\n")
rows := [][]telego.InlineKeyboardButton{
tu.InlineKeyboardRow(t.btn("tgbot.buttons.refresh", cbServer)),
t.backRow(),
}
t.renderScreen(chatID, t.newScreen("status", body, rows...))
t.renderScreen(chatID, t.newScreen("status", body, rows...), delivery...)
}
// screenOnlines lists who is connected right now; client names are plain text
+8 -39
View File
@@ -46,7 +46,7 @@ func (c apiCall) text() string {
return ""
}
func recordingServer(t *testing.T, failEdit bool) (*httptest.Server, func() []apiCall) {
func recordingServer(t *testing.T, failEdit bool, overrides ...map[string]any) (*httptest.Server, func() []apiCall) {
t.Helper()
var mu sync.Mutex
var calls []apiCall
@@ -67,6 +67,13 @@ func recordingServer(t *testing.T, failEdit bool) (*httptest.Server, func() []ap
nextID++
id := nextID
mu.Unlock()
if len(overrides) > 0 {
if response, ok := overrides[0][method]; ok {
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(response)
return
}
}
fail := func(desc string) {
w.Header().Set("Content-Type", "application/json")
@@ -800,44 +807,6 @@ func TestStartClearsTheBroadcastPrompt(t *testing.T) {
}
}
// Once /start has opened the menu the command itself is removed, and a second /start edits the live
// screen instead of leaving two of them.
func TestStartCommandIsRemovedAndEditsTheLiveScreen(t *testing.T) {
tb, calls := newScreenTgbot(t, false)
initReportDB(t)
setTestAdmins(t, 6666)
// The handler deletes the command after answering, exactly as the router
// wires it; calling both keeps the test on the real path.
start := func(messageID int) {
msg := &telego.Message{
MessageID: messageID,
Chat: telego.Chat{ID: 6666},
From: &telego.User{ID: 6666},
Text: "/start",
}
tb.answerCommand(msg, 6666, true)
tb.deleteIncoming(msg)
}
start(12)
if countMethod(calls(), "sendPhoto") != 1 {
t.Fatalf("sendPhoto calls = %d, want 1 for the first /start", countMethod(calls(), "sendPhoto"))
}
if countMethod(calls(), "deleteMessage") < 1 {
t.Error("the /start command message was left in the chat")
}
// A second /start must redraw the tracked screen, not send another one.
start(13)
if got := countMethod(calls(), "sendPhoto"); got != 1 {
t.Errorf("sendPhoto calls = %d, want 1: a second /start duplicated the menu", got)
}
if countMethod(calls(), "editMessageMedia") == 0 {
t.Error("the second /start did not edit the live screen")
}
}
// Callback-data tests pass a button whose key is missing (its label is ""), which is how /start broke;
// this one loads the real en-US file and insists the client's menu has visible labels.
func TestClientMenuButtonsHaveRealLabels(t *testing.T) {
+3 -3
View File
@@ -366,7 +366,7 @@ func (t *Tgbot) clientOwnedByTgUser(tgUserID int64, email string) bool {
}
// getClientUsage retrieves and sends client usage information to the chat.
func (t *Tgbot) getClientUsage(chatId int64, tgUserID int64, email ...string) {
func (t *Tgbot) getClientUsage(chatId int64, tgUserID int64, delivery screenDelivery, email ...string) {
traffics, err := t.inboundService.GetClientTrafficTgBot(tgUserID)
if err != nil {
logger.Warning(err)
@@ -383,7 +383,7 @@ func (t *Tgbot) getClientUsage(chatId int64, tgUserID int64, email ...string) {
// A client sees only its own card, and never the admin keyboard: every button
// on it (reset traffic, limits, IP limit, enable/disable) acts on an account.
if t.levelOf(tgUserID) != levelAdmin {
t.screenClientHome(chatId, tgUserID)
t.screenClientHome(chatId, tgUserID, delivery)
return
}
@@ -416,7 +416,7 @@ func (t *Tgbot) getClientUsage(chatId int64, tgUserID int64, email ...string) {
} else if len(traffics) > 0 {
rows = t.clientRows(traffics[0].Email)
}
t.renderScreen(chatId, t.newScreen("client", output, rows...))
t.renderScreen(chatId, t.newScreen("client", output, rows...), delivery)
}
// getCommonClientButtons returns the shared inline keyboard rows for the
@@ -233,7 +233,7 @@ func TestUsageWithoutAnArgumentDoesNotPanic(t *testing.T) {
t.Fatalf("getClientUsage panicked: %v", r)
}
}()
tb.getClientUsage(3131, 3131)
tb.getClientUsage(3131, 3131, screenEditOrReplace)
if sc, ok := tb.screens().get(3131); !ok || sc.msgID == 0 {
t.Fatalf("the usage path never rendered a screen (ok=%v): the test does not exercise the row-building line", ok)
@@ -247,7 +247,7 @@ func TestUsageGivesAClientNoAdminButtons(t *testing.T) {
tb, _ := newScreenTgbot(t, false)
seedClientForTgUser(t, 4141, "usage-self@example.com")
tb.getClientUsage(4141, 4141)
tb.getClientUsage(4141, 4141, screenEditOrReplace)
sc, ok := tb.screens().get(4141)
if !ok {
@@ -272,7 +272,7 @@ func TestUsageEmailArgumentDoesNotLeakAnotherClient(t *testing.T) {
seedClientForTgUser(t, 5151, "mine@example.com")
seedClientForTgUser(t, 9999, "other@example.com")
tb.getClientUsage(5151, 5151, "other@example.com")
tb.getClientUsage(5151, 5151, screenEditOrReplace, "other@example.com")
sc, ok := tb.screens().get(5151)
if !ok {
@@ -329,7 +329,7 @@ func TestClientUsageDoesNotLoadTheInboundPerClient(t *testing.T) {
}
t.Cleanup(func() { _ = db.Callback().Query().Remove("test:whole_inbound_rows") })
(&Tgbot{}).getClientUsage(ownerTgID, ownerTgID)
(&Tgbot{}).getClientUsage(ownerTgID, ownerTgID, screenEditOrReplace)
if got := strings.Join(sent(), "\n"); !strings.Contains(got, "shared-inbound") {
t.Fatalf("no client listed with its inbound: %q", got)
+7 -7
View File
@@ -276,13 +276,13 @@ func (t *Tgbot) answerCommand(message *telego.Message, chatId int64, isAdmin boo
return
}
t.clearBroadcastPrompt(messageActor(*message))
t.screenHome(chatId, message.From.ID)
t.screenHome(chatId, message.From.ID, screenSendNew)
return
case "status":
// The full server screen carries the host addresses and versions, so it
// stays admin-only; a client asking for status gets liveness alone.
if isAdmin {
t.screenServer(chatId)
t.screenServer(chatId, screenSendNew)
return
}
t.sendNotice(chatId, t.I18nBot("tgbot.commands.statusOk"))
@@ -293,12 +293,12 @@ func (t *Tgbot) answerCommand(message *telego.Message, chatId int64, isAdmin boo
case "usage":
if len(commandArgs) > 0 {
if isAdmin {
t.searchClientScreen(chatId, commandArgs[0])
t.searchClientScreen(chatId, commandArgs[0], screenSendNew)
} else {
t.getClientUsage(chatId, message.From.ID, commandArgs[0])
t.getClientUsage(chatId, message.From.ID, screenSendNew, commandArgs[0])
}
} else {
t.getClientUsage(chatId, message.From.ID)
t.getClientUsage(chatId, message.From.ID, screenSendNew)
}
return
case "inbound":
@@ -329,7 +329,7 @@ func (t *Tgbot) answerCommand(message *telego.Message, chatId int64, isAdmin boo
}
case "clearall":
if isAdmin {
t.renderScreen(chatId, t.resetAllConfirm())
t.renderScreen(chatId, t.resetAllConfirm(), screenSendNew)
return
}
handleUnknownCommand()
@@ -1028,7 +1028,7 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
t.renderScreen(chatId, t.resetAllConfirm())
case "client_traffic":
t.answerSilent(callbackQuery.ID)
t.getClientUsage(chatId, callbackQuery.From.ID)
t.getClientUsage(chatId, callbackQuery.From.ID, screenEditOrReplace)
case "client_sub_links", "client_individual_links", "client_qr_links":
// A client tapped its own link button: its identity already says which
// subscription, so it opens directly. A second one still needs a choice.
+7 -1
View File
@@ -162,12 +162,18 @@ func (t *Tgbot) sendCallbackAnswerTgBot(id string, message string) {
// editMessageCallbackTgBot edits the reply markup of a message.
func (t *Tgbot) editMessageCallbackTgBot(chatId int64, messageID int, inlineKeyboard *telego.InlineKeyboardMarkup) {
b := liveBot()
if b == nil {
return
}
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
defer cancel()
params := telego.EditMessageReplyMarkupParams{
ChatID: tu.ID(chatId),
MessageID: messageID,
ReplyMarkup: inlineKeyboard,
}
if _, err := liveBot().EditMessageReplyMarkup(context.Background(), &params); err != nil {
if _, err := b.EditMessageReplyMarkup(ctx, &params); err != nil {
if isTelegramNotModifiedError(err) {
logger.Debug("Telegram reply markup unchanged, skipping edit")
return