fix: Do not retrieve notification events sent by ignored users by jmartinesp · Pull Request #5081 · matrix-org/matrix-rust-sdk

jmartinesp · 2025-05-23T12:51:56Z

When retrieving notifications using either sliding sync or the /context endpoint, check if the sender is ignored by the user before returning it. If it is, just filter out the notification.

Public API changes documented in changelogs (optional)

Signed-off-by:

codecov · 2025-05-23T13:06:12Z

Codecov Report

Attention: Patch coverage is 70.17544% with 17 lines in your changes missing coverage. Please review.

Project coverage is 85.79%. Comparing base (f0c7370) to head (f23fdc6).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/matrix-sdk/src/test_utils/mocks/mod.rs	58.33%	10 Missing ⚠️
crates/matrix-sdk-base/src/client.rs	50.00%	6 Missing ⚠️
crates/matrix-sdk-ui/src/notification_client.rs	94.73%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5081      +/-   ##
==========================================
- Coverage   85.83%   85.79%   -0.04%     
==========================================
  Files         333      333              
  Lines       36158    36205      +47     
==========================================
+ Hits        31037    31063      +26     
- Misses       5121     5142      +21

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Hywan · 2025-05-26T08:55:54Z

+    pub fn is_user_ignored(&self, user_id: &UserId) -> bool {
+        let raw_user_id = user_id.to_string();
+        let current_ignored_user_list = self.subscribe_to_ignore_user_list_changes().get();
+        current_ignored_user_list.contains(&raw_user_id)
+    }


subscribe_to_ignore_user_list_changes is only “runtime-aware”: it knows the changes that happen during the current run of the SDK. If a user has been ignored in a previous instance of the SDK, you will miss it.

I think it's better to use something like this (not tested):

let state_store = get_it_from_somewhere(); let list = state_store.get_account_data_event_static::<IgnoredUserListEvent>()…;

The new is_user_ignored is nice! That's a good idea. It's just the body of the method that should change.

Also, the method should be on matrix_sdk_base::Client, not matrix_sdk::Client.

Hywan

Almost there! Thanks for the new patch.

Hywan · 2025-05-26T09:48:24Z

+                    current_ignored_user_list.content.ignored_users.contains_key(user_id)
+                }
+                Err(error) => {
+                    warn!("Failed to deserialize ignored user list event: {error:?}.");


Suggested change

warn!("Failed to deserialize ignored user list event: {error:?}.");

warn!(?error, "Failed to deserialize the ignored user list event");

Hywan · 2025-05-26T09:48:44Z

+            },
+            Ok(None) => false,
+            Err(error) => {
+                warn!("Could not get ignored user list. {error:?}.");


Suggested change

warn!("Could not get ignored user list. {error:?}.");

warn!(?error, "Could not get the ignored user list from the state store");

Hywan · 2025-05-26T09:50:40Z

+        let user_id = user_id!("@alice:example.org");
+        let client =
+            BaseClient::new(StoreConfig::new("cross-process-store-locks-holder-name".to_owned()));
+
+        client
+            .activate(
+                SessionMeta { user_id: user_id.to_owned(), device_id: "FOOBAR".into() },
+                RoomLoadSettings::default(),
+                #[cfg(feature = "e2e-encryption")]
+                None,
+            )
+            .await
+            .unwrap();


Suggested change

let user_id = user_id!("@alice:example.org");

let client =

BaseClient::new(StoreConfig::new("cross-process-store-locks-holder-name".to_owned()));

client

.activate(

SessionMeta { user_id: user_id.to_owned(), device_id: "FOOBAR".into() },

RoomLoadSettings::default(),

#[cfg(feature = "e2e-encryption")]

None,

)

.await

.unwrap();

let client = logged_in_base_client(None).await;

Hywan · 2025-05-26T10:19:18Z

+                            {
+                                "content": {
+                                    "avatar_url": sender_avatar_url,
+                                    "displayname": sender_display_name,
+                                    "membership": "join"
+                                },
+                                "room_id": room_id,
+                                "event_id": "$151800140517rfvjc:example.org",
+                                "membership": "join",
+                                "origin_server_ts": 151800140,
+                                "sender": sender,
+                                "state_key": sender,
+                                "type": "m.room.member",
+                                "unsigned": {
+                                    "age": 2970366,
+                                }
+                            },


I think you may want to use the EventFactory here 🙄.

These are mostly copy-pasted from other tests, but sure, I can do that.

You'd be my hero if you also rewrote the other existing tests so they used the event factory <3

I can probably do that on a separate PR.

Ping me and I'll review it with a great pleasure. Thank you, thank you, thank you.

… `Client::is_user_ignored` Also move it to `BaseClient` instead.

jmartinesp requested a review from a team as a code owner May 23, 2025 12:51

jmartinesp requested review from poljar and removed request for a team May 23, 2025 12:51

Hywan requested review from Hywan and removed request for poljar May 26, 2025 08:17

Hywan mentioned this pull request May 26, 2025

Receiving invites from blocked users element-hq/synapse#18479

Open

Hywan requested changes May 26, 2025

View reviewed changes

Hywan approved these changes May 26, 2025

View reviewed changes

jmartinesp added 2 commits May 26, 2025 12:48

fix: Do not retrieve notification events sent by ignored users

c68a2c9

refactor: Fetch the ignored user list account data from the store for…

f23fdc6

… `Client::is_user_ignored` Also move it to `BaseClient` instead.

jmartinesp force-pushed the fix/do-not-retrieve-notifications-from-ignored-users branch from c6f5d7b to f23fdc6 Compare May 26, 2025 10:48

jmartinesp enabled auto-merge (rebase) May 26, 2025 10:58

jmartinesp merged commit e53eaf4 into main May 26, 2025
41 checks passed

jmartinesp deleted the fix/do-not-retrieve-notifications-from-ignored-users branch May 26, 2025 11:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Do not retrieve notification events sent by ignored users#5081

fix: Do not retrieve notification events sent by ignored users#5081
jmartinesp merged 2 commits intomainfrom
fix/do-not-retrieve-notifications-from-ignored-users

jmartinesp commented May 23, 2025 •

edited

Loading

Uh oh!

codecov Bot commented May 23, 2025 •

edited

Loading

Uh oh!

Hywan May 26, 2025

Uh oh!

Hywan left a comment

Uh oh!

Hywan May 26, 2025

Uh oh!

Hywan May 26, 2025

Uh oh!

Hywan May 26, 2025

Uh oh!

Hywan May 26, 2025

Uh oh!

jmartinesp May 26, 2025

Uh oh!

bnjbvr May 26, 2025

Uh oh!

jmartinesp May 26, 2025

Uh oh!

Hywan May 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	warn!("Failed to deserialize ignored user list event: {error:?}.");
	warn!(?error, "Failed to deserialize the ignored user list event");

	warn!("Could not get ignored user list. {error:?}.");
	warn!(?error, "Could not get the ignored user list from the state store");

Conversation

jmartinesp commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hywan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jmartinesp commented May 23, 2025 •

edited

Loading

codecov Bot commented May 23, 2025 •

edited

Loading