Skip to content

Commit

Permalink
fix: Return from dc_get_chatlist(DC_GCL_FOR_FORWARDING) only chats wh…
Browse files Browse the repository at this point in the history
…ere we can send (#4616)

I.e. exclude from the list the following chats as well:
- Read-only mailing lists.
- Chats we're not a member of.
- Chats with broken protection.
  • Loading branch information
iequidoo committed Aug 11, 2023
1 parent 20c8874 commit 3b5e7fa
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,7 @@ impl Chat {
pub(crate) async fn why_cant_send(&self, context: &Context) -> Result<Option<CantSendReason>> {
use CantSendReason::*;

// NB: Don't forget to update Chatlist::try_load() when changing this function!
let reason = if self.id.is_special() {
Some(SpecialChat)
} else if self.is_device_talk() {
Expand Down
61 changes: 52 additions & 9 deletions src/chatlist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@

use anyhow::{ensure, Context as _, Result};

use crate::chat::{update_special_chat_names, Chat, ChatId, ChatVisibility};
use crate::chat::{update_special_chat_names, Chat, ChatId, ChatVisibility, ProtectionStatus};
use crate::constants::{
Blocked, Chattype, DC_CHAT_ID_ALLDONE_HINT, DC_CHAT_ID_ARCHIVED_LINK, DC_GCL_ADD_ALLDONE_HINT,
DC_GCL_ARCHIVED_ONLY, DC_GCL_FOR_FORWARDING, DC_GCL_NO_SPECIALS,
};
use crate::contact::{Contact, ContactId};
use crate::context::Context;
use crate::message::{Message, MessageState, MsgId};
use crate::param::{Param, Params};
use crate::stock_str;
use crate::summary::Summary;
use crate::tools::IsNoneOrEmpty;

/// An object representing a single chatlist in memory.
///
Expand Down Expand Up @@ -212,23 +214,54 @@ impl Chatlist {
} else {
ChatId::new(0)
};
let process_row = |row: &rusqlite::Row| {
let chat_id: ChatId = row.get(0)?;
let typ: Chattype = row.get(1)?;
let param: Params = row.get::<_, String>(2)?.parse().unwrap_or_default();
let msg_id: Option<MsgId> = row.get(3)?;
Ok((chat_id, typ, param, msg_id))
};
let process_rows = |rows: rusqlite::MappedRows<_>| {
rows.filter_map(|row: std::result::Result<(_, _, Params, _), _>| match row {
Ok((chat_id, typ, param, msg_id)) => {
if flag_for_forwarding
&& typ == Chattype::Mailinglist
&& param.get(Param::ListPost).is_none_or_empty()
{
None
} else {
Some(Ok((chat_id, msg_id)))
}
}
Err(e) => Some(Err(e)),
})
.collect::<std::result::Result<Vec<_>, _>>()
.map_err(Into::into)
};
let mut ids = context.sql.query_map(
"SELECT c.id, m.id
"SELECT c.id, c.type, c.param, m.id
FROM chats c
LEFT JOIN msgs m
ON c.id=m.chat_id
AND m.id=(
SELECT id
FROM msgs
WHERE chat_id=c.id
AND (hidden=0 OR state=?1)
AND (hidden=0 OR state=?)
ORDER BY timestamp DESC, id DESC LIMIT 1)
WHERE c.id>9 AND c.id!=?2
AND (c.blocked=0 OR (c.blocked=2 AND NOT ?3))
AND NOT c.archived=?4
WHERE c.id>9 AND c.id!=?
AND (c.blocked=0 OR (c.blocked=2 AND NOT ?))
AND NOT c.archived=?
AND (NOT ? OR c.protected!=?)
AND (NOT ? OR c.type!=? OR c.id IN(SELECT chat_id FROM chats_contacts WHERE contact_id=?))
GROUP BY c.id
ORDER BY c.id=?5 DESC, c.archived=?6 DESC, IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
(MessageState::OutDraft, skip_id, flag_for_forwarding, ChatVisibility::Archived, sort_id_up, ChatVisibility::Pinned),
ORDER BY c.id=? DESC, c.archived=? DESC, IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
(
MessageState::OutDraft, skip_id, flag_for_forwarding, ChatVisibility::Archived,
flag_for_forwarding, ProtectionStatus::ProtectionBroken,
flag_for_forwarding, Chattype::Group, ContactId::SELF,
sort_id_up, ChatVisibility::Pinned,
),
process_row,
process_rows,
).await?;
Expand Down Expand Up @@ -388,7 +421,9 @@ pub async fn get_last_message_for_chat(
#[cfg(test)]
mod tests {
use super::*;
use crate::chat::{create_group_chat, get_chat_contacts, ProtectionStatus};
use crate::chat::{
create_group_chat, get_chat_contacts, remove_contact_from_chat, ProtectionStatus,
};
use crate::message::Viewtype;
use crate::receive_imf::receive_imf;
use crate::stock_str::StockMessage;
Expand Down Expand Up @@ -473,6 +508,14 @@ mod tests {
.await
.unwrap()
.is_self_talk());

remove_contact_from_chat(&t, chats.get_chat_id(1).unwrap(), ContactId::SELF)
.await
.unwrap();
let chats = Chatlist::try_load(&t, DC_GCL_FOR_FORWARDING, None, None)
.await
.unwrap();
assert!(chats.len() == 1);
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
Expand Down
4 changes: 3 additions & 1 deletion src/receive_imf/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::chat::{
};
use crate::chat::{get_chat_msgs, ChatItem, ChatVisibility};
use crate::chatlist::Chatlist;
use crate::constants::DC_GCL_NO_SPECIALS;
use crate::constants::{DC_GCL_FOR_FORWARDING, DC_GCL_NO_SPECIALS};
use crate::imap::prefetch_should_download;
use crate::message::Message;
use crate::test_utils::{get_chat_msg, TestContext, TestContextManager};
Expand Down Expand Up @@ -793,6 +793,8 @@ async fn test_github_mailing_list() -> Result<()> {

let chats = Chatlist::try_load(&t.ctx, 0, None, None).await?;
assert_eq!(chats.len(), 1);
let chats = Chatlist::try_load(&t.ctx, DC_GCL_FOR_FORWARDING, None, None).await?;
assert_eq!(chats.len(), 0);
let contacts = Contact::get_all(&t.ctx, 0, None).await?;
assert_eq!(contacts.len(), 0); // mailing list recipients and senders do not count as "known contacts"

Expand Down
8 changes: 8 additions & 0 deletions src/tests/verified_chats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use anyhow::Result;
use pretty_assertions::assert_eq;

use crate::chat::{Chat, ProtectionStatus};
use crate::chatlist::Chatlist;
use crate::config::Config;
use crate::constants::DC_GCL_FOR_FORWARDING;
use crate::contact::VerifiedStatus;
use crate::contact::{Contact, Origin};
use crate::message::{Message, Viewtype};
Expand Down Expand Up @@ -617,6 +619,8 @@ async fn test_break_protection_then_verify_again() -> Result<()> {

alice.create_chat(&bob).await;
assert_verified(&alice, &bob, ProtectionStatus::Protected).await;
let chats = Chatlist::try_load(&alice, DC_GCL_FOR_FORWARDING, None, None).await?;
assert!(chats.len() == 1);

tcm.section("Bob reinstalls DC");
drop(bob);
Expand All @@ -638,6 +642,8 @@ async fn test_break_protection_then_verify_again() -> Result<()> {
let chat = alice.get_chat(&bob_new).await;
assert_eq!(chat.is_protected(), false);
assert_eq!(chat.is_protection_broken(), true);
let chats = Chatlist::try_load(&alice, DC_GCL_FOR_FORWARDING, None, None).await?;
assert!(chats.is_empty());

{
let alice_bob_chat = alice.get_chat(&bob_new).await;
Expand All @@ -655,6 +661,8 @@ async fn test_break_protection_then_verify_again() -> Result<()> {

tcm.execute_securejoin(&alice, &bob_new).await;
assert_verified(&alice, &bob_new, ProtectionStatus::Protected).await;
let chats = Chatlist::try_load(&alice, DC_GCL_FOR_FORWARDING, None, None).await?;
assert!(chats.len() == 1);

Ok(())
}
Expand Down

0 comments on commit 3b5e7fa

Please sign in to comment.