deltachat-core-rust
deltachat-core-rust copied to clipboard
feat: Sort received outgoing message down if it's fresher than all non fresh messages
Received messages shouldn't mingle with just sent ones and appear somewhere in the middle of the chat, so we go after the newest non fresh message.
But if a received outgoing message is older than some non fresh message, better sort the received message purely by timestamp. We could place it just before that non fresh message, but anyway the user may not notice it.
At least this fixes outgoing messages sorting for shared accounts where messages from other devices should be sorted the same way as incoming ones.
But if a received outgoing message is older than some non fresh message
Why does this happen? If we download messages in order, then message is usually newer than all messages received so far.
Does this PR try to fix some case where messages are pulled from Sent folder when used together with non-DC client?
Why does this happen? If we download messages in order, then message is usually newer than all messages received so far.
This may happen if Sentbox is fetched after Inbox, verified_chats::test_old_message_4() simulates this and i don't want to break this scenario now:
let msg_incoming = receive_imf(
&alice,
b"From: Bob <[email protected]>\n\
To: [email protected]\n\
Message-ID: <[email protected]>\n\
Date: Sun, 08 Dec 2019 19:00:27 +0000\n\
\n\
Thanks, Alice!\n",
true,
)
.await?
.unwrap();
let msg_sent = receive_imf(
&alice,
b"From: [email protected]\n\
To: Bob <[email protected]>\n\
Message-ID: <[email protected]>\n\
Date: Sat, 07 Dec 2019 19:00:27 +0000\n\
\n\
Happy birthday, Bob!\n",
true,
)
.await?
.unwrap();
Of course Delta Chat doesn't always look into folders in this order, it may fetch Sentbox messages first, and if we change the test this way and also give the Sentbox message later Date e.g. 09 Dec 2019, it would appear in the chat before the Inbox message which is wrong and happens because all outgoing messages are currently considered "noticed" (not really, but when it comes to sorting) because they get OutDelivered state in receive_imf::add_parts(). Also this breaks sorting of new incoming messages (as i noted in the code comment). I think this is not quite correct and would suggest to add some MessageState::Out = 17, i.e. somewhat undefined, state for received outgoing messages and sort such messages together with InFresh messages.
Does this PR try to fix some case where messages are pulled from Sent folder when used together with non-DC client?
No, it only tries to add new outgoing messages (e.g. from other devices) to the of the chat when possible, if they are delayed for any reason. At least they should appear in the chat under messages sent from the current device. I rather try to keep the current behaviour when old messages are pulled from Sentbox.
EDIT: Not sure about adding MessageState::Out though, it's not clear how to handle MDNs for them.
Not sure about adding
MessageState::Outthough, it's not clear how to handle MDNs for them.
MessageState::OutMdnRcvd isn't really used in the code, so it can be safely removed. Its value is even more questionable for group chat messages. I.e. it should be left in the enum for compatibility, but there's no need in this state in the db. Having msgs_mdns is sufficient and MessageState::OutMdnRcvd can be easily computed from it w/o significant overhead, there's an index by msg_id. So we can introduce MessageState::Out (or OutRcvd) that doesn't need to be converted to OutMdnRcvd or some other state, and such messages can form message sorting window together with InFresh messages.
No, it only tries to add new outgoing messages (e.g. from other devices) to the of the chat when possible, if they are delayed for any reason. At least they should appear in the chat under messages sent from the current device. I rather try to keep the current behaviour when old messages are pulled from Sentbox.
So this is a fix for Gmail? On most providers outgoing messages from Delta Chat never appears in the Sent folder, they are BCC-ed to self and arrive into the Inbox.
So this is a fix for Gmail? On most providers outgoing messages from Delta Chat never appears in the Sent folder, they are BCC-ed to self and arrive into the Inbox.
This PR is a fix for multi-device, it makes received outgoing messages appear in the bottom of the chat even if they have Date earlier than messages sent from the current device, to make them more visible. This may happen if messages are delayed because of network problems or clocks are not synchronised. Before that was done only for incoming messages.
As for Gmail, particularly if Delta Chat is used together with Gmail client, there's another problem -- Sentbox messages may appear before Inbox ones even if the latter are older and this can be fixed by introducing some MessageState::OutRcvd so that such messages can mingle with fresh incoming messages. OutPending, OutDelivered etc. messages shouldn't mingle with them, they are sent locally by the user and there's no need to make them more noticeable.
But if a received outgoing message is older than some non fresh message, better sort the received message purely by timestamp.
Actually this is an heuristic in this PR in order not to break the Gmail case simulated by test_old_message_4(). Maybe even better to do the following: if a received outgoing message is older than some InSeen message, sort the received message purely by timestamp. EDIT: Done.
Didn't expect that, but this is incompatible with 796b0d77525a18ddf34deb6f7b9cc9c975b448b2, need to debug.
EDIT: Can't reproduce the test_verified_group_vs_delete_server_after failure on my machine...
EDIT: This PR isn't the reason, the test also fails on main: https://github.com/deltachat/deltachat-core-rust/actions/runs/11329131300/job/31507389864. Probably the reason is a recent CI chatmail server change.
EDIT: On my machine at least all the CFFI Python tests pass.
Only test_verified_group_vs_delete_server_after and test_prefer_encrypt failed now which i can't reproduce locally with nine.testrun.org, so i think this may be merged taking into account that it passed the CI before