Skip to content

Commit c8af189

Browse files
committed
fix: Order of messages if Sentbox is synced before Inbox (#7169)
This fixes a Gmail-like scenario when outgoing messages are saved to Sentbox as well and it is fetched before Inbox by chance, by allowing received outgoing messages to mingle with fresh incoming ones. The reason is that a received outgoing message may be a reply (explicit or implicit) to an incoming message received later, so it's better to sort them together purely by timestamp. Another case is the user sharing their account with someone else (using another device) or having some auto-reply bot. This fixes the described scenarios w/o introducing a new message state for outgoing messages because locally sent ones have zero `timestamp_sent`, so we can filter them in `calc_sort_timestamp()`. As for messages sent locally, there's no need to make them more noticeable even if they are newer, so received outgoing messages are added after them.
1 parent 53a3e51 commit c8af189

File tree

9 files changed

+26
-14
lines changed

9 files changed

+26
-14
lines changed

src/chat.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,17 +1450,23 @@ impl ChatId {
14501450
// received message purely by timestamp. We could place it just before that seen
14511451
// message, but anyway the user may not notice it.
14521452
//
1453-
// NB: Received outgoing messages may break sorting of fresh incoming ones, but this
1454-
// shouldn't happen frequently. Seen incoming messages don't really break sorting of
1455-
// fresh ones, they rather mean that older incoming messages are actually seen as well.
1453+
// NB: Seen incoming messages don't really break sorting of fresh ones, they rather mean
1454+
// that older incoming messages are actually seen as well.
1455+
// NB: Locally sent messages have zero `timestamp_sent`.
14561456
context
14571457
.sql
14581458
.query_row_optional(
14591459
"SELECT MAX(timestamp), MAX(IIF(state=?,timestamp_sent,0))
14601460
FROM msgs
14611461
WHERE chat_id=? AND hidden=0 AND state>?
1462+
AND (state!=? OR timestamp_sent=0)
14621463
HAVING COUNT(*) > 0",
1463-
(MessageState::InSeen, self, MessageState::InFresh),
1464+
(
1465+
MessageState::InSeen,
1466+
self,
1467+
MessageState::InFresh,
1468+
MessageState::OutDelivered,
1469+
),
14641470
|row| {
14651471
let ts: i64 = row.get(0)?;
14661472
let ts_sent_seen: i64 = row.get(1)?;

src/events/payload.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ pub enum EventType {
3535
/// Emitted when an IMAP message has been moved
3636
ImapMessageMoved(String),
3737

38-
/// Emitted before going into IDLE on the Inbox folder.
38+
/// Emitted before going into IDLE on any folder.
3939
ImapInboxIdle,
4040

4141
/// Emitted when an new file in the $BLOBDIR was created

src/scheduler.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,7 @@ async fn fetch_idle(
661661

662662
connection.connectivity.set_idle(ctx);
663663

664+
// Maybe we'll remove watching other folders soon, so use this event for all folders for now.
664665
ctx.emit_event(EventType::ImapInboxIdle);
665666

666667
if !session.can_idle() {

src/test_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1521,7 +1521,7 @@ async fn write_msg(context: &Context, prefix: &str, msg: &Message, buf: &mut Str
15211521

15221522
let statestr = match msg.get_state() {
15231523
MessageState::OutPending => " o",
1524-
MessageState::OutDelivered => " √",
1524+
MessageState::OutDelivered if msg.timestamp_sent == 0 => " √",
15251525
MessageState::OutMdnRcvd => " √√",
15261526
MessageState::OutFailed => " !!",
15271527
_ => "",

src/tests/verified_chats.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -316,9 +316,9 @@ async fn test_old_message_4() -> Result<()> {
316316
Ok(())
317317
}
318318

319-
/// Alice is offline for some time.
320-
/// When they come online, first their sentbox is synced and then their inbox.
321-
/// This test tests that the messages are still in the right order.
319+
/// Alice's device#0 is offline for some time.
320+
/// When it comes online, it sees a message from another device and an incoming message. Messages
321+
/// may come from different folders.
322322
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
323323
async fn test_old_message_5() -> Result<()> {
324324
let alice = TestContext::new_alice().await;
@@ -348,7 +348,12 @@ async fn test_old_message_5() -> Result<()> {
348348
.await?
349349
.unwrap();
350350

351-
assert!(msg_sent.sort_timestamp == msg_incoming.sort_timestamp);
351+
// If the messages come from the same folder and `msg_sent` is sent by Alice, it's better to
352+
// sort `msg_incoming` after it so that it's more visible, but if Alice shares her account with
353+
// someone else or has some auto-reply bot, messages should be sorted just by "Date". If the
354+
// messages come from different folders, probably `msg_incoming` is already noticed by Alice on
355+
// another device (and this is the most often case in practice).
356+
assert!(msg_incoming.sort_timestamp < msg_sent.sort_timestamp);
352357
alice
353358
.golden_test_chat(msg_sent.chat_id, "test_old_message_5")
354359
.await;
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Single#Chat#10: [email protected] [[email protected]] Icon: 4138c52e5bc1c576cda7dd44d088c07.png
22
--------------------------------------------------------------------------------
33
Msg#10: Me (Contact#Contact#Self): We share this account √
4-
Msg#11: Me (Contact#Contact#Self): I'm Alice too
4+
Msg#11: Me (Contact#Contact#Self): I'm Alice too
55
--------------------------------------------------------------------------------
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Single#Chat#10: Bob [[email protected]] Icon: 4138c52e5bc1c576cda7dd44d088c07.png
22
--------------------------------------------------------------------------------
3-
Msg#10: Me (Contact#Contact#Self): Happy birthday, Bob! √
43
Msg#11: (Contact#Contact#10): Happy birthday to me, Alice! [FRESH]
4+
Msg#10: Me (Contact#Contact#Self): Happy birthday, Bob!
55
--------------------------------------------------------------------------------
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Single#Chat#10: [email protected] [KEY [email protected]] 🛡️
22
--------------------------------------------------------------------------------
33
Msg#10: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO 🛡️]
4-
Msg#11🔒: Me (Contact#Contact#Self): Test – This is encrypted, signed, and has an Autocrypt Header without prefer-encrypt=mutual.
4+
Msg#11🔒: Me (Contact#Contact#Self): Test – This is encrypted, signed, and has an Autocrypt Header without prefer-encrypt=mutual.
55
--------------------------------------------------------------------------------
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
Single#Chat#11: [email protected] [[email protected]] Icon: 4138c52e5bc1c576cda7dd44d088c07.png
22
--------------------------------------------------------------------------------
3-
Msg#12: Me (Contact#Contact#Self): One classical MUA message
3+
Msg#12: Me (Contact#Contact#Self): One classical MUA message
44
--------------------------------------------------------------------------------

0 commit comments

Comments
 (0)