feat: show reactions in summaries (#5387)

shows the last reaction in chatlist's summaries if there is no
newer message.

the reason to show reactions in the summary, is to make them a _little_
more visible when one is not in the chat. esp. in not-so-chatty or in
one-to-ones chats this becomes handy: imaging a question and someone
"answers" with "thumbs up" ... 

otoh, reactions are still tuned down on purpose: no notifications, chats
are opend as usual, the chatlist is not sorted by reactions and also the
date in the summary refer to the last message - i thought quite a bit
about that, this seems to be good compromise and will raise the fewest
questions. it is somehow clear to the users that reactions are not the
same as a real message. also, it is comparable easy to implement - no
UI changes required :)

all that is very close to what whatsapp is doing (figured that out by
quite some testing ... to cite @adbenitez: if in doubt, we can blame
whatsapp :)

technically, i first wanted to go for the "big solution" and add two
more columns, chat_id and timestamp, however, it seemed a bit bloated if
we really only need the last one. therefore, i just added the last
reaction information to the chat's param, which seems more performant
but also easier to code :)
This commit is contained in:
bjoern
2024-04-03 10:50:05 +02:00
committed by GitHub
parent 3f35b442c3
commit ace281ff6c
8 changed files with 254 additions and 9 deletions

View File

@@ -20,11 +20,12 @@ use std::fmt;
use anyhow::Result;
use crate::chat::{send_msg, ChatId};
use crate::chat::{send_msg, Chat, ChatId};
use crate::contact::ContactId;
use crate::context::Context;
use crate::events::EventType;
use crate::message::{rfc724_mid_exists, Message, MsgId, Viewtype};
use crate::param::Param;
/// A single reaction consisting of multiple emoji sequences.
///
@@ -170,6 +171,7 @@ async fn set_msg_id_reaction(
msg_id: MsgId,
chat_id: ChatId,
contact_id: ContactId,
timestamp: i64,
reaction: Reaction,
) -> Result<()> {
if reaction.is_empty() {
@@ -194,6 +196,17 @@ async fn set_msg_id_reaction(
(msg_id, contact_id, reaction.as_str()),
)
.await?;
let mut chat = Chat::load_from_db(context, chat_id).await?;
if chat
.param
.update_timestamp(Param::LastReactionTimestamp, timestamp)?
{
chat.param
.set_i64(Param::LastReactionMsgId, i64::from(msg_id.to_u32()));
chat.param
.set_i64(Param::LastReactionContactId, i64::from(contact_id.to_u32()));
chat.update_param(context).await?;
}
}
context.emit_event(EventType::ReactionsChanged {
@@ -223,7 +236,15 @@ pub async fn send_reaction(context: &Context, msg_id: MsgId, reaction: &str) ->
let reaction_msg_id = send_msg(context, chat_id, &mut reaction_msg).await?;
// Only set reaction if we successfully sent the message.
set_msg_id_reaction(context, msg_id, msg.chat_id, ContactId::SELF, reaction).await?;
set_msg_id_reaction(
context,
msg_id,
msg.chat_id,
ContactId::SELF,
reaction_msg.timestamp_sort,
reaction,
)
.await?;
Ok(reaction_msg_id)
}
@@ -250,10 +271,11 @@ pub(crate) async fn set_msg_reaction(
in_reply_to: &str,
chat_id: ChatId,
contact_id: ContactId,
timestamp: i64,
reaction: Reaction,
) -> Result<()> {
if let Some((msg_id, _)) = rfc724_mid_exists(context, in_reply_to).await? {
set_msg_id_reaction(context, msg_id, chat_id, contact_id, reaction).await
set_msg_id_reaction(context, msg_id, chat_id, contact_id, timestamp, reaction).await
} else {
info!(
context,
@@ -307,18 +329,68 @@ pub async fn get_msg_reactions(context: &Context, msg_id: MsgId) -> Result<React
Ok(Reactions { reactions })
}
impl Chat {
/// Check if there is a reaction newer than the given timestamp.
///
/// If so, reaction details are returned and can be used to create a summary string.
pub async fn get_last_reaction_if_newer_than(
&self,
context: &Context,
timestamp: i64,
) -> Result<Option<(Message, ContactId, String)>> {
if let Some(reaction_timestamp) = self.param.get_i64(Param::LastReactionTimestamp) {
if reaction_timestamp > timestamp {
let reaction_msg = Message::load_from_db(
context,
MsgId::new(
self.param
.get_int(Param::LastReactionMsgId)
.unwrap_or_default() as u32,
),
)
.await?;
if !reaction_msg.chat_id.is_trash() {
let reaction_contact_id = ContactId::new(
self.param
.get_int(Param::LastReactionContactId)
.unwrap_or_default() as u32,
);
if let Some(reaction) = context
.sql
.query_row_optional(
r#"SELECT reaction FROM reactions WHERE msg_id=? AND contact_id=?"#,
(reaction_msg.id, reaction_contact_id),
|row| {
let reaction: String = row.get(0)?;
Ok(reaction)
},
)
.await?
{
return Ok(Some((reaction_msg, reaction_contact_id, reaction)));
}
}
}
}
Ok(None)
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::chat::{get_chat_msgs, send_text_msg};
use crate::chatlist::Chatlist;
use crate::config::Config;
use crate::constants::DC_CHAT_ID_TRASH;
use crate::contact::{Contact, ContactAddress, Origin};
use crate::download::DownloadState;
use crate::message::MessageState;
use crate::message::{delete_msgs, MessageState};
use crate::receive_imf::{receive_imf, receive_imf_from_inbox};
use crate::test_utils::TestContext;
use crate::test_utils::TestContextManager;
use crate::tools::SystemTime;
use std::time::Duration;
#[test]
fn test_parse_reaction() {
@@ -549,6 +621,107 @@ Here's my footer -- bob@example.net"
Ok(())
}
async fn assert_summary(t: &TestContext, expected: &str) {
let chatlist = Chatlist::try_load(t, 0, None, None).await.unwrap();
let summary = chatlist.get_summary(t, 0, None).await.unwrap();
assert_eq!(summary.text, expected);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_reaction_summary() -> Result<()> {
let alice = TestContext::new_alice().await;
let bob = TestContext::new_bob().await;
alice.set_config(Config::Displayname, Some("ALICE")).await?;
bob.set_config(Config::Displayname, Some("BOB")).await?;
// Alice sends message to Bob
let alice_chat = alice.create_chat(&bob).await;
let alice_msg1 = alice.send_text(alice_chat.id, "Party?").await;
let bob_msg1 = bob.recv_msg(&alice_msg1).await;
// Bob reacts to Alice's message, this is shown in the summaries
SystemTime::shift(Duration::from_secs(10));
bob_msg1.chat_id.accept(&bob).await?;
send_reaction(&bob, bob_msg1.id, "👍").await?;
let bob_send_reaction = bob.pop_sent_msg().await;
let alice_rcvd_reaction = alice.recv_msg(&bob_send_reaction).await;
assert!(alice_rcvd_reaction.get_timestamp() > bob_msg1.get_timestamp());
let chatlist = Chatlist::try_load(&bob, 0, None, None).await?;
let summary = chatlist.get_summary(&bob, 0, None).await?;
assert_eq!(summary.text, "You reacted 👍 to \"Party?\"");
assert_eq!(summary.timestamp, bob_msg1.get_timestamp()); // time refers to message, not to reaction
assert_eq!(summary.state, MessageState::InFresh); // state refers to message, not to reaction
assert!(summary.prefix.is_none());
assert!(summary.thumbnail_path.is_none());
assert_summary(&alice, "BOB reacted 👍 to \"Party?\"").await;
// Alice reacts to own message as well
SystemTime::shift(Duration::from_secs(10));
send_reaction(&alice, alice_msg1.sender_msg_id, "🍿").await?;
let alice_send_reaction = alice.pop_sent_msg().await;
bob.recv_msg(&alice_send_reaction).await;
assert_summary(&alice, "You reacted 🍿 to \"Party?\"").await;
assert_summary(&bob, "ALICE reacted 🍿 to \"Party?\"").await;
// Alice sends a newer message, this overwrites reaction summaries
SystemTime::shift(Duration::from_secs(10));
let alice_msg2 = alice.send_text(alice_chat.id, "kewl").await;
bob.recv_msg(&alice_msg2).await;
assert_summary(&alice, "kewl").await;
assert_summary(&bob, "kewl").await;
// Reactions to older messages still overwrite newer messages
SystemTime::shift(Duration::from_secs(10));
send_reaction(&alice, alice_msg1.sender_msg_id, "🤘").await?;
let alice_send_reaction = alice.pop_sent_msg().await;
bob.recv_msg(&alice_send_reaction).await;
assert_summary(&alice, "You reacted 🤘 to \"Party?\"").await;
assert_summary(&bob, "ALICE reacted 🤘 to \"Party?\"").await;
// Retracted reactions remove all summary reactions
SystemTime::shift(Duration::from_secs(10));
send_reaction(&alice, alice_msg1.sender_msg_id, "").await?;
let alice_remove_reaction = alice.pop_sent_msg().await;
bob.recv_msg(&alice_remove_reaction).await;
assert_summary(&alice, "kewl").await;
assert_summary(&bob, "kewl").await;
// Alice adds another reaction and then deletes the message reacted to; this will also delete reaction summary
SystemTime::shift(Duration::from_secs(10));
send_reaction(&alice, alice_msg1.sender_msg_id, "🧹").await?;
assert_summary(&alice, "You reacted 🧹 to \"Party?\"").await;
delete_msgs(&alice, &[alice_msg1.sender_msg_id]).await?;
assert_summary(&alice, "kewl").await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_reaction_self_chat_multidevice_summary() -> Result<()> {
let alice0 = TestContext::new_alice().await;
let alice1 = TestContext::new_alice().await;
let chat = alice0.get_self_chat().await;
let msg_id = send_text_msg(&alice0, chat.id, "mom's birthday!".to_string()).await?;
alice1.recv_msg(&alice0.pop_sent_msg().await).await;
SystemTime::shift(Duration::from_secs(10));
send_reaction(&alice0, msg_id, "👆").await?;
let sync = alice0.pop_sent_msg().await;
receive_imf(&alice1, sync.payload().as_bytes(), false).await?;
assert_summary(&alice0, "You reacted 👆 to \"mom's birthday!\"").await;
assert_summary(&alice1, "You reacted 👆 to \"mom's birthday!\"").await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_partial_download_and_reaction() -> Result<()> {
let alice = TestContext::new_alice().await;