Code review

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/DefaultIdentityService.kt

@@ -202,6 +202,8 @@ internal class DefaultIdentityService @Inject constructor(
 
             identityStore.setUrl(urlCandidate)
             identityStore.setToken(token)
+            // could we remember if it was previously given?
+            identityStore.setUserConsent(false)
             updateIdentityAPI(urlCandidate)
 
             updateAccountData(urlCandidate)

vector/src/main/java/im/vector/app/core/utils/Dialogs.kt

@@ -20,6 +20,7 @@ import android.content.Context
 import android.webkit.WebView
 import android.webkit.WebViewClient
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
+import im.vector.app.R
 
 /**
  * Open a web view above the current activity.
@@ -38,3 +39,14 @@ fun Context.displayInWebView(url: String) {
             .setPositiveButton(android.R.string.ok, null)
             .show()
 }
+
+fun Context.showIdentityServerConsentDialog(configuredIdentityServer: String?, consentCallBack: (() -> Unit)) {
+    MaterialAlertDialogBuilder(this)
+            .setTitle(R.string.identity_server_consent_dialog_title)
+            .setMessage(getString(R.string.identity_server_consent_dialog_content, configuredIdentityServer ?: ""))
+            .setPositiveButton(R.string.yes) { _, _ ->
+                consentCallBack?.invoke()
+            }
+            .setNegativeButton(R.string.no, null)
+            .show()
+}

vector/src/main/java/im/vector/app/features/contactsbook/ContactsBookFragment.kt

@@ -23,14 +23,13 @@ import android.view.ViewGroup
 import androidx.core.view.isVisible
 import com.airbnb.mvrx.activityViewModel
 import com.airbnb.mvrx.withState
-import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import com.jakewharton.rxbinding3.widget.checkedChanges
 import com.jakewharton.rxbinding3.widget.textChanges
-import im.vector.app.R
 import im.vector.app.core.extensions.cleanup
 import im.vector.app.core.extensions.configureWith
 import im.vector.app.core.extensions.hideKeyboard
 import im.vector.app.core.platform.VectorBaseFragment
+import im.vector.app.core.utils.showIdentityServerConsentDialog
 import im.vector.app.databinding.FragmentContactsBookBinding
 import im.vector.app.features.userdirectory.PendingSelection
 import im.vector.app.features.userdirectory.UserListAction
@@ -76,14 +75,9 @@ class ContactsBookFragment @Inject constructor(
     private fun setupConsentView() {
         views.phoneBookSearchForMatrixContacts.setOnClickListener {
             withState(contactsBookViewModel) { state ->
-                MaterialAlertDialogBuilder(requireActivity())
+                requireContext().showIdentityServerConsentDialog(state.identityServerUrl) {
-                        .setTitle(R.string.identity_server_consent_dialog_title)
+                    contactsBookViewModel.handle(ContactsBookAction.UserConsentGranted)
-                        .setMessage(getString(R.string.identity_server_consent_dialog_content, state.identityServerUrl ?: ""))
+                }
-                        .setPositiveButton(R.string.yes) { _, _ ->
-                            contactsBookViewModel.handle(ContactsBookAction.UserConsentGranted)
-                        }
-                        .setNegativeButton(R.string.no, null)
-                        .show()
             }
         }
     }

vector/src/main/java/im/vector/app/features/discovery/DiscoverySettingsFragment.kt

@@ -32,6 +32,7 @@ import im.vector.app.core.extensions.observeEvent
 import im.vector.app.core.extensions.registerStartForActivityResult
 import im.vector.app.core.platform.VectorBaseFragment
 import im.vector.app.core.utils.ensureProtocol
+import im.vector.app.core.utils.showIdentityServerConsentDialog
 import im.vector.app.databinding.FragmentGenericRecyclerBinding
 import im.vector.app.features.discovery.change.SetIdentityServerFragment
 import im.vector.app.features.settings.VectorSettingsActivity
@@ -179,14 +180,9 @@ class DiscoverySettingsFragment @Inject constructor(
     override fun onTapUpdateUserConsent(newValue: Boolean) {
         if (newValue) {
             withState(viewModel) { state ->
-                MaterialAlertDialogBuilder(requireActivity())
+                requireContext().showIdentityServerConsentDialog(state.identityServer.invoke()) {
-                        .setTitle(R.string.identity_server_consent_dialog_title)
+                    viewModel.handle(DiscoverySettingsAction.UpdateUserConsent(true))
-                        .setMessage(getString(R.string.identity_server_consent_dialog_content, state.identityServer.invoke()))
+                }
-                        .setPositiveButton(R.string.yes) { _, _ ->
-                            viewModel.handle(DiscoverySettingsAction.UpdateUserConsent(true))
-                        }
-                        .setNegativeButton(R.string.no, null)
-                        .show()
             }
         } else {
             viewModel.handle(DiscoverySettingsAction.UpdateUserConsent(false))

vector/src/main/java/im/vector/app/features/discovery/DiscoverySettingsViewModel.kt

@@ -65,7 +65,7 @@ class DiscoverySettingsViewModel @AssistedInject constructor(
             setState {
                 copy(
                         identityServer = Success(identityServerUrl),
-                        userConsent = false
+                        userConsent = identityService.getUserConsent()
                 )
             }
             if (currentIS != identityServerUrl) retrieveBinding()

vector/src/main/java/im/vector/app/features/userdirectory/InviteByEmailItem.kt

@@ -28,7 +28,7 @@ import im.vector.app.core.epoxy.VectorEpoxyModel
 import im.vector.app.features.home.AvatarRenderer
 
 @EpoxyModelClass(layout = R.layout.item_invite_by_mail)
-abstract class FoundThreePidItem : VectorEpoxyModel<FoundThreePidItem.Holder>() {
+abstract class InviteByEmailItem : VectorEpoxyModel<InviteByEmailItem.Holder>() {
 
     @EpoxyAttribute lateinit var avatarRenderer: AvatarRenderer
     @EpoxyAttribute lateinit var foundItem: ThreePidUser

vector/src/main/java/im/vector/app/features/userdirectory/UserListController.kt

@@ -93,45 +93,46 @@ class UserListController @Inject constructor(private val session: Session,
 
         when (val matchingEmail = currentState.matchingEmail) {
             is Success -> {
-                userListHeaderItem {
+                matchingEmail()?.let { threePidUser ->
-                    id("is_matching")
+                    userListHeaderItem {
-                    header(host.stringProvider.getString(R.string.discovery_section, currentState.configuredIdentityServer ?: ""))
+                        id("identity_server_result_header")
-                }
+                        header(host.stringProvider.getString(R.string.discovery_section, currentState.configuredIdentityServer ?: ""))
-                val invoke = matchingEmail()
+                    }
-                val isSelected = currentState.pendingSelections.indexOfFirst { pendingSelection ->
+                    val isSelected = currentState.pendingSelections.any { pendingSelection ->
-                    when (pendingSelection) {
+                        when (pendingSelection) {
-                        is PendingSelection.ThreePidPendingSelection -> {
+                            is PendingSelection.ThreePidPendingSelection -> {
-                            when (pendingSelection.threePid) {
+                                when (pendingSelection.threePid) {
-                                is ThreePid.Email  -> pendingSelection.threePid.email == invoke?.email
+                                    is ThreePid.Email  -> pendingSelection.threePid.email == threePidUser.email
-                                is ThreePid.Msisdn -> false
+                                    is ThreePid.Msisdn -> false
+                                }
+                            }
+                            is PendingSelection.UserPendingSelection     -> {
+                                threePidUser.user != null && threePidUser.user.userId == pendingSelection.user.userId
                             }
                         }
-                        is PendingSelection.UserPendingSelection     -> {
-                            invoke?.user != null && invoke.user.userId == pendingSelection.user.userId
-                        }
                     }
-                }  != -1
+                    if (threePidUser.user == null) {
-                if (invoke?.user == null) {
+                        inviteByEmailItem {
-                    foundThreePidItem {
+                            id("email_${threePidUser.email}")
-                        id("email_${invoke?.email}")
+                            foundItem(threePidUser)
-                        foundItem(invoke!!)
+                            selected(isSelected)
-                        selected(isSelected)
+                            clickListener {
-                        clickListener {
+                                host.callback?.onThreePidClick(ThreePid.Email(threePidUser.email))
-                            host.callback?.onThreePidClick(ThreePid.Email(invoke.email))
+                            }
                         }
-                    }
+                    } else {
-                } else {
+                        userDirectoryUserItem {
-                    userDirectoryUserItem {
+                            id(threePidUser.user.userId)
-                        id(invoke.user.userId)
+                            selected(isSelected)
-                        selected(isSelected)
+                            matrixItem(threePidUser.user.toMatrixItem().let {
-                        matrixItem(invoke.user.toMatrixItem().let {
+                                it.copy(
-                            it.copy(
+                                        displayName = "${it.getBestName()} [${threePidUser.email}]"
-                                    displayName = "${it.displayName} [${invoke.email}]"
+                                )
-                            )
+                            })
-                        })
+                            avatarRenderer(host.avatarRenderer)
-                        avatarRenderer(host.avatarRenderer)
+                            clickListener {
-                        clickListener {
+                                host.callback?.onItemClick(threePidUser.user)
-                            host.callback?.onItemClick(invoke.user)
+                            }
                         }
                     }
                 }
@@ -191,7 +192,7 @@ class UserListController @Inject constructor(private val session: Session,
             }
             is Loading -> {
                 userListHeaderItem {
-                    id("is_matching")
+                    id("identity_server_result_header_loading")
                     header(host.stringProvider.getString(R.string.discovery_section, currentState.configuredIdentityServer ?: ""))
                 }
                 loadingItem {

vector/src/main/java/im/vector/app/features/userdirectory/UserListFragment.kt

@@ -31,7 +31,6 @@ import com.airbnb.mvrx.args
 import com.airbnb.mvrx.fragmentViewModel
 import com.airbnb.mvrx.withState
 import com.google.android.material.chip.Chip
-import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import com.jakewharton.rxbinding3.widget.textChanges
 import im.vector.app.R
 import im.vector.app.core.extensions.cleanup
@@ -40,6 +39,7 @@ import im.vector.app.core.extensions.hideKeyboard
 import im.vector.app.core.extensions.setupAsSearch
 import im.vector.app.core.platform.VectorBaseFragment
 import im.vector.app.core.utils.DimensionConverter
+import im.vector.app.core.utils.showIdentityServerConsentDialog
 import im.vector.app.core.utils.startSharePlainTextIntent
 import im.vector.app.databinding.FragmentUserListBinding
 import im.vector.app.features.homeserver.HomeServerCapabilitiesViewModel
@@ -228,14 +228,9 @@ class UserListFragment @Inject constructor(
 
     override fun giveIdentityServerConsent() {
         withState(viewModel) { state ->
-            MaterialAlertDialogBuilder(requireActivity())
+            requireContext().showIdentityServerConsentDialog(state.configuredIdentityServer) {
-                    .setTitle(R.string.identity_server_consent_dialog_title)
+                viewModel.handle(UserListAction.UpdateUserConsent(true))
-                    .setMessage(getString(R.string.identity_server_consent_dialog_content, state.configuredIdentityServer ?: ""))
+            }
-                    .setPositiveButton(R.string.yes) { _, _ ->
-                        viewModel.handle(UserListAction.UpdateUserConsent(true))
-                    }
-                    .setNegativeButton(R.string.no, null)
-                    .show()
         }
     }
 

vector/src/main/res/layout/item_invite_by_mail.xml

@@ -1,6 +1,4 @@
 <?xml version="1.0" encoding="utf-8"?>
-
-
 <androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:app="http://schemas.android.com/apk/res-auto"
     xmlns:tools="http://schemas.android.com/tools"
@@ -13,7 +11,7 @@
     android:padding="8dp">
 
     <FrameLayout
-        android:id="@+id/knownUserAvatarContainer"
+        android:id="@+id/iconContainer"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:layout_marginStart="8dp"
@@ -58,7 +56,7 @@
         app:layout_constraintBottom_toTopOf="@+id/itemDescription"
         app:layout_constraintEnd_toEndOf="parent"
         app:layout_constraintHorizontal_bias="0.5"
-        app:layout_constraintStart_toEndOf="@+id/knownUserAvatarContainer"
+        app:layout_constraintStart_toEndOf="@+id/iconContainer"
         app:layout_constraintTop_toTopOf="parent"
         tools:text="foo@example.com" />
 

vector/src/main/res/values/strings.xml

@@ -3483,6 +3483,7 @@
     <string name="finish_setting_up_discovery">Finish setting up discovery.</string>
     <string name="discovery_invite">Invite by email, find contacts and more…</string>
     <string name="finish_setup">Finish setup</string>
+    <!-- %s will be replaced by the user identity server domain, e.g vector.im -->
     <string name="discovery_section">Discovery (%s)</string>