-
-
Notifications
You must be signed in to change notification settings - Fork 114
feat: send pre-message on messages with large attachments #7410
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.
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
Conversation
a283803 to
fcdd28c
Compare
e564a9b to
05a1967
Compare
90b61a1 to
1f0cc45
Compare
e97a448 to
2b900c3
Compare
and modify it to employ message reordering instead of a partially downloaded instance
full-messages and fix exclusion of gossip header, which was broken
`receive_imf::receive_imf_tests::test_dont_reverify_by_self_on_outgoing_msg`
2b900c3 to
305498d
Compare
and full message doesn't have these headers
easier solution for `should_do_gossip` with less changes. thanks to iequidoo for suggesting it apply test naming suggestion remove autoxrypt and self avatar check in `test_sending_pre_message` because it is already in `test_pre_message_contains_selfavatar_and_gossip_and_full_message_does_not` set self.attach_selfavatar to false in render method if pre message mode is full message merge `test_not_sending_pre_message_for_large_text` into `test_not_sending_pre_message_no_attachment`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I finally got around to review, sorry it took so long!
LGTM once comments are addressed.
src/download.rs
Outdated
| .await; | ||
|
|
||
| let mut msg = Message::new(Viewtype::File); | ||
| msg.set_file_from_bytes(&alice.ctx, "test.bin", &[0u8; 300_000], None)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a heads-up: AFAIK, &[0u8; 300_000] actually includes 300kb of bytes into the test binary, which slows down compile times a bit. I'm not sure right now what could be a good fix for this; maybe it's fine. Maybe vec![300_000] would be better. But we can just ignore the problem for now; if we want to improve compile times, then doing something about the many include_str!()s in tests has likely the higher impact.
Co-authored-by: Hocuri <hocuri@gmx.de>
| Ok(message) | ||
| } | ||
|
|
||
| pub fn will_be_encrypted(&self) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename to should_encrypt()? It sounds more consistent with other should_* functions like should_skip_autocrypt()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "should" sounds to vague in this case. it is used to set a variable with the name is_encrypted.
Co-authored-by: iequidoo <117991069+iequidoo@users.noreply.github.com>
part of #7367
progress / what's to do:
OutDeliveredis set when a message is sent out and has no remaining send jobs in the smtp table for this message id - so already works since full message and pre message have same msgId in that table.OutMdnRcvdis a "virtual" state ([Tracking Issue] Pre-Messages #7367 (comment)), so going back toOutDeliveredcan't happen anymoreChatFullMessageIdwith<and>like the other message idsreceive_imf::receive_imf_tests::test_dont_reverify_by_self_on_outgoing_msg