Merge pull request #7130 from vector-im/feature/bma/fix_verification

Fix empty verification bottom sheet

changelog.d/7130.bugfix

@@ -0,0 +1 @@
+ Fix empty verification bottom sheet.

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/SessionExtensions.kt

@@ -32,5 +32,13 @@ fun Session.getRoomSummary(roomIdOrAlias: String): RoomSummary? = roomService().
 
 /**
  * Get a user using the UserService of a Session.
+ * @param userId the userId to look for.
+ * @return a user with userId or null if the User is not known yet by the SDK.
+ * See [org.matrix.android.sdk.api.session.user.UserService.resolveUser] to ensure that a User is retrieved.
  */
 fun Session.getUser(userId: String): User? = userService().getUser(userId)
+
+/**
+ * Similar to [getUser], but fallback to a User without details if the User is not known by the SDK, or if Session is null.
+ */
+fun Session?.getUserOrDefault(userId: String): User = this?.userService()?.getUser(userId) ?: User(userId)

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/user/UserService.kt

@@ -29,7 +29,7 @@ interface UserService {
     /**
      * Get a user from a userId.
      * @param userId the userId to look for.
-     * @return a user with userId or null
+     * @return a user with userId or null if the User is not known yet by the SDK. See [resolveUser] to ensure that a User is retrieved.
      */
     fun getUser(userId: String): User?
 

vector/src/main/java/im/vector/app/features/createdirect/CreateDirectRoomViewModel.kt

@@ -37,11 +37,10 @@ import kotlinx.coroutines.launch
 import org.matrix.android.sdk.api.extensions.orFalse
 import org.matrix.android.sdk.api.raw.RawService
 import org.matrix.android.sdk.api.session.Session
-import org.matrix.android.sdk.api.session.getUser
+import org.matrix.android.sdk.api.session.getUserOrDefault
 import org.matrix.android.sdk.api.session.permalinks.PermalinkData
 import org.matrix.android.sdk.api.session.permalinks.PermalinkParser
 import org.matrix.android.sdk.api.session.room.model.create.CreateRoomParams
-import org.matrix.android.sdk.api.session.user.model.User
 
 class CreateDirectRoomViewModel @AssistedInject constructor(
         @Assisted initialState: CreateDirectRoomViewState,
@@ -78,11 +77,7 @@ class CreateDirectRoomViewModel @AssistedInject constructor(
                 _viewEvents.post(CreateDirectRoomViewEvents.DmSelf)
             } else {
                 // Try to get user from known users and fall back to creating a User object from MXID
-                val qrInvitee = if (session.getUser(mxid) != null) {
-                    session.getUser(mxid)!!
-                } else {
-                    User(mxid, null, null)
-                }
+                val qrInvitee = session.getUserOrDefault(mxid)
                 onSubmitInvitees(setOf(PendingSelection.UserPendingSelection(qrInvitee)))
             }
         }

vector/src/main/java/im/vector/app/features/crypto/verification/IncomingVerificationRequestHandler.kt

@@ -31,6 +31,7 @@ import org.matrix.android.sdk.api.session.crypto.verification.PendingVerificatio
 import org.matrix.android.sdk.api.session.crypto.verification.VerificationService
 import org.matrix.android.sdk.api.session.crypto.verification.VerificationTransaction
 import org.matrix.android.sdk.api.session.crypto.verification.VerificationTxState
+import org.matrix.android.sdk.api.session.getUserOrDefault
 import org.matrix.android.sdk.api.util.toMatrixItem
 import timber.log.Timber
 import javax.inject.Inject
@@ -67,8 +68,8 @@ class IncomingVerificationRequestHandler @Inject constructor(
         when (tx.state) {
             is VerificationTxState.OnStarted -> {
                 // Add a notification for every incoming request
-                val user = session?.userService()?.getUser(tx.otherUserId)
-                val name = user?.toMatrixItem()?.getBestName() ?: tx.otherUserId
+                val user = session.getUserOrDefault(tx.otherUserId).toMatrixItem()
+                val name = user.getBestName()
                 val alert = VerificationVectorAlert(
                         uid,
                         context.getString(R.string.sas_incoming_request_notif_title),
@@ -86,7 +87,7 @@ class IncomingVerificationRequestHandler @Inject constructor(
                         }
                 )
                         .apply {
-                            viewBinder = VerificationVectorAlert.ViewBinder(user?.toMatrixItem(), avatarRenderer.get())
+                            viewBinder = VerificationVectorAlert.ViewBinder(user, avatarRenderer.get())
                             contentAction = Runnable {
                                 (weakCurrentActivity?.get() as? VectorBaseActivity<*>)?.let {
                                     it.navigator.performDeviceVerification(it, tx.otherUserId, tx.transactionId)
@@ -131,8 +132,8 @@ class IncomingVerificationRequestHandler @Inject constructor(
                 // XXX this is a bit hard coded :/
                 popupAlertManager.cancelAlert("review_login")
             }
-            val user = session?.userService()?.getUser(pr.otherUserId)?.toMatrixItem()
-            val name = user?.getBestName() ?: pr.otherUserId
+            val user = session.getUserOrDefault(pr.otherUserId).toMatrixItem()
+            val name = user.getBestName()
             val description = if (name == pr.otherUserId) {
                 name
             } else {

vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheet.kt

@@ -152,29 +152,25 @@ class VerificationBottomSheet : VectorBaseBottomSheetDialogFragment<BottomSheetV
     }
 
     override fun invalidate() = withState(viewModel) { state ->
-        state.otherUserMxItem?.let { matrixItem ->
-            if (state.isMe) {
-                avatarRenderer.render(matrixItem, views.otherUserAvatarImageView)
-                if (state.sasTransactionState == VerificationTxState.Verified ||
-                        state.qrTransactionState == VerificationTxState.Verified ||
-                        state.verifiedFromPrivateKeys) {
-                    views.otherUserShield.render(RoomEncryptionTrustLevel.Trusted)
-                } else {
-                    views.otherUserShield.render(RoomEncryptionTrustLevel.Warning)
-                }
-                views.otherUserNameText.text = getString(
-                        if (state.selfVerificationMode) R.string.crosssigning_verify_this_session else R.string.crosssigning_verify_session
-                )
+        avatarRenderer.render(state.otherUserMxItem, views.otherUserAvatarImageView)
+        if (state.isMe) {
+            if (state.sasTransactionState == VerificationTxState.Verified ||
+                    state.qrTransactionState == VerificationTxState.Verified ||
+                    state.verifiedFromPrivateKeys) {
+                views.otherUserShield.render(RoomEncryptionTrustLevel.Trusted)
             } else {
-                avatarRenderer.render(matrixItem, views.otherUserAvatarImageView)
-
-                if (state.sasTransactionState == VerificationTxState.Verified || state.qrTransactionState == VerificationTxState.Verified) {
-                    views.otherUserNameText.text = getString(R.string.verification_verified_user, matrixItem.getBestName())
-                    views.otherUserShield.render(RoomEncryptionTrustLevel.Trusted)
-                } else {
-                    views.otherUserNameText.text = getString(R.string.verification_verify_user, matrixItem.getBestName())
-                    views.otherUserShield.render(null)
-                }
+                views.otherUserShield.render(RoomEncryptionTrustLevel.Warning)
+            }
+            views.otherUserNameText.text = getString(
+                    if (state.selfVerificationMode) R.string.crosssigning_verify_this_session else R.string.crosssigning_verify_session
+            )
+        } else {
+            if (state.sasTransactionState == VerificationTxState.Verified || state.qrTransactionState == VerificationTxState.Verified) {
+                views.otherUserNameText.text = getString(R.string.verification_verified_user, state.otherUserMxItem.getBestName())
+                views.otherUserShield.render(RoomEncryptionTrustLevel.Trusted)
+            } else {
+                views.otherUserNameText.text = getString(R.string.verification_verify_user, state.otherUserMxItem.getBestName())
+                views.otherUserShield.render(null)
             }
         }
 
@@ -235,7 +231,7 @@ class VerificationBottomSheet : VectorBaseBottomSheetDialogFragment<BottomSheetV
                     showFragment(
                             VerificationEmojiCodeFragment::class,
                             VerificationArgs(
-                                    state.otherUserMxItem?.id ?: "",
+                                    state.otherUserId,
                                     // If it was outgoing it.transaction id would be null, but the pending request
                                     // would be updated (from localId to txId)
                                     state.pendingRequest.invoke()?.transactionId ?: state.transactionId
@@ -271,7 +267,7 @@ class VerificationBottomSheet : VectorBaseBottomSheetDialogFragment<BottomSheetV
                         VerificationQRWaitingFragment::class,
                         VerificationQRWaitingFragment.Args(
                                 isMe = state.isMe,
-                                otherUserName = state.otherUserMxItem?.getBestName() ?: ""
+                                otherUserName = state.otherUserMxItem.getBestName()
                         )
                 )
                 return@withState
@@ -319,7 +315,7 @@ class VerificationBottomSheet : VectorBaseBottomSheetDialogFragment<BottomSheetV
                 showFragment(
                         VerificationChooseMethodFragment::class,
                         VerificationArgs(
-                                otherUserId = state.otherUserMxItem?.id ?: "",
+                                otherUserId = state.otherUserId,
                                 verificationId = state.pendingRequest.invoke()?.transactionId
                         )
                 )
@@ -328,7 +324,7 @@ class VerificationBottomSheet : VectorBaseBottomSheetDialogFragment<BottomSheetV
                 showFragment(
                         VerificationRequestFragment::class,
                         VerificationArgs(
-                                otherUserId = state.otherUserMxItem?.id ?: "",
+                                otherUserId = state.otherUserId,
                                 verificationId = state.pendingRequest.invoke()?.transactionId,
                                 verificationLocalId = state.roomId
                         )
@@ -340,7 +336,7 @@ class VerificationBottomSheet : VectorBaseBottomSheetDialogFragment<BottomSheetV
             showFragment(
                     VerificationChooseMethodFragment::class,
                     VerificationArgs(
-                            otherUserId = state.otherUserMxItem?.id ?: "",
+                            otherUserId = state.otherUserId,
                             verificationId = state.pendingRequest.invoke()?.transactionId
                     )
             )

vector/src/main/java/im/vector/app/features/crypto/verification/VerificationBottomSheetViewModel.kt

@@ -37,6 +37,7 @@ import kotlinx.coroutines.launch
 import org.matrix.android.sdk.api.Matrix
 import org.matrix.android.sdk.api.MatrixCallback
 import org.matrix.android.sdk.api.extensions.orFalse
+import org.matrix.android.sdk.api.extensions.tryOrNull
 import org.matrix.android.sdk.api.raw.RawService
 import org.matrix.android.sdk.api.session.Session
 import org.matrix.android.sdk.api.session.crypto.crosssigning.KEYBACKUP_SECRET_SSSS_NAME
@@ -70,7 +71,7 @@ data class VerificationBottomSheetViewState(
         val roomId: String?,
         // true when we display the loading and we wait for the other (incoming request)
         val selfVerificationMode: Boolean,
-        val otherUserMxItem: MatrixItem? = null,
+        val otherUserMxItem: MatrixItem,
         val pendingRequest: Async<PendingVerificationRequest> = Uninitialized,
         val pendingLocalId: String? = null,
         val sasTransactionState: VerificationTxState? = null,
@@ -92,7 +93,8 @@ data class VerificationBottomSheetViewState(
             otherUserId = args.otherUserId,
             verificationId = args.verificationId,
             roomId = args.roomId,
-            selfVerificationMode = args.selfVerificationMode
+            selfVerificationMode = args.selfVerificationMode,
+            otherUserMxItem = MatrixItem.UserItem(args.otherUserId),
     )
 }
 
@@ -126,7 +128,7 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(
             }
         }
 
-        val userItem = session.getUser(initialState.otherUserId)
+        fetchOtherUserProfile(initialState.otherUserId)
 
         var autoReady = false
         val pr = if (initialState.selfVerificationMode) {
@@ -160,7 +162,6 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(
 
         setState {
             copy(
-                    otherUserMxItem = userItem?.toMatrixItem(),
                     sasTransactionState = sasTx?.state,
                     qrTransactionState = qrTx?.state,
                     transactionId = pr?.transactionId ?: initialState.verificationId,
@@ -183,6 +184,28 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(
         }
     }
 
+    private fun fetchOtherUserProfile(otherUserId: String) {
+        session.getUser(otherUserId)?.toMatrixItem()?.let {
+            setState {
+                copy(
+                        otherUserMxItem = it
+                )
+            }
+        }
+        // Always fetch the latest User data
+        viewModelScope.launch {
+            tryOrNull { session.userService().resolveUser(otherUserId) }
+                    ?.toMatrixItem()
+                    ?.let {
+                        setState {
+                            copy(
+                                    otherUserMxItem = it
+                            )
+                        }
+                    }
+        }
+    }
+
     override fun onCleared() {
         session.cryptoService().verificationService().removeListener(this)
         super.onCleared()
@@ -216,12 +239,12 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(
 
     private fun cancelAllPendingVerifications(state: VerificationBottomSheetViewState) {
         session.cryptoService()
-                .verificationService().getExistingVerificationRequest(state.otherUserMxItem?.id ?: "", state.transactionId)?.let {
+                .verificationService().getExistingVerificationRequest(state.otherUserId, state.transactionId)?.let {
                     session.cryptoService().verificationService().cancelVerificationRequest(it)
                 }
         session.cryptoService()
                 .verificationService()
-                .getExistingTransaction(state.otherUserMxItem?.id ?: "", state.transactionId ?: "")
+                .getExistingTransaction(state.otherUserId, state.transactionId ?: "")
                 ?.cancel(CancelCode.User)
     }
 
@@ -249,7 +272,7 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(
     }
 
     override fun handle(action: VerificationAction) = withState { state ->
-        val otherUserId = state.otherUserMxItem?.id ?: return@withState
+        val otherUserId = state.otherUserId
         val roomId = state.roomId
                 ?: session.roomService().getExistingDirectRoomWithUser(otherUserId)
 
@@ -514,7 +537,7 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(
     override fun transactionUpdated(tx: VerificationTransaction) = withState { state ->
         if (state.selfVerificationMode && state.transactionId == null) {
             // is this an incoming with that user
-            if (tx.isIncoming && tx.otherUserId == state.otherUserMxItem?.id) {
+            if (tx.isIncoming && tx.otherUserId == state.otherUserId) {
                 // Also auto accept incoming if needed!
                 if (tx is IncomingSasVerificationTransaction) {
                     if (tx.uxState == IncomingSasVerificationTransaction.UxState.SHOW_ACCEPT) {
@@ -564,7 +587,7 @@ class VerificationBottomSheetViewModel @AssistedInject constructor(
 
         if (state.selfVerificationMode && state.pendingRequest.invoke() == null && state.transactionId == null) {
             // is this an incoming with that user
-            if (pr.isIncoming && pr.otherUserId == state.otherUserMxItem?.id) {
+            if (pr.isIncoming && pr.otherUserId == state.otherUserId) {
                 if (!pr.isReady) {
                     // auto ready in this case, as we are waiting
                     // TODO, can I be here in DM mode? in this case should test if roomID is null?

vector/src/main/java/im/vector/app/features/crypto/verification/cancel/VerificationCancelController.kt

@@ -26,6 +26,7 @@ import im.vector.app.core.utils.colorizeMatchingText
 import im.vector.app.features.crypto.verification.VerificationBottomSheetViewState
 import im.vector.app.features.crypto.verification.epoxy.bottomSheetVerificationActionItem
 import im.vector.app.features.crypto.verification.epoxy.bottomSheetVerificationNoticeItem
+import im.vector.app.features.displayname.getBestName
 import im.vector.lib.core.utils.epoxy.charsequence.EpoxyCharSequence
 import im.vector.lib.core.utils.epoxy.charsequence.toEpoxyCharSequence
 import javax.inject.Inject
@@ -60,8 +61,8 @@ class VerificationCancelController @Inject constructor(
                 }
             }
         } else {
-            val otherUserID = state.otherUserMxItem?.id ?: ""
-            val otherDisplayName = state.otherUserMxItem?.displayName ?: ""
+            val otherUserID = state.otherUserId
+            val otherDisplayName = state.otherUserMxItem.getBestName()
             bottomSheetVerificationNoticeItem {
                 id("notice")
                 notice(

vector/src/main/java/im/vector/app/features/crypto/verification/choose/VerificationChooseMethodFragment.kt

@@ -78,7 +78,7 @@ class VerificationChooseMethodFragment :
     override fun doVerifyBySas() = withState(sharedViewModel) { state ->
         sharedViewModel.handle(
                 VerificationAction.StartSASVerification(
-                        state.otherUserMxItem?.id ?: "",
+                        state.otherUserId,
                         state.pendingRequest.invoke()?.transactionId ?: ""
                 )
         )
@@ -130,7 +130,7 @@ class VerificationChooseMethodFragment :
     private fun onRemoteQrCodeScanned(remoteQrCode: String) = withState(sharedViewModel) { state ->
         sharedViewModel.handle(
                 VerificationAction.RemoteQrCodeScanned(
-                        state.otherUserMxItem?.id ?: "",
+                        state.otherUserId,
                         state.pendingRequest.invoke()?.transactionId ?: "",
                         remoteQrCode
                 )

vector/src/main/java/im/vector/app/features/crypto/verification/emoji/VerificationEmojiCodeController.kt

@@ -136,7 +136,7 @@ class VerificationEmojiCodeController @Inject constructor(
         if (state.isWaitingFromOther) {
             bottomSheetVerificationWaitingItem {
                 id("waiting")
-                title(host.stringProvider.getString(R.string.verification_request_waiting_for, state.otherUser?.getBestName() ?: ""))
+                title(host.stringProvider.getString(R.string.verification_request_waiting_for, state.otherUser.getBestName()))
             }
         } else {
             bottomSheetVerificationActionItem {

vector/src/main/java/im/vector/app/features/crypto/verification/emoji/VerificationEmojiCodeFragment.kt

@@ -68,13 +68,13 @@ class VerificationEmojiCodeFragment :
     }
 
     override fun onMatchButtonTapped() = withState(viewModel) { state ->
-        val otherUserId = state.otherUser?.id ?: return@withState
+        val otherUserId = state.otherUser.id
         val txId = state.transactionId ?: return@withState
         sharedViewModel.handle(VerificationAction.SASMatchAction(otherUserId, txId))
     }
 
     override fun onDoNotMatchButtonTapped() = withState(viewModel) { state ->
-        val otherUserId = state.otherUser?.id ?: return@withState
+        val otherUserId = state.otherUser.id
         val txId = state.transactionId ?: return@withState
         sharedViewModel.handle(VerificationAction.SASDoNotMatchAction(otherUserId, txId))
     }