Skip to content

Commit

Permalink
read receipts: add an extra num_unread_notifications field
Browse files Browse the repository at this point in the history
This helps supporting cases where we want to show that a room has some activity (unread messages) but no notifications.
  • Loading branch information
bnjbvr committed Dec 19, 2023
1 parent 546637d commit e9fe6b8
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 6 deletions.
8 changes: 8 additions & 0 deletions bindings/matrix-sdk-ffi/src/room_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ pub struct RoomInfo {
user_defined_notification_mode: Option<RoomNotificationMode>,
has_room_call: bool,
active_room_call_participants: Vec<String>,
/// "Interesting" messages received in that room, independently of the
/// notification settings.
num_unread_messages: u64,
/// Events that will notify the user, according to their
/// notification settings.
num_unread_notifications: u64,
/// Events causing mentions/highlights for the user, according to their
/// notification settings.
num_unread_mentions: u64,
}

Expand Down Expand Up @@ -78,6 +85,7 @@ impl RoomInfo {
.map(|u| u.to_string())
.collect(),
num_unread_messages: room.num_unread_messages(),
num_unread_notifications: room.num_unread_notifications(),
num_unread_mentions: room.num_unread_mentions(),
})
}
Expand Down
12 changes: 8 additions & 4 deletions crates/matrix-sdk-base/src/read_receipts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ pub(crate) async fn compute_notifications(
// There's no new read-receipt here. We assume the cached events have been
// properly processed, and we only need to process the new events based
// on the previous receipt.
trace!("Couldn't find the event attached to the latest receipt; looking if the past latest known receipt refers to a new event...");
trace!("No new receipts, or couldn't find attached event; looking if the past latest known receipt refers to a new event...");
if find_and_count_events(&receipt_event_id, user_id, new_events.iter(), room_info) {
// We found the event to which the previous receipt attached to, so our work is
// done here.
Expand All @@ -114,7 +114,7 @@ pub(crate) async fn compute_notifications(
//
// In that case, accumulate all events as part of the current batch, and wait
// for the next receipt.
trace!("All other ways failed, including all new events for the receipts count.");
trace!("Default path: including all new events for the receipts count.");
for event in new_events {
count_unread_and_mentions(event, user_id, room_info);
}
Expand All @@ -128,9 +128,12 @@ fn count_unread_and_mentions(
user_id: &UserId,
room_info: &mut RoomInfo,
) {
if marks_as_unread(&event.event, user_id) {
room_info.read_receipts.num_unread += 1;
}
for action in &event.push_actions {
if action.should_notify() && marks_as_unread(&event.event, user_id) {
room_info.read_receipts.num_unread += 1;
if action.should_notify() {
room_info.read_receipts.num_notifications += 1;
}
if action.is_highlight() {
room_info.read_receipts.num_mentions += 1;
Expand Down Expand Up @@ -158,6 +161,7 @@ fn find_and_count_events<'a>(
// previous counts.
trace!("Found the event the receipt was referring to! Starting to count.");
room_info.read_receipts.num_unread = 0;
room_info.read_receipts.num_notifications = 0;
room_info.read_receipts.num_mentions = 0;
counting_receipts = true;
}
Expand Down
12 changes: 12 additions & 0 deletions crates/matrix-sdk-base/src/rooms/normal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,14 @@ impl Room {
self.inner.read().read_receipts.num_unread
}

/// Get the number of unread notifications (computed client-side).
///
/// This might be more precise than [`Self::unread_notification_counts`] for
/// encrypted rooms.
pub fn num_unread_notifications(&self) -> u64 {
self.inner.read().read_receipts.num_notifications
}

/// Get the number of unread mentions (computed client-side), that is,
/// messages causing a highlight in a room.
///
Expand Down Expand Up @@ -733,6 +741,9 @@ pub struct RoomReadReceipts {
/// Does the room have unread messages?
pub(crate) num_unread: u64,

/// Does the room have unread events that should notify?
pub(crate) num_notifications: u64,

/// Does the room have messages causing highlights for the users? (aka
/// mentions)
pub(crate) num_mentions: u64,
Expand Down Expand Up @@ -1345,6 +1356,7 @@ mod tests {
"read_receipts": {
"num_unread": 0,
"num_mentions": 0,
"num_notifications": 0,
"latest_read_receipt_event_id": null,
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use matrix_sdk::{
api::client::{
receipt::create_receipt::v3::ReceiptType,
room::create_room::v3::Request as CreateRoomRequest,
sync::sync_events::v4::{E2EEConfig, ReceiptsConfig, ToDeviceConfig},
sync::sync_events::v4::{
AccountDataConfig, E2EEConfig, ReceiptsConfig, ToDeviceConfig,
},
},
assign,
events::{
Expand Down Expand Up @@ -237,6 +239,9 @@ async fn test_room_notification_count() -> Result<()> {
let sync = alice
.sliding_sync("main")?
.with_receipt_extension(assign!(ReceiptsConfig::default(), { enabled: Some(true) }))
.with_account_data_extension(
assign!(AccountDataConfig::default(), { enabled: Some(true) }),
)
.add_list(
SlidingSyncList::builder("all")
.sync_mode(SlidingSyncMode::new_selective().add_range(0..=20)),
Expand Down Expand Up @@ -306,6 +311,7 @@ async fn test_room_notification_count() -> Result<()> {
// At first, nothing has happened, so we shouldn't have any notifications.
assert_eq!(alice_room.num_unread_messages(), 0);
assert_eq!(alice_room.num_unread_mentions(), 0);
assert_eq!(alice_room.num_unread_notifications(), 0);

assert_pending!(info_updates);

Expand All @@ -316,6 +322,7 @@ async fn test_room_notification_count() -> Result<()> {

assert_eq!(alice_room.num_unread_messages(), 0);
assert_eq!(alice_room.num_unread_mentions(), 0);
assert_eq!(alice_room.num_unread_notifications(), 0);
assert!(alice_room.latest_event().is_none());

assert_pending!(info_updates);
Expand All @@ -328,12 +335,12 @@ async fn test_room_notification_count() -> Result<()> {
assert!(info_updates.next().await.is_some());

assert_eq!(alice_room.num_unread_messages(), 1);
assert_eq!(alice_room.num_unread_notifications(), 1);
assert_eq!(alice_room.num_unread_mentions(), 0);

assert_pending!(info_updates);

// Bob sends a mention message.
let bob_room = bob.get_room(&room_id).expect("bob knows about alice's room");
bob_room
.send(
RoomMessageEventContent::text_plain("Hello my dear friend Alice!")
Expand All @@ -352,6 +359,7 @@ async fn test_room_notification_count() -> Result<()> {

// The highlight also counts as a notification.
assert_eq!(alice_room.num_unread_messages(), 2);
assert_eq!(alice_room.num_unread_notifications(), 2);
// One new highlight.
assert_eq!(alice_room.num_unread_mentions(), 1);
break;
Expand All @@ -377,6 +385,7 @@ async fn test_room_notification_count() -> Result<()> {
}

assert_eq!(alice_room.num_unread_messages(), 0);
assert_eq!(alice_room.num_unread_notifications(), 0);
assert_eq!(alice_room.num_unread_mentions(), 0);
break;
}
Expand All @@ -390,15 +399,71 @@ async fn test_room_notification_count() -> Result<()> {
assert!(info_updates.next().await.is_some());

assert_eq!(alice_room.num_unread_messages(), 0);
assert_eq!(alice_room.num_unread_notifications(), 0);
assert_eq!(alice_room.num_unread_mentions(), 0);

// Remote echo for our own message.
assert!(info_updates.next().await.is_some());

assert_eq!(alice_room.num_unread_messages(), 0);
assert_eq!(alice_room.num_unread_notifications(), 0);
assert_eq!(alice_room.num_unread_mentions(), 0);

assert_pending!(info_updates);

// Now Alice is only interesting in mentions of their name.
let settings = alice.notification_settings().await;

tracing::warn!("Updating room notification mode to mentions and keywords only...");
settings
.set_room_notification_mode(
alice_room.room_id(),
matrix_sdk::notification_settings::RoomNotificationMode::MentionsAndKeywordsOnly,
)
.await?;
tracing::warn!("Done!");

// Wait for remote echo.
let _ = settings.subscribe_to_changes().recv().await;

bob_room.send(RoomMessageEventContent::text_plain("I said hello!")).await?;

assert!(info_updates.next().await.is_some());

// The message doesn't contain a mention, so it doesn't notify Alice. But it
// exists.
assert_eq!(alice_room.num_unread_messages(), 1);
assert_eq!(alice_room.num_unread_notifications(), 0);
// One new highlight.
assert_eq!(alice_room.num_unread_mentions(), 0);

assert_pending!(info_updates);

// Bob sends a mention message.
bob_room
.send(
RoomMessageEventContent::text_plain("Why, hello there Alice!")
.set_mentions(Mentions::with_user_ids([alice.user_id().unwrap().to_owned()])),
)
.await?;

loop {
assert!(info_updates.next().await.is_some());

// FIXME we receive multiple spurious room info updates.
if alice_room.num_unread_messages() == 1 && alice_room.num_unread_mentions() == 0 {
tracing::warn!("ignoring");
continue;
}

// The highlight also counts as a notification.
assert_eq!(alice_room.num_unread_messages(), 2);
assert_eq!(alice_room.num_unread_notifications(), 1);
assert_eq!(alice_room.num_unread_mentions(), 1);
break;
}

assert_pending!(info_updates);

Ok(())
}

0 comments on commit e9fe6b8

Please sign in to comment.