fix: add change-ban/info buttons to reaction-spammer ban notifications#403
Merged
Conversation
…fications 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.
- [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
- [minor] [admin.go ReportReactionBan] dry/training notifications now read "[dry run|training] would have permanently banned <link> 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
Deploying tg-spam with
|
| Latest commit: |
891faca
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3a5ef5d1.tg-spam.pages.dev |
| Branch Preview URL: | https://fix-reaction-ban-buttons.tg-spam.pages.dev |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the admin experience for reaction-spam bans by routing reaction-ban notifications through the same “change ban” + “info” inline-keyboard flow used for message-based bans, enabling unban/approve actions directly from the notification while correctly handling the “no underlying message” (msgID == 0) reaction case.
Changes:
- Add
admin.ReportReactionBanand use it fromprocReactionto send reaction-ban notifications with the standard action buttons. - Update ban-confirmation flow to skip Telegram message deletion when
msgID == 0(reaction sentinel), avoiding attempts to delete message 0 in training-mode confirmations. - Add/extend tests and documentation to cover the reaction notification format, callbacks, and parsing behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents that reaction bans now include change ban and info buttons in admin notifications. |
| CLAUDE.md | Adds implementation notes about reaction-ban notifications and the msgID == 0 sentinel behavior. |
| app/events/listener.go | Routes reaction-ban admin notification through ReportReactionBan instead of sending a plain message. |
| app/events/listener_test.go | Adds assertions that reaction-ban notifications include the expected inline keyboard and callback data (msgID=0). |
| app/events/admin.go | Introduces ReportReactionBan and updates deleteAndBan to skip deletion when msgID == 0. |
| app/events/admin_test.go | Adds coverage for reaction notification parsing and callback flows with msgID == 0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
reaction-spammer bans were reported to the admin chat as a plain message with no action buttons, so a wrongly banned reaction spammer could not be unbanned from the notification. A normal message-based ban shows
change banandinfobuttons; the reaction ban showed nothing.this routes the reaction-ban notification through a new
ReportReactionBanthat reuses the samechange ban+infoinline keyboard asReportBan, so admins get the full unban/approve flow. Tappingchange banopens the usual unban/keep-banned confirmation;Unban for realunbans the user and adds them to the approved list.reactions have no underlying message to delete, so the callback carries
msgID = 0.deleteAndBanskips the Telegram delete whenmsgID == 0(and the unban callback already ignores msgID), so confirming a ban in training mode no longer tries to delete a non-existent message.the notification keeps the
[user](tg://user?id=N)link immediately after "permanently banned" in all modes (normal/dry/training). Telegram strips markdown from callback text, soextractUsernamewould otherwise capture the "reaction spammer" descriptor as part of the approved-user name on unban.not changed: dry-mode notifications carry live unban buttons exactly like message-based bans always have. That behavior is shared by all ban types and is out of scope here.
docs: README reaction-spam section updated, plus a CLAUDE.md note on the
msgID == 0reaction sentinel.