feat: mention listener, handler for @admin#69
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a new MentionListener middleware wired into the bot, extracts reporting logic into an exported Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Bot
participant MentionListener
participant logReport
participant tgLogger
User->>Bot: Send message containing "@admin" in reply to a message
Bot->>MentionListener: message:entities:mention event
MentionListener->>MentionListener: Filter mentions for text == "@admin"
MentionListener->>Bot: Delete triggering message
MentionListener->>MentionListener: Verify replied-to message exists and has .from
MentionListener->>logReport: Call logReport(context, repliedTo)
logReport->>tgLogger: tgLogger.report(repliedTo, context.from)
tgLogger-->>logReport: Success / Failure
logReport->>Bot: Reply with success or failure message
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/middlewares/mention-listener.ts (2)
10-18: Consider error handling implications with.fork().Using
.fork()runs the handler in a separate branch, meaning exceptions inhandleReport(including fromctx.deleteMessage()) won't propagate through the main middleware chain tobot.catch(). This may be intentional, but verify that errors are still being captured appropriately.If fail-fast behavior is desired per existing patterns, consider whether
.fork()aligns with that intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middlewares/mention-listener.ts` around lines 10 - 18, The current use of .fork() on the composer branch prevents exceptions from handleReport (and calls like ctx.deleteMessage) from bubbling to the main middleware error handler; to fix this either remove .fork() so errors from the composer.on("message:entities:mention") handler propagate normally, or keep .fork() but wrap the call to this.handleReport(ctx) in an async try/catch that forwards caught errors to the global error handler (e.g., rethrow or call the bot.catch/global error emitter) so failures in handleReport/ctx.deleteMessage are captured; locate the composer.on("message:entities:mention") chain and the handleReport method to implement the change.
24-33: Silent failure when user doesn't reply to a message.When a user mentions
@adminwithout replying to a message, the function logs an error and returns silently. Consider providing user feedback so they know to reply to the message they want to report.💡 Proposed enhancement to provide user feedback
private async handleReport(ctx: MentionContext<C>) { await ctx.deleteMessage() const repliedTo = ctx.message.reply_to_message if (!repliedTo?.from) { logger.error("report: no repliedTo or repliedTo.from field (the msg was sent in a channel)") + await ctx.reply("⚠️ Please reply to the message you want to report.", { + disable_notification: true, + }) return } await report(ctx, repliedTo) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middlewares/mention-listener.ts` around lines 24 - 33, handleReport currently logs and returns when ctx.message.reply_to_message is missing, leaving the user uninformed; instead, after ctx.deleteMessage() and before returning, send a user-facing message (via ctx.reply or the project's standard reply/send API) telling the user they must reply to the message they want to report, keep the logger.error for server-side diagnostics, and only return afterward; update the logic around ctx.message.reply_to_message in handleReport to perform this user notification and then skip calling report(ctx, repliedTo).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bot.ts`:
- Line 24: The import ordering in src/bot.ts is failing the pipeline because the
MentionListener import is out of alphabetical order; move the line importing
MentionListener so it is alphabetically ordered with the other local imports
(i.e., place the `import { MentionListener } from
"./middlewares/mention-listener"` line in the correct position among the other
local imports) to satisfy the linter.
In `@src/commands/report.ts`:
- Around line 9-23: The report function currently uses context.from without
checking for undefined before calling modules.get("tgLogger").report; add a null
check for context.from at the start of report (the function named report) and
handle the undefined case by not calling modules.get("tgLogger").report and
instead sending an explanatory reply to the user (similar style to the existing
reply messages) or otherwise passing an explicit fallback/identity if your
tgLogger.report accepts it; ensure you reference context.from and repliedTo when
forming the response so anonymous/channel-post cases are handled safely and no
undefined is passed into tgLogger.report.
---
Nitpick comments:
In `@src/middlewares/mention-listener.ts`:
- Around line 10-18: The current use of .fork() on the composer branch prevents
exceptions from handleReport (and calls like ctx.deleteMessage) from bubbling to
the main middleware error handler; to fix this either remove .fork() so errors
from the composer.on("message:entities:mention") handler propagate normally, or
keep .fork() but wrap the call to this.handleReport(ctx) in an async try/catch
that forwards caught errors to the global error handler (e.g., rethrow or call
the bot.catch/global error emitter) so failures in
handleReport/ctx.deleteMessage are captured; locate the
composer.on("message:entities:mention") chain and the handleReport method to
implement the change.
- Around line 24-33: handleReport currently logs and returns when
ctx.message.reply_to_message is missing, leaving the user uninformed; instead,
after ctx.deleteMessage() and before returning, send a user-facing message (via
ctx.reply or the project's standard reply/send API) telling the user they must
reply to the message they want to report, keep the logger.error for server-side
diagnostics, and only return afterward; update the logic around
ctx.message.reply_to_message in handleReport to perform this user notification
and then skip calling report(ctx, repliedTo).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cac8639a-e88e-4ca5-a42d-3a39e8167a96
📒 Files selected for processing (3)
src/bot.tssrc/commands/report.tssrc/middlewares/mention-listener.ts
Closes #65