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.

Sign up for GitHub

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: Set BccSelf to true when receiving a sync message #6434

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: Set BccSelf to true when receiving a sync message #6434

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a1e0b35
feat: Set BccSelf to true when receiving a sync message
Hocuri Jan 13, 2025
a587755
Always do it when we receive a message from a second device, not just…
Hocuri Jan 13, 2025
468f926
Improve test
Hocuri Jan 13, 2025
c1e8cfa
Revert "Improve test"
Hocuri Jan 13, 2025
51b9c21
Revert "Always do it when we receive a message from a second device, …
Hocuri Jan 13, 2025
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
48 changes: 48 additions & 0 deletions src/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,17 @@ impl Context {
.log_err(self)
.ok();
}

// Since there was a sync message, we know that there is a second device.
// Set BccSelf to true if it isn't already.
if !items.items.is_empty() && !self.get_config_bool(Config::BccSelf).await.unwrap_or(false)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!= Ok(true) is easier to read, but i'd make it == Ok(false), i think we don't want to set the config on failures.

{
self.set_config_ex(Sync::Nosync, Config::BccSelf, Some("1"))
.await
.log_err(self)
.ok();
}
// TODO check that a test fails if we do this unconditionally
}

async fn add_qr_token(&self, token: &QrTokenData) -> Result<()> {
Expand Down Expand Up @@ -578,6 +589,43 @@ mod tests {
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_send_sync_msg_enables_bccself() -> Result<()> {
let alice1 = TestContext::new_alice().await;
let alice2 = TestContext::new_alice().await;

alice1.set_config_bool(Config::IsChatmail, true).await?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also test for non-chatmail, this should work for non-chatmail too.

alice2.set_config_bool(Config::IsChatmail, true).await?;

// SyncMsgs defaults to true on real devices, but in tests it defaults to false,
// so we need to enable it
alice1.set_config_bool(Config::SyncMsgs, true).await?;
alice2.set_config_bool(Config::SyncMsgs, true).await?;

alice1.set_config_bool(Config::BccSelf, true).await?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also check that alice2 has BccSelf disabled. In case of non-chatmail, disable it manually.


alice1
.add_sync_item(SyncData::AddQrToken(QrTokenData {
invitenumber: "in".to_string(),
auth: "testtoken".to_string(),
grpid: None,
}))
.await?;
alice1.send_sync_msg().await?.unwrap();
let sent_msg = alice1.pop_sent_sync_msg().await;

// On chatmail accounts, BccSelf defaults to false.
// When receiving a sync message from another device,
// there obviously is a multi-device-setup, and BccSelf
// should be enabled.
assert_eq!(alice2.get_config_bool(Config::BccSelf).await?, false);

alice2.recv_msg_trash(&sent_msg).await;
assert_eq!(alice2.get_config_bool(Config::BccSelf).await?, true);

Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_bot_no_sync_msgs() -> Result<()> {
let mut tcm = TestContextManager::new();
Expand Down
Loading