change(crypto): have the RoomIdentityProvider return processing changes when identities transition to IdentityState::Verified too (#4670)

While implementing user verification on Element X I realized that the
`IdentityStatusChanges` stream does not notify us when an user becomes
`Verified`, which is a shame as it's perfect for powering app updates
after verifying users (i.e. all the decorations we show throughout the
app: room header, room details, room member list, room member profile
etc.)

Had a look at the existing code and, while that seems completely
intentional, there is no reason why we can't expand its purview.

crates/matrix-sdk-crypto/CHANGELOG.md

@@ -21,6 +21,10 @@ All notable changes to this project will be documented in this file.
 
 - Room keys are not shared with unsigned dehydrated devices.
   ([#4551](https://github.com/matrix-org/matrix-rust-sdk/pull/4551))
+  
+- Have the `RoomIdentityProvider` return processing changes when identities transition
+  to `IdentityState::Verified` too.
+  ([#4670](https://github.com/matrix-org/matrix-rust-sdk/pull/4670))
 
 ## [0.9.0] - 2024-12-18
 

crates/matrix-sdk-crypto/src/identities/room_identity_state.rs

@@ -146,6 +146,14 @@ impl<R: RoomIdentityProvider> RoomIdentityState<R> {
                 if let Some(user_identity @ UserIdentity::Other(_)) =
                     self.room.user_identity(user_id).await
                 {
+                    // Don't notify on membership changes of verified or pinned identities
+                    if matches!(
+                        self.room.state_of(&user_identity),
+                        IdentityState::Verified | IdentityState::Pinned
+                    ) {
+                        return vec![];
+                    }
+
                     match event.content.membership {
                         MembershipState::Join | MembershipState::Invite => {
                             // They are joining the room - check whether we need to display a
@@ -192,50 +200,20 @@ impl<R: RoomIdentityProvider> RoomIdentityState<R> {
     }
 
     /// Updates our internal state for this user to the supplied `new_state`. If
-    /// the change of state is significant (it requires something to change
-    /// in the UI, like a warning being added or removed), returns the
-    /// change information we will surface to the UI.
+    /// the state changed it returns the change information we will surface to
+    /// the UI.
     fn update_user_state_to(
         &mut self,
         user_id: &UserId,
         new_state: IdentityState,
     ) -> Option<IdentityStatusChange> {
-        use IdentityState::*;
-
         let old_state = self.known_states.get(user_id);
 
-        match (old_state, &new_state) {
-            // good -> bad - report so we can add a message
-            (Pinned, PinViolation) |
-            (Pinned, VerificationViolation) |
-            (Verified, PinViolation) |
-            (Verified, VerificationViolation) |
-
-            // bad -> good - report so we can remove a message
-            (PinViolation, Pinned) |
-            (PinViolation, Verified) |
-            (VerificationViolation, Pinned) |
-            (VerificationViolation, Verified) |
-
-            // Changed the type of bad - report so can change the message
-            (PinViolation, VerificationViolation) |
-            (VerificationViolation, PinViolation) => Some(self.set_state(user_id, new_state)),
-
-            // good -> good - don't report - no message needed in either case
-            (Pinned, Verified) |
-            (Verified, Pinned) => {
-                // The state has changed, so we update it
-                self.set_state(user_id, new_state);
-                // but there is no need to report a change to the UI
-                None
-            }
-
-            // State didn't change - don't report - nothing changed
-            (Pinned, Pinned) |
-            (Verified, Verified) |
-            (PinViolation, PinViolation) |
-            (VerificationViolation, VerificationViolation) => None,
+        if old_state == new_state {
+            return None;
         }
+
+        Some(self.set_state(user_id, new_state))
     }
 
     fn set_state(&mut self, user_id: &UserId, new_state: IdentityState) -> IdentityStatusChange {
@@ -397,7 +375,7 @@ mod tests {
     }
 
     #[async_test]
-    async fn test_verifying_a_pinned_identity_in_the_room_does_nothing() {
+    async fn test_verifying_a_pinned_identity_in_the_room_notifies() {
         // Given someone in the room is pinned
         let user_id = user_id!("@u:s.co");
         let mut room = FakeRoom::new();
@@ -409,8 +387,14 @@ mod tests {
             identity_change(&mut room, user_id, IdentityState::Verified, false, false).await;
         let update = state.process_change(updates).await;
 
-        // Then we emit no update
-        assert_eq!(update, vec![]);
+        // Then we emit an update
+        assert_eq!(
+            update,
+            vec![IdentityStatusChange {
+                user_id: user_id.to_owned(),
+                changed_to: IdentityState::Verified
+            }]
+        );
     }
 
     #[async_test]
@@ -722,7 +706,7 @@ mod tests {
 
     #[async_test]
     async fn test_a_verified_identity_leaving_the_room_does_nothing() {
-        // Given a pinned user is in the room
+        // Given a verified user is in the room
         let user_id = user_id!("@u:s.co");
         let mut room = FakeRoom::new();
         room.member(other_user_identity(user_id).await, IdentityState::Verified);

crates/matrix-sdk/src/room/identity_status_changes.rs

@@ -276,7 +276,7 @@ mod tests {
     }
 
     #[async_test]
-    async fn test_when_user_becomes_verified_we_dont_report_it() {
+    async fn test_when_user_becomes_verified_we_report_it() {
         // Given a room containing us and Bob
         let t = TestSetup::new_room_with_other_bob().await;
 
@@ -287,10 +287,16 @@ mod tests {
         // When Bob becomes verified
         t.verify_bob().await;
 
+        // Then we are notified about Bob being verified
+        let change = assert_next_with_timeout!(stream);
+        assert_eq!(change[0].user_id, t.bob_user_id());
+        assert_eq!(change[0].changed_to, IdentityState::Verified);
+        assert_eq!(change.len(), 1);
+
         // (And then unpinned, so we have something to come through the stream)
         t.unpin_bob().await;
 
-        // Then we are only notified about the unpinning part
+        // Then we are notified about the unpinning part
         let change = assert_next_with_timeout!(stream);
         assert_eq!(change[0].user_id, t.bob_user_id());
         assert_eq!(change[0].changed_to, IdentityState::VerificationViolation);