From cb458cb7754dc126faf853fce1650e61856ca4a4 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 25 Jun 2026 17:58:11 -0500 Subject: [PATCH 1/3] fix(events): add change-ban/info buttons to reaction-spammer ban notifications reaction-spammer bans were reported to the admin chat as a plain message without action buttons, so a wrongly banned user could not be unbanned. route the notification through a new ReportReactionBan that reuses the same unban/info markup as ReportBan. reactions have no underlying message, so the callback carries msgID 0 and deleteAndBan skips message deletion when it is 0. --- README.md | 2 ++ app/events/admin.go | 31 +++++++++++++++++++++----- app/events/admin_test.go | 18 +++++++++++++++ app/events/listener.go | 13 +---------- app/events/listener_test.go | 44 +++++++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index ee32384d..864c03db 100644 --- a/README.md +++ b/README.md @@ -221,6 +221,8 @@ This option is disabled by default. When enabled, the bot tracks emoji reactions Only applies to unapproved users — approved users are exempt from reaction spam detection. +Reaction bans are reported to the admin chat with the same `change ban` and `info` buttons as message-based bans, so a wrongly banned reaction spammer can be unbanned and approved with one tap. + Note: the bot always subscribes to `message_reaction` updates from Telegram on startup, regardless of whether `--reactions.max-reactions` is set. This is a change from previous versions where no reaction updates were requested. Configure with: diff --git a/app/events/admin.go b/app/events/admin.go index 3f130811..6f95cb22 100644 --- a/app/events/admin.go +++ b/app/events/admin.go @@ -88,6 +88,22 @@ func (a *admin) ReportBan(banUserStr string, msg *bot.Message) { } } +// ReportReactionBan sends a reaction-spammer ban notification to admin chat with the same unban/info buttons as ReportBan. +// reactions have no underlying message, so msgID is 0; the unban path ignores it and deleteAndBan skips deletion for 0. +func (a *admin) ReportReactionBan(banUserStr string, user bot.User) { + link := fmt.Sprintf("[%s](tg://user?id=%d)", escapeMarkDownV1Text(banUserStr), user.ID) + text := fmt.Sprintf("**permanently banned reaction spammer %s**\n\n", link) + switch { + case a.trainingMode: + text = fmt.Sprintf("**[training] reaction spammer detected: %s**\n\n", link) + case a.dry: + text = fmt.Sprintf("**[dry run] would ban reaction spammer %s**\n\n", link) + } + if err := a.sendWithUnbanMarkup(text, "change ban", user, 0, a.adminChatID); err != nil { + log.Printf("[WARN] failed to send reaction ban notification: %v", err) + } +} + // MsgHandler handles messages received on admin chat. this is usually forwarded spam failed // to be detected by the bot. we need to update spam filter with this message and ban the user. // the user will be banned even in training mode, but not in the dry mode. @@ -1055,13 +1071,16 @@ func (a *admin) deleteAndBan(query *tbapi.CallbackQuery, userID int64, msgID int } } + // reaction bans have no underlying message (msgID 0), so there is nothing to delete. // we allow deleting messages from supers. This can be useful if super is training the bot by adding spam messages - _, err := a.tbAPI.Request(tbapi.DeleteMessageConfig{BaseChatMessage: tbapi.BaseChatMessage{ - MessageID: msgID, - ChatConfig: tbapi.ChatConfig{ChatID: a.primChatID}, - }}) - if err != nil { - return fmt.Errorf("failed to delete message %d: %w", query.Message.MessageID, err) + if msgID != 0 { + _, err := a.tbAPI.Request(tbapi.DeleteMessageConfig{BaseChatMessage: tbapi.BaseChatMessage{ + MessageID: msgID, + ChatConfig: tbapi.ChatConfig{ChatID: a.primChatID}, + }}) + if err != nil { + return fmt.Errorf("failed to delete message %d: %w", query.Message.MessageID, err) + } } // any errors happened above will be returned diff --git a/app/events/admin_test.go b/app/events/admin_test.go index 419ffe44..b7550f7c 100644 --- a/app/events/admin_test.go +++ b/app/events/admin_test.go @@ -1228,6 +1228,24 @@ func TestAdmin_InlineCallbacks(t *testing.T) { assert.Equal(t, int64(123), lastCall.C.(tbapi.DeleteMessageConfig).ChatID) }) + t.Run("callbackBanConfirmed_TrainingMode_reactionNoMsgID", func(t *testing.T) { + mockAPI, _, adm, query := setupCallback(true, false) + query.Data = "+12345:0" // reaction ban has no underlying message to delete + + err := adm.callbackBanConfirmed(query) + require.NoError(t, err) + + banned := false + for _, c := range mockAPI.RequestCalls() { + _, isDelete := c.C.(tbapi.DeleteMessageConfig) + assert.False(t, isDelete, "no message deletion when msgID is 0") + if _, ok := c.C.(tbapi.BanChatMemberConfig); ok { + banned = true + } + } + assert.True(t, banned, "user still banned for real on training-mode confirm") + }) + t.Run("callbackBanConfirmed_SoftBan", func(t *testing.T) { mockAPI, botMock, adm, query := setupCallback(false, true) diff --git a/app/events/listener.go b/app/events/listener.go index cc0f6e07..68205620 100644 --- a/app/events/listener.go +++ b/app/events/listener.go @@ -808,18 +808,7 @@ func (l *TelegramListener) procReaction(ctx context.Context, r *tbapi.MessageRea return fmt.Errorf("failed to ban reaction spammer %s: %w", banUserStr, err) } if l.adminChatID != 0 && resp.User.ID != 0 { - // reactions don't have a message ID to delete, so send a plain notification without action buttons - notifText := fmt.Sprintf("permanently banned reaction spammer %s", banUserStr) - switch { - case l.TrainingMode: - notifText = fmt.Sprintf("[training] reaction spammer detected: %s", banUserStr) - case l.Dry: - notifText = fmt.Sprintf("[dry run] would ban reaction spammer %s", banUserStr) - } - notif := tbapi.NewMessage(l.adminChatID, notifText) - if _, err := l.TbAPI.Send(notif); err != nil { - log.Printf("[WARN] failed to send reaction ban notification: %v", err) - } + l.adminHandler.ReportReactionBan(banUserStr, resp.User) } return nil } diff --git a/app/events/listener_test.go b/app/events/listener_test.go index c42e7e85..f9139ade 100644 --- a/app/events/listener_test.go +++ b/app/events/listener_test.go @@ -4561,6 +4561,50 @@ func TestProcReaction(t *testing.T) { assert.Equal(t, "[reaction spam]", spamLoggerMock.SaveCalls()[0].Msg.Text) }) + t.Run("admin notification includes change-ban and info buttons", func(t *testing.T) { + mockAPI := makeAPI(t) + locatorMock := &mocks.LocatorMock{ + AddSpamFunc: func(ctx context.Context, userID int64, checks []spamcheck.Response) error { return nil }, + } + spamLoggerMock := &mocks.SpamLoggerMock{SaveFunc: func(msg *bot.Message, response *bot.Response) {}} + botMock := &mocks.BotMock{ + OnReactionFunc: func(userID int64, userName string) bot.Response { + return bot.Response{ + BanInterval: bot.PermanentBanDuration, + User: bot.User{ID: 42, Username: "spammer"}, + CheckResults: []spamcheck.Response{{Name: "reactions", Spam: true, Details: "5 reactions in 1s"}}, + } + }, + } + l := TelegramListener{TbAPI: mockAPI, Bot: botMock, Locator: locatorMock, SpamLogger: spamLoggerMock} + l.chatID = 123 + l.adminChatID = 999 + l.adminHandler = &admin{tbAPI: mockAPI, bot: botMock, locator: locatorMock, adminChatID: 999, primChatID: 123} + + err := l.procReaction(context.Background(), &tbapi.MessageReactionUpdated{ + Chat: tbapi.Chat{ID: 123}, + User: &tbapi.User{ID: 42, UserName: "spammer"}, + NewReaction: []tbapi.ReactionType{{Type: "emoji", Emoji: "👍"}}, + }) + require.NoError(t, err) + + require.Len(t, mockAPI.SendCalls(), 1, "notification should be sent to admin chat") + sentMsg, ok := mockAPI.SendCalls()[0].C.(tbapi.MessageConfig) + require.True(t, ok) + assert.Equal(t, int64(999), sentMsg.ChatID) + assert.Contains(t, sentMsg.Text, "permanently banned reaction spammer") + assert.Contains(t, sentMsg.Text, "tg://user?id=42") + + markup, ok := sentMsg.ReplyMarkup.(tbapi.InlineKeyboardMarkup) + require.True(t, ok, "notification should carry inline keyboard") + require.Len(t, markup.InlineKeyboard, 1) + require.Len(t, markup.InlineKeyboard[0], 2, "should have change-ban and info buttons") + require.NotNil(t, markup.InlineKeyboard[0][0].CallbackData) + require.NotNil(t, markup.InlineKeyboard[0][1].CallbackData) + assert.Equal(t, "?42:0", *markup.InlineKeyboard[0][0].CallbackData, "change-ban callback, msgID 0 for reactions") + assert.Equal(t, "!42:0", *markup.InlineKeyboard[0][1].CallbackData, "info callback, msgID 0 for reactions") + }) + t.Run("superuser reaction ignored", func(t *testing.T) { mockAPI := makeAPI(t) botMock := &mocks.BotMock{} From a15fe990050b59124e1b3a9b1b8f0684d21c784d Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 25 Jun 2026 18:16:07 -0500 Subject: [PATCH 2/3] fix(review-loop): iteration 1 addressed 11 findings - [major] [admin.go ReportReactionBan] move user link before "reaction spammer" so extractUsername parses the username cleanly on unban (telegram strips markdown from callback text) - [minor] [admin.go deleteAndBan] report msgID in delete-failure error, not the admin-chat message id - [minor] [admin.go deleteAndBan] drop "message 0 deleted" from success log when msgID is 0 - [doc-gap] [admin.go deleteAndBan] godoc notes deletion is skipped for msgID 0 - [doc-gap] [README.md] reword reaction-ban "one tap" to reflect the change-ban confirmation step - [doc-gap] [CLAUDE.md] add Reaction Ban Notifications subsection (msgID==0 sentinel, link placement, getCleanMessage) - [test-gap] [admin_test.go] add callbackUnbanConfirmed_reaction round-trip asserting clean approved-user name - [test-gap] [admin_test.go] add extractUsername reaction cases (markdown + rendered) and getCleanMessage reaction shape - [test-gap] [admin_test.go] training reaction subtest uses reaction body and asserts the confirm edit - [test-gap] [listener_test.go] assert change-ban/info button labels, not just callback data - [test-gap] [listener_test.go] assert ReplyMarkup on dry-run and training reaction notifications --- CLAUDE.md | 6 ++++ README.md | 2 +- app/events/admin.go | 17 +++++++---- app/events/admin_test.go | 60 ++++++++++++++++++++++++++++++++++++- app/events/listener_test.go | 18 +++++++++-- 5 files changed, 94 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 710508c4..a146c411 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -123,6 +123,12 @@ - Training/dry/soft-ban: feature emits a normal spam result through the existing pipeline, so the listener's mode handling intercepts identically to other spam checks (no special-casing) - Naturally-terse legitimate users carry a bounded false-positive risk during the evaluation window only; once the user crosses `FirstMessagesCount` they are approved and immune. Recommended baseline: `MaxShortMsgCount >= 3` with low `FirstMessagesCount` (1–2) +### Reaction Ban Notifications +- Reaction-spammer bans (`procReaction` in `app/events/listener.go`) are reported to the admin chat via `admin.ReportReactionBan`, which reuses the same `change ban`/`info` inline keyboard as `ReportBan` (`sendWithUnbanMarkup`) so admins can unban+approve a wrongly banned reaction spammer +- Reactions have no underlying message, so the callback carries `msgID = 0` as a sentinel. Two paths must honor it: `deleteAndBan` skips the Telegram delete when `msgID == 0` (removing the `if msgID != 0` guard would make every reaction-ban confirmation try to delete message 0), and `callbackUnbanConfirmed` already discards msgID (`userID, _, err := parseCallbackData(...)`) +- The notification text keeps the `[user](tg://user?id=N)` link immediately after "permanently banned" because Telegram strips markdown from callback text; `extractUsername`'s plain-channel regex would otherwise capture any words placed between "permanently banned" and the trailing `(id)` as the approved-user name +- `getCleanMessage` returns empty/error on the short reaction notification (no message body), so the unban path correctly skips `UpdateHam` — no `[reaction spam]` sample is learned + ### LLM Checker Structure - Shared provider-agnostic LLM flow lives in `lib/tgspam/llm.go` - Keep provider-specific transport and request construction in `lib/tgspam/openai.go`, `lib/tgspam/gemini.go`, etc diff --git a/README.md b/README.md index 864c03db..bce6baa0 100644 --- a/README.md +++ b/README.md @@ -221,7 +221,7 @@ This option is disabled by default. When enabled, the bot tracks emoji reactions Only applies to unapproved users — approved users are exempt from reaction spam detection. -Reaction bans are reported to the admin chat with the same `change ban` and `info` buttons as message-based bans, so a wrongly banned reaction spammer can be unbanned and approved with one tap. +Reaction bans are reported to the admin chat with the same `change ban` and `info` buttons as message-based bans, so a wrongly banned reaction spammer can be unbanned and approved straight from the notification (`change ban` opens an unban/keep-banned confirmation). Note: the bot always subscribes to `message_reaction` updates from Telegram on startup, regardless of whether `--reactions.max-reactions` is set. This is a change from previous versions where no reaction updates were requested. diff --git a/app/events/admin.go b/app/events/admin.go index 6f95cb22..3e2dce9e 100644 --- a/app/events/admin.go +++ b/app/events/admin.go @@ -92,7 +92,9 @@ func (a *admin) ReportBan(banUserStr string, msg *bot.Message) { // reactions have no underlying message, so msgID is 0; the unban path ignores it and deleteAndBan skips deletion for 0. func (a *admin) ReportReactionBan(banUserStr string, user bot.User) { link := fmt.Sprintf("[%s](tg://user?id=%d)", escapeMarkDownV1Text(banUserStr), user.ID) - text := fmt.Sprintf("**permanently banned reaction spammer %s**\n\n", link) + // keep the user link immediately after "permanently banned" so extractUsername parses it cleanly on unban; + // telegram strips markdown from callback text, so any words placed between the two get captured as the name + text := fmt.Sprintf("**permanently banned %s reaction spammer**\n\n", link) switch { case a.trainingMode: text = fmt.Sprintf("**[training] reaction spammer detected: %s**\n\n", link) @@ -1048,7 +1050,8 @@ func (a *admin) callbackShowInfo(query *tbapi.CallbackQuery) error { return nil } -// deleteAndBan deletes the message and bans the user +// deleteAndBan bans the user and deletes the message; deletion is skipped when msgID is 0 +// (reaction bans have no underlying message). func (a *admin) deleteAndBan(query *tbapi.CallbackQuery, userID int64, msgID int) error { errs := new(multierror.Error) userName := a.locator.UserNameByID(context.TODO(), userID) @@ -1079,7 +1082,7 @@ func (a *admin) deleteAndBan(query *tbapi.CallbackQuery, userID int64, msgID int ChatConfig: tbapi.ChatConfig{ChatID: a.primChatID}, }}) if err != nil { - return fmt.Errorf("failed to delete message %d: %w", query.Message.MessageID, err) + return fmt.Errorf("failed to delete message %d: %w", msgID, err) } } @@ -1093,10 +1096,14 @@ func (a *admin) deleteAndBan(query *tbapi.CallbackQuery, userID int64, msgID int return errors.New(strings.Join(errMsgs, "\n")) // reformat to be md friendly } + deletedPart := "" + if msgID != 0 { + deletedPart = fmt.Sprintf("message %d deleted, ", msgID) + } if msgFromSuper { - log.Printf("[INFO] message %d deleted, user %q (%d) is super, not banned", msgID, userName, userID) + log.Printf("[INFO] %suser %q (%d) is super, not banned", deletedPart, userName, userID) } else { - log.Printf("[INFO] message %d deleted, user %q (%d) banned", msgID, userName, userID) + log.Printf("[INFO] %suser %q (%d) banned", deletedPart, userName, userID) } return nil } diff --git a/app/events/admin_test.go b/app/events/admin_test.go index b7550f7c..fa407273 100644 --- a/app/events/admin_test.go +++ b/app/events/admin_test.go @@ -126,6 +126,12 @@ func TestAdmin_getCleanMessage(t *testing.T) { expected: "", err: true, }, + { + name: "reaction ban notification has no body", + input: "permanently banned @spammer (42) reaction spammer", + expected: "", + err: true, + }, } for _, tt := range tests { @@ -173,6 +179,8 @@ func TestAdmin_extractUsername(t *testing.T) { {name: "t.me channel link", banMessage: "**permanently banned [spamchannel](https://t.me/spamchannel)**\n\nspam text", expectedResult: "spamchannel"}, {name: "plain channel with ID", banMessage: "**permanently banned mychannel (-100999888)**\n\nspam text", expectedResult: "mychannel"}, {name: "plain channel multi-word title", banMessage: "**permanently banned Spam News Channel (-100999888)**\n\ntext", expectedResult: "Spam News Channel"}, + {name: "reaction markdown", banMessage: "**permanently banned [spammer](tg://user?id=42) reaction spammer**\n\n", expectedResult: "spammer"}, + {name: "reaction rendered", banMessage: "permanently banned @spammer (42) reaction spammer", expectedResult: "@spammer"}, {name: "invalid format", banMessage: "permanently banned John_Doe some message text", expectError: true}, } @@ -1230,7 +1238,8 @@ func TestAdmin_InlineCallbacks(t *testing.T) { t.Run("callbackBanConfirmed_TrainingMode_reactionNoMsgID", func(t *testing.T) { mockAPI, _, adm, query := setupCallback(true, false) - query.Data = "+12345:0" // reaction ban has no underlying message to delete + query.Data = "+12345:0" // reaction ban has no underlying message to delete + query.Message.Text = "permanently banned @spammer (12345) reaction spammer" // reaction notif has no message body err := adm.callbackBanConfirmed(query) require.NoError(t, err) @@ -1244,6 +1253,55 @@ func TestAdmin_InlineCallbacks(t *testing.T) { } } assert.True(t, banned, "user still banned for real on training-mode confirm") + + // admin message is edited to confirm the ban + require.GreaterOrEqual(t, len(mockAPI.SendCalls()), 1) + editMsg, ok := mockAPI.SendCalls()[0].C.(tbapi.EditMessageTextConfig) + require.True(t, ok) + assert.Contains(t, editMsg.Text, "ban confirmed by admin") + }) + + t.Run("callbackUnbanConfirmed_reaction", func(t *testing.T) { + mockAPI, _, adm, _ := setupCallback(false, false) + botMock := &mocks.BotMock{ + UpdateHamFunc: func(msg string) error { return nil }, + AddApprovedUserFunc: func(id int64, name string) error { return nil }, + } + adm.bot = botMock + + // reaction ban: msgID 0, rendered (markdown-stripped) notification text as telegram returns it on callback + query := &tbapi.CallbackQuery{ + ID: "test-callback-id", + Data: "42:0", + Message: &tbapi.Message{ + MessageID: 789, + Chat: tbapi.Chat{ID: 456}, + Text: "permanently banned @spammer (42) reaction spammer", + From: &tbapi.User{UserName: "bot"}, + }, + From: &tbapi.User{UserName: "admin", ID: 111}, + } + + err := adm.callbackUnbanConfirmed(query) + require.NoError(t, err) + + // regular user unban uses UnbanChatMemberConfig (not channel), no message deletion for msgID 0 + var foundUnban bool + for _, call := range mockAPI.RequestCalls() { + _, isDelete := call.C.(tbapi.DeleteMessageConfig) + assert.False(t, isDelete, "no message deletion on reaction unban") + if unbanCall, ok := call.C.(tbapi.UnbanChatMemberConfig); ok { + foundUnban = true + assert.Equal(t, int64(42), unbanCall.UserID) + assert.Equal(t, int64(123), unbanCall.ChatID) + } + } + assert.True(t, foundUnban, "expected UnbanChatMemberConfig for reaction spammer") + + // approved user added with the clean extracted username, not "reaction spammer @spammer" + require.Len(t, botMock.AddApprovedUserCalls(), 1) + assert.Equal(t, int64(42), botMock.AddApprovedUserCalls()[0].ID) + assert.Equal(t, "@spammer", botMock.AddApprovedUserCalls()[0].Name) }) t.Run("callbackBanConfirmed_SoftBan", func(t *testing.T) { diff --git a/app/events/listener_test.go b/app/events/listener_test.go index f9139ade..ed7227cc 100644 --- a/app/events/listener_test.go +++ b/app/events/listener_test.go @@ -4592,8 +4592,10 @@ func TestProcReaction(t *testing.T) { sentMsg, ok := mockAPI.SendCalls()[0].C.(tbapi.MessageConfig) require.True(t, ok) assert.Equal(t, int64(999), sentMsg.ChatID) - assert.Contains(t, sentMsg.Text, "permanently banned reaction spammer") - assert.Contains(t, sentMsg.Text, "tg://user?id=42") + assert.Contains(t, sentMsg.Text, "permanently banned") + assert.Contains(t, sentMsg.Text, "reaction spammer") + // user link sits immediately after "permanently banned" so extractUsername parses it cleanly on unban + assert.Contains(t, sentMsg.Text, "permanently banned [@spammer (42)](tg://user?id=42)") markup, ok := sentMsg.ReplyMarkup.(tbapi.InlineKeyboardMarkup) require.True(t, ok, "notification should carry inline keyboard") @@ -4601,6 +4603,8 @@ func TestProcReaction(t *testing.T) { require.Len(t, markup.InlineKeyboard[0], 2, "should have change-ban and info buttons") require.NotNil(t, markup.InlineKeyboard[0][0].CallbackData) require.NotNil(t, markup.InlineKeyboard[0][1].CallbackData) + assert.Contains(t, markup.InlineKeyboard[0][0].Text, "change ban", "first button is change ban") + assert.Contains(t, markup.InlineKeyboard[0][1].Text, "info", "second button is info") assert.Equal(t, "?42:0", *markup.InlineKeyboard[0][0].CallbackData, "change-ban callback, msgID 0 for reactions") assert.Equal(t, "!42:0", *markup.InlineKeyboard[0][1].CallbackData, "info callback, msgID 0 for reactions") }) @@ -4804,6 +4808,11 @@ func TestProcReaction(t *testing.T) { msg := sendCalls[0].C.(tbapi.MessageConfig) assert.Equal(t, int64(456), msg.ChatID) assert.Contains(t, msg.Text, "[dry run]") + // buttons are attached regardless of mode + markup, ok := msg.ReplyMarkup.(tbapi.InlineKeyboardMarkup) + require.True(t, ok, "dry-run notification should carry inline keyboard") + require.Len(t, markup.InlineKeyboard, 1) + require.Len(t, markup.InlineKeyboard[0], 2) }) t.Run("threshold reached, training mode admin notified", func(t *testing.T) { @@ -4844,5 +4853,10 @@ func TestProcReaction(t *testing.T) { msg := sendCalls[0].C.(tbapi.MessageConfig) assert.Equal(t, int64(456), msg.ChatID) assert.Contains(t, msg.Text, "[training]") + // buttons are attached regardless of mode + markup, ok := msg.ReplyMarkup.(tbapi.InlineKeyboardMarkup) + require.True(t, ok, "training notification should carry inline keyboard") + require.Len(t, markup.InlineKeyboard, 1) + require.Len(t, markup.InlineKeyboard[0], 2) }) } From 891facab991f799b5249dae332e11ab8799dc35d Mon Sep 17 00:00:00 2001 From: Umputun Date: Fri, 26 Jun 2026 03:29:23 -0500 Subject: [PATCH 3/3] =?UTF-8?q?fix(review-loop):=20final=20cleanup=20pass?= =?UTF-8?q?=20=E2=80=94=20addressed=203=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - [minor] [admin.go ReportReactionBan] dry/training notifications now read "[dry run|training] would have permanently banned reaction spammer" so extractUsername parses the username on unban in all modes, not just normal (regular bans were already parseable in every mode) - [minor] [admin.go deleteAndBan] drop the now-unused query parameter (iteration-1's error-id fix removed its last use; revive flagged it) - [test] [admin_test.go] fix the synthetic "reaction markdown" extractUsername case to the production link shape and add rendered dry/training cases as regression guards --- app/events/admin.go | 8 ++++---- app/events/admin_test.go | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/events/admin.go b/app/events/admin.go index 3e2dce9e..7dd8f3aa 100644 --- a/app/events/admin.go +++ b/app/events/admin.go @@ -97,9 +97,9 @@ func (a *admin) ReportReactionBan(banUserStr string, user bot.User) { text := fmt.Sprintf("**permanently banned %s reaction spammer**\n\n", link) switch { case a.trainingMode: - text = fmt.Sprintf("**[training] reaction spammer detected: %s**\n\n", link) + text = fmt.Sprintf("**[training] would have permanently banned %s reaction spammer**\n\n", link) case a.dry: - text = fmt.Sprintf("**[dry run] would ban reaction spammer %s**\n\n", link) + text = fmt.Sprintf("**[dry run] would have permanently banned %s reaction spammer**\n\n", link) } if err := a.sendWithUnbanMarkup(text, "change ban", user, 0, a.adminChatID); err != nil { log.Printf("[WARN] failed to send reaction ban notification: %v", err) @@ -856,7 +856,7 @@ func (a *admin) callbackBanConfirmed(query *tbapi.CallbackQuery) error { if a.trainingMode { // in training mode, the user is not banned automatically, here we do the real ban & delete the message - if err := a.deleteAndBan(query, userID, msgID); err != nil { + if err := a.deleteAndBan(userID, msgID); err != nil { return fmt.Errorf("failed to ban user %d: %w", userID, err) } } @@ -1052,7 +1052,7 @@ func (a *admin) callbackShowInfo(query *tbapi.CallbackQuery) error { // deleteAndBan bans the user and deletes the message; deletion is skipped when msgID is 0 // (reaction bans have no underlying message). -func (a *admin) deleteAndBan(query *tbapi.CallbackQuery, userID int64, msgID int) error { +func (a *admin) deleteAndBan(userID int64, msgID int) error { errs := new(multierror.Error) userName := a.locator.UserNameByID(context.TODO(), userID) banReq := banRequest{ diff --git a/app/events/admin_test.go b/app/events/admin_test.go index fa407273..25a39a2a 100644 --- a/app/events/admin_test.go +++ b/app/events/admin_test.go @@ -179,8 +179,10 @@ func TestAdmin_extractUsername(t *testing.T) { {name: "t.me channel link", banMessage: "**permanently banned [spamchannel](https://t.me/spamchannel)**\n\nspam text", expectedResult: "spamchannel"}, {name: "plain channel with ID", banMessage: "**permanently banned mychannel (-100999888)**\n\nspam text", expectedResult: "mychannel"}, {name: "plain channel multi-word title", banMessage: "**permanently banned Spam News Channel (-100999888)**\n\ntext", expectedResult: "Spam News Channel"}, - {name: "reaction markdown", banMessage: "**permanently banned [spammer](tg://user?id=42) reaction spammer**\n\n", expectedResult: "spammer"}, + {name: "reaction markdown", banMessage: "**permanently banned [@spammer (42)](tg://user?id=42) reaction spammer**\n\n", expectedResult: "@spammer (42)"}, {name: "reaction rendered", banMessage: "permanently banned @spammer (42) reaction spammer", expectedResult: "@spammer"}, + {name: "reaction rendered dry", banMessage: "[dry run] would have permanently banned @spammer (42) reaction spammer", expectedResult: "@spammer"}, + {name: "reaction rendered training", banMessage: "[training] would have permanently banned @spammer (42) reaction spammer", expectedResult: "@spammer"}, {name: "invalid format", banMessage: "permanently banned John_Doe some message text", expectError: true}, }