Merge pull request #3630 from element-hq/feature/bma/noIdentityChangeAlertInClearRoom

Do not render pin violation in clear rooms.

Author: bmarty

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt

@@ -25,9 +25,13 @@ import kotlinx.collections.immutable.PersistentList
 import kotlinx.collections.immutable.persistentListOf
 import kotlinx.collections.immutable.toPersistentList
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
+import kotlinx.coroutines.flow.filter
+import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
 import timber.log.Timber
@@ -56,22 +60,31 @@ class IdentityChangeStatePresenter @Inject constructor(
         )
     }
 
+    @OptIn(ExperimentalCoroutinesApi::class)
     private fun ProduceStateScope<PersistentList<RoomMemberIdentityStateChange>>.observeRoomMemberIdentityStateChange() {
-        combine(room.identityStateChangesFlow, room.membersStateFlow) { identityStateChanges, membersState ->
-            identityStateChanges.map { identityStateChange ->
-                val member = membersState.roomMembers()
-                    ?.firstOrNull { roomMember -> roomMember.userId == identityStateChange.userId }
-                    ?.toIdentityRoomMember()
-                    ?: createDefaultRoomMemberForIdentityChange(identityStateChange.userId)
-                RoomMemberIdentityStateChange(
-                    identityRoomMember = member,
-                    identityState = identityStateChange.identityState,
-                )
+        room.syncUpdateFlow
+            .filter {
+                // Room cannot become unencrypted, so we can just apply a filter here.
+                room.isEncrypted
             }
-        }
             .distinctUntilChanged()
-            .onEach { roomMemberIdentityStateChanges ->
-                value = roomMemberIdentityStateChanges.toPersistentList()
+            .flatMapLatest {
+                combine(room.identityStateChangesFlow, room.membersStateFlow,) { identityStateChanges, membersState ->
+                    identityStateChanges.map { identityStateChange ->
+                        val member = membersState.roomMembers()
+                            ?.firstOrNull { roomMember -> roomMember.userId == identityStateChange.userId }
+                            ?.toIdentityRoomMember()
+                            ?: createDefaultRoomMemberForIdentityChange(identityStateChange.userId)
+                        RoomMemberIdentityStateChange(
+                            identityRoomMember = member,
+                            identityState = identityStateChange.identityState,
+                        )
+                    }
+                }
+                    .distinctUntilChanged()
+                    .onEach { roomMemberIdentityStateChanges ->
+                        value = roomMemberIdentityStateChanges.toPersistentList()
+                    }
             }
             .launchIn(this)
     }

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt

@@ -44,7 +44,7 @@ class IdentityChangeStatePresenterTest {
 
     @Test
     fun `present - when the room emits identity change, the presenter emits new state`() = runTest {
-        val room = FakeMatrixRoom()
+        val room = FakeMatrixRoom(isEncrypted = true)
         val presenter = createIdentityChangeStatePresenter(room)
         presenter.test {
             val initialState = awaitItem()
@@ -65,10 +65,37 @@ class IdentityChangeStatePresenterTest {
         }
     }
 
+    @Test
+    fun `present - when the clear room emits identity change, the presenter does not emits new state`() = runTest {
+        val room = FakeMatrixRoom(isEncrypted = false)
+        val presenter = createIdentityChangeStatePresenter(room)
+        presenter.test {
+            val initialState = awaitItem()
+            assertThat(initialState.roomMemberIdentityStateChanges).isEmpty()
+            room.emitIdentityStateChanges(
+                listOf(
+                    IdentityStateChange(
+                        userId = A_USER_ID_2,
+                        identityState = IdentityState.PinViolation,
+                    ),
+                )
+            )
+            // No item emitted.
+            expectNoEvents()
+            // Room become encrypted.
+            room.enableEncryption()
+            val finalItem = awaitItem()
+            assertThat(finalItem.roomMemberIdentityStateChanges).hasSize(1)
+            val value = finalItem.roomMemberIdentityStateChanges.first()
+            assertThat(value.identityRoomMember.userId).isEqualTo(A_USER_ID_2)
+            assertThat(value.identityState).isEqualTo(IdentityState.PinViolation)
+        }
+    }
+
     @Test
     fun `present - when the room emits identity change, the presenter emits new state with member details`() =
         runTest {
-            val room = FakeMatrixRoom().apply {
+            val room = FakeMatrixRoom(isEncrypted = true).apply {
                 givenRoomMembersState(
                     MatrixRoomMembersState.Ready(
                         listOf(

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt

@@ -60,6 +60,7 @@ import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.asStateFlow
 import java.io.File
 
 class FakeMatrixRoom(
@@ -68,7 +69,7 @@ class FakeMatrixRoom(
     override val displayName: String = "",
     override val topic: String? = null,
     override val avatarUrl: String? = null,
-    override val isEncrypted: Boolean = false,
+    override var isEncrypted: Boolean = false,
     override val alias: RoomAlias? = null,
     override val alternativeAliases: List<RoomAlias> = emptyList(),
     override val isPublic: Boolean = true,
@@ -181,7 +182,17 @@ class FakeMatrixRoom(
         return Result.success(Unit)
     }
 
-    override val syncUpdateFlow: StateFlow<Long> = MutableStateFlow(0L)
+    fun enableEncryption() {
+        isEncrypted = true
+        emitSyncUpdate()
+    }
+
+    private val _syncUpdateFlow = MutableStateFlow(0L)
+    override val syncUpdateFlow: StateFlow<Long> = _syncUpdateFlow.asStateFlow()
+
+    fun emitSyncUpdate() {
+        _syncUpdateFlow.tryEmit(_syncUpdateFlow.value + 1)
+    }
 
     override suspend fun timelineFocusedOnEvent(eventId: EventId): Result<Timeline> = simulateLongTask {
         timelineFocusedOnEventResult(eventId)