Skip to content

Commit 74e2e76

Browse files
Hywanstefanceriu
Hywan
authored and
stefanceriu
committed
fix(base): Add RoomNotableUpdateReasons::NONE to… fix a possible regression.
This patch introduces a temporary hack. So here is the thing. Ideally, we DO NOT want to emit this reason. It does not makes sense. However, all notable update reasons are not clearly identified so far. Why is it a problem? The `matrix_sdk_ui::room_list_service::RoomList` is listening this stream of [`RoomInfoNotableUpdate`], and emits an update on a room item if it receives a notable reason. Because all reasons are not identified, we are likely to miss particular updates, and it can feel broken. Ultimately, we want to clearly identify all the notable update reasons, and remove this one.
1 parent 3ec831f commit 74e2e76

File tree

3 files changed

+111
-7
lines changed

3 files changed

+111
-7
lines changed

‎crates/matrix-sdk-base/src/rooms/normal.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,18 @@ bitflags! {
116116

117117
/// The display name has changed.
118118
const DISPLAY_NAME = 0b0010_0000;
119+
120+
/// This is a temporary hack.
121+
///
122+
/// So here is the thing. Ideally, we DO NOT want to emit this reason. It does not
123+
/// makes sense. However, all notable update reasons are not clearly identified
124+
/// so far. Why is it a problem? The `matrix_sdk_ui::room_list_service::RoomList`
125+
/// is listening this stream of [`RoomInfoNotableUpdate`], and emits an update on a
126+
/// room item if it receives a notable reason. Because all reasons are not
127+
/// identified, we are likely to miss particular updates, and it can feel broken.
128+
/// Ultimately, we want to clearly identify all the notable update reasons, and
129+
/// remove this one.
130+
const NONE = 0b1000_0000;
119131
}
120132
}
121133

@@ -1074,6 +1086,14 @@ impl Room {
10741086
room_id: self.room_id.clone(),
10751087
reasons: room_info_notable_update_reasons,
10761088
});
1089+
} else {
1090+
// TODO: remove this block!
1091+
// Read `RoomInfoNotableUpdateReasons::NONE` to understand why it must be
1092+
// removed.
1093+
let _ = self.room_info_notable_update_sender.send(RoomInfoNotableUpdate {
1094+
room_id: self.room_id.clone(),
1095+
reasons: RoomInfoNotableUpdateReasons::NONE,
1096+
});
10771097
}
10781098
}
10791099

‎crates/matrix-sdk-base/src/sliding_sync.rs‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,13 @@ mod tests {
11071107
let room = client.get_room(room_id).expect("No room found");
11081108
assert_eq!(room.cached_display_name().unwrap().to_string(), "Hello World");
11091109

1110+
assert_matches!(
1111+
room_info_notable_update.recv().await,
1112+
Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons }) => {
1113+
assert_eq!(received_room_id, room_id);
1114+
assert!(reasons.contains(RoomInfoNotableUpdateReasons::NONE));
1115+
}
1116+
);
11101117
assert_matches!(
11111118
room_info_notable_update.recv().await,
11121119
Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons }) => {
@@ -1813,6 +1820,13 @@ mod tests {
18131820
assert!(!received_reasons.contains(RoomInfoNotableUpdateReasons::RECENCY_STAMP));
18141821
}
18151822
);
1823+
assert_matches!(
1824+
room_info_notable_update_stream.recv().await,
1825+
Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => {
1826+
assert_eq!(received_room_id, room_id);
1827+
assert!(received_reasons.contains(RoomInfoNotableUpdateReasons::DISPLAY_NAME));
1828+
}
1829+
);
18161830
assert!(room_info_notable_update_stream.is_empty());
18171831

18181832
// When I send sliding sync response containing a room with a recency stamp.
@@ -1860,6 +1874,13 @@ mod tests {
18601874
assert!(!received_reasons.contains(RoomInfoNotableUpdateReasons::READ_RECEIPT));
18611875
}
18621876
);
1877+
assert_matches!(
1878+
room_info_notable_update_stream.recv().await,
1879+
Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => {
1880+
assert_eq!(received_room_id, room_id);
1881+
assert!(received_reasons.contains(RoomInfoNotableUpdateReasons::DISPLAY_NAME));
1882+
}
1883+
);
18631884
assert!(room_info_notable_update_stream.is_empty());
18641885

