code review: renaming, comments, extract common code

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

@@ -10,6 +10,6 @@ package io.element.android.features.messages.impl.crypto.identity
 import io.element.android.libraries.matrix.api.core.UserId
 
 sealed interface IdentityChangeEvent {
-    data class PinViolation(val userId: UserId) : IdentityChangeEvent
+    data class PinIdentity(val userId: UserId) : IdentityChangeEvent
-    data class VerificationViolation(val userId: UserId) : IdentityChangeEvent
+    data class WithdrawVerification(val userId: UserId) : IdentityChangeEvent
 }

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

@@ -8,10 +8,10 @@
 package io.element.android.features.messages.impl.crypto.identity
 
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.ProduceStateScope
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.produceState
 import androidx.compose.runtime.rememberCoroutineScope
+import io.element.android.features.messages.impl.messagecomposer.observeRoomMemberIdentityStateChange
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.designsystem.components.avatar.AvatarData
 import io.element.android.libraries.designsystem.components.avatar.AvatarSize
@@ -19,19 +19,9 @@ import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.encryption.EncryptionService
 import io.element.android.libraries.matrix.api.room.MatrixRoom
 import io.element.android.libraries.matrix.api.room.RoomMember
-import io.element.android.libraries.matrix.api.room.roomMembers
 import io.element.android.libraries.matrix.ui.model.getAvatarData
-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.onEach
 import kotlinx.coroutines.launch
 import timber.log.Timber
 import javax.inject.Inject
