Skip to content

fix: Emit MsgsChanged, not IncomingMsg, for messages only having special parts (#8157)#8170

Merged
iequidoo merged 2 commits intomainfrom
iequidoo/location-only-no-IncomingMsg
May 1, 2026
Merged

fix: Emit MsgsChanged, not IncomingMsg, for messages only having special parts (#8157)#8170
iequidoo merged 2 commits intomainfrom
iequidoo/location-only-no-IncomingMsg

Conversation

@iequidoo
Copy link
Copy Markdown
Collaborator

@iequidoo iequidoo commented Apr 24, 2026

An example of such messages is location-only messages.
Fix #8157

@iequidoo iequidoo marked this pull request as ready for review April 24, 2026 23:48
@iequidoo iequidoo requested review from adbenitez and link2xt April 24, 2026 23:48
@Hocuri
Copy link
Copy Markdown
Collaborator

Hocuri commented Apr 28, 2026

This didn't fix the bug when I tried it out on Android together with @adbenitez. Adding a debug log:

diff --git a/src/receive_imf.rs b/src/receive_imf.rs
index 9c7a2395c..7a48025e1 100644
--- a/src/receive_imf.rs
+++ b/src/receive_imf.rs
@@ -1080,6 +1080,10 @@ pub(crate) async fn receive_imf_inner(
             && fresh
             && !is_old_contact_request
             && !skip_bot_notify;
+        info!(
+            context,
+            "dbg important: {important}, parts: {:?}", mime_parser.parts
+        );
 
         for msg_id in &received_msg.msg_ids {
             chat_id.emit_msg_event(context, *msg_id, important);

yields:

04-28 16:35:36.015  1491  1638 I DeltaChat: [accId=4] src/receive_imf.rs:1083: dbg important: true, parts: [Part { typ: Text, mimetype: None, msg: "", msg_raw: None, bytes: 0, param: Params { inner: {GuaranteeE2ee: "1"} }, org_filename: None, error: None, dehtml_failed: false, is_related: false, is_reaction: false }]
04-28 16:36:38.307  1491  1638 I DeltaChat: [accId=4] src/receive_imf.rs:1083: dbg important: true, parts: [Part { typ: Text, mimetype: None, msg: "", msg_raw: None, bytes: 0, param: Params { inner: {GuaranteeE2ee: "1"} }, org_filename: None, error: None, dehtml_failed: false, is_related: false, is_reaction: false }]
04-28 16:37:39.979  1491  1638 I DeltaChat: [accId=4] src/receive_imf.rs:1083: dbg important: true, parts: [Part { typ: Text, mimetype: None, msg: "", msg_raw: None, bytes: 0, param: Params { inner: {GuaranteeE2ee: "1"} }, org_filename: None, error: None, dehtml_failed: false, is_related: false, is_reaction: false }]

so, the location message did contain a text part, with empty text. It's weird that this seems to be different in the Rust tests written by @iequidoo

@iequidoo iequidoo marked this pull request as draft April 28, 2026 14:43
@iequidoo iequidoo force-pushed the iequidoo/location-only-no-IncomingMsg branch 2 times, most recently from 7884c51 to c71346e Compare April 30, 2026 02:07
Comment thread src/receive_imf.rs Outdated
@iequidoo iequidoo marked this pull request as ready for review April 30, 2026 02:37
@iequidoo iequidoo force-pushed the iequidoo/location-only-no-IncomingMsg branch from c71346e to 1e67d1c Compare April 30, 2026 03:48
@iequidoo iequidoo requested a review from Hocuri April 30, 2026 03:50
Copy link
Copy Markdown
Collaborator

@Hocuri Hocuri left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

Comment thread src/test_utils.rs Outdated
#[derive(Debug)]
pub struct EventTracker(EventEmitter);

pub mod event_tracker {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why put this into an extra module? Since all the other event tracking logic is not in this module, it looks like this should also not be in an extra module.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is just a syntactic sugar because EventTracker is a tuple struct and can't have GetMatchingArgs inside. It's also what Rust suggests when i try to add it to impl EventTracker:

error: struct is not supported in `trait`s or `impl`s
[...]
     = help: consider moving the struct out to a nearby module scope

I want to have GetMatchingArgs as a struct so that get_matching_ex() is really extensible, i.e. if new args with reasonable defaults are added, you don't need to fix up all usages of the function.

Copy link
Copy Markdown
Collaborator

@Hocuri Hocuri Apr 30, 2026

Choose a reason for hiding this comment

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

Seems surprising to have a module called event_tracker, but EventTracker is outside of it. Just not putting it into a module, and then renaming it, looks cleaner:

diff --git a/src/location.rs b/src/location.rs
index d78edc645..ac0ff2a47 100644
--- a/src/location.rs
+++ b/src/location.rs
@@ -873,3 +873,3 @@ mod tests {
     use crate::receive_imf::receive_imf;
-    use crate::test_utils::{TestContext, TestContextManager, event_tracker::GetMatchingArgs};
+    use crate::test_utils::{ExpectedEvents, TestContext, TestContextManager};
     use crate::tools::SystemTime;
@@ -1127,3 +1127,3 @@ async fn test_delete_expired_locations() -> Result<()> {
                 bob,
-                GetMatchingArgs {
+                ExpectedEvents {
                     expected: |e| matches!(e, EventType::MsgsChanged { .. }),
diff --git a/src/test_utils.rs b/src/test_utils.rs
index 2470d95b9..94bdce166 100644
--- a/src/test_utils.rs
+++ b/src/test_utils.rs
@@ -1433,7 +1433,4 @@ pub fn fiona_keypair() -> SignedSecretKey {
 
-pub mod event_tracker {
-    use super::EventType;
-
 /// See [`super::EventTracker::get_matching_ex`].
-    pub struct GetMatchingArgs<E: Fn(&EventType) -> bool, U: Fn(&EventType) -> bool> {
+pub struct ExpectedEvents<E: Fn(&EventType) -> bool, U: Fn(&EventType) -> bool> {
     pub expected: E,
@@ -1441,3 +1438,2 @@ pub struct GetMatchingArgs<E: Fn(&EventType) -> bool, U: Fn(&EventType) -> bool>
 }
-}
 
@@ -1487,3 +1483,3 @@ pub async fn get_matching_opt<F: Fn(&EventType) -> bool>(
             ctx,
-            event_tracker::GetMatchingArgs {
+            ExpectedEvents {
                 expected: event_matcher,
@@ -1500,3 +1496,3 @@ pub async fn get_matching_ex<E: Fn(&EventType) -> bool, U: Fn(&EventType) -> boo
         ctx: &Context,
-        args: event_tracker::GetMatchingArgs<E, U>,
+        args: ExpectedEvents<E, U>,
     ) -> Option<EventType> {

Copy link
Copy Markdown
Collaborator

@Hocuri Hocuri Apr 30, 2026

Choose a reason for hiding this comment

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

I went ahead and pushed a commit with this change because Github doesn't have a good UI for suggesting medium-sized diffs

…ial parts (#8157)

An example of such messages is location-only messages.

Co-authored-by: Hocuri <hocuri@gmx.de>
@iequidoo iequidoo force-pushed the iequidoo/location-only-no-IncomingMsg branch from 4eeffd1 to 53bcad3 Compare May 1, 2026 01:00
@iequidoo iequidoo merged commit 4d53754 into main May 1, 2026
19 checks passed
@iequidoo iequidoo deleted the iequidoo/location-only-no-IncomingMsg branch May 1, 2026 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

location streaming: hidden location message triggering notification

2 participants