From c94969dee7ee8dc862fe4bc236f7a99a423d53f2 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Thu, 15 Feb 2024 23:29:52 +0100 Subject: [PATCH 01/23] make help-forward-test independent from saved-messages behavior --- src/html.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/html.rs b/src/html.rs index c58f099c83..f96f0bb226 100644 --- a/src/html.rs +++ b/src/html.rs @@ -291,7 +291,7 @@ pub fn new_html_mimepart(html: String) -> PartBuilder { mod tests { use super::*; use crate::chat; - use crate::chat::forward_msgs; + use crate::chat::{forward_msgs, ProtectionStatus}; use crate::config::Config; use crate::contact::ContactId; use crate::message::{MessengerMessage, Viewtype}; @@ -518,8 +518,10 @@ test some special html-characters as < > and & but also " and &#x // forward the message to saved-messages, // this will encrypt the message as new_alice() has set up keys - let chat = alice.get_self_chat().await; - forward_msgs(&alice, &[msg.get_id()], chat.get_id()) + let chat_id = alice + .create_group_with_members(ProtectionStatus::Protected, "foo", &[]) + .await; + forward_msgs(&alice, &[msg.get_id()], chat_id) .await .unwrap(); let msg = alice.pop_sent_msg().await; @@ -531,7 +533,6 @@ test some special html-characters as < > and & but also " and &#x .await .unwrap(); let msg = alice.recv_msg(&msg).await; - assert_eq!(msg.chat_id, alice.get_self_chat().await.id); assert_eq!(msg.get_from_id(), ContactId::SELF); assert_eq!(msg.is_dc_message, MessengerMessage::Yes); assert!(msg.get_showpadlock()); From 4117f667034f29fd9fbdba63e86fcdd2e0d7de20 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Sat, 17 Feb 2024 11:48:58 +0100 Subject: [PATCH 02/23] improve forwarding to 'Saved Messages' - keep original metadata as far as possible (they are not sent out again) - add api to get original message; this allows the UI to add a corrsponding button - add tests --- deltachat-ffi/deltachat.h | 24 +++++-- deltachat-ffi/src/lib.rs | 24 +++++++ src/chat.rs | 127 +++++++++++++++++++++++++++++++++++++- src/html.rs | 32 ++++++++++ src/message.rs | 51 ++++++++++++++- src/param.rs | 3 + src/sync.rs | 14 ++++- 7 files changed, 266 insertions(+), 9 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index edff3dfb21..ba69aa8e46 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1960,10 +1960,10 @@ void dc_delete_msgs (dc_context_t* context, const uint3 /** * Forward messages to another chat. * - * All types of messages can be forwarded, - * however, they will be flagged as such (dc_msg_is_forwarded() is set). - * - * Original sender, info-state and webxdc updates are not forwarded on purpose. + * Unless forwarded to "Saved Messages", + * UI should mark forwarded messages as such. + * Whether to mark a message as forwarded can be determined by dc_msg_is_forwarded(). + * Original sender, webxdc updates and other metadata are not forwarded on purpose. * * @memberof dc_context_t * @param context The context object. @@ -4384,10 +4384,9 @@ int dc_msg_is_sent (const dc_msg_t* msg); /** - * Check if the message is a forwarded message. + * Check if the message should be marked as forwarded message. * * Forwarded messages may not be created by the contact given as "from". - * * Typically, the UI shows a little text for a symbol above forwarded messages. * * For privacy reasons, we do not provide the name or the e-mail address of the @@ -4868,6 +4867,19 @@ dc_msg_t* dc_msg_get_quoted_msg (const dc_msg_t* msg); dc_msg_t* dc_msg_get_parent (const dc_msg_t* msg); +/** + * Get original message for a saved message. + * + * For messages from the "Saved Messages" chat, + * this function returns the original message ID, if possible. + * + * @param msg The message object as returned for the "Saved Messages" chat. + * @return The message object of the original message. + * NULL of the given message object is not a "Saved Message" + * or if the original does no longer exist. + */ +dc_msg_t* dc_msg_get_original_msg (const dc_msg_t* msg); + /** * Force the message to be sent in plain text. * diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index bef03049f6..763f90f237 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -3980,6 +3980,30 @@ pub unsafe extern "C" fn dc_msg_get_parent(msg: *const dc_msg_t) -> *mut dc_msg_ } } +#[no_mangle] +pub unsafe extern "C" fn dc_msg_get_original_msg(msg: *const dc_msg_t) -> *mut dc_msg_t { + if msg.is_null() { + eprintln!("ignoring careless call to dc_msg_get_original_msg()"); + return ptr::null_mut(); + } + let ffi_msg: &MessageWrapper = &*msg; + let context = &*ffi_msg.context; + let res = block_on(async move { + ffi_msg + .message + .get_original_msg(context) + .await + .context("failed to get original message") + .log_err(context) + .unwrap_or(None) + }); + + match res { + Some(message) => Box::into_raw(Box::new(MessageWrapper { context, message })), + None => ptr::null_mut(), + } +} + #[no_mangle] pub unsafe extern "C" fn dc_msg_force_plaintext(msg: *mut dc_msg_t) { if msg.is_null() { diff --git a/src/chat.rs b/src/chat.rs index 89d4f3e81e..5330808790 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4153,6 +4153,11 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) if let Some(reason) = chat.why_cant_send(context).await? { bail!("cannot send to {}: {}", chat_id, reason); } + + if chat.is_self_talk() { + return save_copies_in_self_talk_and_sync(context, msg_ids).await; + } + curr_timestamp = create_smeared_timestamps(context, msg_ids.len()); let mut msgs = Vec::with_capacity(msg_ids.len()); for id in msg_ids { @@ -4184,6 +4189,7 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) msg.param.remove(Param::ForcePlaintext); msg.param.remove(Param::Cmd); msg.param.remove(Param::OverrideSenderDisplayname); + msg.param.remove(Param::OriginalMsgId); msg.param.remove(Param::WebxdcDocument); msg.param.remove(Param::WebxdcDocumentTimestamp); msg.param.remove(Param::WebxdcSummary); @@ -4209,6 +4215,66 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) Ok(()) } +async fn save_copies_in_self_talk_and_sync(context: &Context, msg_ids: &[MsgId]) -> Result<()> { + let self_addr = context.get_primary_self_addr().await?; + for src_msg_id in msg_ids { + let dest_rfc724_mid = create_outgoing_rfc724_mid(&self_addr); + let src_rfc724_mid = save_copy_in_self_talk(context, src_msg_id, &dest_rfc724_mid).await?; + context + .add_sync_item(SyncData::SaveMessage { + src: src_rfc724_mid, + dest: dest_rfc724_mid, + }) + .await?; + } + context.send_sync_msg().await?; + Ok(()) +} + +/// Saves a copy of the given message in "Saved Messages" using the given RFC724 id. +/// Returns data needed to add a `SaveMessage` sync item. +pub(crate) async fn save_copy_in_self_talk( + context: &Context, + src_msg_id: &MsgId, + dest_rfc724_mid: &String, +) -> Result { + let chat_id = ChatId::create_for_contact(context, ContactId::SELF).await?; + let mut msg = Message::load_from_db(context, *src_msg_id).await?; + msg.param.remove(Param::Cmd); + msg.param.remove(Param::WebxdcDocument); + msg.param.remove(Param::WebxdcDocumentTimestamp); + msg.param.remove(Param::WebxdcSummary); + msg.param.remove(Param::WebxdcSummaryTimestamp); + msg.param + .set_int(Param::OriginalMsgId, src_msg_id.to_u32() as i32); + let copy_fields = "from_id, to_id, timestamp_sent, timestamp_rcvd, type, txt, txt_raw, \ + mime_modified, mime_headers, mime_compressed, mime_in_reply_to, subject, msgrmsg"; + let row_id = context + .sql + .insert( + &*format!( + "INSERT INTO msgs ({copy_fields}, chat_id, rfc724_mid, state, timestamp, param) \ + SELECT {copy_fields}, ?, ?, ?, ?, ? \ + FROM msgs WHERE id=?;" + ), + ( + chat_id, + dest_rfc724_mid, + if msg.from_id == ContactId::SELF { + MessageState::OutDelivered + } else { + MessageState::InSeen + }, + create_smeared_timestamp(context), + msg.param.to_string(), + src_msg_id, + ), + ) + .await?; + context.emit_msgs_changed(chat_id, MsgId::new(row_id.try_into()?)); + Ok(msg.rfc724_mid) +} + /// Resends given messages with the same Message-ID. /// /// This is primarily intended to make existing webxdcs available to new chat members. @@ -4702,8 +4768,9 @@ mod tests { use super::*; use crate::chatlist::get_archived_cnt; use crate::constants::{DC_GCL_ARCHIVED_ONLY, DC_GCL_NO_SPECIALS}; + use crate::contact::{Contact, ContactAddress}; use crate::headerdef::HeaderDef; - use crate::message::delete_msgs; + use crate::message::{delete_msgs, MessengerMessage}; use crate::receive_imf::receive_imf; use crate::test_utils::{sync, TestContext, TestContextManager, TimeShiftFalsePositiveNote}; use strum::IntoEnumIterator; @@ -6836,6 +6903,64 @@ mod tests { Ok(()) } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_forward_to_saved() -> Result<()> { + let alice = TestContext::new_alice().await; + let bob = TestContext::new_bob().await; + let alice_chat = alice.create_chat(&bob).await; + + let sent = alice.send_text(alice_chat.get_id(), "hi, bob").await; + let sent_msg = Message::load_from_db(&alice, sent.sender_msg_id).await?; + let self_chat = alice.get_self_chat().await; + forward_msgs(&alice, &[sent.sender_msg_id], self_chat.id).await?; + let msg = alice.get_last_msg_in(self_chat.id).await; + assert_ne!(msg.get_id(), sent.sender_msg_id); + assert_eq!(msg.get_text(), "hi, bob"); + assert!(!msg.is_forwarded()); // UI should not flag "saved messages" as "forwarded" + assert_eq!(msg.is_dc_message, MessengerMessage::Yes); + assert_eq!(msg.get_from_id(), ContactId::SELF); + assert_eq!(msg.get_state(), MessageState::OutDelivered); + assert_ne!(msg.rfc724_mid(), sent_msg.rfc724_mid()); + + let rcvd_msg = bob.recv_msg(&sent).await; + let self_chat = bob.get_self_chat().await; + forward_msgs(&bob, &[rcvd_msg.id], self_chat.id).await?; + let msg = bob.get_last_msg_in(self_chat.id).await; + assert_ne!(msg.get_id(), rcvd_msg.id); + assert_eq!(msg.get_text(), "hi, bob"); + assert!(!msg.is_forwarded()); + assert_eq!(msg.is_dc_message, MessengerMessage::Yes); + assert_ne!(msg.get_from_id(), ContactId::SELF); + assert_eq!(msg.get_state(), MessageState::InSeen); + assert_ne!(msg.rfc724_mid(), rcvd_msg.rfc724_mid()); + + Ok(()) + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_forward_to_saved_is_not_added_to_shared_chats() -> Result<()> { + let alice = TestContext::new_alice().await; + let bob = TestContext::new_alice().await; + let alice_chat = alice.create_chat(&bob).await; + let bob_chat = bob.create_chat(&alice).await; + + let sent = alice.send_text(alice_chat.id, "hi, bob").await; + bob.recv_msg(&sent).await; + let msg = bob.get_last_msg_in(bob_chat.id).await; + + let self_chat = bob.get_self_chat().await; + forward_msgs(&bob, &[msg.id], self_chat.id).await?; + let msg = bob.get_last_msg_in(self_chat.id).await; + let contact = Contact::get_by_id(&bob, msg.get_from_id()).await?; + assert_eq!(contact.get_addr(), "alice@example.org"); + + let shared_chats = Chatlist::try_load(&bob, 0, None, Some(contact.id)).await?; + assert_eq!(shared_chats.len(), 1); + assert_eq!(shared_chats.get_chat_id(0).unwrap(), bob_chat.id); + + Ok(()) + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_resend_own_message() -> Result<()> { // Alice creates group with Bob and sends an initial message diff --git a/src/html.rs b/src/html.rs index f96f0bb226..9633304cca 100644 --- a/src/html.rs +++ b/src/html.rs @@ -499,6 +499,38 @@ test some special html-characters as < > and & but also " and &#x assert!(html.contains("this is html")); } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_html_forward_to_saved_messages() -> Result<()> { + // Alice receives a non-delta html-message + let alice = TestContext::new_alice().await; + let chat = alice + .create_chat_with_contact("", "sender@testrun.org") + .await; + let raw = include_bytes!("../test-data/message/text_alt_plain_html.eml"); + receive_imf(&alice, raw, false).await?; + let msg = alice.get_last_msg_in(chat.get_id()).await; + + // Alice forwards the message to "Saved Messages" + let self_chat = alice.get_self_chat().await; + forward_msgs(&alice, &[msg.id], self_chat.id).await?; + let saved_msg = alice.get_last_msg_in(self_chat.get_id()).await; + assert_ne!(saved_msg.id, msg.id); + assert_eq!( + saved_msg.get_original_msg(&alice).await?.unwrap().id, + msg.id + ); + assert!(!saved_msg.is_forwarded()); // UI should not flag "saved messages" as "forwarded" + assert_ne!(saved_msg.get_from_id(), ContactId::SELF); + assert_eq!(saved_msg.get_from_id(), msg.get_from_id()); + assert_eq!(saved_msg.is_dc_message, MessengerMessage::No); + assert!(saved_msg.get_text().contains("this is plain")); + assert!(saved_msg.has_html()); + let html = saved_msg.get_id().get_html(&alice).await?.unwrap(); + assert!(html.contains("this is html")); + + Ok(()) + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_html_forwarding_encrypted() { // Alice receives a non-delta html-message diff --git a/src/message.rs b/src/message.rs index b12e3d4844..93371e6991 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1256,6 +1256,19 @@ impl Message { Ok(None) } + /// Returns original message object for message from "Saved Messages". + pub async fn get_original_msg(&self, context: &Context) -> Result> { + if let Some(msg_id) = self.param.get_int(Param::OriginalMsgId) { + let msg = Message::load_from_db(context, MsgId::new(msg_id as u32)).await?; + return if msg.chat_id.is_trash() { + Ok(None) + } else { + Ok(Some(msg)) + }; + } + Ok(None) + } + /// Force the message to be sent in plain text. pub fn force_plaintext(&mut self) { self.param.set_int(Param::ForcePlaintext, 1); @@ -2168,7 +2181,8 @@ mod tests { use super::*; use crate::chat::{ - self, add_contact_to_chat, marknoticed_chat, send_text_msg, ChatItem, ProtectionStatus, + self, add_contact_to_chat, forward_msgs, marknoticed_chat, send_text_msg, ChatItem, + ProtectionStatus, }; use crate::chatlist::Chatlist; use crate::config::Config; @@ -2477,6 +2491,41 @@ mod tests { ); } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_get_original_msg() -> Result<()> { + let alice = TestContext::new_alice().await; + let bob = TestContext::new_bob().await; + + // normal sending of messages does not have an original ID + let one2one_chat = alice.create_chat(&bob).await; + let sent = alice.send_text(one2one_chat.id, "foo").await; + let orig_msg = Message::load_from_db(&alice, sent.sender_msg_id).await?; + assert!(orig_msg.get_original_msg(&alice).await?.is_none()); + assert!(orig_msg.parent(&alice).await?.is_none()); + assert!(orig_msg.quoted_message(&alice).await?.is_none()); + + // forwarding to "Saved Messages", the message gets the original ID attached + let self_chat = alice.get_self_chat().await; + forward_msgs(&alice, &[sent.sender_msg_id], self_chat.get_id()).await?; + let saved_msg = alice.get_last_msg_in(self_chat.get_id()).await; + assert_ne!(saved_msg.get_id(), orig_msg.get_id()); + assert_eq!( + saved_msg.get_original_msg(&alice).await?.unwrap().get_id(), + orig_msg.get_id() + ); + assert!(saved_msg.parent(&alice).await?.is_none()); + assert!(saved_msg.quoted_message(&alice).await?.is_none()); + + // forwarding from "Saved Messages" back to another chat, the original ID attached + forward_msgs(&alice, &[saved_msg.get_id()], one2one_chat.get_id()).await?; + let forwarded_msg = alice.get_last_msg_in(one2one_chat.get_id()).await; + assert_ne!(forwarded_msg.get_id(), saved_msg.get_id()); + assert_ne!(forwarded_msg.get_id(), orig_msg.get_id()); + assert!(forwarded_msg.get_original_msg(&alice).await?.is_none()); + + Ok(()) + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_markseen_msgs() -> Result<()> { let alice = TestContext::new_alice().await; diff --git a/src/param.rs b/src/param.rs index e9255d69f4..9f2263dbd6 100644 --- a/src/param.rs +++ b/src/param.rs @@ -114,6 +114,9 @@ pub enum Param { /// For Messages WebrtcRoom = b'V', + /// For Messages + OriginalMsgId = b'y', + /// For Messages: space-separated list of messaged IDs of forwarded copies. /// /// This is used when a [crate::message::Message] is in the diff --git a/src/sync.rs b/src/sync.rs index d2671e22bc..7e2badabfe 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -16,7 +16,7 @@ use crate::param::Param; use crate::sync::SyncData::{AddQrToken, AlterChat, DeleteQrToken}; use crate::token::Namespace; use crate::tools::time; -use crate::{stock_str, token}; +use crate::{message, stock_str, token}; /// Whether to send device sync messages. Aimed for usage in the internal API. #[derive(Debug, PartialEq)] @@ -62,6 +62,10 @@ pub(crate) enum SyncData { key: Config, val: String, }, + SaveMessage { + src: String, // RFC724 id + dest: String, // RFC724 id + }, } #[derive(Debug, Serialize, Deserialize)] @@ -259,6 +263,7 @@ impl Context { DeleteQrToken(token) => self.delete_qr_token(token).await, AlterChat { id, action } => self.sync_alter_chat(id, action).await, SyncData::Config { key, val } => self.sync_config(key, val).await, + SyncData::SaveMessage { src, dest } => self.save_message(src, dest).await, }, SyncDataOrUnknown::Unknown(data) => { warn!(self, "Ignored unknown sync item: {data}."); @@ -282,6 +287,13 @@ impl Context { token::delete(self, Namespace::Auth, &token.auth).await?; Ok(()) } + + async fn save_message(&self, src_rfc724_mid: &String, dest_rfc724_mid: &String) -> Result<()> { + if let Some((src_msg_id, _)) = message::rfc724_mid_exists(self, src_rfc724_mid).await? { + chat::save_copy_in_self_talk(self, &src_msg_id, dest_rfc724_mid).await?; + } + Ok(()) + } } #[cfg(test)] From ef581b1dd3008d6915ef8430e67f8611543a6f69 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Thu, 22 Feb 2024 00:23:36 +0100 Subject: [PATCH 03/23] save original message in separate column for easier access on deletion --- src/chat.rs | 8 +++----- src/message.rs | 7 +++++-- src/param.rs | 3 --- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 5330808790..6d0b89364b 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4189,7 +4189,6 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) msg.param.remove(Param::ForcePlaintext); msg.param.remove(Param::Cmd); msg.param.remove(Param::OverrideSenderDisplayname); - msg.param.remove(Param::OriginalMsgId); msg.param.remove(Param::WebxdcDocument); msg.param.remove(Param::WebxdcDocumentTimestamp); msg.param.remove(Param::WebxdcSummary); @@ -4245,16 +4244,14 @@ pub(crate) async fn save_copy_in_self_talk( msg.param.remove(Param::WebxdcDocumentTimestamp); msg.param.remove(Param::WebxdcSummary); msg.param.remove(Param::WebxdcSummaryTimestamp); - msg.param - .set_int(Param::OriginalMsgId, src_msg_id.to_u32() as i32); let copy_fields = "from_id, to_id, timestamp_sent, timestamp_rcvd, type, txt, txt_raw, \ mime_modified, mime_headers, mime_compressed, mime_in_reply_to, subject, msgrmsg"; let row_id = context .sql .insert( &*format!( - "INSERT INTO msgs ({copy_fields}, chat_id, rfc724_mid, state, timestamp, param) \ - SELECT {copy_fields}, ?, ?, ?, ?, ? \ + "INSERT INTO msgs ({copy_fields}, chat_id, rfc724_mid, state, timestamp, param, starred) \ + SELECT {copy_fields}, ?, ?, ?, ?, ?, ? \ FROM msgs WHERE id=?;" ), ( @@ -4268,6 +4265,7 @@ pub(crate) async fn save_copy_in_self_talk( create_smeared_timestamp(context), msg.param.to_string(), src_msg_id, + src_msg_id, ), ) .await?; diff --git a/src/message.rs b/src/message.rs index 93371e6991..750a45cea6 100644 --- a/src/message.rs +++ b/src/message.rs @@ -470,6 +470,7 @@ pub struct Message { /// `In-Reply-To` header value. pub(crate) in_reply_to: Option, pub(crate) is_dc_message: MessengerMessage, + pub(crate) original_msg_id: MsgId, pub(crate) mime_modified: bool, pub(crate) chat_blocked: Blocked, pub(crate) location_id: u32, @@ -536,6 +537,7 @@ impl Message { " m.download_state AS download_state,", " m.error AS error,", " m.msgrmsg AS msgrmsg,", + " m.starred AS original_msg_id,", " m.mime_modified AS mime_modified,", " m.txt AS txt,", " m.subject AS subject,", @@ -592,6 +594,7 @@ impl Message { error: Some(row.get::<_, String>("error")?) .filter(|error| !error.is_empty()), is_dc_message: row.get("msgrmsg")?, + original_msg_id: row.get("original_msg_id")?, mime_modified: row.get("mime_modified")?, text, subject: row.get("subject")?, @@ -1258,8 +1261,8 @@ impl Message { /// Returns original message object for message from "Saved Messages". pub async fn get_original_msg(&self, context: &Context) -> Result> { - if let Some(msg_id) = self.param.get_int(Param::OriginalMsgId) { - let msg = Message::load_from_db(context, MsgId::new(msg_id as u32)).await?; + if !self.original_msg_id.is_special() { + let msg = Message::load_from_db(context, self.original_msg_id).await?; return if msg.chat_id.is_trash() { Ok(None) } else { diff --git a/src/param.rs b/src/param.rs index 9f2263dbd6..e9255d69f4 100644 --- a/src/param.rs +++ b/src/param.rs @@ -114,9 +114,6 @@ pub enum Param { /// For Messages WebrtcRoom = b'V', - /// For Messages - OriginalMsgId = b'y', - /// For Messages: space-separated list of messaged IDs of forwarded copies. /// /// This is used when a [crate::message::Message] is in the From ee1892469674acdc15f27d87d86a3737865f86d2 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Thu, 22 Feb 2024 17:51:51 +0100 Subject: [PATCH 04/23] improve tests --- src/chat.rs | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 6d0b89364b..ab7dc7d28c 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4244,6 +4244,11 @@ pub(crate) async fn save_copy_in_self_talk( msg.param.remove(Param::WebxdcDocumentTimestamp); msg.param.remove(Param::WebxdcSummary); msg.param.remove(Param::WebxdcSummaryTimestamp); + let original_msg_id = if !msg.original_msg_id.is_special() { + msg.original_msg_id + } else { + *src_msg_id + }; let copy_fields = "from_id, to_id, timestamp_sent, timestamp_rcvd, type, txt, txt_raw, \ mime_modified, mime_headers, mime_compressed, mime_in_reply_to, subject, msgrmsg"; let row_id = context @@ -4264,7 +4269,7 @@ pub(crate) async fn save_copy_in_self_talk( }, create_smeared_timestamp(context), msg.param.to_string(), - src_msg_id, + original_msg_id, src_msg_id, ), ) @@ -6913,6 +6918,10 @@ mod tests { forward_msgs(&alice, &[sent.sender_msg_id], self_chat.id).await?; let msg = alice.get_last_msg_in(self_chat.id).await; assert_ne!(msg.get_id(), sent.sender_msg_id); + assert_eq!( + msg.get_original_msg(&alice).await?.unwrap().id, + sent.sender_msg_id + ); assert_eq!(msg.get_text(), "hi, bob"); assert!(!msg.is_forwarded()); // UI should not flag "saved messages" as "forwarded" assert_eq!(msg.is_dc_message, MessengerMessage::Yes); @@ -6925,6 +6934,7 @@ mod tests { forward_msgs(&bob, &[rcvd_msg.id], self_chat.id).await?; let msg = bob.get_last_msg_in(self_chat.id).await; assert_ne!(msg.get_id(), rcvd_msg.id); + assert_eq!(msg.get_original_msg(&bob).await?.unwrap().id, rcvd_msg.id); assert_eq!(msg.get_text(), "hi, bob"); assert!(!msg.is_forwarded()); assert_eq!(msg.is_dc_message, MessengerMessage::Yes); @@ -6938,7 +6948,7 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_forward_to_saved_is_not_added_to_shared_chats() -> Result<()> { let alice = TestContext::new_alice().await; - let bob = TestContext::new_alice().await; + let bob = TestContext::new_bob().await; let alice_chat = alice.create_chat(&bob).await; let bob_chat = bob.create_chat(&alice).await; @@ -6959,6 +6969,32 @@ mod tests { Ok(()) } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_forward_from_saved_to_saved() -> Result<()> { + let alice = TestContext::new_alice().await; + let bob = TestContext::new_bob().await; + let sent = alice.send_text(alice.create_chat(&bob).await.id, "k").await; + + bob.recv_msg(&sent).await; + let orig = bob.get_last_msg().await; + let self_chat = bob.get_self_chat().await; + forward_msgs(&bob, &[orig.id], self_chat.id).await?; + let saved1 = bob.get_last_msg().await; + assert_eq!( + saved1.get_original_msg(&bob).await?.unwrap().id, + sent.sender_msg_id + ); + + forward_msgs(&bob, &[saved1.id], self_chat.id).await?; + let saved2 = bob.get_last_msg().await; + assert_eq!( + saved2.get_original_msg(&bob).await?.unwrap().id, + sent.sender_msg_id + ); + + Ok(()) + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_resend_own_message() -> Result<()> { // Alice creates group with Bob and sends an initial message From ae4af5027db49576db764a7f289255e7c0d413ba Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 20 May 2024 14:10:45 +0200 Subject: [PATCH 05/23] adapt to changed create_outgoing_rfc724_mid() api --- src/chat.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index ab7dc7d28c..1d893a5a36 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4215,9 +4215,8 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) } async fn save_copies_in_self_talk_and_sync(context: &Context, msg_ids: &[MsgId]) -> Result<()> { - let self_addr = context.get_primary_self_addr().await?; for src_msg_id in msg_ids { - let dest_rfc724_mid = create_outgoing_rfc724_mid(&self_addr); + let dest_rfc724_mid = create_outgoing_rfc724_mid(); let src_rfc724_mid = save_copy_in_self_talk(context, src_msg_id, &dest_rfc724_mid).await?; context .add_sync_item(SyncData::SaveMessage { @@ -4771,7 +4770,6 @@ mod tests { use super::*; use crate::chatlist::get_archived_cnt; use crate::constants::{DC_GCL_ARCHIVED_ONLY, DC_GCL_NO_SPECIALS}; - use crate::contact::{Contact, ContactAddress}; use crate::headerdef::HeaderDef; use crate::message::{delete_msgs, MessengerMessage}; use crate::receive_imf::receive_imf; From 1749b365cd209384b92cc4d682eafd580b7dae66 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 20 May 2024 14:16:47 +0200 Subject: [PATCH 06/23] make clippy happy --- src/chat.rs | 2 +- src/sync.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 1d893a5a36..dc7ad12aa1 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4253,7 +4253,7 @@ pub(crate) async fn save_copy_in_self_talk( let row_id = context .sql .insert( - &*format!( + &format!( "INSERT INTO msgs ({copy_fields}, chat_id, rfc724_mid, state, timestamp, param, starred) \ SELECT {copy_fields}, ?, ?, ?, ?, ?, ? \ FROM msgs WHERE id=?;" diff --git a/src/sync.rs b/src/sync.rs index 7e2badabfe..0411edd387 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -288,7 +288,7 @@ impl Context { Ok(()) } - async fn save_message(&self, src_rfc724_mid: &String, dest_rfc724_mid: &String) -> Result<()> { + async fn save_message(&self, src_rfc724_mid: &str, dest_rfc724_mid: &String) -> Result<()> { if let Some((src_msg_id, _)) = message::rfc724_mid_exists(self, src_rfc724_mid).await? { chat::save_copy_in_self_talk(self, &src_msg_id, dest_rfc724_mid).await?; } From ebeff88f486ea99ee9bd26c1e118856a350824d2 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Sun, 13 Oct 2024 14:32:27 +0200 Subject: [PATCH 07/23] add api to get original chat_id --- deltachat-ffi/deltachat.h | 21 +++++++++++++++++++-- deltachat-ffi/src/lib.rs | 21 +++++++++++++++++++++ src/chat.rs | 11 +++++++++++ src/message.rs | 12 ++++++++++++ src/param.rs | 4 +++- 5 files changed, 66 insertions(+), 3 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index ba69aa8e46..1683931cb1 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -4873,13 +4873,30 @@ dc_msg_t* dc_msg_get_parent (const dc_msg_t* msg); * For messages from the "Saved Messages" chat, * this function returns the original message ID, if possible. * + * If this function returns NULL, try dc_msg_get_original_chat_id(). + * * @param msg The message object as returned for the "Saved Messages" chat. * @return The message object of the original message. - * NULL of the given message object is not a "Saved Message" - * or if the original does no longer exist. + * NULL if the given message object is not a "Saved Message" + * or if the original message does no longer exist. */ dc_msg_t* dc_msg_get_original_msg (const dc_msg_t* msg); + +/** + * Get original chat ID for a saved message. + * + * This may succeed even if the original message was deleted + * and dc_msg_get_original_msg() return NULL. + * + * @param msg The message object as returned for the "Saved Messages" chat. + * @return The chat ID of the original message. + * 0 if the given message object is not a "Saved Message" + * or if the original chat does no longer exist. + */ +uint32_t dc_msg_get_original_chat_id (const dc_msg_t* msg); + + /** * Force the message to be sent in plain text. * diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 763f90f237..7162f040d4 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4004,6 +4004,27 @@ pub unsafe extern "C" fn dc_msg_get_original_msg(msg: *const dc_msg_t) -> *mut d } } +#[no_mangle] +pub unsafe extern "C" fn dc_msg_get_original_chat_id(msg: *const dc_msg_t) -> u32 { + if msg.is_null() { + eprintln!("ignoring careless call to dc_msg_get_original_chat_id()"); + return 0; + } + let ffi_msg: &MessageWrapper = &*msg; + let context = &*ffi_msg.context; + block_on(async move { + ffi_msg + .message + .get_original_chat_id(context) + .await + .context("failed to get original message") + .log_err(context) + .unwrap_or_default() + .map(|id| id.to_u32()) + .unwrap_or(0) + }) +} + #[no_mangle] pub unsafe extern "C" fn dc_msg_force_plaintext(msg: *mut dc_msg_t) { if msg.is_null() { diff --git a/src/chat.rs b/src/chat.rs index dc7ad12aa1..4b63294780 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4230,6 +4230,9 @@ async fn save_copies_in_self_talk_and_sync(context: &Context, msg_ids: &[MsgId]) } /// Saves a copy of the given message in "Saved Messages" using the given RFC724 id. +/// To allow UIs to have a "show in context" button, +/// the copy contains a reference to the original message +/// as well as to the original chat in case the original message gets deleted. /// Returns data needed to add a `SaveMessage` sync item. pub(crate) async fn save_copy_in_self_talk( context: &Context, @@ -4243,11 +4246,14 @@ pub(crate) async fn save_copy_in_self_talk( msg.param.remove(Param::WebxdcDocumentTimestamp); msg.param.remove(Param::WebxdcSummary); msg.param.remove(Param::WebxdcSummaryTimestamp); + msg.param + .set_int(Param::OriginalChatId, msg.chat_id.to_u32() as i32); let original_msg_id = if !msg.original_msg_id.is_special() { msg.original_msg_id } else { *src_msg_id }; + let copy_fields = "from_id, to_id, timestamp_sent, timestamp_rcvd, type, txt, txt_raw, \ mime_modified, mime_headers, mime_compressed, mime_in_reply_to, subject, msgrmsg"; let row_id = context @@ -6920,6 +6926,7 @@ mod tests { msg.get_original_msg(&alice).await?.unwrap().id, sent.sender_msg_id ); + assert_eq!(msg.get_original_chat_id(&alice).await?.unwrap(), sent_msg.chat_id); assert_eq!(msg.get_text(), "hi, bob"); assert!(!msg.is_forwarded()); // UI should not flag "saved messages" as "forwarded" assert_eq!(msg.is_dc_message, MessengerMessage::Yes); @@ -6933,6 +6940,10 @@ mod tests { let msg = bob.get_last_msg_in(self_chat.id).await; assert_ne!(msg.get_id(), rcvd_msg.id); assert_eq!(msg.get_original_msg(&bob).await?.unwrap().id, rcvd_msg.id); + assert_eq!( + msg.get_original_chat_id(&bob).await?.unwrap(), + rcvd_msg.chat_id + ); assert_eq!(msg.get_text(), "hi, bob"); assert!(!msg.is_forwarded()); assert_eq!(msg.is_dc_message, MessengerMessage::Yes); diff --git a/src/message.rs b/src/message.rs index 750a45cea6..a5a7dccd14 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1272,6 +1272,18 @@ impl Message { Ok(None) } + /// Returns original chat id for message from "Saved Messages". + /// This may work although get_original_msg() returns None as the message was deleted. + pub async fn get_original_chat_id(&self, context: &Context) -> Result> { + if let Some(chat_id) = self.param.get_int(Param::OriginalChatId) { + let chat_id = ChatId::new(u32::try_from(chat_id)?); + if Chat::load_from_db(context, chat_id).await.is_ok() { + return Ok(Some(chat_id)); + } + } + Ok(None) + } + /// Force the message to be sent in plain text. pub fn force_plaintext(&mut self) { self.param.set_int(Param::ForcePlaintext, 1); diff --git a/src/param.rs b/src/param.rs index e9255d69f4..5bd54b86c0 100644 --- a/src/param.rs +++ b/src/param.rs @@ -207,7 +207,9 @@ pub enum Param { /// For messages: Whether [crate::message::Viewtype::Sticker] should be forced. ForceSticker = b'X', - // 'L' was defined as ProtectionSettingsTimestamp for Chats, however, never used in production. + + /// For saved messages: The original ChatId, in case the original message is deleted + OriginalChatId = b'L', } /// An object for handling key=value parameter lists. From fd1d0eaae697e0ba9c07a5af469ab19e463071cf Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Sun, 13 Oct 2024 14:47:29 +0200 Subject: [PATCH 08/23] fix and test deletion of original message --- src/chat.rs | 19 ++++++++++++++++++- src/message.rs | 14 ++++++++------ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 4b63294780..9328268acb 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -6926,7 +6926,10 @@ mod tests { msg.get_original_msg(&alice).await?.unwrap().id, sent.sender_msg_id ); - assert_eq!(msg.get_original_chat_id(&alice).await?.unwrap(), sent_msg.chat_id); + assert_eq!( + msg.get_original_chat_id(&alice).await?.unwrap(), + sent_msg.chat_id + ); assert_eq!(msg.get_text(), "hi, bob"); assert!(!msg.is_forwarded()); // UI should not flag "saved messages" as "forwarded" assert_eq!(msg.is_dc_message, MessengerMessage::Yes); @@ -6951,6 +6954,20 @@ mod tests { assert_eq!(msg.get_state(), MessageState::InSeen); assert_ne!(msg.rfc724_mid(), rcvd_msg.rfc724_mid()); + // delete original message + rcvd_msg.id.delete_from_db(&bob).await?; + let msg = Message::load_from_db(&bob, msg.id).await?; + assert!(msg.get_original_msg(&bob).await?.is_none()); + assert_eq!( + msg.get_original_chat_id(&bob).await?.unwrap(), + rcvd_msg.chat_id + ); + + // delete original chat + rcvd_msg.chat_id.delete(&bob).await?; + let msg = Message::load_from_db(&bob, msg.id).await?; + assert!(msg.get_original_chat_id(&bob).await?.is_none()); + Ok(()) } diff --git a/src/message.rs b/src/message.rs index a5a7dccd14..0e09158803 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1262,12 +1262,14 @@ impl Message { /// Returns original message object for message from "Saved Messages". pub async fn get_original_msg(&self, context: &Context) -> Result> { if !self.original_msg_id.is_special() { - let msg = Message::load_from_db(context, self.original_msg_id).await?; - return if msg.chat_id.is_trash() { - Ok(None) - } else { - Ok(Some(msg)) - }; + if let Some(msg) = Message::load_from_db_optional(context, self.original_msg_id).await? + { + return if msg.chat_id.is_trash() { + Ok(None) + } else { + Ok(Some(msg)) + }; + } } Ok(None) } From 2ae3cfbcf9ad3b5695fcba610f43edaf46335f50 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 14 Oct 2024 00:32:35 +0200 Subject: [PATCH 09/23] add api to get saved msg_id --- deltachat-ffi/deltachat.h | 13 ++++++++ deltachat-ffi/src/lib.rs | 21 +++++++++++++ src/chat.rs | 65 ++++++++++++++++++++++++--------------- src/message.rs | 14 +++++++++ 4 files changed, 89 insertions(+), 24 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 1683931cb1..802e60de26 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -4897,6 +4897,19 @@ dc_msg_t* dc_msg_get_original_msg (const dc_msg_t* msg); uint32_t dc_msg_get_original_chat_id (const dc_msg_t* msg); +/** + * Check if a message was saved and return its ID inside "Saved Messages". + * + * The returned ID can be used to un-save a message. + * The state "is saved" can be used to show some icon to indicate that a message was saved. + * + * @param msg The message object. + * @return The message ID inside "Saved Messages", if any. + * 0 if the given message object is not saved. + */ +uint32_t dc_msg_get_saved_msg_id (const dc_msg_t* msg); + + /** * Force the message to be sent in plain text. * diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 7162f040d4..cdfceeb761 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4025,6 +4025,27 @@ pub unsafe extern "C" fn dc_msg_get_original_chat_id(msg: *const dc_msg_t) -> u3 }) } +#[no_mangle] +pub unsafe extern "C" fn dc_msg_get_saved_msg_id(msg: *const dc_msg_t) -> u32 { + if msg.is_null() { + eprintln!("ignoring careless call to dc_msg_get_saved_msg_id()"); + return 0; + } + let ffi_msg: &MessageWrapper = &*msg; + let context = &*ffi_msg.context; + block_on(async move { + ffi_msg + .message + .get_saved_msg_id(context) + .await + .context("failed to get original message") + .log_err(context) + .unwrap_or_default() + .map(|id| id.to_u32()) + .unwrap_or(0) + }) +} + #[no_mangle] pub unsafe extern "C" fn dc_msg_force_plaintext(msg: *mut dc_msg_t) { if msg.is_null() { diff --git a/src/chat.rs b/src/chat.rs index 9328268acb..577ac9ee3f 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -6918,54 +6918,71 @@ mod tests { let sent = alice.send_text(alice_chat.get_id(), "hi, bob").await; let sent_msg = Message::load_from_db(&alice, sent.sender_msg_id).await?; + assert!(sent_msg.get_saved_msg_id(&alice).await?.is_none()); + assert!(sent_msg.get_original_msg(&alice).await?.is_none()); + assert!(sent_msg.get_original_chat_id(&alice).await?.is_none()); + let self_chat = alice.get_self_chat().await; forward_msgs(&alice, &[sent.sender_msg_id], self_chat.id).await?; - let msg = alice.get_last_msg_in(self_chat.id).await; - assert_ne!(msg.get_id(), sent.sender_msg_id); + + let saved_msg = alice.get_last_msg_in(self_chat.id).await; + assert_ne!(saved_msg.get_id(), sent.sender_msg_id); + assert!(saved_msg.get_saved_msg_id(&alice).await?.is_none()); assert_eq!( - msg.get_original_msg(&alice).await?.unwrap().id, + saved_msg.get_original_msg(&alice).await?.unwrap().id, sent.sender_msg_id ); assert_eq!( - msg.get_original_chat_id(&alice).await?.unwrap(), + saved_msg.get_original_chat_id(&alice).await?.unwrap(), sent_msg.chat_id ); - assert_eq!(msg.get_text(), "hi, bob"); - assert!(!msg.is_forwarded()); // UI should not flag "saved messages" as "forwarded" - assert_eq!(msg.is_dc_message, MessengerMessage::Yes); - assert_eq!(msg.get_from_id(), ContactId::SELF); - assert_eq!(msg.get_state(), MessageState::OutDelivered); - assert_ne!(msg.rfc724_mid(), sent_msg.rfc724_mid()); + assert_eq!(saved_msg.get_text(), "hi, bob"); + assert!(!saved_msg.is_forwarded()); // UI should not flag "saved messages" as "forwarded" + assert_eq!(saved_msg.is_dc_message, MessengerMessage::Yes); + assert_eq!(saved_msg.get_from_id(), ContactId::SELF); + assert_eq!(saved_msg.get_state(), MessageState::OutDelivered); + assert_ne!(saved_msg.rfc724_mid(), sent_msg.rfc724_mid()); + + let sent_msg = Message::load_from_db(&alice, sent.sender_msg_id).await?; + assert_eq!( + sent_msg.get_saved_msg_id(&alice).await?.unwrap(), + saved_msg.id + ); + assert!(sent_msg.get_original_msg(&alice).await?.is_none()); + assert!(sent_msg.get_original_chat_id(&alice).await?.is_none()); let rcvd_msg = bob.recv_msg(&sent).await; let self_chat = bob.get_self_chat().await; forward_msgs(&bob, &[rcvd_msg.id], self_chat.id).await?; - let msg = bob.get_last_msg_in(self_chat.id).await; - assert_ne!(msg.get_id(), rcvd_msg.id); - assert_eq!(msg.get_original_msg(&bob).await?.unwrap().id, rcvd_msg.id); + let saved_msg = bob.get_last_msg_in(self_chat.id).await; + assert_ne!(saved_msg.get_id(), rcvd_msg.id); assert_eq!( - msg.get_original_chat_id(&bob).await?.unwrap(), + saved_msg.get_original_msg(&bob).await?.unwrap().id, + rcvd_msg.id + ); + assert_eq!( + saved_msg.get_original_chat_id(&bob).await?.unwrap(), rcvd_msg.chat_id ); - assert_eq!(msg.get_text(), "hi, bob"); - assert!(!msg.is_forwarded()); - assert_eq!(msg.is_dc_message, MessengerMessage::Yes); - assert_ne!(msg.get_from_id(), ContactId::SELF); - assert_eq!(msg.get_state(), MessageState::InSeen); - assert_ne!(msg.rfc724_mid(), rcvd_msg.rfc724_mid()); + assert_eq!(saved_msg.get_text(), "hi, bob"); + assert!(!saved_msg.is_forwarded()); + assert_eq!(saved_msg.is_dc_message, MessengerMessage::Yes); + assert_ne!(saved_msg.get_from_id(), ContactId::SELF); + assert_eq!(saved_msg.get_state(), MessageState::InSeen); + assert_ne!(saved_msg.rfc724_mid(), rcvd_msg.rfc724_mid()); // delete original message rcvd_msg.id.delete_from_db(&bob).await?; - let msg = Message::load_from_db(&bob, msg.id).await?; - assert!(msg.get_original_msg(&bob).await?.is_none()); + let saved_msg = Message::load_from_db(&bob, saved_msg.id).await?; + assert!(saved_msg.get_original_msg(&bob).await?.is_none()); assert_eq!( - msg.get_original_chat_id(&bob).await?.unwrap(), + saved_msg.get_original_chat_id(&bob).await?.unwrap(), rcvd_msg.chat_id ); // delete original chat rcvd_msg.chat_id.delete(&bob).await?; - let msg = Message::load_from_db(&bob, msg.id).await?; + let msg = Message::load_from_db(&bob, saved_msg.id).await?; assert!(msg.get_original_chat_id(&bob).await?.is_none()); Ok(()) diff --git a/src/message.rs b/src/message.rs index 0e09158803..e323491d56 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1286,6 +1286,20 @@ impl Message { Ok(None) } + /// Check if the message was saved and returns the corresponding message inside "Saved Messages". + /// UI can use this to show a symbol beside the message, indicating it was saved. + /// The message can be un-saved by deleting the returned message. + pub async fn get_saved_msg_id(&self, context: &Context) -> Result> { + let res: Option = context + .sql + .query_get_value( + "SELECT id FROM msgs WHERE starred=? AND chat_id!=?", + (self.id, DC_CHAT_ID_TRASH), + ) + .await?; + Ok(res) + } + /// Force the message to be sent in plain text. pub fn force_plaintext(&mut self) { self.param.set_int(Param::ForcePlaintext, 1); From 3c43dc58b14d71a7ff3f9c98f2427f3af9b5594b Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 14 Oct 2024 01:05:12 +0200 Subject: [PATCH 10/23] emit DC_EVENT_MSGS_CHANGED also for the source message on saving (to let UI update saved indicator) --- src/chat.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 577ac9ee3f..ea87a2e9b5 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4239,7 +4239,7 @@ pub(crate) async fn save_copy_in_self_talk( src_msg_id: &MsgId, dest_rfc724_mid: &String, ) -> Result { - let chat_id = ChatId::create_for_contact(context, ContactId::SELF).await?; + let dest_chat_id = ChatId::create_for_contact(context, ContactId::SELF).await?; let mut msg = Message::load_from_db(context, *src_msg_id).await?; msg.param.remove(Param::Cmd); msg.param.remove(Param::WebxdcDocument); @@ -4265,7 +4265,7 @@ pub(crate) async fn save_copy_in_self_talk( FROM msgs WHERE id=?;" ), ( - chat_id, + dest_chat_id, dest_rfc724_mid, if msg.from_id == ContactId::SELF { MessageState::OutDelivered @@ -4279,7 +4279,11 @@ pub(crate) async fn save_copy_in_self_talk( ), ) .await?; - context.emit_msgs_changed(chat_id, MsgId::new(row_id.try_into()?)); + let dest_msg_id = MsgId::new(row_id.try_into()?); + + context.emit_msgs_changed(msg.chat_id, *src_msg_id); + context.emit_msgs_changed(dest_chat_id, dest_msg_id); + Ok(msg.rfc724_mid) } From ccd06ed689de7dbd50fa55b600886d55e6b21b13 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 14 Oct 2024 14:28:54 +0200 Subject: [PATCH 11/23] unify get original/saved API to return IDs --- deltachat-ffi/deltachat.h | 34 +++++++++++++++++----------------- deltachat-ffi/src/lib.rs | 29 +++++++++++++---------------- src/chat.rs | 14 +++++++------- src/html.rs | 2 +- src/message.rs | 38 +++++++++++++++++++------------------- 5 files changed, 57 insertions(+), 60 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 802e60de26..218d463da3 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -4868,33 +4868,33 @@ dc_msg_t* dc_msg_get_parent (const dc_msg_t* msg); /** - * Get original message for a saved message. + * Get original chat ID for a saved message from the "Saved Messages" chat. * - * For messages from the "Saved Messages" chat, - * this function returns the original message ID, if possible. + * Additionally, you can use dc_msg_get_original_msg_id() to find out the original message, + * which, however, may be deleted. * - * If this function returns NULL, try dc_msg_get_original_chat_id(). + * To save a message, use dc_forward_msgs(). + * To check if a message is saved, use dc_msgs_get_saved_msg_id(). * - * @param msg The message object as returned for the "Saved Messages" chat. - * @return The message object of the original message. - * NULL if the given message object is not a "Saved Message" - * or if the original message does no longer exist. + * @param msg The message object. Usually, this refers to a a message inside "Saved Messages". + * @return The chat ID of the original message. + * 0 if the given message object is not a "Saved Message" + * or if the original chat does no longer exist. */ -dc_msg_t* dc_msg_get_original_msg (const dc_msg_t* msg); +uint32_t dc_msg_get_original_chat_id (const dc_msg_t* msg); /** - * Get original chat ID for a saved message. + * Get original message ID for a saved message from the "Saved Messages" chat. * - * This may succeed even if the original message was deleted - * and dc_msg_get_original_msg() return NULL. + * Usually, UI will check dc_msg_get_original_chat_id() as well. * - * @param msg The message object as returned for the "Saved Messages" chat. - * @return The chat ID of the original message. + * @param msg The message object. Usually, this refers to a a message inside "Saved Messages". + * @return The message ID of the original message. * 0 if the given message object is not a "Saved Message" - * or if the original chat does no longer exist. + * or if the original message does no longer exist. */ -uint32_t dc_msg_get_original_chat_id (const dc_msg_t* msg); +uint32_t dc_msg_get_original_msg_id (const dc_msg_t* msg); /** @@ -4903,7 +4903,7 @@ uint32_t dc_msg_get_original_chat_id (const dc_msg_t* msg); * The returned ID can be used to un-save a message. * The state "is saved" can be used to show some icon to indicate that a message was saved. * - * @param msg The message object. + * @param msg The message object. Usually, this refers to a a message outside "Saved Messages". * @return The message ID inside "Saved Messages", if any. * 0 if the given message object is not saved. */ diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index cdfceeb761..bb38f7fa37 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -3981,33 +3981,30 @@ pub unsafe extern "C" fn dc_msg_get_parent(msg: *const dc_msg_t) -> *mut dc_msg_ } #[no_mangle] -pub unsafe extern "C" fn dc_msg_get_original_msg(msg: *const dc_msg_t) -> *mut dc_msg_t { +pub unsafe extern "C" fn dc_msg_get_original_chat_id(msg: *const dc_msg_t) -> u32 { if msg.is_null() { - eprintln!("ignoring careless call to dc_msg_get_original_msg()"); - return ptr::null_mut(); + eprintln!("ignoring careless call to dc_msg_get_original_chat_id()"); + return 0; } let ffi_msg: &MessageWrapper = &*msg; let context = &*ffi_msg.context; - let res = block_on(async move { + block_on(async move { ffi_msg .message - .get_original_msg(context) + .get_original_chat_id(context) .await - .context("failed to get original message") + .context("failed to get original chat") .log_err(context) - .unwrap_or(None) - }); - - match res { - Some(message) => Box::into_raw(Box::new(MessageWrapper { context, message })), - None => ptr::null_mut(), - } + .unwrap_or_default() + .map(|id| id.to_u32()) + .unwrap_or(0) + }) } #[no_mangle] -pub unsafe extern "C" fn dc_msg_get_original_chat_id(msg: *const dc_msg_t) -> u32 { +pub unsafe extern "C" fn dc_msg_get_original_msg_id(msg: *const dc_msg_t) -> u32 { if msg.is_null() { - eprintln!("ignoring careless call to dc_msg_get_original_chat_id()"); + eprintln!("ignoring careless call to dc_msg_get_original_msg_id()"); return 0; } let ffi_msg: &MessageWrapper = &*msg; @@ -4015,7 +4012,7 @@ pub unsafe extern "C" fn dc_msg_get_original_chat_id(msg: *const dc_msg_t) -> u3 block_on(async move { ffi_msg .message - .get_original_chat_id(context) + .get_original_msg_id(context) .await .context("failed to get original message") .log_err(context) diff --git a/src/chat.rs b/src/chat.rs index ea87a2e9b5..d34c8c4ca8 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -6923,7 +6923,7 @@ mod tests { let sent = alice.send_text(alice_chat.get_id(), "hi, bob").await; let sent_msg = Message::load_from_db(&alice, sent.sender_msg_id).await?; assert!(sent_msg.get_saved_msg_id(&alice).await?.is_none()); - assert!(sent_msg.get_original_msg(&alice).await?.is_none()); + assert!(sent_msg.get_original_msg_id(&alice).await?.is_none()); assert!(sent_msg.get_original_chat_id(&alice).await?.is_none()); let self_chat = alice.get_self_chat().await; @@ -6933,7 +6933,7 @@ mod tests { assert_ne!(saved_msg.get_id(), sent.sender_msg_id); assert!(saved_msg.get_saved_msg_id(&alice).await?.is_none()); assert_eq!( - saved_msg.get_original_msg(&alice).await?.unwrap().id, + saved_msg.get_original_msg_id(&alice).await?.unwrap(), sent.sender_msg_id ); assert_eq!( @@ -6952,7 +6952,7 @@ mod tests { sent_msg.get_saved_msg_id(&alice).await?.unwrap(), saved_msg.id ); - assert!(sent_msg.get_original_msg(&alice).await?.is_none()); + assert!(sent_msg.get_original_msg_id(&alice).await?.is_none()); assert!(sent_msg.get_original_chat_id(&alice).await?.is_none()); let rcvd_msg = bob.recv_msg(&sent).await; @@ -6961,7 +6961,7 @@ mod tests { let saved_msg = bob.get_last_msg_in(self_chat.id).await; assert_ne!(saved_msg.get_id(), rcvd_msg.id); assert_eq!( - saved_msg.get_original_msg(&bob).await?.unwrap().id, + saved_msg.get_original_msg_id(&bob).await?.unwrap(), rcvd_msg.id ); assert_eq!( @@ -6978,7 +6978,7 @@ mod tests { // delete original message rcvd_msg.id.delete_from_db(&bob).await?; let saved_msg = Message::load_from_db(&bob, saved_msg.id).await?; - assert!(saved_msg.get_original_msg(&bob).await?.is_none()); + assert!(saved_msg.get_original_msg_id(&bob).await?.is_none()); assert_eq!( saved_msg.get_original_chat_id(&bob).await?.unwrap(), rcvd_msg.chat_id @@ -7028,14 +7028,14 @@ mod tests { forward_msgs(&bob, &[orig.id], self_chat.id).await?; let saved1 = bob.get_last_msg().await; assert_eq!( - saved1.get_original_msg(&bob).await?.unwrap().id, + saved1.get_original_msg_id(&bob).await?.unwrap(), sent.sender_msg_id ); forward_msgs(&bob, &[saved1.id], self_chat.id).await?; let saved2 = bob.get_last_msg().await; assert_eq!( - saved2.get_original_msg(&bob).await?.unwrap().id, + saved2.get_original_msg_id(&bob).await?.unwrap(), sent.sender_msg_id ); diff --git a/src/html.rs b/src/html.rs index 9633304cca..decc46cf06 100644 --- a/src/html.rs +++ b/src/html.rs @@ -516,7 +516,7 @@ test some special html-characters as < > and & but also " and &#x let saved_msg = alice.get_last_msg_in(self_chat.get_id()).await; assert_ne!(saved_msg.id, msg.id); assert_eq!( - saved_msg.get_original_msg(&alice).await?.unwrap().id, + saved_msg.get_original_msg_id(&alice).await?.unwrap(), msg.id ); assert!(!saved_msg.is_forwarded()); // UI should not flag "saved messages" as "forwarded" diff --git a/src/message.rs b/src/message.rs index e323491d56..a09fba9d74 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1259,33 +1259,33 @@ impl Message { Ok(None) } - /// Returns original message object for message from "Saved Messages". - pub async fn get_original_msg(&self, context: &Context) -> Result> { + /// Returns original chat ID for message from "Saved Messages". + /// This may work although get_original_msg_id() returns 0 as the message was deleted. + pub async fn get_original_chat_id(&self, context: &Context) -> Result> { + if let Some(chat_id) = self.param.get_int(Param::OriginalChatId) { + let chat_id = ChatId::new(u32::try_from(chat_id)?); + if Chat::load_from_db(context, chat_id).await.is_ok() { + return Ok(Some(chat_id)); + } + } + Ok(None) + } + + /// Returns original message ID for message from "Saved Messages". + pub async fn get_original_msg_id(&self, context: &Context) -> Result> { if !self.original_msg_id.is_special() { if let Some(msg) = Message::load_from_db_optional(context, self.original_msg_id).await? { return if msg.chat_id.is_trash() { Ok(None) } else { - Ok(Some(msg)) + Ok(Some(msg.id)) }; } } Ok(None) } - /// Returns original chat id for message from "Saved Messages". - /// This may work although get_original_msg() returns None as the message was deleted. - pub async fn get_original_chat_id(&self, context: &Context) -> Result> { - if let Some(chat_id) = self.param.get_int(Param::OriginalChatId) { - let chat_id = ChatId::new(u32::try_from(chat_id)?); - if Chat::load_from_db(context, chat_id).await.is_ok() { - return Ok(Some(chat_id)); - } - } - Ok(None) - } - /// Check if the message was saved and returns the corresponding message inside "Saved Messages". /// UI can use this to show a symbol beside the message, indicating it was saved. /// The message can be un-saved by deleting the returned message. @@ -2523,7 +2523,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_get_original_msg() -> Result<()> { + async fn test_get_original_msg_id() -> Result<()> { let alice = TestContext::new_alice().await; let bob = TestContext::new_bob().await; @@ -2531,7 +2531,7 @@ mod tests { let one2one_chat = alice.create_chat(&bob).await; let sent = alice.send_text(one2one_chat.id, "foo").await; let orig_msg = Message::load_from_db(&alice, sent.sender_msg_id).await?; - assert!(orig_msg.get_original_msg(&alice).await?.is_none()); + assert!(orig_msg.get_original_msg_id(&alice).await?.is_none()); assert!(orig_msg.parent(&alice).await?.is_none()); assert!(orig_msg.quoted_message(&alice).await?.is_none()); @@ -2541,7 +2541,7 @@ mod tests { let saved_msg = alice.get_last_msg_in(self_chat.get_id()).await; assert_ne!(saved_msg.get_id(), orig_msg.get_id()); assert_eq!( - saved_msg.get_original_msg(&alice).await?.unwrap().get_id(), + saved_msg.get_original_msg_id(&alice).await?.unwrap(), orig_msg.get_id() ); assert!(saved_msg.parent(&alice).await?.is_none()); @@ -2552,7 +2552,7 @@ mod tests { let forwarded_msg = alice.get_last_msg_in(one2one_chat.get_id()).await; assert_ne!(forwarded_msg.get_id(), saved_msg.get_id()); assert_ne!(forwarded_msg.get_id(), orig_msg.get_id()); - assert!(forwarded_msg.get_original_msg(&alice).await?.is_none()); + assert!(forwarded_msg.get_original_msg_id(&alice).await?.is_none()); Ok(()) } From bf72686fea3be107a9ea62a4ee006ccd1db35924 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 21 Oct 2024 21:49:08 +0200 Subject: [PATCH 12/23] adapt to new deletion API --- src/chat.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chat.rs b/src/chat.rs index d34c8c4ca8..922a04e9cb 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -6976,7 +6976,7 @@ mod tests { assert_ne!(saved_msg.rfc724_mid(), rcvd_msg.rfc724_mid()); // delete original message - rcvd_msg.id.delete_from_db(&bob).await?; + delete_msgs(&bob, &[rcvd_msg.id]).await?; let saved_msg = Message::load_from_db(&bob, saved_msg.id).await?; assert!(saved_msg.get_original_msg_id(&bob).await?.is_none()); assert_eq!( From 33e1a9c6bf948e9c727e6dcde3530e1e21501659 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Thu, 16 Jan 2025 16:43:07 +0100 Subject: [PATCH 13/23] use a dedicated save_msgs() api; this would allow to force-sync even deleted messages --- deltachat-ffi/deltachat.h | 38 +++++++++++++++++++++++++++++++++----- deltachat-ffi/src/lib.rs | 20 ++++++++++++++++++++ src/chat.rs | 26 +++++++++++--------------- src/html.rs | 15 +++++++-------- src/message.rs | 8 ++++---- 5 files changed, 75 insertions(+), 32 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 218d463da3..f3aa8c7fe9 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1960,10 +1960,10 @@ void dc_delete_msgs (dc_context_t* context, const uint3 /** * Forward messages to another chat. * - * Unless forwarded to "Saved Messages", - * UI should mark forwarded messages as such. - * Whether to mark a message as forwarded can be determined by dc_msg_is_forwarded(). - * Original sender, webxdc updates and other metadata are not forwarded on purpose. + * All types of messages can be forwarded, + * however, they will be flagged as such (dc_msg_is_forwarded() is set). + * + * Original sender, info-state and webxdc updates are not forwarded on purpose. * * @memberof dc_context_t * @param context The context object. @@ -1974,6 +1974,33 @@ void dc_delete_msgs (dc_context_t* context, const uint3 void dc_forward_msgs (dc_context_t* context, const uint32_t* msg_ids, int msg_cnt, uint32_t chat_id); +/** + * Save a copy of messages in "Saved Messages". + * + * In contrast to forwarding messages, + * information as author, date and origin are preserved. + * The action completes locally, so "Saved Messages" do not show sending errors in case one is offline. + * Still, a sync message is emitted, so that other devices will save the same message, + * as long as not deleted before. + * + * To check if a message was saved, use dc_msg_get_saved_msg_id(), + * UI may show an indicator and offer an "Unsave" instead of a "Save" button then. + * + * The other way round, from inside the "Saved Messages" chat, + * UI may show in indicator checking dc_msg_get_original_msg_id() and dc_msg_get_original_chat_id() + * and offer a button to go the original chat. + * + * "Unsave" is done by deleting the saved message. + * Webxdc updates are not copied on purpose. + * + * @memberof dc_context_t + * @param context The context object. + * @param msg_ids An array of uint32_t containing all message IDs that should be saved. + * @param msg_cnt The number of messages IDs in the msg_ids array. + */ +void dc_save_msgs (dc_context_t* context, const uint32_t* msg_ids, int msg_cnt); + + /** * Resend messages and make information available for newly added chat members. * Resending sends out the original message, however, recipients and webxdc-status may differ. @@ -4384,9 +4411,10 @@ int dc_msg_is_sent (const dc_msg_t* msg); /** - * Check if the message should be marked as forwarded message. + * Check if the message is a forwarded message. * * Forwarded messages may not be created by the contact given as "from". + * * Typically, the UI shows a little text for a symbol above forwarded messages. * * For privacy reasons, we do not provide the name or the e-mail address of the diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index bb38f7fa37..150c75ec09 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -1979,6 +1979,26 @@ pub unsafe extern "C" fn dc_forward_msgs( }) } +#[no_mangle] +pub unsafe extern "C" fn dc_save_msgs( + context: *mut dc_context_t, + msg_ids: *const u32, + msg_cnt: libc::c_int, +) { + if context.is_null() || msg_ids.is_null() || msg_cnt <= 0 { + eprintln!("ignoring careless call to dc_save_msgs()"); + return; + } + let msg_ids = convert_and_prune_message_ids(msg_ids, msg_cnt); + let ctx = &*context; + + block_on(async move { + chat::save_msgs(ctx, &msg_ids[..]) + .await + .unwrap_or_log_default(ctx, "Failed to save message") + }) +} + #[no_mangle] pub unsafe extern "C" fn dc_resend_msgs( context: *mut dc_context_t, diff --git a/src/chat.rs b/src/chat.rs index 922a04e9cb..b53af03efd 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4153,11 +4153,6 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) if let Some(reason) = chat.why_cant_send(context).await? { bail!("cannot send to {}: {}", chat_id, reason); } - - if chat.is_self_talk() { - return save_copies_in_self_talk_and_sync(context, msg_ids).await; - } - curr_timestamp = create_smeared_timestamps(context, msg_ids.len()); let mut msgs = Vec::with_capacity(msg_ids.len()); for id in msg_ids { @@ -4214,7 +4209,9 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) Ok(()) } -async fn save_copies_in_self_talk_and_sync(context: &Context, msg_ids: &[MsgId]) -> Result<()> { +/// Save a copy of the message in "Saved Messages" +/// and send a sync messages so that other devices save the message as well, unless deleted there. +pub async fn save_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> { for src_msg_id in msg_ids { let dest_rfc724_mid = create_outgoing_rfc724_mid(); let src_rfc724_mid = save_copy_in_self_talk(context, src_msg_id, &dest_rfc724_mid).await?; @@ -6915,7 +6912,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_forward_to_saved() -> Result<()> { + async fn test_save_msgs() -> Result<()> { let alice = TestContext::new_alice().await; let bob = TestContext::new_bob().await; let alice_chat = alice.create_chat(&bob).await; @@ -6927,7 +6924,7 @@ mod tests { assert!(sent_msg.get_original_chat_id(&alice).await?.is_none()); let self_chat = alice.get_self_chat().await; - forward_msgs(&alice, &[sent.sender_msg_id], self_chat.id).await?; + save_msgs(&alice, &[sent.sender_msg_id]).await?; let saved_msg = alice.get_last_msg_in(self_chat.id).await; assert_ne!(saved_msg.get_id(), sent.sender_msg_id); @@ -6957,7 +6954,7 @@ mod tests { let rcvd_msg = bob.recv_msg(&sent).await; let self_chat = bob.get_self_chat().await; - forward_msgs(&bob, &[rcvd_msg.id], self_chat.id).await?; + save_msgs(&bob, &[rcvd_msg.id]).await?; let saved_msg = bob.get_last_msg_in(self_chat.id).await; assert_ne!(saved_msg.get_id(), rcvd_msg.id); assert_eq!( @@ -7004,7 +7001,7 @@ mod tests { let msg = bob.get_last_msg_in(bob_chat.id).await; let self_chat = bob.get_self_chat().await; - forward_msgs(&bob, &[msg.id], self_chat.id).await?; + save_msgs(&bob, &[msg.id]).await?; let msg = bob.get_last_msg_in(self_chat.id).await; let contact = Contact::get_by_id(&bob, msg.get_from_id()).await?; assert_eq!(contact.get_addr(), "alice@example.org"); @@ -7025,19 +7022,18 @@ mod tests { bob.recv_msg(&sent).await; let orig = bob.get_last_msg().await; let self_chat = bob.get_self_chat().await; - forward_msgs(&bob, &[orig.id], self_chat.id).await?; + save_msgs(&bob, &[orig.id]).await?; let saved1 = bob.get_last_msg().await; assert_eq!( saved1.get_original_msg_id(&bob).await?.unwrap(), sent.sender_msg_id ); + assert_ne!(saved1.from_id, ContactId::SELF); forward_msgs(&bob, &[saved1.id], self_chat.id).await?; let saved2 = bob.get_last_msg().await; - assert_eq!( - saved2.get_original_msg_id(&bob).await?.unwrap(), - sent.sender_msg_id - ); + assert!(saved2.get_original_msg_id(&bob).await?.is_none(),); + assert_eq!(saved2.from_id, ContactId::SELF); Ok(()) } diff --git a/src/html.rs b/src/html.rs index decc46cf06..1e5e5097c9 100644 --- a/src/html.rs +++ b/src/html.rs @@ -291,7 +291,7 @@ pub fn new_html_mimepart(html: String) -> PartBuilder { mod tests { use super::*; use crate::chat; - use crate::chat::{forward_msgs, ProtectionStatus}; + use crate::chat::{forward_msgs, save_msgs}; use crate::config::Config; use crate::contact::ContactId; use crate::message::{MessengerMessage, Viewtype}; @@ -500,7 +500,7 @@ test some special html-characters as < > and & but also " and &#x } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_html_forward_to_saved_messages() -> Result<()> { + async fn test_html_save_msg() -> Result<()> { // Alice receives a non-delta html-message let alice = TestContext::new_alice().await; let chat = alice @@ -510,9 +510,9 @@ test some special html-characters as < > and & but also " and &#x receive_imf(&alice, raw, false).await?; let msg = alice.get_last_msg_in(chat.get_id()).await; - // Alice forwards the message to "Saved Messages" + // Alice saves the message let self_chat = alice.get_self_chat().await; - forward_msgs(&alice, &[msg.id], self_chat.id).await?; + save_msgs(&alice, &[msg.id]).await?; let saved_msg = alice.get_last_msg_in(self_chat.get_id()).await; assert_ne!(saved_msg.id, msg.id); assert_eq!( @@ -550,10 +550,8 @@ test some special html-characters as < > and & but also " and &#x // forward the message to saved-messages, // this will encrypt the message as new_alice() has set up keys - let chat_id = alice - .create_group_with_members(ProtectionStatus::Protected, "foo", &[]) - .await; - forward_msgs(&alice, &[msg.get_id()], chat_id) + let chat = alice.get_self_chat().await; + forward_msgs(&alice, &[msg.get_id()], chat.get_id()) .await .unwrap(); let msg = alice.pop_sent_msg().await; @@ -565,6 +563,7 @@ test some special html-characters as < > and & but also " and &#x .await .unwrap(); let msg = alice.recv_msg(&msg).await; + assert_eq!(msg.chat_id, alice.get_self_chat().await.id); assert_eq!(msg.get_from_id(), ContactId::SELF); assert_eq!(msg.is_dc_message, MessengerMessage::Yes); assert!(msg.get_showpadlock()); diff --git a/src/message.rs b/src/message.rs index a09fba9d74..06d8ba934b 100644 --- a/src/message.rs +++ b/src/message.rs @@ -2212,8 +2212,8 @@ mod tests { use super::*; use crate::chat::{ - self, add_contact_to_chat, forward_msgs, marknoticed_chat, send_text_msg, ChatItem, - ProtectionStatus, + self, add_contact_to_chat, forward_msgs, marknoticed_chat, save_msgs, send_text_msg, + ChatItem, ProtectionStatus, }; use crate::chatlist::Chatlist; use crate::config::Config; @@ -2537,7 +2537,7 @@ mod tests { // forwarding to "Saved Messages", the message gets the original ID attached let self_chat = alice.get_self_chat().await; - forward_msgs(&alice, &[sent.sender_msg_id], self_chat.get_id()).await?; + save_msgs(&alice, &[sent.sender_msg_id]).await?; let saved_msg = alice.get_last_msg_in(self_chat.get_id()).await; assert_ne!(saved_msg.get_id(), orig_msg.get_id()); assert_eq!( @@ -2547,7 +2547,7 @@ mod tests { assert!(saved_msg.parent(&alice).await?.is_none()); assert!(saved_msg.quoted_message(&alice).await?.is_none()); - // forwarding from "Saved Messages" back to another chat, the original ID attached + // forwarding from "Saved Messages" back to another chat, detaches original ID forward_msgs(&alice, &[saved_msg.get_id()], one2one_chat.get_id()).await?; let forwarded_msg = alice.get_last_msg_in(one2one_chat.get_id()).await; assert_ne!(forwarded_msg.get_id(), saved_msg.get_id()); From 9d492106036b58e620cbe61a3300d40baa42a55e Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Fri, 17 Jan 2025 12:39:37 +0100 Subject: [PATCH 14/23] we need to emit more events, chatlist*changed was added recently --- src/chat.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/chat.rs b/src/chat.rs index b53af03efd..55576484c7 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4280,6 +4280,8 @@ pub(crate) async fn save_copy_in_self_talk( context.emit_msgs_changed(msg.chat_id, *src_msg_id); context.emit_msgs_changed(dest_chat_id, dest_msg_id); + chatlist_events::emit_chatlist_changed(context); + chatlist_events::emit_chatlist_item_changed(context, dest_chat_id); Ok(msg.rfc724_mid) } From c1c75e816817e4a04f14b7093c7a1cf93ba9ddac Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Fri, 17 Jan 2025 12:44:34 +0100 Subject: [PATCH 15/23] adapt test name to new save_msgs() api --- src/chat.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chat.rs b/src/chat.rs index 55576484c7..3c1a5186c4 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -6992,7 +6992,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_forward_to_saved_is_not_added_to_shared_chats() -> Result<()> { + async fn test_saved_msgs_not_added_to_shared_chats() -> Result<()> { let alice = TestContext::new_alice().await; let bob = TestContext::new_bob().await; let alice_chat = alice.create_chat(&bob).await; From 5d5e71423dcf9862eb3cc8e0159962043d73837d Mon Sep 17 00:00:00 2001 From: bjoern Date: Fri, 17 Jan 2025 17:15:08 +0100 Subject: [PATCH 16/23] Update src/chat.rs Co-authored-by: Hocuri --- src/chat.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 3c1a5186c4..c5ee34889e 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -6993,14 +6993,11 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_saved_msgs_not_added_to_shared_chats() -> Result<()> { - let alice = TestContext::new_alice().await; - let bob = TestContext::new_bob().await; - let alice_chat = alice.create_chat(&bob).await; - let bob_chat = bob.create_chat(&alice).await; + let mut tcm = TestContextManager::new(); + let alice = tcm.alice().await; + let bob = tcm.bob().await; - let sent = alice.send_text(alice_chat.id, "hi, bob").await; - bob.recv_msg(&sent).await; - let msg = bob.get_last_msg_in(bob_chat.id).await; + let msg = tcm.send_recv_accept(&alice, &bob, "hi, bob").await; let self_chat = bob.get_self_chat().await; save_msgs(&bob, &[msg.id]).await?; From 38d926f6b1077401aaef78ddd7c3b16f290fb6a0 Mon Sep 17 00:00:00 2001 From: bjoern Date: Fri, 17 Jan 2025 17:15:30 +0100 Subject: [PATCH 17/23] Update src/chat.rs Co-authored-by: Hocuri --- src/chat.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/chat.rs b/src/chat.rs index c5ee34889e..291200ff55 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -7007,7 +7007,10 @@ mod tests { let shared_chats = Chatlist::try_load(&bob, 0, None, Some(contact.id)).await?; assert_eq!(shared_chats.len(), 1); - assert_eq!(shared_chats.get_chat_id(0).unwrap(), bob_chat.id); + assert_eq!( + shared_chats.get_chat_id(0).unwrap(), + bob.get_chat(&alice).await.id + ); Ok(()) } From 74a22cb3f6f19569253ff25aada383947d43065c Mon Sep 17 00:00:00 2001 From: bjoern Date: Fri, 17 Jan 2025 17:15:42 +0100 Subject: [PATCH 18/23] Update src/sync.rs Co-authored-by: Hocuri --- src/sync.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sync.rs b/src/sync.rs index 0411edd387..fbfdf1b07c 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -63,8 +63,8 @@ pub(crate) enum SyncData { val: String, }, SaveMessage { - src: String, // RFC724 id - dest: String, // RFC724 id + src: String, // RFC724 id (i.e. "Message-Id" header) + dest: String, // RFC724 id (i.e. "Message-Id" header) }, } From 0aa254219df9fee12bb3e28def705034b7550936 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Sun, 19 Jan 2025 00:10:47 +0100 Subject: [PATCH 19/23] tweak and fix documentation --- deltachat-ffi/deltachat.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index f3aa8c7fe9..89fa7eb89e 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1987,12 +1987,15 @@ void dc_forward_msgs (dc_context_t* context, const uint3 * UI may show an indicator and offer an "Unsave" instead of a "Save" button then. * * The other way round, from inside the "Saved Messages" chat, - * UI may show in indicator checking dc_msg_get_original_msg_id() and dc_msg_get_original_chat_id() + * UI may show an indicator checking dc_msg_get_original_msg_id() and dc_msg_get_original_chat_id() * and offer a button to go the original chat. * * "Unsave" is done by deleting the saved message. * Webxdc updates are not copied on purpose. * + * For performance reasons, esp. when saving lots of messages, + * UI should call this function from a background thread. + * * @memberof dc_context_t * @param context The context object. * @param msg_ids An array of uint32_t containing all message IDs that should be saved. From 933c1f545225f3bc9ec0db6e057c6643b91ad803 Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Sun, 19 Jan 2025 13:20:47 +0100 Subject: [PATCH 20/23] remove get_original_msg_id() agree with @Hoccuri after thinking it over, the limited cornercase of deleted message but not deleted chat seems not to be worth additional effort and states - and may even worsen UX as it is unclear what happens when tapping the "goto original" button (or one need to use different icons or sth) if we want to do sth. about that, it might be better to go the way we went for quotes: so, eg. keep a copy of the chatname. but let's see if this is really needed and an issue at scale. --- deltachat-ffi/deltachat.h | 24 ++++-------------------- deltachat-ffi/src/lib.rs | 21 --------------------- src/chat.rs | 18 +----------------- src/message.rs | 12 ------------ src/param.rs | 4 +--- 5 files changed, 6 insertions(+), 73 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 89fa7eb89e..1280140543 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -1987,8 +1987,8 @@ void dc_forward_msgs (dc_context_t* context, const uint3 * UI may show an indicator and offer an "Unsave" instead of a "Save" button then. * * The other way round, from inside the "Saved Messages" chat, - * UI may show an indicator checking dc_msg_get_original_msg_id() and dc_msg_get_original_chat_id() - * and offer a button to go the original chat. + * UI may show the indicator and "Unsave" button checking dc_msg_get_original_msg_id() + * and offer a button to go the original message. * * "Unsave" is done by deleting the saved message. * Webxdc updates are not copied on purpose. @@ -4898,27 +4898,11 @@ dc_msg_t* dc_msg_get_quoted_msg (const dc_msg_t* msg); dc_msg_t* dc_msg_get_parent (const dc_msg_t* msg); -/** - * Get original chat ID for a saved message from the "Saved Messages" chat. - * - * Additionally, you can use dc_msg_get_original_msg_id() to find out the original message, - * which, however, may be deleted. - * - * To save a message, use dc_forward_msgs(). - * To check if a message is saved, use dc_msgs_get_saved_msg_id(). - * - * @param msg The message object. Usually, this refers to a a message inside "Saved Messages". - * @return The chat ID of the original message. - * 0 if the given message object is not a "Saved Message" - * or if the original chat does no longer exist. - */ -uint32_t dc_msg_get_original_chat_id (const dc_msg_t* msg); - - /** * Get original message ID for a saved message from the "Saved Messages" chat. * - * Usually, UI will check dc_msg_get_original_chat_id() as well. + * Can be used by UI to show a button to go the original message + * and an option to "Unsave" the message. * * @param msg The message object. Usually, this refers to a a message inside "Saved Messages". * @return The message ID of the original message. diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 150c75ec09..aaaf8a4ab7 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4000,27 +4000,6 @@ pub unsafe extern "C" fn dc_msg_get_parent(msg: *const dc_msg_t) -> *mut dc_msg_ } } -#[no_mangle] -pub unsafe extern "C" fn dc_msg_get_original_chat_id(msg: *const dc_msg_t) -> u32 { - if msg.is_null() { - eprintln!("ignoring careless call to dc_msg_get_original_chat_id()"); - return 0; - } - let ffi_msg: &MessageWrapper = &*msg; - let context = &*ffi_msg.context; - block_on(async move { - ffi_msg - .message - .get_original_chat_id(context) - .await - .context("failed to get original chat") - .log_err(context) - .unwrap_or_default() - .map(|id| id.to_u32()) - .unwrap_or(0) - }) -} - #[no_mangle] pub unsafe extern "C" fn dc_msg_get_original_msg_id(msg: *const dc_msg_t) -> u32 { if msg.is_null() { diff --git a/src/chat.rs b/src/chat.rs index 291200ff55..f853f4cf99 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4243,8 +4243,6 @@ pub(crate) async fn save_copy_in_self_talk( msg.param.remove(Param::WebxdcDocumentTimestamp); msg.param.remove(Param::WebxdcSummary); msg.param.remove(Param::WebxdcSummaryTimestamp); - msg.param - .set_int(Param::OriginalChatId, msg.chat_id.to_u32() as i32); let original_msg_id = if !msg.original_msg_id.is_special() { msg.original_msg_id } else { @@ -6923,7 +6921,6 @@ mod tests { let sent_msg = Message::load_from_db(&alice, sent.sender_msg_id).await?; assert!(sent_msg.get_saved_msg_id(&alice).await?.is_none()); assert!(sent_msg.get_original_msg_id(&alice).await?.is_none()); - assert!(sent_msg.get_original_chat_id(&alice).await?.is_none()); let self_chat = alice.get_self_chat().await; save_msgs(&alice, &[sent.sender_msg_id]).await?; @@ -6935,10 +6932,6 @@ mod tests { saved_msg.get_original_msg_id(&alice).await?.unwrap(), sent.sender_msg_id ); - assert_eq!( - saved_msg.get_original_chat_id(&alice).await?.unwrap(), - sent_msg.chat_id - ); assert_eq!(saved_msg.get_text(), "hi, bob"); assert!(!saved_msg.is_forwarded()); // UI should not flag "saved messages" as "forwarded" assert_eq!(saved_msg.is_dc_message, MessengerMessage::Yes); @@ -6952,7 +6945,6 @@ mod tests { saved_msg.id ); assert!(sent_msg.get_original_msg_id(&alice).await?.is_none()); - assert!(sent_msg.get_original_chat_id(&alice).await?.is_none()); let rcvd_msg = bob.recv_msg(&sent).await; let self_chat = bob.get_self_chat().await; @@ -6963,10 +6955,6 @@ mod tests { saved_msg.get_original_msg_id(&bob).await?.unwrap(), rcvd_msg.id ); - assert_eq!( - saved_msg.get_original_chat_id(&bob).await?.unwrap(), - rcvd_msg.chat_id - ); assert_eq!(saved_msg.get_text(), "hi, bob"); assert!(!saved_msg.is_forwarded()); assert_eq!(saved_msg.is_dc_message, MessengerMessage::Yes); @@ -6978,15 +6966,11 @@ mod tests { delete_msgs(&bob, &[rcvd_msg.id]).await?; let saved_msg = Message::load_from_db(&bob, saved_msg.id).await?; assert!(saved_msg.get_original_msg_id(&bob).await?.is_none()); - assert_eq!( - saved_msg.get_original_chat_id(&bob).await?.unwrap(), - rcvd_msg.chat_id - ); // delete original chat rcvd_msg.chat_id.delete(&bob).await?; let msg = Message::load_from_db(&bob, saved_msg.id).await?; - assert!(msg.get_original_chat_id(&bob).await?.is_none()); + assert!(msg.get_original_msg_id(&bob).await?.is_none()); Ok(()) } diff --git a/src/message.rs b/src/message.rs index 06d8ba934b..a9c908bd5f 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1259,18 +1259,6 @@ impl Message { Ok(None) } - /// Returns original chat ID for message from "Saved Messages". - /// This may work although get_original_msg_id() returns 0 as the message was deleted. - pub async fn get_original_chat_id(&self, context: &Context) -> Result> { - if let Some(chat_id) = self.param.get_int(Param::OriginalChatId) { - let chat_id = ChatId::new(u32::try_from(chat_id)?); - if Chat::load_from_db(context, chat_id).await.is_ok() { - return Ok(Some(chat_id)); - } - } - Ok(None) - } - /// Returns original message ID for message from "Saved Messages". pub async fn get_original_msg_id(&self, context: &Context) -> Result> { if !self.original_msg_id.is_special() { diff --git a/src/param.rs b/src/param.rs index 5bd54b86c0..e9255d69f4 100644 --- a/src/param.rs +++ b/src/param.rs @@ -207,9 +207,7 @@ pub enum Param { /// For messages: Whether [crate::message::Viewtype::Sticker] should be forced. ForceSticker = b'X', - - /// For saved messages: The original ChatId, in case the original message is deleted - OriginalChatId = b'L', + // 'L' was defined as ProtectionSettingsTimestamp for Chats, however, never used in production. } /// An object for handling key=value parameter lists. From b818e1aa2aab9dc9182b7dcd70aa34a3d0d1000d Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Sun, 19 Jan 2025 13:49:23 +0100 Subject: [PATCH 21/23] trying to save an already saved message is an error; UI should offer 'Unsave' instead --- src/chat.rs | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index f853f4cf99..8c501b0fa2 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4243,11 +4243,10 @@ pub(crate) async fn save_copy_in_self_talk( msg.param.remove(Param::WebxdcDocumentTimestamp); msg.param.remove(Param::WebxdcSummary); msg.param.remove(Param::WebxdcSummaryTimestamp); - let original_msg_id = if !msg.original_msg_id.is_special() { - msg.original_msg_id - } else { - *src_msg_id - }; + + if !msg.original_msg_id.is_unset() { + bail!("message already saved."); + } let copy_fields = "from_id, to_id, timestamp_sent, timestamp_rcvd, type, txt, txt_raw, \ mime_modified, mime_headers, mime_compressed, mime_in_reply_to, subject, msgrmsg"; @@ -4269,7 +4268,7 @@ pub(crate) async fn save_copy_in_self_talk( }, create_smeared_timestamp(context), msg.param.to_string(), - original_msg_id, + src_msg_id, src_msg_id, ), ) @@ -7024,6 +7023,23 @@ mod tests { Ok(()) } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_save_from_saved_to_saved_failing() -> Result<()> { + let alice = TestContext::new_alice().await; + let bob = TestContext::new_bob().await; + let sent = alice.send_text(alice.create_chat(&bob).await.id, "k").await; + + bob.recv_msg(&sent).await; + let orig = bob.get_last_msg().await; + save_msgs(&bob, &[orig.id]).await?; + let saved1 = bob.get_last_msg().await; + + let result = save_msgs(&bob, &[saved1.id]).await; + assert!(result.is_err()); + + Ok(()) + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_resend_own_message() -> Result<()> { // Alice creates group with Bob and sends an initial message From fd3e0c607781c797c27e811e972533572b89e5ff Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Sun, 19 Jan 2025 13:59:55 +0100 Subject: [PATCH 22/23] remove outdated comment --- src/sql/migrations.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index a7bb6236c5..2532d68ad9 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -116,8 +116,6 @@ CREATE INDEX msgs_mdns_index1 ON msgs_mdns (msg_id);"#, r#" ALTER TABLE chats ADD COLUMN archived INTEGER DEFAULT 0; CREATE INDEX chats_index2 ON chats (archived); --- 'starred' column is not used currently --- (dropping is not easily doable and stop adding it will make reusing it complicated) ALTER TABLE msgs ADD COLUMN starred INTEGER DEFAULT 0; CREATE INDEX msgs_index5 ON msgs (starred);"#, 17, From f8c13836a8311e73eb11cb305878546556ff6c76 Mon Sep 17 00:00:00 2001 From: bjoern Date: Sun, 19 Jan 2025 16:30:49 +0100 Subject: [PATCH 23/23] Update deltachat-ffi/deltachat.h Co-authored-by: Hocuri --- deltachat-ffi/deltachat.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 1280140543..66ae3b2fd6 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -4915,7 +4915,7 @@ uint32_t dc_msg_get_original_msg_id (const dc_msg_t* msg); /** * Check if a message was saved and return its ID inside "Saved Messages". * - * The returned ID can be used to un-save a message. + * Deleting the returned message will un-save the message. * The state "is saved" can be used to show some icon to indicate that a message was saved. * * @param msg The message object. Usually, this refers to a a message outside "Saved Messages".