Skip to content

feat: Display vCard contact name in the message summary #5619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion deltachat-ffi/deltachat.h
Original file line number Diff line number Diff line change
Expand Up @@ -7366,7 +7366,7 @@ void dc_event_unref(dc_event_t* event);
/// Used as info message.
#define DC_STR_SECUREJOIN_WAIT_TIMEOUT 191

/// "Contact"
/// "Contact". Deprecated, currently unused.
#define DC_STR_CONTACT 200

/**
Expand Down
21 changes: 18 additions & 3 deletions src/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ use crate::tools::{
create_smeared_timestamps, get_abs_path, gm2local_offset, improve_single_line_input,
smeared_time, time, IsNoneOrEmpty, SystemTime,
};
use crate::webxdc::WEBXDC_SUFFIX;

/// An chat item, such as a message or a marker.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -894,8 +893,20 @@ impl ChatId {
.await?
.context("no file stored in params")?;
msg.param.set(Param::File, blob.as_name());
if blob.suffix() == Some(WEBXDC_SUFFIX) {
msg.viewtype = Viewtype::Webxdc;
if msg.viewtype == Viewtype::File {
if let Some((better_type, _)) =
message::guess_msgtype_from_suffix(&blob.to_abs_path())
// We do not do an automatic conversion to other viewtypes here so that
// users can send images as "files" to preserve the original quality
// (usually we compress images). The remaining conversions are done by
// `prepare_msg_blob()` later.
.filter(|&(vt, _)| vt == Viewtype::Webxdc || vt == Viewtype::Vcard)
{
msg.viewtype = better_type;
}
}
if msg.viewtype == Viewtype::Vcard {
msg.try_set_vcard(context, &blob.to_abs_path()).await?;
}
}
}
Expand Down Expand Up @@ -2649,6 +2660,10 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
.await?;
}

if msg.viewtype == Viewtype::Vcard {
msg.try_set_vcard(context, &blob.to_abs_path()).await?;
}

let mut maybe_sticker = msg.viewtype == Viewtype::Sticker;
if !send_as_is
&& (msg.viewtype == Viewtype::Image
Expand Down
26 changes: 26 additions & 0 deletions src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use std::collections::BTreeSet;
use std::path::{Path, PathBuf};
use std::str;

use anyhow::{ensure, format_err, Context as _, Result};
use deltachat_contact_tools::{parse_vcard, VcardContact};
Expand Down Expand Up @@ -1093,6 +1094,18 @@ impl Message {
.await
}

/// Updates message state from the vCard attachment.
pub(crate) async fn try_set_vcard(&mut self, context: &Context, path: &Path) -> Result<()> {
let vcard = fs::read(path).await.context("Could not read {path}")?;
if let Some(summary) = get_vcard_summary(&vcard) {
self.param.set(Param::Summary1, summary);
} else {
warn!(context, "try_set_vcard: Not a valid DeltaChat vCard.");
self.viewtype = Viewtype::File;
}
Ok(())
}

/// Set different sender name for a message.
/// This overrides the name set by the `set_config()`-option `displayname`.
pub fn set_override_sender_name(&mut self, name: Option<String>) {
Expand Down Expand Up @@ -1947,6 +1960,19 @@ pub(crate) async fn get_by_rfc724_mids(
Ok(latest)
}

/// Returns the 1st part of summary text (i.e. before the dash if any) for a valid DeltaChat vCard.
pub(crate) fn get_vcard_summary(vcard: &[u8]) -> Option<String> {
let vcard = str::from_utf8(vcard).ok()?;
let contacts = deltachat_contact_tools::parse_vcard(vcard);
let [c] = &contacts[..] else {
return None;
};
if !deltachat_contact_tools::may_be_valid_addr(&c.addr) {
return None;
}
Some(c.display_name().to_string())
}

/// How a message is primarily displayed.
#[derive(
Debug,
Expand Down
31 changes: 15 additions & 16 deletions src/mimeparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ use crate::events::EventType;
use crate::headerdef::{HeaderDef, HeaderDefMap};
use crate::key::{self, load_self_secret_keyring, DcKey, Fingerprint, SignedPublicKey};
use crate::message::{
self, set_msg_failed, update_msg_state, Message, MessageState, MsgId, Viewtype,
self, get_vcard_summary, set_msg_failed, update_msg_state, Message, MessageState, MsgId,
Viewtype,
};
use crate::param::{Param, Params};
use crate::peerstate::Peerstate;
Expand Down Expand Up @@ -1234,6 +1235,7 @@ impl MimeMessage {
return Ok(());
}
}
let mut part = Part::default();
let msg_type = if context
.is_webxdc_file(filename, decoded_data)
.await
Expand Down Expand Up @@ -1277,6 +1279,13 @@ impl MimeMessage {
.unwrap_or_default();
self.webxdc_status_update = Some(serialized);
return Ok(());
} else if msg_type == Viewtype::Vcard {
if let Some(summary) = get_vcard_summary(decoded_data) {
part.param.set(Param::Summary1, summary);
msg_type
} else {
Viewtype::File
}
} else {
msg_type
};
Expand All @@ -1296,8 +1305,6 @@ impl MimeMessage {
};
info!(context, "added blobfile: {:?}", blob.as_name());

/* create and register Mime part referencing the new Blob object */
let mut part = Part::default();
if mime_type.type_() == mime::IMAGE {
if let Ok((width, height)) = get_filemeta(decoded_data) {
part.param.set_int(Param::Width, width as i32);
Expand Down Expand Up @@ -1929,7 +1936,10 @@ pub struct Part {
pub(crate) is_reaction: bool,
}

/// return mimetype and viewtype for a parsed mail
/// Returns the mimetype and viewtype for a parsed mail.
///
/// This only looks at the metadata, not at the content;
/// the viewtype may later be corrected in `do_add_single_file_part()`.
fn get_mime_type(
mail: &mailparse::ParsedMail<'_>,
filename: &Option<String>,
Expand All @@ -1938,7 +1948,7 @@ fn get_mime_type(

let viewtype = match mimetype.type_() {
mime::TEXT => match mimetype.subtype() {
mime::VCARD if is_valid_deltachat_vcard(mail) => Viewtype::Vcard,
mime::VCARD => Viewtype::Vcard,
mime::PLAIN | mime::HTML if !is_attachment_disposition(mail) => Viewtype::Text,
_ => Viewtype::File,
},
Expand Down Expand Up @@ -1989,17 +1999,6 @@ fn is_attachment_disposition(mail: &mailparse::ParsedMail<'_>) -> bool {
.any(|(key, _value)| key.starts_with("filename"))
}

fn is_valid_deltachat_vcard(mail: &mailparse::ParsedMail) -> bool {
let Ok(body) = &mail.get_body() else {
return false;
};
let contacts = deltachat_contact_tools::parse_vcard(body);
if let [c] = &contacts[..] {
return deltachat_contact_tools::may_be_valid_addr(&c.addr);
}
false
}

/// Tries to get attachment filename.
///
/// If filename is explicitly specified in Content-Disposition, it is
Expand Down
3 changes: 3 additions & 0 deletions src/param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ pub enum Param {
/// For Messages: quoted text.
Quote = b'q',

/// For Messages: the 1st part of summary text (i.e. before the dash if any).
Summary1 = b'4',

/// For Messages
Cmd = b'S',

Expand Down
29 changes: 24 additions & 5 deletions src/receive_imf/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4705,10 +4705,15 @@ async fn test_receive_vcard() -> Result<()> {
let alice = tcm.alice().await;
let bob = tcm.bob().await;

for vcard_contains_address in [true, false] {
let mut msg = Message::new(Viewtype::Vcard);
async fn test(
alice: &TestContext,
bob: &TestContext,
vcard_contains_address: bool,
viewtype: Viewtype,
) -> Result<()> {
let mut msg = Message::new(viewtype);
msg.set_file_from_bytes(
&alice,
alice,
"claire.vcf",
format!(
"BEGIN:VCARD\n\
Expand All @@ -4728,19 +4733,24 @@ async fn test_receive_vcard() -> Result<()> {
.await
.unwrap();

let alice_bob_chat = alice.create_chat(&bob).await;
let alice_bob_chat = alice.create_chat(bob).await;
let sent = alice.send_msg(alice_bob_chat.id, &mut msg).await;
let rcvd = bob.recv_msg(&sent).await;
let sent = Message::load_from_db(alice, sent.sender_msg_id).await?;

if vcard_contains_address {
assert_eq!(sent.viewtype, Viewtype::Vcard);
assert_eq!(sent.get_summary_text(alice).await, "👤 Claire");
assert_eq!(rcvd.viewtype, Viewtype::Vcard);
assert_eq!(rcvd.get_summary_text(bob).await, "👤 Claire");
} else {
// VCards without an email address are not "deltachat contacts",
// so they are shown as files
assert_eq!(sent.viewtype, Viewtype::File);
assert_eq!(rcvd.viewtype, Viewtype::File);
}

let vcard = tokio::fs::read(rcvd.get_file(&bob).unwrap()).await?;
let vcard = tokio::fs::read(rcvd.get_file(bob).unwrap()).await?;
let vcard = std::str::from_utf8(&vcard)?;
let parsed = deltachat_contact_tools::parse_vcard(vcard);
assert_eq!(parsed.len(), 1);
Expand All @@ -4749,6 +4759,13 @@ async fn test_receive_vcard() -> Result<()> {
} else {
assert_eq!(&parsed[0].addr, "");
}
Ok(())
}

for vcard_contains_address in [true, false] {
for viewtype in [Viewtype::File, Viewtype::Vcard] {
test(&alice, &bob, vcard_contains_address, viewtype).await?;
}
}

Ok(())
Expand Down Expand Up @@ -4776,7 +4793,9 @@ async fn test_make_n_send_vcard() -> Result<()> {
let sent = Message::load_from_db(alice, sent.sender_msg_id).await?;

assert_eq!(sent.viewtype, Viewtype::Vcard);
assert_eq!(sent.get_summary_text(alice).await, "👤 Claire");
assert_eq!(rcvd.viewtype, Viewtype::Vcard);
assert_eq!(rcvd.get_summary_text(bob).await, "👤 Claire");

let vcard = tokio::fs::read(rcvd.get_file(bob).unwrap()).await?;
let vcard = std::str::from_utf8(&vcard)?;
Expand Down
8 changes: 0 additions & 8 deletions src/stock_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,9 +443,6 @@ pub enum StockMessage {
fallback = "Could not yet establish guaranteed end-to-end encryption, but you may already send a message."
))]
SecurejoinWaitTimeout = 191,

#[strum(props(fallback = "Contact"))]
Contact = 200,
}

impl StockMessage {
Expand Down Expand Up @@ -1101,11 +1098,6 @@ pub(crate) async fn videochat_invite_msg_body(context: &Context, url: &str) -> S
.replace1(url)
}

/// Stock string: `Contact`.
pub(crate) async fn contact(context: &Context) -> String {
translated(context, StockMessage::Contact).await
}

/// Stock string: `Error:\n\n“%1$s”`.
pub(crate) async fn configuration_failed(context: &Context, details: &str) -> String {
translated(context, StockMessage::ConfigurationFailed)
Expand Down
57 changes: 35 additions & 22 deletions src/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::contact::{Contact, ContactId};
use crate::context::Context;
use crate::message::{Message, MessageState, Viewtype};
use crate::mimeparser::SystemMessage;
use crate::param::Param;
use crate::stock_str;
use crate::stock_str::msg_reacted;
use crate::tools::truncate;
Expand Down Expand Up @@ -149,7 +150,7 @@ impl Summary {

impl Message {
/// Returns a summary text.
async fn get_summary_text(&self, context: &Context) -> String {
pub(crate) async fn get_summary_text(&self, context: &Context) -> String {
let summary = self.get_summary_text_without_prefix(context).await;

if self.is_forwarded() {
Expand Down Expand Up @@ -231,8 +232,8 @@ impl Message {
}
Viewtype::Vcard => {
emoji = Some("👤");
type_name = Some(stock_str::contact(context).await);
type_file = None;
type_name = None;
type_file = self.param.get(Param::Summary1).map(|s| s.to_string());
append_text = true;
}
Viewtype::Text | Viewtype::Unknown => {
Expand Down Expand Up @@ -284,6 +285,7 @@ impl Message {
#[cfg(test)]
mod tests {
use super::*;
use crate::chat::ChatId;
use crate::param::Param;
use crate::test_utils as test;

Expand All @@ -296,7 +298,9 @@ mod tests {
async fn test_get_summary_text() {
let d = test::TestContext::new().await;
let ctx = &d.ctx;

let chat_id = ChatId::create_for_contact(ctx, ContactId::SELF)
.await
.unwrap();
let some_text = " bla \t\n\tbla\n\t".to_string();

let mut msg = Message::new(Viewtype::Text);
Expand Down Expand Up @@ -367,25 +371,34 @@ mod tests {
assert_summary_texts(&msg, ctx, "Video chat invitation").await; // text is not added for videochat invitations

let mut msg = Message::new(Viewtype::Vcard);
msg.set_file("foo.vcf", None);
assert_summary_texts(&msg, ctx, "👤 Contact").await;
msg.set_file_from_bytes(ctx, "foo.vcf", b"", None)
.await
.unwrap();
chat_id.set_draft(ctx, Some(&mut msg)).await.unwrap();
// If a vCard can't be parsed, the message becomes `Viewtype::File`.
assert_eq!(msg.viewtype, Viewtype::File);
assert_summary_texts(&msg, ctx, "📎 foo.vcf").await;
msg.set_text(some_text.clone());
assert_summary_texts(&msg, ctx, "👤 bla bla").await;

let mut msg = Message::new(Viewtype::Vcard);
msg.set_file_from_bytes(
ctx,
"alice.vcf",
b"BEGIN:VCARD\n\
VERSION:4.0\n\
FN:Alice Wonderland\n\
EMAIL;TYPE=work:[email protected]\n\
END:VCARD",
None,
)
.await
.unwrap();
assert_summary_texts(&msg, ctx, "👤 Contact").await;
assert_summary_texts(&msg, ctx, "📎 foo.vcf \u{2013} bla bla").await;

for vt in [Viewtype::Vcard, Viewtype::File] {
let mut msg = Message::new(vt);
msg.set_file_from_bytes(
ctx,
"alice.vcf",
b"BEGIN:VCARD\n\
VERSION:4.0\n\
FN:Alice Wonderland\n\
EMAIL;TYPE=work:[email protected]\n\
END:VCARD",
None,
)
.await
.unwrap();
chat_id.set_draft(ctx, Some(&mut msg)).await.unwrap();
assert_eq!(msg.viewtype, Viewtype::Vcard);
assert_summary_texts(&msg, ctx, "👤 Alice Wonderland").await;
}

// Forwarded
let mut msg = Message::new(Viewtype::Text);
Expand Down
Loading