Prefer Chat-Group-ID over In-Reply-To and References #5551
Conversation
Chat-Group-ID always correctly identifies the chat message was sent to, while In-Reply-To and References may point to a message that has itself been incorrectly assigned to a chat.
When there are no parent references, Delta Chat inserts Message-ID into References. Such references should be ignored because otherwise fully downloaded message may be assigned to the same chat as previously incorrectly assigned partially downloaded message. Fixes receive_imf::tests::test_create_group_with_big_msg
link2xt
force-pushed
the
link2xt/chat-group-id-priority
branch
from
d3a6811
to
a784a6b
Compare
| if chat_id.is_none() { | ||
| if let Some(grpid) = mime_parser.get_chat_group_id() { | ||
| if let Some((id, _protected, blocked)) = | ||
| chat::get_chat_id_by_grpid(context, grpid).await? |
There was a problem hiding this comment.
But this doesn't create a group, only looks it up, maybe we need to call create_or_lookup_group() instead? And also otherwise we have the duplicated logic here and in create_or_lookup_group()
There was a problem hiding this comment.
Yes, this only looks up the group and only using Chat-Group-ID header. This ensures that it only happens for fully downloaded messages and group ID is not exctracted from Message-ID or References to messages send by old Delta Chat that included group ID into Message-IDs.
I want to make a minimal change to always assign fully downloaded group messages to known groups first. create_or_lookup_group() I think should be split to factor out group lookup and only call it when we were not able to assign to existing group, but this refactoring is for later PR.
| @@ -810,6 +811,12 @@ impl MimeMessage { | |||
| self.headers.get(headerdef.get_headername()) | |||
| } | |||
|
|
|||
| /// Returns `Chat-Group-ID` header value if it is a valid group ID. | |||
| pub fn get_chat_group_id(&self) -> Option<&String> { | |||
There was a problem hiding this comment.
This could probably return the more idiomatic &str? Not that it matters much, feel free to ignore
There was a problem hiding this comment.
This is what .get_header() returns, I just passed it though.
There was a problem hiding this comment.
I made a follow-up PR fixing both functions: #5559
| let parent = get_parent_message(context, mime_parser) | ||
| .await? | ||
| .filter(|p| Some(p.id) != replace_msg_id); |
There was a problem hiding this comment.
Note that this may lead to the message suddenly disappear after clicking the "Download" button, which may be equally confusing. OTOH, we can just merge it and see whether people complain, and then think about which behavior is better (or whether whether we should even send a toast "Oops, this message actually belongs to group XY, fixed it now", which ofc would need support from the UIs)
There was a problem hiding this comment.
Note that this may lead to the message suddenly disappear after clicking the "Download" button
This was already the case before, most messages don't reference themselves. Delta Chat puts self-reference into References only if there are no better references found.
Fixes #5549