From 0a9ff58a691d4d1c5d3f96821c07b29e830f1a09 Mon Sep 17 00:00:00 2001 From: YoursFunny Date: Thu, 17 Sep 2026 01:31:30 +0800 Subject: [PATCH] refactor: cover the edit/answer surface in MediaSender, test the button flows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docs/architecture-refactor.md §3 sketched the trait with "按需扩展: edit_message_caption / delete_message / answer_callback_query …", but only the five send methods landed, so `callback.rs` and the edit-before-forward caption swap were stuck on the concrete `Bot` and remained untested (AGENTS.md still lists callback.rs as untestable). - `MediaSender` gains `answer_callback_query`, `edit_message_caption` (HTML parse mode baked in, every caller uses it) and `delete_message`; the mock records call order plus the texts, captions and answer toasts, so tests can assert what the user saw. - `send_message` now returns the sent message id instead of the whole `Message`: the only consumer of the value is the edit-before-forward prompt (which keys its record by it), and returning a `Message` forced every mock to build a teloxide type. `reply`/`reply_html` follow. - `callback.rs`: the dptree entry only unpacks the update; `handle_callback` takes plain values + `&AppContext`. `handlers/mod.rs::edit_message_handler` likewise takes the values the reply carries. Admin/setup APIs (`get_chat`, `get_chat_administrators`, `get_me`, `set_my_commands`) stay on the concrete `Bot`: they are not user flows worth a trait. - The scripted mock moves to `parking_lot::Mutex` (no poisoning unwraps). Tests: +11 (template button, forward ok/no-channel/retryable, expired+unknown prompt, caption swap via template, escaping of user text into the caption, failed swap still consuming the reply, prompt record written by post_send). fmt/clippy clean, 70 + 69 tests pass. --- crates/xmedia-bot/src/ctx.rs | 4 + crates/xmedia-bot/src/handlers/callback.rs | 313 +++++++++++++++++---- crates/xmedia-bot/src/handlers/mod.rs | 161 +++++++++-- crates/xmedia-bot/src/media_sender.rs | 172 ++++++++++- crates/xmedia-bot/src/send.rs | 34 ++- 5 files changed, 576 insertions(+), 108 deletions(-) diff --git a/crates/xmedia-bot/src/ctx.rs b/crates/xmedia-bot/src/ctx.rs index e4ef800..e6a1f10 100644 --- a/crates/xmedia-bot/src/ctx.rs +++ b/crates/xmedia-bot/src/ctx.rs @@ -84,6 +84,10 @@ pub(crate) mod test_support { } } + pub(crate) fn chat_store(&self) -> &ChatStore { + &self.chat_store + } + pub(crate) fn link_cache(&self) -> &LinkCache { &self.link_cache } diff --git a/crates/xmedia-bot/src/handlers/callback.rs b/crates/xmedia-bot/src/handlers/callback.rs index 7e5743e..45aa4f4 100644 --- a/crates/xmedia-bot/src/handlers/callback.rs +++ b/crates/xmedia-bot/src/handlers/callback.rs @@ -1,56 +1,76 @@ //! Callback query handling: the edit-before-forward prompt's `"forward"` and //! `"template|"` buttons. +//! +//! [`callback_query_handler`] is the dptree entry; it only pulls the plain +//! values out of the teloxide update and hands them to [`handle_callback`], +//! which holds the button logic and is driven directly by tests. -use super::{CHAT_STORE, CONFIG, TASK_QUEUE}; +use crate::ctx::AppContext; use crate::db::unix_now; use crate::send::{self, Task}; use teloxide::RequestError; use teloxide::prelude::*; -use teloxide::types::{CallbackQuery, ChatId, MessageId, ParseMode}; +use teloxide::types::{CallbackQuery, CallbackQueryId, MessageId}; + +/// The `"forward"` button's data. +const FORWARD: &str = "forward"; +/// Prefix of a template button's data: `"template|"`. +const TEMPLATE_PREFIX: &str = "template|"; pub async fn callback_query_handler(bot: Bot, query: CallbackQuery) -> Result<(), RequestError> { - let callback_query_id = query.id; - let data = query.data.clone(); let Some(message) = &query.message else { return respond(()); }; - let chat_id = message.chat().id.0; - let prompt_message_id = message.id().0 as i64; - let ttl_secs = CONFIG.edit_message_ttl.as_secs() as i64; - let chat_data = CHAT_STORE.get(chat_id).await; + let Some(data) = query.data.clone() else { + return respond(()); + }; + let ctx = AppContext::from_statics(&bot); + handle_callback( + &ctx, + query.id.clone(), + message.chat().id.0, + message.id().0 as i64, + &data, + ) + .await; + respond(()) +} + +/// Handles one button press on the edit-before-forward prompt. +async fn handle_callback( + ctx: &AppContext<'_>, + callback_query_id: CallbackQueryId, + chat_id: i64, + prompt_message_id: i64, + data: &str, +) { + let ttl_secs = ctx.config.edit_message_ttl.as_secs() as i64; + let chat_data = ctx.chat_store.get(chat_id).await; let edit = chat_data.edit_message.get(&prompt_message_id).cloned(); let Some(edit) = edit else { - log::debug!( - "callback from {}: no edit record for prompt {prompt_message_id}", - chat_id - ); - bot.answer_callback_query(callback_query_id) - .text("Expired") - .await?; - return respond(()); + log::debug!("callback from {chat_id}: no edit record for prompt {prompt_message_id}"); + let _ = ctx + .sender + .answer_callback_query(callback_query_id, Some("Expired".to_string())) + .await; + return; }; // Lazy expiry: a stale record (past the TTL, not yet swept) is dropped. if edit.created_at + ttl_secs <= unix_now() { - CHAT_STORE + ctx.chat_store .update(chat_id, |data| { data.edit_message.remove(&prompt_message_id); }) .await; - bot.answer_callback_query(callback_query_id) - .text("Expired") - .await?; - return respond(()); + let _ = ctx + .sender + .answer_callback_query(callback_query_id, Some("Expired".to_string())) + .await; + return; } - let Some(data) = data else { - return respond(()); - }; - log::info!( - "callback from {} on prompt {prompt_message_id}: {data}", - chat_id - ); - if data == "forward" { - let ctx = crate::ctx::AppContext::from_statics(&bot); + log::info!("callback from {chat_id} on prompt {prompt_message_id}: {data}"); + if data == FORWARD { match chat_data.forward_channel_id { Some(channel_id) => { let forward_task = Task::ForwardMessages { @@ -60,62 +80,72 @@ pub async fn callback_query_handler(bot: Bot, query: CallbackQuery) -> Result<() notify_chat_id: Some(chat_id), notify_message_id: Some(prompt_message_id), }; - match send::forward_messages(&ctx, &forward_task).await { + let (answer, settled) = match send::forward_messages(ctx, &forward_task).await { Ok(()) => { log::info!( "forwarded {} message(s) to channel {channel_id}", edit.forward_message_ids.len() ); - bot.answer_callback_query(callback_query_id) - .text("✅ Forwarded") - .await?; - let _ = bot - .delete_message(ChatId(chat_id), MessageId(prompt_message_id as i32)) - .await; - CHAT_STORE - .update(chat_id, |data| { - data.edit_message.remove(&prompt_message_id); - }) - .await; + ("✅ Forwarded".to_string(), true) } Err(send::SendError::Retryable { delay_seconds, task, }) => { log::info!("forward queued for retry in {delay_seconds:.1}s"); - send::enqueue_retry(&TASK_QUEUE, *task, delay_seconds).await; - bot.answer_callback_query(callback_query_id) - .text("Forward queued for retry.") - .await?; + send::enqueue_retry(ctx.task_queue, *task, delay_seconds).await; + ("Forward queued for retry.".to_string(), false) } Err(send::SendError::Permanent { message, .. }) => { log::error!("forward failed permanently: {message}"); - bot.answer_callback_query(callback_query_id) - .text(format!("Forward failed: {message}")) - .await?; + (format!("Forward failed: {message}"), false) } + }; + if settled { + // The prompt is done: drop it and its record. + let _ = ctx + .sender + .delete_message(ChatId(chat_id), MessageId(prompt_message_id as i32)) + .await; + ctx.chat_store + .update(chat_id, |data| { + data.edit_message.remove(&prompt_message_id); + }) + .await; } + let _ = ctx + .sender + .answer_callback_query(callback_query_id, Some(answer)) + .await; } None => { log::debug!("forward callback without a forward channel set"); - bot.answer_callback_query(callback_query_id) - .text("No forward channel set.") - .await?; + let _ = ctx + .sender + .answer_callback_query( + callback_query_id, + Some("No forward channel set.".to_string()), + ) + .await; } } - return respond(()); + return; } - if let Some(name) = data.strip_prefix("template|") { + + if let Some(name) = data.strip_prefix(TEMPLATE_PREFIX) { if let Some(template_html) = chat_data.template.get(name).cloned() && let Some(first_forward_id) = edit.forward_message_ids.first().copied() { // Raw template including the [] placeholder (Python parity). - let _ = bot - .edit_message_caption(ChatId(chat_id), MessageId(first_forward_id as i32)) - .caption(template_html) - .parse_mode(ParseMode::Html) + let _ = ctx + .sender + .edit_message_caption( + ChatId(chat_id), + MessageId(first_forward_id as i32), + template_html, + ) .await; - CHAT_STORE + ctx.chat_store .update(chat_id, |data| { if let Some(entry) = data.edit_message.get_mut(&prompt_message_id) { entry.template = name.to_string(); @@ -124,7 +154,168 @@ pub async fn callback_query_handler(bot: Bot, query: CallbackQuery) -> Result<() .await; log::info!("template '{name}' applied to prompt {prompt_message_id}"); } - bot.answer_callback_query(callback_query_id).await?; + let _ = ctx + .sender + .answer_callback_query(callback_query_id, None) + .await; + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::ctx::test_support::TestStores; + use crate::media_sender::test_support::{MockSender, Outcome}; + use crate::state::EditMessage; + use teloxide::ApiError; + + /// The edit-before-forward prompt's message id in these tests. + const PROMPT_ID: i64 = 7; + /// The message the prompt refers to (the one whose caption is swapped). + const FORWARDED_ID: i64 = 9; + + fn api_error() -> RequestError { + RequestError::Api(ApiError::Unknown("Bad Request: chat not found".into())) + } + + fn callback_id() -> CallbackQueryId { + CallbackQueryId("cb-1".to_string()) + } + + /// Seeds a live prompt record plus a forward channel and a template; + /// `created_at` backdates the record for the expiry cases. + async fn seed_prompt(ctx: &AppContext<'_>, created_at: i64) { + ctx.chat_store + .update(1, |data| { + data.forward_channel_id = Some(2); + data.template + .insert("tpl".to_string(), "[]".to_string()); + data.edit_message.insert( + PROMPT_ID, + EditMessage { + url: "https://x.com/u/status/1".into(), + chat_id: 1, + forward_message_ids: vec![FORWARDED_ID], + template: String::new(), + created_at, + }, + ); + }) + .await; + } + + #[tokio::test] + async fn template_button_swaps_the_caption_and_records_the_choice() { + let sender = MockSender::scripted(vec![Outcome::EditOk], api_error); + let stores = TestStores::new(); + let ctx = stores.ctx(&sender); + seed_prompt(&ctx, crate::db::unix_now()).await; + + handle_callback(&ctx, callback_id(), 1, PROMPT_ID, "template|tpl").await; + + assert_eq!( + sender.calls(), + vec!["edit_message_caption", "answer_callback_query"] + ); + // The raw template, including the [] the user edits into. + assert_eq!(sender.captions(), vec!["[]"]); + assert_eq!(sender.answers(), vec![None]); + let data = ctx.chat_store.get(1).await; + assert_eq!(data.edit_message[&PROMPT_ID].template, "tpl"); + } + + #[tokio::test] + async fn forward_button_copies_then_clears_the_prompt() { + let sender = MockSender::scripted(vec![Outcome::CopyOk], api_error); + let stores = TestStores::new(); + let ctx = stores.ctx(&sender); + seed_prompt(&ctx, crate::db::unix_now()).await; + + handle_callback(&ctx, callback_id(), 1, PROMPT_ID, "forward").await; + + assert_eq!( + sender.calls(), + vec!["copy_messages", "delete_message", "answer_callback_query"] + ); + assert_eq!(sender.answers(), vec![Some("✅ Forwarded".to_string())]); + assert!( + ctx.chat_store.get(1).await.edit_message.is_empty(), + "a settled prompt must drop its record" + ); + } + + #[tokio::test] + async fn forward_without_a_channel_is_reported() { + let sender = MockSender::scripted(vec![], api_error); + let stores = TestStores::new(); + let ctx = stores.ctx(&sender); + seed_prompt(&ctx, crate::db::unix_now()).await; + ctx.chat_store + .update(1, |data| data.forward_channel_id = None) + .await; + + handle_callback(&ctx, callback_id(), 1, PROMPT_ID, "forward").await; + + assert_eq!(sender.calls(), vec!["answer_callback_query"]); + assert_eq!( + sender.answers(), + vec![Some("No forward channel set.".to_string())] + ); + } + + #[tokio::test] + async fn retryable_forward_is_queued_and_keeps_the_prompt() { + use teloxide::types::Seconds; + let sender = MockSender::scripted(vec![Outcome::CopyErr], || { + RequestError::RetryAfter(Seconds::from_seconds(7)) + }); + let stores = TestStores::new(); + let ctx = stores.ctx(&sender); + seed_prompt(&ctx, crate::db::unix_now()).await; + + handle_callback(&ctx, callback_id(), 1, PROMPT_ID, "forward").await; + + assert_eq!( + sender.calls(), + vec!["copy_messages", "answer_callback_query"] + ); + assert_eq!( + sender.answers(), + vec![Some("Forward queued for retry.".to_string())] + ); + assert_eq!(stores.queued_tasks().await, 1); + // The prompt is not settled: the queued retry still needs the record. + assert!( + ctx.chat_store + .get(1) + .await + .edit_message + .contains_key(&PROMPT_ID) + ); + } + + #[tokio::test] + async fn unknown_and_expired_prompts_answer_expired() { + let sender = MockSender::scripted(vec![], api_error); + let stores = TestStores::new(); + let ctx = stores.ctx(&sender); + + // No record at all. + handle_callback(&ctx, callback_id(), 1, PROMPT_ID, "forward").await; + assert_eq!(sender.answers(), vec![Some("Expired".to_string())]); + + // A record past its TTL (nothing swept it yet) is dropped on use. + let stale = crate::db::unix_now() - ctx.config.edit_message_ttl.as_secs() as i64 - 1; + seed_prompt(&ctx, stale).await; + handle_callback(&ctx, callback_id(), 1, PROMPT_ID, "forward").await; + assert_eq!( + sender.answers(), + vec![Some("Expired".to_string()), Some("Expired".to_string())] + ); + assert!( + ctx.chat_store.get(1).await.edit_message.is_empty(), + "the expired record must be dropped" + ); + assert_eq!(sender.calls(), vec!["answer_callback_query"; 2]); } - respond(()) } diff --git a/crates/xmedia-bot/src/handlers/mod.rs b/crates/xmedia-bot/src/handlers/mod.rs index a3e6310..f971abd 100644 --- a/crates/xmedia-bot/src/handlers/mod.rs +++ b/crates/xmedia-bot/src/handlers/mod.rs @@ -18,6 +18,7 @@ pub use inline::inline_query_handler; pub use statics::{CHAT_STORE, CONFIG, LINK_CACHE, TASK_QUEUE}; pub use urls::{start_url_workers, stop_url_workers}; +use crate::ctx::AppContext; use crate::media_sender::MediaSender; use commands::{Command, execute_command}; use teloxide::RequestError; @@ -27,16 +28,13 @@ use teloxide::utils::command::BotCommands; use urls::{URL_JOBS, extract_urls}; /// Reply to a message by id, keeping the reply decoration even if the -/// original was already deleted. -pub(crate) async fn reply( +/// original was already deleted. Returns the reply's message id. +pub(crate) async fn reply( sender: &dyn MediaSender, chat_id: i64, reply_to: MessageId, - text: T, -) -> Result -where - T: Into, -{ + text: impl Into, +) -> Result { sender .send_message(ChatId(chat_id), text.into(), Some(reply_to), None) .await @@ -50,13 +48,14 @@ pub(crate) async fn reply_html( chat_id: i64, reply_to: MessageId, text: String, -) -> Result { +) -> Result { // `::` disambiguates from the MediaSender trait's // same-named method (see media_sender.rs). ::send_message(bot, ChatId(chat_id), text) .parse_mode(ParseMode::Html) .reply_parameters(ReplyParameters::new(reply_to).allow_sending_without_reply()) .await + .map(|message| message.id.0 as i64) } /// Log prefix tying the whole lifecycle of one link (fetch → send → cache → @@ -69,16 +68,16 @@ pub fn log_key(url: &str) -> String { /// Edit-before-forward: a reply to the prompt swaps the caption of the first /// forwarded message. Returns true when the message was consumed as an edit. -async fn edit_message_handler(bot: &Bot, message: &Message) -> bool { - let Some(reply) = message.reply_to_message() else { - return false; - }; - let chat_id = message.chat.id.0; - let Some(text) = message.text() else { - return false; - }; - let chat_data = CHAT_STORE.get(chat_id).await; - let Some(edit) = chat_data.edit_message.get(&(reply.id.0 as i64)) else { +/// Body of [`message_handler`]'s edit branch, without teloxide update types so +/// it can be driven by tests. +async fn edit_message_handler( + ctx: &AppContext<'_>, + chat_id: i64, + reply_to_message_id: i64, + text: &str, +) -> bool { + let chat_data = ctx.chat_store.get(chat_id).await; + let Some(edit) = chat_data.edit_message.get(&reply_to_message_id) else { return false; }; let Some(first_forward_id) = edit.forward_message_ids.first() else { @@ -98,15 +97,17 @@ async fn edit_message_handler(bot: &Bot, message: &Message) -> bool { .map(|template| template.replace("[]", &link)) .unwrap_or(link) }; - let result = bot - .edit_message_caption(ChatId(chat_id), MessageId(*first_forward_id as i32)) - .caption(new_text) - .parse_mode(ParseMode::Html) - .await; - match result { - Ok(_) => log::info!( - "edit-before-forward: caption swapped on message {first_forward_id} for prompt {}", - reply.id.0 + match ctx + .sender + .edit_message_caption( + ChatId(chat_id), + MessageId(*first_forward_id as i32), + new_text, + ) + .await + { + Ok(()) => log::info!( + "edit-before-forward: caption swapped on message {first_forward_id} for prompt {reply_to_message_id}" ), Err(e) => log::error!("edit_message_caption failed: {e}"), } @@ -133,7 +134,17 @@ pub async fn message_handler(bot: Bot, message: Message) -> Result<(), RequestEr message.chat.id ); // URL/edit flows only run in private chats; commands run in any chat. - if is_private && edit_message_handler(&bot, &message).await { + if is_private + && let Some(reply) = message.reply_to_message() + && let Some(text) = message.text() + && edit_message_handler( + &AppContext::from_statics(&bot), + message.chat.id.0, + reply.id.0 as i64, + text, + ) + .await + { return respond(()); } if let Some(text) = message.text() @@ -167,3 +178,97 @@ pub async fn message_handler(bot: Bot, message: Message) -> Result<(), RequestEr } respond(()) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::ctx::test_support::TestStores; + use crate::media_sender::test_support::{MockSender, Outcome}; + use crate::state::EditMessage; + use teloxide::ApiError; + + const PROMPT_ID: i64 = 7; + const FORWARDED_ID: i64 = 9; + + fn api_error() -> RequestError { + RequestError::Api(ApiError::Unknown("Bad Request: message not found".into())) + } + + /// Seeds a prompt record; `template` names the chat template used for it + /// (empty = none, the caption gets the bare link). + async fn seed_prompt(ctx: &AppContext<'_>, template: &str) { + ctx.chat_store + .update(1, |data| { + data.template + .insert("tpl".to_string(), "[]".to_string()); + data.edit_message.insert( + PROMPT_ID, + EditMessage { + url: "https://x.com/u/status/1".into(), + chat_id: 1, + forward_message_ids: vec![FORWARDED_ID], + template: template.to_string(), + created_at: crate::db::unix_now(), + }, + ); + }) + .await; + } + + #[tokio::test] + async fn reply_to_a_prompt_swaps_the_caption_through_its_template() { + let sender = MockSender::scripted(vec![Outcome::EditOk], api_error); + let stores = TestStores::new(); + let ctx = stores.ctx(&sender); + seed_prompt(&ctx, "tpl").await; + + let consumed = edit_message_handler(&ctx, 1, PROMPT_ID, "new caption").await; + + assert!(consumed, "a reply to the prompt must be consumed"); + assert_eq!( + sender.captions(), + vec!["new caption"] + ); + } + + #[tokio::test] + async fn reply_text_and_url_are_escaped_into_the_caption() { + let sender = MockSender::scripted(vec![Outcome::EditOk], api_error); + let stores = TestStores::new(); + let ctx = stores.ctx(&sender); + seed_prompt(&ctx, "").await; + + edit_message_handler(&ctx, 1, PROMPT_ID, "").await; + + // No raw markup from user text may reach the HTML caption. + assert_eq!( + sender.captions(), + vec!["<script>alert(1)</script>"] + ); + } + + #[tokio::test] + async fn a_failed_caption_swap_still_consumes_the_reply() { + let sender = MockSender::scripted(vec![Outcome::EditErr], api_error); + let stores = TestStores::new(); + let ctx = stores.ctx(&sender); + seed_prompt(&ctx, "tpl").await; + + // The edit failed (message deleted etc.); the reply must still be + // swallowed instead of being treated as a link to fetch. + assert!(edit_message_handler(&ctx, 1, PROMPT_ID, "new caption").await); + assert_eq!(sender.calls(), vec!["edit_message_caption"]); + } + + #[tokio::test] + async fn reply_to_an_unrelated_message_is_not_consumed() { + let sender = MockSender::scripted(vec![], api_error); + let stores = TestStores::new(); + let ctx = stores.ctx(&sender); + + // No prompt record for that message id → the reply runs the normal + // (URL/command) path instead. + assert!(!edit_message_handler(&ctx, 1, PROMPT_ID, "hello").await); + assert!(sender.calls().is_empty()); + } +} diff --git a/crates/xmedia-bot/src/media_sender.rs b/crates/xmedia-bot/src/media_sender.rs index c1b103e..30cf575 100644 --- a/crates/xmedia-bot/src/media_sender.rs +++ b/crates/xmedia-bot/src/media_sender.rs @@ -8,8 +8,8 @@ use teloxide::RequestError; use teloxide::prelude::Requester; use teloxide::prelude::*; use teloxide::types::{ - ChatAction, ChatId, InlineKeyboardMarkup, InputFile, InputMedia, Message, MessageId, ParseMode, - ReplyParameters, + CallbackQueryId, ChatAction, ChatId, InlineKeyboardMarkup, InputFile, InputMedia, Message, + MessageId, ParseMode, ReplyParameters, }; /// Boxed, `Send` future returned by a [`MediaSender`] method (`async fn` in @@ -48,14 +48,42 @@ pub trait MediaSender: Send + Sync { ) -> BoxFuture<'_, Result, RequestError>>; /// Sends a plain text message, optionally replying to `reply_to` and - /// attaching `reply_markup`. + /// attaching `reply_markup`. Returns the sent message's id: the bot only + /// ever needs that (the edit-before-forward prompt's record is keyed by + /// it), and returning the whole `Message` would force every test mock to + /// construct one. fn send_message( &self, chat_id: ChatId, text: String, reply_to: Option, reply_markup: Option, - ) -> BoxFuture<'_, Result>; + ) -> BoxFuture<'_, Result>; + + /// Answers a callback query, optionally with a toast `text` shown to the + /// user who pressed the button. + fn answer_callback_query( + &self, + id: CallbackQueryId, + text: Option, + ) -> BoxFuture<'_, Result<(), RequestError>>; + + /// Rewrites a message's caption, always with HTML parse mode (every caller + /// in this bot renders escaped HTML: templates and edit-before-forward + /// links). + fn edit_message_caption( + &self, + chat_id: ChatId, + message_id: MessageId, + caption: String, + ) -> BoxFuture<'_, Result<(), RequestError>>; + + /// Deletes a message (the edit-before-forward prompt after a forward). + fn delete_message( + &self, + chat_id: ChatId, + message_id: MessageId, + ) -> BoxFuture<'_, Result<(), RequestError>>; /// Sets the chat's "typing / uploading …" indicator (cosmetic). fn send_chat_action( @@ -129,7 +157,7 @@ impl MediaSender for Bot { text: String, reply_to: Option, reply_markup: Option, - ) -> BoxFuture<'_, Result> { + ) -> BoxFuture<'_, Result> { Box::pin(async move { let mut request = ::send_message(self, chat_id, text); if let Some(reply_to) = reply_to { @@ -139,7 +167,48 @@ impl MediaSender for Bot { if let Some(markup) = reply_markup { request = request.reply_markup(markup); } - request.await + request.await.map(|message| message.id.0 as i64) + }) + } + + fn answer_callback_query( + &self, + id: CallbackQueryId, + text: Option, + ) -> BoxFuture<'_, Result<(), RequestError>> { + Box::pin(async move { + let mut request = ::answer_callback_query(self, id); + if let Some(text) = text { + request = request.text(text); + } + request.await.map(|_| ()) + }) + } + + fn edit_message_caption( + &self, + chat_id: ChatId, + message_id: MessageId, + caption: String, + ) -> BoxFuture<'_, Result<(), RequestError>> { + Box::pin(async move { + ::edit_message_caption(self, chat_id, message_id) + .caption(caption) + .parse_mode(ParseMode::Html) + .await + .map(|_| ()) + }) + } + + fn delete_message( + &self, + chat_id: ChatId, + message_id: MessageId, + ) -> BoxFuture<'_, Result<(), RequestError>> { + Box::pin(async move { + ::delete_message(self, chat_id, message_id) + .await + .map(|_| ()) }) } @@ -162,7 +231,7 @@ impl MediaSender for Bot { #[cfg(test)] pub(crate) mod test_support { use super::*; - use std::sync::Mutex; + use parking_lot::Mutex; /// One scripted outcome, consumed front-to-back; the last entry repeats /// for further calls of the same method kind. @@ -176,19 +245,30 @@ pub(crate) mod test_support { /// An error from `send_message` (replies are fire-and-forget, so an /// error is fine for tests). MessageErr, + /// A successful `send_message`, returning message id [`MockSender::SENT_ID`]. + MessageOk, + EditOk, + EditErr, } - /// Replays a script and records the method names that were called. + /// Replays a script and records what was sent, so tests can assert the + /// user-visible text a path produced. pub(crate) struct MockSender { script: Mutex>, cursor: Mutex, calls: Mutex>, + messages: Mutex>, + captions: Mutex>, + answers: Mutex>>, /// Builds the error every `*Err` outcome returns (RequestError is not /// cloneable, so the factory recreates it per call). error: Box RequestError + Send + Sync>, } impl MockSender { + /// The message id a successful `send_message` reports. + pub(crate) const SENT_ID: i64 = 1; + pub(crate) fn scripted( script: Vec, error: impl Fn() -> RequestError + Send + Sync + 'static, @@ -197,6 +277,9 @@ pub(crate) mod test_support { script: Mutex::new(script), cursor: Mutex::new(0), calls: Mutex::new(Vec::new()), + messages: Mutex::new(Vec::new()), + captions: Mutex::new(Vec::new()), + answers: Mutex::new(Vec::new()), error: Box::new(error), } } @@ -204,13 +287,28 @@ pub(crate) mod test_support { /// Method names in call order (e.g. `["send_media_group", /// "send_media_group"]` proves the fallback re-sent). pub(crate) fn calls(&self) -> Vec<&'static str> { - self.calls.lock().unwrap().clone() + self.calls.lock().clone() + } + + /// Texts of the plain messages sent, in order. + pub(crate) fn messages(&self) -> Vec { + self.messages.lock().clone() + } + + /// Captions passed to `edit_message_caption`, in order. + pub(crate) fn captions(&self) -> Vec { + self.captions.lock().clone() + } + + /// Toast texts of the answered callback queries, in order. + pub(crate) fn answers(&self) -> Vec> { + self.answers.lock().clone() } fn next(&self, kind: &'static str) -> Outcome { - self.calls.lock().unwrap().push(kind); - let script = self.script.lock().unwrap(); - let mut cursor = self.cursor.lock().unwrap(); + self.calls.lock().push(kind); + let script = self.script.lock(); + let mut cursor = self.cursor.lock(); if script.is_empty() { panic!("mock script exhausted: {kind}"); } @@ -274,25 +372,69 @@ pub(crate) mod test_support { fn send_message( &self, _chat_id: ChatId, - _text: String, + text: String, _reply_to: Option, _reply_markup: Option, - ) -> BoxFuture<'_, Result> { + ) -> BoxFuture<'_, Result> { Box::pin(async move { + self.messages.lock().push(text); match self.next("send_message") { + Outcome::MessageOk => Ok(MockSender::SENT_ID), Outcome::MessageErr => Err(self.error()), other => panic!("unexpected outcome {other:?} for send_message"), } }) } + fn answer_callback_query( + &self, + _id: CallbackQueryId, + text: Option, + ) -> BoxFuture<'_, Result<(), RequestError>> { + // Always succeeds: the toast is cosmetic, so the script stays + // focused on the outcomes a test cares about. + Box::pin(async move { + self.calls.lock().push("answer_callback_query"); + self.answers.lock().push(text); + Ok(()) + }) + } + + fn edit_message_caption( + &self, + _chat_id: ChatId, + _message_id: MessageId, + caption: String, + ) -> BoxFuture<'_, Result<(), RequestError>> { + Box::pin(async move { + self.captions.lock().push(caption); + match self.next("edit_message_caption") { + Outcome::EditOk => Ok(()), + Outcome::EditErr => Err(self.error()), + other => panic!("unexpected outcome {other:?} for edit_message_caption"), + } + }) + } + + fn delete_message( + &self, + _chat_id: ChatId, + _message_id: MessageId, + ) -> BoxFuture<'_, Result<(), RequestError>> { + // Deletion is fire-and-forget in every caller; always succeeds. + Box::pin(async move { + self.calls.lock().push("delete_message"); + Ok(()) + }) + } + fn send_chat_action( &self, _chat_id: ChatId, _action: ChatAction, ) -> BoxFuture<'_, Result<(), RequestError>> { Box::pin(async move { - self.calls.lock().unwrap().push("send_chat_action"); + self.calls.lock().push("send_chat_action"); Ok(()) }) } diff --git a/crates/xmedia-bot/src/send.rs b/crates/xmedia-bot/src/send.rs index 75c8822..4a63d91 100644 --- a/crates/xmedia-bot/src/send.rs +++ b/crates/xmedia-bot/src/send.rs @@ -1269,13 +1269,11 @@ pub async fn post_send_actions(ctx: &AppContext<'_>, task: &Task, message_ids: V ) .await; match prompt { - Ok(prompt) => { + Ok(prompt_id) => { log::info!( - "edit-before-forward prompt {} opened for {} message(s)", - prompt.id.0, + "edit-before-forward prompt {prompt_id} opened for {} message(s)", message_ids.len() ); - let prompt_id = prompt.id.0 as i64; let source_url = source_url.clone(); ctx.chat_store .update(chat_id, move |data| { @@ -1923,6 +1921,34 @@ mod tests { ); } + #[tokio::test] + async fn post_send_opens_and_records_the_edit_prompt() { + let sender = MockSender::scripted(vec![Outcome::MessageOk], media_fetch_error); + let stores = TestStores::new(); + let ctx = stores.ctx(&sender); + let mut task = sent_task(None, false); + if let Task::SendMediaSequence { + edit_before_forward, + .. + } = &mut task + { + *edit_before_forward = true; + } + + post_send_actions(&ctx, &task, vec![10, 11]).await; + + assert_eq!(sender.calls(), vec!["send_message"]); + assert_eq!(sender.messages(), vec!["Reply to edit message."]); + // The prompt's own message id keys the record the reply will edit. + let data = stores.chat_store().get(1).await; + let record = data + .edit_message + .get(&MockSender::SENT_ID) + .expect("the prompt record must be stored"); + assert_eq!(record.url, "https://x.com/u/status/1"); + assert_eq!(record.forward_message_ids, vec![10, 11]); + } + #[tokio::test] async fn post_send_forwards_immediately_when_configured() { let sender = MockSender::scripted(vec![Outcome::CopyOk], media_fetch_error);