fix: don't try /context when a notification has been filtered out

This commit is contained in:
Benjamin Bouvier
2023-08-04 15:52:55 +02:00
parent aea0b00ac2
commit dd3cab9409
3 changed files with 39 additions and 30 deletions

View File

@@ -98,10 +98,12 @@ impl NotificationClient {
room_id: &RoomId,
event_id: &EventId,
) -> Result<Option<NotificationItem>, 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<Option<NotificationItem>, Error> {
) -> Result<NotificationStatus, Error> {
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`.

View File

@@ -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);

View File

@@ -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();