@@ -44,15 +34,15 @@ class IdentityChangeStatePresenter @Inject constructor(
     override fun present(): IdentityChangeState {
         val coroutineScope = rememberCoroutineScope()
         val roomMemberIdentityStateChange by produceState(persistentListOf()) {
-            observeRoomMemberIdentityStateChange()
+            observeRoomMemberIdentityStateChange(room)
         }
 
         fun handleEvent(event: IdentityChangeEvent) {
             when (event) {
-                is IdentityChangeEvent.VerificationViolation -> {
+                is IdentityChangeEvent.WithdrawVerification -> {
-                    coroutineScope.withdrawVerificationRequirement(event.userId)
+                    coroutineScope.withdrawVerification(event.userId)
                 }
-                is IdentityChangeEvent.PinViolation -> {
+                is IdentityChangeEvent.PinIdentity -> {
                     coroutineScope.pinUserIdentity(event.userId)
                 }
             }
@@ -64,35 +54,6 @@ class IdentityChangeStatePresenter @Inject constructor(
         )
     }
 
-    @OptIn(ExperimentalCoroutinesApi::class)
-    private fun ProduceStateScope<PersistentList<RoomMemberIdentityStateChange>>.observeRoomMemberIdentityStateChange() {
-        room.syncUpdateFlow
-            .filter {
-                // Room cannot become unencrypted, so we can just apply a filter here.
-                room.isEncrypted
-            }
-            .distinctUntilChanged()
-            .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)
-    }
-
     private fun CoroutineScope.pinUserIdentity(userId: UserId) = launch {
         encryptionService.pinUserIdentity(userId)
             .onFailure {
@@ -100,8 +61,8 @@ class IdentityChangeStatePresenter @Inject constructor(
             }
     }
 
-    private fun CoroutineScope.withdrawVerificationRequirement(userId: UserId) = launch {
+    private fun CoroutineScope.withdrawVerification(userId: UserId) = launch {
-        encryptionService.withdrawVerificationRequirement(userId)
+        encryptionService.withdrawVerification(userId)
             .onFailure {
                 Timber.e(it, "Failed to withdraw verification for user $userId")
             }

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

@@ -30,25 +30,24 @@ fun IdentityChangeStateView(
     onLinkClick: (String, Boolean) -> Unit,
     modifier: Modifier = Modifier,
 ) {
-    // Pick the first identity change to PinViolation
+    // Pick the first identity change that is in Pin or Verification violation
-    val pinViolationIdentityChange = state.roomMemberIdentityStateChanges.firstOrNull {
+    val maybeIdentityChangeViolation = state.roomMemberIdentityStateChanges.firstOrNull {
-        // For now only render PinViolation
         it.identityState == IdentityState.PinViolation ||
             it.identityState == IdentityState.VerificationViolation
     }
-    if (pinViolationIdentityChange != null) {
+    if (maybeIdentityChangeViolation != null) {
         ComposerAlertMolecule(
             modifier = modifier,
-            avatar = pinViolationIdentityChange.identityRoomMember.avatarData,
+            avatar = maybeIdentityChangeViolation.identityRoomMember.avatarData,
             content = buildAnnotatedString {
                 val learnMoreStr = stringResource(CommonStrings.action_learn_more)
-                val displayName = pinViolationIdentityChange.identityRoomMember.displayNameOrDefault
+                val displayName = maybeIdentityChangeViolation.identityRoomMember.displayNameOrDefault
                 val userIdStr = stringResource(
                     CommonStrings.crypto_identity_change_pin_violation_new_user_id,
-                    pinViolationIdentityChange.identityRoomMember.userId,
+                    maybeIdentityChangeViolation.identityRoomMember.userId,
                 )
 
-                val fullText = if (pinViolationIdentityChange.identityState == IdentityState.PinViolation) {
+                val fullText = if (maybeIdentityChangeViolation.identityState == IdentityState.PinViolation) {
                     stringResource(
                         id = CommonStrings.crypto_identity_change_pin_violation_new,
                         displayName,
@@ -93,19 +92,19 @@ fun IdentityChangeStateView(
                     end = learnMoreStartIndex + learnMoreStr.length,
                 )
             },
-            submitText = if (pinViolationIdentityChange.identityState == IdentityState.VerificationViolation) {
+            submitText = if (maybeIdentityChangeViolation.identityState == IdentityState.VerificationViolation) {
                 stringResource(CommonStrings.crypto_identity_change_withdraw_verification_action)
             } else {
                 stringResource(CommonStrings.action_ok)
             },
             onSubmitClick = {
-                if (pinViolationIdentityChange.identityState == IdentityState.VerificationViolation) {
+                if (maybeIdentityChangeViolation.identityState == IdentityState.VerificationViolation) {
-                    state.eventSink(IdentityChangeEvent.VerificationViolation(pinViolationIdentityChange.identityRoomMember.userId))
+                    state.eventSink(IdentityChangeEvent.WithdrawVerification(maybeIdentityChangeViolation.identityRoomMember.userId))
                 } else {
-                    state.eventSink(IdentityChangeEvent.PinViolation(pinViolationIdentityChange.identityRoomMember.userId))
+                    state.eventSink(IdentityChangeEvent.PinIdentity(maybeIdentityChangeViolation.identityRoomMember.userId))
                 }
             },
-            isCritical = pinViolationIdentityChange.identityState == IdentityState.VerificationViolation,
+            isCritical = maybeIdentityChangeViolation.identityState == IdentityState.VerificationViolation,
         )
     }
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt

@@ -403,7 +403,7 @@ class MessageComposerPresenter @AssistedInject constructor(
         }
 
         val roomMemberIdentityStateChange by produceState(persistentListOf()) {
-            observeRoomMemberIdentityStateChange()
+            observeRoomMemberIdentityStateChange(room)
         }
 
         return MessageComposerState(

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerUtils.kt

@@ -0,0 +1,53 @@
+/*
+ * Copyright 2025 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+package io.element.android.features.messages.impl.messagecomposer
+
+import androidx.compose.runtime.ProduceStateScope
+import io.element.android.features.messages.impl.crypto.identity.RoomMemberIdentityStateChange
+import io.element.android.features.messages.impl.crypto.identity.createDefaultRoomMemberForIdentityChange
+import io.element.android.features.messages.impl.crypto.identity.toIdentityRoomMember
+import io.element.android.libraries.matrix.api.room.MatrixRoom
+import io.element.android.libraries.matrix.api.room.roomMembers
+import kotlinx.collections.immutable.PersistentList
+import kotlinx.collections.immutable.toPersistentList
+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.onEach
+
+@OptIn(ExperimentalCoroutinesApi::class)
+fun ProduceStateScope<PersistentList<RoomMemberIdentityStateChange>>.observeRoomMemberIdentityStateChange(room: MatrixRoom) {
+    room.syncUpdateFlow
+        .filter {
+            // Room cannot become unencrypted, so we can just apply a filter here.
+            room.isEncrypted
+        }
+        .distinctUntilChanged()
+        .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

@@ -143,7 +143,7 @@ class IdentityChangeStatePresenterTest {
             val presenter = createIdentityChangeStatePresenter(encryptionService = encryptionService)
             presenter.test {
                 val initialState = awaitItem()
-                initialState.eventSink(IdentityChangeEvent.PinViolation(A_USER_ID))
+                initialState.eventSink(IdentityChangeEvent.PinIdentity(A_USER_ID))
                 lambda.assertions().isCalledOnce().with(value(A_USER_ID))
             }
         }
@@ -158,7 +158,7 @@ class IdentityChangeStatePresenterTest {
             val presenter = createIdentityChangeStatePresenter(encryptionService = encryptionService)
             presenter.test {
                 val initialState = awaitItem()
-                initialState.eventSink(IdentityChangeEvent.VerificationViolation(A_USER_ID))
+                initialState.eventSink(IdentityChangeEvent.WithdrawVerification(A_USER_ID))
                 lambda.assertions().isCalledOnce().with(value(A_USER_ID))
             }
         }

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

@@ -47,7 +47,7 @@ class IdentityChangeStateViewTest {
         rule.onNodeWithText("Alice", substring = true).assertExists("should display user displayname")
 
         rule.clickOn(res = CommonStrings.action_ok)
-        eventsRecorder.assertSingle(IdentityChangeEvent.PinViolation(UserId("@alice:localhost")))
+        eventsRecorder.assertSingle(IdentityChangeEvent.PinIdentity(UserId("@alice:localhost")))
     }
 
     @Test
@@ -70,7 +70,7 @@ class IdentityChangeStateViewTest {
         rule.onNodeWithText("Alice", substring = true).assertExists("should display user displayname")
 
         rule.clickOn(res = CommonStrings.crypto_identity_change_withdraw_verification_action)
-        eventsRecorder.assertSingle(IdentityChangeEvent.VerificationViolation(UserId("@alice:localhost")))
+        eventsRecorder.assertSingle(IdentityChangeEvent.WithdrawVerification(UserId("@alice:localhost")))
     }
 
     @Test

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/encryption/EncryptionService.kt

@@ -66,7 +66,15 @@ interface EncryptionService {
      * Remember this identity, ensuring it does not result in a pin violation.
      */
     suspend fun pinUserIdentity(userId: UserId): Result<Unit>
-    suspend fun withdrawVerificationRequirement(userId: UserId): Result<Unit>
+
+    /**
+     * Withdraw the verification for that user (also pin the identity).
+     *
+     * Useful when a user that was verified is not anymore, but it is not
+     * possible to re-verify immediately. This allows to restore communication by reverting the
+     * user trust from verified to TOFU verified.
+     */
+    suspend fun withdrawVerification(userId: UserId): Result<Unit>
 }
 
 /**

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/encryption/RustEncryptionService.kt

@@ -209,7 +209,7 @@ internal class RustEncryptionService(
         getUserIdentity(userId).pin()
     }
 
-    override suspend fun withdrawVerificationRequirement(userId: UserId): Result<Unit> = runCatching {
+    override suspend fun withdrawVerification(userId: UserId): Result<Unit> = runCatching {
         getUserIdentity(userId).withdrawVerification()
     }
 

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/encryption/FakeEncryptionService.kt

@@ -125,7 +125,7 @@ class FakeEncryptionService(
         return pinUserIdentityResult(userId)
     }
 
-    override suspend fun withdrawVerificationRequirement(userId: UserId): Result<Unit> {
+    override suspend fun withdrawVerification(userId: UserId): Result<Unit> {
         return withdrawVerificationResult(userId)
     }