Skip to content
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

feat: Make broadcast lists create their own chat #4644

Merged
merged 8 commits into from
Oct 17, 2023
24 changes: 6 additions & 18 deletions deltachat-ffi/deltachat.h
Original file line number Diff line number Diff line change
Expand Up @@ -1717,24 +1717,12 @@ uint32_t dc_create_group_chat (dc_context_t* context, int protect
* Create a new broadcast list.
*
* Broadcast lists are similar to groups on the sending device,
* however, recipients get the messages in normal one-to-one chats
* and will not be aware of other members.
*
* Replies to broadcasts go only to the sender
* and not to all broadcast recipients.
* Moreover, replies will not appear in the broadcast list
* but in the one-to-one chat with the person answering.
*
* The name and the image of the broadcast list is set automatically
* and is visible to the sender only.
* Not asking for these data allows more focused creation
* and we bypass the question who will get which data.
* Also, many users will have at most one broadcast list
* so, a generic name and image is sufficient at the first place.
*
* Later on, however, the name can be changed using dc_set_chat_name().
* The image cannot be changed to have a unique, recognizable icon in the chat lists.
* All in all, this is also what other messengers are doing here.
* however, recipients get the messages in a read-only chat
* and will see who the other members are.
*
* For historical reasons, this function does not take a name directly,
* instead you have to set the name using dc_set_chat_name()
* after creating the broadcast list.
*
* @memberof dc_context_t
* @param context The context object.
Expand Down
24 changes: 6 additions & 18 deletions deltachat-jsonrpc/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -812,24 +812,12 @@ impl CommandApi {
/// Create a new broadcast list.
///
/// Broadcast lists are similar to groups on the sending device,
/// however, recipients get the messages in normal one-to-one chats
/// and will not be aware of other members.
///
/// Replies to broadcasts go only to the sender
/// and not to all broadcast recipients.
/// Moreover, replies will not appear in the broadcast list
/// but in the one-to-one chat with the person answering.
///
/// The name and the image of the broadcast list is set automatically
/// and is visible to the sender only.
/// Not asking for these data allows more focused creation
/// and we bypass the question who will get which data.
/// Also, many users will have at most one broadcast list
/// so, a generic name and image is sufficient at the first place.
///
/// Later on, however, the name can be changed using dc_set_chat_name().
/// The image cannot be changed to have a unique, recognizable icon in the chat lists.
/// All in all, this is also what other messengers are doing here.
/// however, recipients get the messages in a read-only chat
/// and will see who the other members are.
///
/// For historical reasons, this function does not take a name directly,
/// instead you have to set the name using dc_set_chat_name()
/// after creating the broadcast list.
async fn create_broadcast_list(&self, account_id: u32) -> Result<u32> {
let ctx = self.get_context(account_id).await?;
chat::create_broadcast_list(&ctx)
Expand Down
46 changes: 32 additions & 14 deletions src/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6094,22 +6094,40 @@ mod tests {
get_chat_contacts(&alice, chat_bob.id).await?.pop().unwrap(),
)
.await?;
let chat = Chat::load_from_db(&alice, broadcast_id).await?;
assert_eq!(chat.typ, Chattype::Broadcast);
assert_eq!(chat.name, stock_str::broadcast_list(&alice).await);
assert!(!chat.is_self_talk());
set_chat_name(&alice, broadcast_id, "Broadcast list").await?;
{
let chat = Chat::load_from_db(&alice, broadcast_id).await?;
assert_eq!(chat.typ, Chattype::Broadcast);
assert_eq!(chat.name, "Broadcast list");
assert!(!chat.is_self_talk());

send_text_msg(&alice, broadcast_id, "ola!".to_string()).await?;
let msg = alice.get_last_msg().await;
assert_eq!(msg.chat_id, chat.id);
}

send_text_msg(&alice, broadcast_id, "ola!".to_string()).await?;
let msg = alice.get_last_msg().await;
assert_eq!(msg.chat_id, chat.id);
{
let msg = bob.recv_msg(&alice.pop_sent_msg().await).await;
assert_eq!(msg.get_text(), "ola!");
assert_eq!(msg.subject, "Broadcast list");
assert!(!msg.get_showpadlock()); // avoid leaking recipients in encryption data
let chat = Chat::load_from_db(&bob, msg.chat_id).await?;
assert_eq!(chat.typ, Chattype::Mailinglist);
assert_ne!(chat.id, chat_bob.id);
assert_eq!(chat.name, "Broadcast list");
assert!(!chat.is_self_talk());
}

let msg = bob.recv_msg(&alice.pop_sent_msg().await).await;
assert_eq!(msg.get_text(), "ola!");
assert!(!msg.get_showpadlock()); // avoid leaking recipients in encryption data
let chat = Chat::load_from_db(&bob, msg.chat_id).await?;
assert_eq!(chat.typ, Chattype::Single);
assert_eq!(chat.id, chat_bob.id);
assert!(!chat.is_self_talk());
{
// Alice changes the name:
set_chat_name(&alice, broadcast_id, "My great broadcast").await?;
let sent = alice.send_text(broadcast_id, "I changed the title!").await;

let msg = bob.recv_msg(&sent).await;
assert_eq!(msg.subject, "Re: My great broadcast");
let bob_chat = Chat::load_from_db(&bob, msg.chat_id).await?;
assert_eq!(bob_chat.name, "My great broadcast");
}

Ok(())
}
Expand Down
29 changes: 19 additions & 10 deletions src/mimefactory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,9 @@ impl<'a> MimeFactory<'a> {
return Ok(self.msg.subject.clone());
}

if chat.typ == Chattype::Group && quoted_msg_subject.is_none_or_empty() {
if (chat.typ == Chattype::Group || chat.typ == Chattype::Broadcast)
&& quoted_msg_subject.is_none_or_empty()
{
let re = if self.in_reply_to.is_empty() {
""
} else {
Expand All @@ -424,15 +426,13 @@ impl<'a> MimeFactory<'a> {
return Ok(format!("{}{}", re, chat.name));
}

if chat.typ != Chattype::Broadcast {
let parent_subject = if quoted_msg_subject.is_none_or_empty() {
chat.param.get(Param::LastSubject)
} else {
quoted_msg_subject.as_deref()
};
if let Some(last_subject) = parent_subject {
return Ok(format!("Re: {}", remove_subject_prefix(last_subject)));
}
let parent_subject = if quoted_msg_subject.is_none_or_empty() {
chat.param.get(Param::LastSubject)
} else {
quoted_msg_subject.as_deref()
};
if let Some(last_subject) = parent_subject {
return Ok(format!("Re: {}", remove_subject_prefix(last_subject)));
}

let self_name = &match context.get_config(Config::Displayname).await? {
Expand Down Expand Up @@ -594,6 +594,15 @@ impl<'a> MimeFactory<'a> {
));
}

if let Loaded::Message { chat } = &self.loaded {
if chat.typ == Chattype::Broadcast {
headers.protected.push(Header::new(
"List-ID".into(),
format!("{} <{}>", chat.name, chat.grpid),
));
}
}

// Non-standard headers.
headers
.unprotected
Expand Down
39 changes: 12 additions & 27 deletions src/mimeparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,23 +123,6 @@ pub(crate) enum AvatarAction {
Change(String),
}

#[derive(Debug, PartialEq)]
pub(crate) enum MailinglistType {
/// The message belongs to a mailing list and has a `ListId:`-header
/// that should be used to get a unique id.
ListIdBased,

/// The message belongs to a mailing list, but there is no `ListId:`-header;
/// `Sender:`-header should be used to get a unique id.
/// This method is used by implementations as Majordomo.
/// Note, that the `Sender:` header alone is not sufficient to detect these lists,
/// `get_mailinglist_type()` check additional conditions therefore.
SenderBased,

/// The message does not belong to a mailing list.
None,
}

/// System message type.
#[derive(
Debug, Default, Display, Clone, Copy, PartialEq, Eq, FromPrimitive, ToPrimitive, ToSql, FromSql,
Expand Down Expand Up @@ -1340,26 +1323,28 @@ impl MimeMessage {
self.parts.push(part);
}

pub(crate) fn get_mailinglist_type(&self) -> MailinglistType {
if self.get_header(HeaderDef::ListId).is_some() {
return MailinglistType::ListIdBased;
} else if self.get_header(HeaderDef::Sender).is_some() {
pub(crate) fn get_mailinglist_header(&self) -> Option<&str> {
if let Some(list_id) = self.get_header(HeaderDef::ListId) {
// The message belongs to a mailing list and has a `ListId:`-header
// that should be used to get a unique id.
return Some(list_id);
} else if let Some(sender) = self.get_header(HeaderDef::Sender) {
// the `Sender:`-header alone is no indicator for mailing list
// as also used for bot-impersonation via `set_override_sender_name()`
if let Some(precedence) = self.get_header(HeaderDef::Precedence) {
if precedence == "list" || precedence == "bulk" {
return MailinglistType::SenderBased;
// The message belongs to a mailing list, but there is no `ListId:`-header;
// `Sender:`-header is be used to get a unique id.
// This method is used by implementations as Majordomo.
return Some(sender);
}
}
}
MailinglistType::None
None
}

pub(crate) fn is_mailinglist_message(&self) -> bool {
match self.get_mailinglist_type() {
MailinglistType::ListIdBased | MailinglistType::SenderBased => true,
MailinglistType::None => false,
}
self.get_mailinglist_header().is_some()
}

pub fn repl_msg_by_error(&mut self, error_msg: &str) {
Expand Down
Loading