18651886
// When I send sliding sync response containing a couple of events with no read
@@ -1906,6 +1927,13 @@ mod tests {
19061927
.expect("Failed to process sync");
19071928

19081929
// Other notable update reason. We don't really care about them here.
1930+
assert_matches!(
1931+
room_info_notable_update_stream.recv().await,
1932+
Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => {
1933+
assert_eq!(received_room_id, room_id);
1934+
assert!(received_reasons.contains(RoomInfoNotableUpdateReasons::NONE));
1935+
}
1936+
);
19091937
assert_matches!(
19101938
room_info_notable_update_stream.recv().await,
19111939
Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => {
@@ -1938,6 +1966,13 @@ mod tests {
19381966
.expect("Failed to process sync");
19391967

19401968
// Room was already joined, no `MEMBERSHIP` update should be triggered here
1969+
assert_matches!(
1970+
room_info_notable_update_stream.recv().await,
1971+
Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => {
1972+
assert_eq!(received_room_id, room_id);
1973+
assert!(received_reasons.contains(RoomInfoNotableUpdateReasons::NONE));
1974+
}
1975+
);
19411976
assert!(room_info_notable_update_stream.is_empty());
19421977

19431978
let events = vec![Raw::from_json_string(
@@ -1987,7 +2022,14 @@ mod tests {
19872022
.await
19882023
.expect("Failed to process sync");
19892024

1990-
// Another notable update is received, but not the one we are interested by.
2025+
// Other notable updates are received, but not the ones we are interested by.
2026+
assert_matches!(
2027+
room_info_notable_update_stream.recv().await,
2028+
Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => {
2029+
assert_eq!(received_room_id, room_id);
2030+
assert!(received_reasons.contains(RoomInfoNotableUpdateReasons::NONE), "{received_reasons:?}");
2031+
}
2032+
);
19912033
assert_matches!(
19922034
room_info_notable_update_stream.recv().await,
19932035
Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => {
@@ -2034,6 +2076,13 @@ mod tests {
20342076
.await
20352077
.expect("Failed to process sync");
20362078

2079+
assert_matches!(
2080+
room_info_notable_update_stream.recv().await,
2081+
Ok(RoomInfoNotableUpdate { room_id: received_room_id, reasons: received_reasons }) => {
2082+
assert_eq!(received_room_id, room_id);
2083+
assert!(received_reasons.contains(RoomInfoNotableUpdateReasons::NONE), "{received_reasons:?}");
2084+
}
2085+
);
20372086
assert!(room_info_notable_update_stream.is_empty());
20382087

20392088
// …Unless its value changes!

‎crates/matrix-sdk-ui/tests/integration/room_list_service.rs‎

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,8 +1488,20 @@ async fn test_dynamic_entries_stream() -> Result<(), Error> {
14881488
push front [ "!r4:bar.org" ];
14891489
end;
14901490
};
1491-
// TODO (@hywan): Must be removed once we restore `RoomInfoNotableUpdate`
1492-
// filtering inside `RoomList`.
1491+
// TODO (@hywan): Remove as soon as `RoomInfoNotableUpdateReasons::NONE` is
1492+
// removed.
1493+
assert_entries_batch! {
1494+
[dynamic_entries_stream]
1495+
set [ 1 ] [ "!r1:bar.org" ];
1496+
end;
1497+
};
1498+
assert_entries_batch! {
1499+
[dynamic_entries_stream]
1500+
set [ 0 ] [ "!r4:bar.org" ];
1501+
end;
1502+
};
1503+
// TODO (@hywan): Remove as soon as `RoomInfoNotableUpdateReasons::NONE` is
1504+
// removed.
14931505
assert_entries_batch! {
14941506
[dynamic_entries_stream]
14951507
set [ 1 ] [ "!r1:bar.org" ];
@@ -1581,8 +1593,8 @@ async fn test_dynamic_entries_stream() -> Result<(), Error> {
15811593
push front [ "!r7:bar.org" ];
15821594
end;
15831595
};
1584-
// TODO (@hywan): Must be removed once we restore `RoomInfoNotableUpdate`
1585-
// filtering inside `RoomList`.
1596+
// TODO (@hywan): Remove as soon as `RoomInfoNotableUpdateReasons::NONE` is
1597+
// removed.
15861598
assert_entries_batch! {
15871599
[dynamic_entries_stream]
15881600
set [ 1 ] [ "!r5:bar.org" ];
@@ -1594,6 +1606,21 @@ async fn test_dynamic_entries_stream() -> Result<(), Error> {
15941606
end;
15951607
};
15961608

1609+
// TODO (@hywan): Remove as soon as `RoomInfoNotableUpdateReasons::NONE` is
1610+
// removed.
1611+
assert_entries_batch! {
1612+
[dynamic_entries_stream]
1613+
set [ 1 ] [ "!r5:bar.org" ];
1614+
end;
1615+
};
1616+
// TODO (@hywan): Remove as soon as `RoomInfoNotableUpdateReasons::NONE` is
1617+
// removed.
1618+
assert_entries_batch! {
1619+
[dynamic_entries_stream]
1620+
set [ 0 ] [ "!r7:bar.org" ];
1621+
end;
1622+
};
1623+
15971624
assert_pending!(dynamic_entries_stream);
15981625

15991626
// Now, let's change the dynamic entries!
@@ -1990,8 +2017,16 @@ async fn test_room_sorting() -> Result<(), Error> {
19902017
// | 4 | !r1 | 6 | Aaa |
19912018
// | 5 | !r4 | 5 | |
19922019

1993-
// TODO (@hywan): Must be removed once we restore `RoomInfoNotableUpdate`
1994-
// filtering inside `RoomList`.
2020+
// TODO (@hywan): Remove as soon as `RoomInfoNotableUpdateReasons::NONE` is
2021+
// removed.
2022+
assert_entries_batch! {
2023+
[stream]
2024+
set [ 2 ] [ "!r6:bar.org" ];
2025+
end;
2026+
};
2027+
2028+
// TODO (@hywan): Remove as soon as `RoomInfoNotableUpdateReasons::NONE` is
2029+
// removed.
19952030
assert_entries_batch! {
19962031
[stream]
19972032
set [ 2 ] [ "!r6:bar.org" ];

0 commit comments

Comments
 (0)