From dd3cab94091123b7a1e163eca052d9e99e60e465 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 4 Aug 2023 15:52:55 +0200 Subject: [PATCH] fix: don't try /context when a notification has been filtered out --- .../matrix-sdk-ui/src/notification_client.rs | 41 ++++++++++--------- .../tests/integration/notification_client.rs | 8 +++- .../src/notification_client.rs | 20 +++++---- 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/crates/matrix-sdk-ui/src/notification_client.rs b/crates/matrix-sdk-ui/src/notification_client.rs index 2948c5273..a4b24b49b 100644 --- a/crates/matrix-sdk-ui/src/notification_client.rs +++ b/crates/matrix-sdk-ui/src/notification_client.rs @@ -98,10 +98,12 @@ impl NotificationClient { room_id: &RoomId, event_id: &EventId, ) -> Result, Error> { - if let Some(found) = self.get_notification_with_sliding_sync(room_id, event_id).await? { - Ok(Some(found)) - } else { - self.get_notification_with_context(room_id, event_id).await + match self.get_notification_with_sliding_sync(room_id, event_id).await? { + NotificationStatus::Event(event) => Ok(Some(event)), + NotificationStatus::EventFilteredOut => Ok(None), + NotificationStatus::EventNotFound => { + self.get_notification_with_context(room_id, event_id).await + } } } @@ -320,21 +322,11 @@ impl NotificationClient { &self, room_id: &RoomId, event_id: &EventId, - ) -> Result, Error> { + ) -> Result { tracing::info!("fetching notification event with a sliding sync"); - let mut raw_event = match self.try_sliding_sync(room_id, event_id).await { - Ok(Some(raw_event)) => raw_event, - - Ok(None) => { - tracing::debug!("notification sync hasn't found the event"); - return Ok(None); - } - - Err(err) => { - tracing::warn!("notification sync has run into an error: {err:#}"); - return Ok(None); - } + let Some(mut raw_event) = self.try_sliding_sync(room_id, event_id).await? else { + return Ok(NotificationStatus::EventNotFound); }; // At this point it should have been added by the sync, if it's not, give up. @@ -360,11 +352,11 @@ impl NotificationClient { if let Some(push_actions) = &push_actions { if self.filter_by_push_rules && !push_actions.iter().any(|a| a.should_notify()) { - return Ok(None); + return Ok(NotificationStatus::EventFilteredOut); } } - Ok(Some( + Ok(NotificationStatus::Event( NotificationItem::new(&room, &raw_event, push_actions.as_deref(), Vec::new()).await?, )) } @@ -376,6 +368,11 @@ impl NotificationClient { /// notification should already be there. In particular, the room containing /// the event MUST be known (via a sliding sync for invites, or another /// sliding sync). + /// + /// An error result means that we couldn't resolve the notification; in that + /// case, a dummy notification may be displayed instead. A `None` result + /// means the notification has been filtered out by the user's push + /// rules. pub async fn get_notification_with_context( &self, room_id: &RoomId, @@ -418,6 +415,12 @@ impl NotificationClient { } } +pub enum NotificationStatus { + Event(NotificationItem), + EventNotFound, + EventFilteredOut, +} + /// Builder for a `NotificationClient`. /// /// Fields have the same meaning as in `NotificationClient`. diff --git a/crates/matrix-sdk-ui/tests/integration/notification_client.rs b/crates/matrix-sdk-ui/tests/integration/notification_client.rs index 6e17cca00..aa5527d43 100644 --- a/crates/matrix-sdk-ui/tests/integration/notification_client.rs +++ b/crates/matrix-sdk-ui/tests/integration/notification_client.rs @@ -2,7 +2,9 @@ use std::{sync::Mutex, time::Duration}; use matrix_sdk::config::SyncSettings; use matrix_sdk_test::{async_test, JoinedRoomBuilder, SyncResponseBuilder, TimelineTestEvent}; -use matrix_sdk_ui::notification_client::{NotificationClient, NotificationEvent}; +use matrix_sdk_ui::notification_client::{ + NotificationClient, NotificationEvent, NotificationStatus, +}; use ruma::{event_id, events::TimelineEventType, room_id, user_id}; use serde_json::json; use wiremock::{ @@ -282,7 +284,9 @@ async fn test_notification_client_sliding_sync() { ) .await; - let item = item.expect("the notification should be found"); + let NotificationStatus::Event(item) = item else { + panic!("notification not found"); + }; assert_matches::assert_matches!(item.event, NotificationEvent::Timeline(event) => { assert_eq!(event.event_type(), TimelineEventType::RoomMessage); diff --git a/testing/sliding-sync-integration-test/src/notification_client.rs b/testing/sliding-sync-integration-test/src/notification_client.rs index dafbdb457..3ff92e7bf 100644 --- a/testing/sliding-sync-integration-test/src/notification_client.rs +++ b/testing/sliding-sync-integration-test/src/notification_client.rs @@ -17,7 +17,7 @@ use matrix_sdk::{ }; use matrix_sdk_integration_testing::helpers::get_client_for_user; use matrix_sdk_ui::notification_client::{ - Error, NotificationClient, NotificationEvent, NotificationItem, + Error, NotificationClient, NotificationEvent, NotificationItem, NotificationStatus, }; use tracing::warn; @@ -76,10 +76,11 @@ async fn test_notification() -> Result<()> { // Try with sliding sync first. let notification_client = NotificationClient::builder(bob.clone()).await.unwrap().build(); - let notification = notification_client - .get_notification_with_sliding_sync(&room_id, &event_id) - .await? - .expect("missing notification for the invite"); + let NotificationStatus::Event(notification) = + notification_client.get_notification_with_sliding_sync(&room_id, &event_id).await? + else { + panic!("event not found"); + }; warn!("sliding_sync: checking invite notification"); @@ -190,10 +191,11 @@ async fn test_notification() -> Result<()> { }; let notification_client = NotificationClient::builder(bob.clone()).await.unwrap().build(); - let notification = notification_client - .get_notification_with_sliding_sync(&room_id, &event_id) - .await? - .expect("missing notification for the message"); + let NotificationStatus::Event(notification) = + notification_client.get_notification_with_sliding_sync(&room_id, &event_id).await? + else { + panic!("event not found"); + }; check_notification(true, notification); let notification_client = NotificationClient::builder(bob.clone()).await.unwrap().build();