Refactor: Centralize callback query message extraction#71
Conversation
Move message extraction from individual inline button handlers to the centralized dispatcher in bot.rs. This eliminates repetitive boilerplate code and simplifies handler signatures by passing Message directly.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThis PR refactors Telegram inline button action handlers to accept Message parameters directly rather than extracting messages from CallbackQuery objects, with central message extraction occurring in the bot worker. Adds minor build configurations (.gitignore rule for ".claude", lazysql recipe). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/workers/bot.rs (1)
1-1:⚠️ Potential issue | 🟠 MajorRemove the stale
formatdocimport.
cargo clippy --all-targets --all-features --no-deps -- -D warningsis already failing on this unused import, so the PR cannot go green until it is removed.🧹 Minimal fix
-use indoc::formatdoc;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workers/bot.rs` at line 1, Remove the unused import `formatdoc` from the top of the file to silence the Clippy warning: locate the `use indoc::formatdoc;` statement in src/workers/bot.rs and delete it (or replace it with only the necessary indoc imports if any are used elsewhere in the file), then run cargo clippy to confirm the warning is resolved.src/telegram/handlers/inline_buttons.rs (1)
26-108:⚠️ Potential issue | 🟠 MajorCentralize callback acknowledgements in the dispatcher.
After this refactor, six dispatched handlers (
handle_inline_regenerate, admin user paging/details,dislike,song_links,skippage) no longer callanswer_callback_query, indicated by their unused_qparameters. Onlyword_definition::handle_inline_listandai_slop_detection::handle_inlineacknowledge callbacks themselves. Telegram keeps the pressed button in its loading state until the callback is acknowledged, so these paths now have inconsistent UX. Adding one empty ack here before dispatch would make the behavior uniform and lets downstream handlers drop their unused_qparameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/telegram/handlers/inline_buttons.rs` around lines 26 - 108, Add a uniform callback acknowledgment in the dispatcher before it matches on AdminInlineButtons so Telegram no longer leaves buttons loading: call app.bot().answer_callback_query(q.id.clone()).await? (or the equivalent empty ack used elsewhere) immediately after verifying admin and before dispatching to handlers like actions::word_definition::handle_inline_regenerate, actions::admin_users::list::handle_inline, actions::admin_users::details::handle_inline, and others; remove or keep downstream answer_callback_query calls as desired so downstream handlers (e.g., handle_inline_list, ai_slop_detection::handle_inline) can continue to acknowledge themselves or rely on this centralized ack for consistent UX.
🤖 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/workers/bot.rs`:
- Around line 111-114: Replace the localized call
t!("error.inaccessible-message", locale = "en") with a plain English string
literal for this internal Telegram failure path: update the chain on
app.bot().answer_callback_query(q.id.clone()).text(...) to pass a Rust-side
constant (e.g., "Message is inaccessible") instead of routing through t!,
leaving answer_callback_query and show_alert(true) unchanged; remove the
dependency on the "error.inaccessible-message" catalog entry so the alert
remains a fixed English technical message.
---
Outside diff comments:
In `@src/telegram/handlers/inline_buttons.rs`:
- Around line 26-108: Add a uniform callback acknowledgment in the dispatcher
before it matches on AdminInlineButtons so Telegram no longer leaves buttons
loading: call app.bot().answer_callback_query(q.id.clone()).await? (or the
equivalent empty ack used elsewhere) immediately after verifying admin and
before dispatching to handlers like
actions::word_definition::handle_inline_regenerate,
actions::admin_users::list::handle_inline,
actions::admin_users::details::handle_inline, and others; remove or keep
downstream answer_callback_query calls as desired so downstream handlers (e.g.,
handle_inline_list, ai_slop_detection::handle_inline) can continue to
acknowledge themselves or rely on this centralized ack for consistent UX.
In `@src/workers/bot.rs`:
- Line 1: Remove the unused import `formatdoc` from the top of the file to
silence the Clippy warning: locate the `use indoc::formatdoc;` statement in
src/workers/bot.rs and delete it (or replace it with only the necessary indoc
imports if any are used elsewhere in the file), then run cargo clippy to confirm
the warning is resolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1cb15124-b2b7-42e7-bdca-eceb94b82a33
📒 Files selected for processing (16)
.gitignoreJustfilelocales/errors.ymlsrc/telegram/actions/admin_users/details.rssrc/telegram/actions/admin_users/list.rssrc/telegram/actions/ai_slop_detection.rssrc/telegram/actions/analyze.rssrc/telegram/actions/dislike.rssrc/telegram/actions/ignore.rssrc/telegram/actions/magic.rssrc/telegram/actions/recommendasion.rssrc/telegram/actions/skippage.rssrc/telegram/actions/song_links.rssrc/telegram/actions/word_definition.rssrc/telegram/handlers/inline_buttons.rssrc/workers/bot.rs
No longer needed after switching error messages to use t!() macro and plain text for inaccessible message edge case.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/workers/bot.rs`:
- Line 123: The current assignment pins and boxes the future when calling
rustify::telegram::handlers::inline_buttons::handle, causing an unnecessary heap
allocation; change the call site that sets let result = Box::pin(...).await to
directly await the async function (i.e., call
rustify::telegram::handlers::inline_buttons::handle(...).await) so the future is
not boxed—update the expression that produces result accordingly.
- Around line 119-121: The code is passing m.chat.id into App::user_state which
keys state by the clicking user; change it to derive the user id from m.from
(the callback sender) and pass that to App::user_state (e.g., use the User id
from m.from, converting to string), handling the Option<User> case appropriately
(return an error or early-exit if m.from is None) so locale/account/error
handling runs under the correct identity; update the variables around
chat_id/m.chat.id references to use the derived user_id when calling
App::user_state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2705c2c6-d63c-44f2-a66d-0a47559d0abd
📒 Files selected for processing (1)
src/workers/bot.rs
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
1 participant
Moves message extraction from individual inline button handlers to the centralized dispatcher in bot.rs. Eliminates ~170 lines of repetitive boilerplate code across 13 handlers.