do not add legacy info-messages on resending (#3389)

* do not wipe info for drafts

* drafts and received webxdc-instances, resent or not, do not trigger info-messages

* test that resending a webxdc does not not add legacy info-messages

* make clippy happy

* update CHANGELOG
This commit is contained in:
bjoern
2022-06-01 21:38:15 +02:00
committed by GitHub
parent 9e0decb6cb
commit e1e5803067
2 changed files with 114 additions and 67 deletions

View File

@@ -10,6 +10,7 @@
### Fixes ### Fixes
- delete outgoing MDNs found in the Sent folder on Gmail #3372 - delete outgoing MDNs found in the Sent folder on Gmail #3372
- fix searching one-to-one chats #3377 - fix searching one-to-one chats #3377
- do not add legacy info-messages on resending webxdc #3389
## 1.84.0 ## 1.84.0

View File

@@ -202,41 +202,33 @@ impl Context {
instance: &mut Message, instance: &mut Message,
update_str: &str, update_str: &str,
timestamp: i64, timestamp: i64,
can_info_msg: bool,
) -> Result<StatusUpdateSerial> { ) -> Result<StatusUpdateSerial> {
let update_str = update_str.trim(); let update_str = update_str.trim();
if update_str.is_empty() { if update_str.is_empty() {
bail!("create_status_update_record: empty update."); bail!("create_status_update_record: empty update.");
} }
let status_update_item: StatusUpdateItem = { let status_update_item: StatusUpdateItem =
if let Ok(item) = serde_json::from_str::<StatusUpdateItem>(update_str) { if let Ok(item) = serde_json::from_str::<StatusUpdateItem>(update_str) {
match instance.state { item
MessageState::Undefined
| MessageState::OutPreparing
| MessageState::OutDraft => StatusUpdateItem {
payload: item.payload,
info: None, // no info-messages in draft mode
document: item.document,
summary: item.summary,
},
_ => item,
}
} else { } else {
bail!("create_status_update_record: no valid update item."); bail!("create_status_update_record: no valid update item.");
} };
};
if let Some(ref info) = status_update_item.info { if can_info_msg {
chat::add_info_msg_with_cmd( if let Some(ref info) = status_update_item.info {
self, chat::add_info_msg_with_cmd(
instance.chat_id, self,
info.as_str(), instance.chat_id,
SystemMessage::Unknown, info.as_str(),
timestamp, SystemMessage::Unknown,
None, timestamp,
Some(instance), None,
) Some(instance),
.await?; )
.await?;
}
} }
let mut param_changed = false; let mut param_changed = false;
@@ -305,46 +297,50 @@ impl Context {
let chat = Chat::load_from_db(self, instance.chat_id).await?; let chat = Chat::load_from_db(self, instance.chat_id).await?;
ensure!(chat.can_send(self).await?, "cannot send to {}", chat.id); ensure!(chat.can_send(self).await?, "cannot send to {}", chat.id);
let send_now = !matches!(
instance.state,
MessageState::Undefined | MessageState::OutPreparing | MessageState::OutDraft
);
let status_update_serial = self let status_update_serial = self
.create_status_update_record( .create_status_update_record(
&mut instance, &mut instance,
update_str, update_str,
dc_create_smeared_timestamp(self).await, dc_create_smeared_timestamp(self).await,
send_now,
) )
.await?; .await?;
match instance.state {
MessageState::Undefined | MessageState::OutPreparing | MessageState::OutDraft => { if send_now {
// send update once the instance is actually send // send update now
Ok(None) // (also send updates on MessagesState::Failed, maybe only one member cannot receive)
} let mut status_update = Message {
_ => { chat_id: instance.chat_id,
// send update now viewtype: Viewtype::Text,
// (also send updates on MessagesState::Failed, maybe only one member cannot receive) text: Some(descr.to_string()),
let mut status_update = Message { hidden: true,
chat_id: instance.chat_id, ..Default::default()
viewtype: Viewtype::Text, };
text: Some(descr.to_string()), status_update
hidden: true, .param
..Default::default() .set_cmd(SystemMessage::WebxdcStatusUpdate);
}; status_update.param.set(
status_update Param::Arg,
.param self.render_webxdc_status_update_object(
.set_cmd(SystemMessage::WebxdcStatusUpdate); instance_msg_id,
status_update.param.set( Some(status_update_serial),
Param::Arg, )
self.render_webxdc_status_update_object( .await?
instance_msg_id, .ok_or_else(|| format_err!("Status object expected."))?,
Some(status_update_serial), );
) status_update.set_quote(self, Some(&instance)).await?;
.await? status_update.param.remove(Param::GuaranteeE2ee); // may be set by set_quote(), if #2985 is done, this line can be removed
.ok_or_else(|| format_err!("Status object expected."))?, let status_update_msg_id =
); chat::send_msg(self, instance.chat_id, &mut status_update).await?;
status_update.set_quote(self, Some(&instance)).await?; Ok(Some(status_update_msg_id))
status_update.param.remove(Param::GuaranteeE2ee); // may be set by set_quote(), if #2985 is done, this line can be removed } else {
let status_update_msg_id = // send update once the instance is actually send
chat::send_msg(self, instance.chat_id, &mut status_update).await?; Ok(None)
Ok(Some(status_update_msg_id))
}
} }
} }
@@ -368,11 +364,11 @@ impl Context {
/// the array is parsed using serde, the single payloads are used as is. /// the array is parsed using serde, the single payloads are used as is.
pub(crate) async fn receive_status_update(&self, msg_id: MsgId, json: &str) -> Result<()> { pub(crate) async fn receive_status_update(&self, msg_id: MsgId, json: &str) -> Result<()> {
let msg = Message::load_from_db(self, msg_id).await?; let msg = Message::load_from_db(self, msg_id).await?;
let (timestamp, mut instance) = if msg.viewtype == Viewtype::Webxdc { let (timestamp, mut instance, can_info_msg) = if msg.viewtype == Viewtype::Webxdc {
(msg.timestamp_sort, msg) (msg.timestamp_sort, msg, false)
} else if let Some(parent) = msg.parent(self).await? { } else if let Some(parent) = msg.parent(self).await? {
if parent.viewtype == Viewtype::Webxdc { if parent.viewtype == Viewtype::Webxdc {
(msg.timestamp_sort, parent) (msg.timestamp_sort, parent, true)
} else { } else {
bail!("receive_status_update: message is not the child of a webxdc message.") bail!("receive_status_update: message is not the child of a webxdc message.")
} }
@@ -386,6 +382,7 @@ impl Context {
&mut instance, &mut instance,
&*serde_json::to_string(&update_item)?, &*serde_json::to_string(&update_item)?,
timestamp, timestamp,
can_info_msg,
) )
.await?; .await?;
} }
@@ -620,8 +617,8 @@ mod tests {
use async_std::io::WriteExt; use async_std::io::WriteExt;
use crate::chat::{ use crate::chat::{
add_contact_to_chat, create_group_chat, forward_msgs, send_msg, send_text_msg, ChatId, add_contact_to_chat, create_group_chat, forward_msgs, resend_msgs, send_msg, send_text_msg,
ProtectionStatus, ChatId, ProtectionStatus,
}; };
use crate::chatlist::Chatlist; use crate::chatlist::Chatlist;
use crate::config::Config; use crate::config::Config;
@@ -801,6 +798,51 @@ mod tests {
Ok(()) Ok(())
} }
#[async_std::test]
async fn test_resend_webxdc_instance_and_info() -> Result<()> {
// Alice uses webxdc in a group
let alice = TestContext::new_alice().await;
let alice_grp = create_group_chat(&alice, ProtectionStatus::Unprotected, "grp").await?;
let alice_instance = send_webxdc_instance(&alice, alice_grp).await?;
assert_eq!(alice_grp.get_msg_cnt(&alice).await?, 1);
alice
.send_webxdc_status_update(
alice_instance.id,
r#"{"payload":7,"info": "i","summary":"s"}"#,
"d",
)
.await?;
assert_eq!(alice_grp.get_msg_cnt(&alice).await?, 2);
assert!(alice.get_last_msg_in(alice_grp).await.is_info());
// Alice adds Bob and resend already used webxdc
add_contact_to_chat(
&alice,
alice_grp,
Contact::create(&alice, "", "bob@example.net").await?,
)
.await?;
assert_eq!(alice_grp.get_msg_cnt(&alice).await?, 3);
resend_msgs(&alice, &[alice_instance.id]).await?;
let sent1 = alice.pop_sent_msg().await;
// Bob received webxdc, legacy info-messages updates are received but not added to the chat
let bob = TestContext::new_bob().await;
let bob_instance = bob.recv_msg(&sent1).await;
assert_eq!(bob_instance.viewtype, Viewtype::Webxdc);
assert!(!bob_instance.is_info());
assert_eq!(
bob.get_webxdc_status_updates(bob_instance.id, StatusUpdateSerial(0))
.await?,
r#"[{"payload":7,"info":"i","summary":"s","serial":1,"max_serial":1}]"#
);
let bob_grp = bob_instance.chat_id;
assert_eq!(bob.get_last_msg_in(bob_grp).await.id, bob_instance.id);
assert_eq!(bob_grp.get_msg_cnt(&bob).await?, 1);
Ok(())
}
#[async_std::test] #[async_std::test]
async fn test_receive_webxdc_instance() -> Result<()> { async fn test_receive_webxdc_instance() -> Result<()> {
let t = TestContext::new_alice().await; let t = TestContext::new_alice().await;
@@ -926,6 +968,7 @@ mod tests {
&mut instance, &mut instance,
"\n\n{\"payload\": {\"foo\":\"bar\"}}\n", "\n\n{\"payload\": {\"foo\":\"bar\"}}\n",
1640178619, 1640178619,
true,
) )
.await?; .await?;
assert_eq!( assert_eq!(
@@ -935,11 +978,11 @@ mod tests {
); );
assert!(t assert!(t
.create_status_update_record(&mut instance, "\n\n\n", 1640178619) .create_status_update_record(&mut instance, "\n\n\n", 1640178619, true)
.await .await
.is_err()); .is_err());
assert!(t assert!(t
.create_status_update_record(&mut instance, "bad json", 1640178619) .create_status_update_record(&mut instance, "bad json", 1640178619, true)
.await .await
.is_err()); .is_err());
assert_eq!( assert_eq!(
@@ -953,13 +996,14 @@ mod tests {
&mut instance, &mut instance,
r#"{"payload" : { "foo2":"bar2"}}"#, r#"{"payload" : { "foo2":"bar2"}}"#,
1640178619, 1640178619,
true,
) )
.await?; .await?;
assert_eq!( assert_eq!(
t.get_webxdc_status_updates(instance.id, update_id1).await?, t.get_webxdc_status_updates(instance.id, update_id1).await?,
r#"[{"payload":{"foo2":"bar2"},"serial":2,"max_serial":2}]"# r#"[{"payload":{"foo2":"bar2"},"serial":2,"max_serial":2}]"#
); );
t.create_status_update_record(&mut instance, r#"{"payload":true}"#, 1640178619) t.create_status_update_record(&mut instance, r#"{"payload":true}"#, 1640178619, true)
.await?; .await?;
assert_eq!( assert_eq!(
t.get_webxdc_status_updates(instance.id, StatusUpdateSerial(0)) t.get_webxdc_status_updates(instance.id, StatusUpdateSerial(0))
@@ -974,6 +1018,7 @@ mod tests {
&mut instance, &mut instance,
r#"{"payload" : 1, "sender": "that is not used"}"#, r#"{"payload" : 1, "sender": "that is not used"}"#,
1640178619, 1640178619,
true,
) )
.await?; .await?;
assert_eq!( assert_eq!(
@@ -1243,8 +1288,9 @@ mod tests {
bob.get_webxdc_status_updates(bob_instance.id, StatusUpdateSerial(0)) bob.get_webxdc_status_updates(bob_instance.id, StatusUpdateSerial(0))
.await?, .await?,
r#"[{"payload":{"foo":"bar"},"serial":1,"max_serial":2}, r#"[{"payload":{"foo":"bar"},"serial":1,"max_serial":2},
{"payload":42,"serial":2,"max_serial":2}]"# // 'info: "i"' ignored as sent in draft mode {"payload":42,"info":"i","serial":2,"max_serial":2}]"#
); );
assert!(!bob.get_last_msg().await.is_info()); // 'info: "i"' message not added in draft mode
Ok(()) Ok(())
} }