Add some guard, and allow to cancel adding 3pid

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

@@ -107,6 +107,12 @@ interface ProfileService {
                                accountPassword: String?,
                                matrixCallback: MatrixCallback<Unit>): Cancelable
 
+    /**
+     * Cancel adding a threepid. It will remove data store locally about this ThreePid
+     */
+    fun cancelAddingThreePid(threePid: ThreePid,
+                             matrixCallback: MatrixCallback<Unit>): Cancelable
+
     /**
      * Delete a 3Pids.
      */

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/profile/DefaultProfileService.kt

@@ -163,7 +163,25 @@ internal class DefaultProfileService @Inject constructor(private val taskExecuto
                                         accountPassword: String?,
                                         matrixCallback: MatrixCallback<Unit>): Cancelable {
         return finalizeAddingThreePidTask
-                .configureWith(FinalizeAddingThreePidTask.Params(threePid, uiaSession, accountPassword)) {
+                .configureWith(FinalizeAddingThreePidTask.Params(
+                        threePid = threePid,
+                        session = uiaSession,
+                        accountPassword = accountPassword,
+                        userWantsToCancel = false
+                )) {
+                    callback = alsoRefresh(matrixCallback)
+                }
+                .executeBy(taskExecutor)
+    }
+
+    override fun cancelAddingThreePid(threePid: ThreePid, matrixCallback: MatrixCallback<Unit>): Cancelable {
+        return finalizeAddingThreePidTask
+                .configureWith(FinalizeAddingThreePidTask.Params(
+                        threePid = threePid,
+                        session = null,
+                        accountPassword = null,
+                        userWantsToCancel = true
+                )) {
                     callback = alsoRefresh(matrixCallback)
                 }
                 .executeBy(taskExecutor)

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/profile/FinalizeAddingThreePidTask.kt

@@ -36,7 +36,8 @@ internal abstract class FinalizeAddingThreePidTask : Task<FinalizeAddingThreePid
     data class Params(
             val threePid: ThreePid,
             val session: String?,
-            val accountPassword: String?
+            val accountPassword: String?,
+            val userWantsToCancel: Boolean
     )
 }
 
@@ -48,35 +49,41 @@ internal class DefaultFinalizeAddingThreePidTask @Inject constructor(
         private val eventBus: EventBus) : FinalizeAddingThreePidTask() {
 
     override suspend fun execute(params: Params) {
-        // Get the required pending data
+        if (params.userWantsToCancel.not()) {
-        val pendingThreePids = monarchy.fetchAllMappedSync(
+            // Get the required pending data
-                { it.where(PendingThreePidEntity::class.java) },
+            val pendingThreePids = monarchy.fetchAllMappedSync(
-                { pendingThreePidMapper.map(it) }
+                    { it.where(PendingThreePidEntity::class.java) },
-        )
+                    { pendingThreePidMapper.map(it) }
-                .firstOrNull { it.threePid == params.threePid }
+            )
-                ?: throw IllegalArgumentException("unknown threepid")
+                    .firstOrNull { it.threePid == params.threePid }
+                    ?: throw IllegalArgumentException("unknown threepid")
 
-        try {
+            try {
-            executeRequest<Unit>(eventBus) {
+                executeRequest<Unit>(eventBus) {
-                val body = FinalizeAddThreePidBody(
+                    val body = FinalizeAddThreePidBody(
-                        clientSecret = pendingThreePids.clientSecret,
+                            clientSecret = pendingThreePids.clientSecret,
-                        sid = pendingThreePids.sid,
+                            sid = pendingThreePids.sid,
-                        auth = if (params.session != null && params.accountPassword != null) {
+                            auth = if (params.session != null && params.accountPassword != null) {
-                            UserPasswordAuth(
+                                UserPasswordAuth(
-                                    session = params.session,
+                                        session = params.session,
-                                    user = userId,
+                                        user = userId,
-                                    password = params.accountPassword
+                                        password = params.accountPassword
-                            )
+                                )
-                        } else null
+                            } else null
-                )
+                    )
-                apiCall = profileAPI.finalizeAddThreePid(body)
+                    apiCall = profileAPI.finalizeAddThreePid(body)
+                }
+            } catch (throwable: Throwable) {
+                throw throwable.toRegistrationFlowResponse()
+                        ?.let { Failure.RegistrationFlowError(it) }
+                        ?: throwable
             }
-        } catch (throwable: Throwable) {
-            throw throwable.toRegistrationFlowResponse()
-                    ?.let { Failure.RegistrationFlowError(it) }
-                    ?: throwable
         }
 
+        cleanupDatabase(params)
+    }
+
+    private suspend fun cleanupDatabase(params: Params) {
         // Delete the pending three pid
         monarchy.awaitTransaction { realm ->
             realm.where(PendingThreePidEntity::class.java)

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

@@ -341,25 +341,22 @@ class DiscoverySettingsController @Inject constructor(
     private fun buildContinueCancel(threePid: ThreePid) {
         settingsContinueCancelItem {
             id("bottom${threePid.value}")
-            interactionListener(object : SettingsContinueCancelItem.Listener {
+            continueOnClick {
-                override fun onContinue() {
+                when (threePid) {
-                    when (threePid) {
+                    is ThreePid.Email  -> {
-                        is ThreePid.Email  -> {
+                        listener?.checkEmailVerification(threePid)
-                            listener?.checkEmailVerification(threePid)
+                    }
-                        }
+                    is ThreePid.Msisdn -> {
-                        is ThreePid.Msisdn -> {
+                        val code = codes[threePid]
-                            val code = codes[threePid]
+                        if (code != null) {
-                            if (code != null) {
+                            listener?.sendMsisdnVerificationCode(threePid, code)
-                                listener?.sendMsisdnVerificationCode(threePid, code)
-                            }
                         }
                     }
                 }
-
+            }
-                override fun onCancel() {
+            cancelOnClick {
-                    listener?.cancelBinding(threePid)
+                listener?.cancelBinding(threePid)
-                }
+            }
-            })
         }
     }
 

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

@@ -20,33 +20,28 @@ import com.airbnb.epoxy.EpoxyAttribute
 import com.airbnb.epoxy.EpoxyModelClass
 import com.airbnb.epoxy.EpoxyModelWithHolder
 import im.vector.app.R
+import im.vector.app.core.epoxy.ClickListener
 import im.vector.app.core.epoxy.VectorEpoxyHolder
+import im.vector.app.core.epoxy.onClick
 
 @EpoxyModelClass(layout = R.layout.item_settings_continue_cancel)
 abstract class SettingsContinueCancelItem : EpoxyModelWithHolder<SettingsContinueCancelItem.Holder>() {
 
     @EpoxyAttribute
-    var interactionListener: Listener? = null
+    var continueOnClick: ClickListener? = null
+
+    @EpoxyAttribute
+    var cancelOnClick: ClickListener? = null
 
     override fun bind(holder: Holder) {
         super.bind(holder)
 
-        holder.cancelButton.setOnClickListener {
+        holder.cancelButton.onClick(cancelOnClick)
-            interactionListener?.onCancel()
+        holder.continueButton.onClick(continueOnClick)
-        }
-
-        holder.continueButton.setOnClickListener {
-            interactionListener?.onContinue()
-        }
     }
 
     class Holder : VectorEpoxyHolder() {
         val cancelButton by bind<Button>(R.id.settings_item_cancel_button)
         val continueButton by bind<Button>(R.id.settings_item_continue_button)
     }
-
-    interface Listener {
-        fun onContinue()
-        fun onCancel()
-    }
 }

vector/src/main/java/im/vector/app/features/settings/threepids/ThreePidsSettingsAction.kt

@@ -22,6 +22,7 @@ import org.matrix.android.sdk.api.session.identity.ThreePid
 sealed class ThreePidsSettingsAction : VectorViewModelAction {
     data class AddThreePid(val threePid: ThreePid) : ThreePidsSettingsAction()
     data class ContinueThreePid(val threePid: ThreePid) : ThreePidsSettingsAction()
+    data class CancelThreePid(val threePid: ThreePid) : ThreePidsSettingsAction()
     data class AccountPassword(val password: String) : ThreePidsSettingsAction()
     data class DeleteThreePid(val threePid: ThreePid) : ThreePidsSettingsAction()
 }

vector/src/main/java/im/vector/app/features/settings/threepids/ThreePidsSettingsController.kt

@@ -30,6 +30,8 @@ import im.vector.app.core.ui.list.GenericItem
 import im.vector.app.core.ui.list.genericButtonItem
 import im.vector.app.core.ui.list.genericFooterItem
 import im.vector.app.core.ui.list.genericItem
+import im.vector.app.features.discovery.settingsContinueCancelItem
+import im.vector.app.features.discovery.settingsInformationItem
 import im.vector.app.features.discovery.settingsSectionTitleItem
 import org.matrix.android.sdk.api.session.identity.ThreePid
 import javax.inject.Inject
@@ -43,6 +45,7 @@ class ThreePidsSettingsController @Inject constructor(
         fun addEmail()
         fun addMsisdn()
         fun continueThreePid(threePid: ThreePid)
+        fun cancelThreePid(threePid: ThreePid)
         fun deleteThreePid(threePid: ThreePid)
     }
 
@@ -80,12 +83,12 @@ class ThreePidsSettingsController @Inject constructor(
             title(stringProvider.getString(R.string.settings_emails))
         }
 
-        emails.forEach { buildThreePid("email_", it) }
+        emails.forEach { buildThreePid("email ", it) }
 
         // Pending threePids
         pendingThreePids.invoke()
                 ?.filterIsInstance(ThreePid.Email::class.java)
-                ?.forEach { buildPendingThreePid("email_", it) }
+                ?.forEach { buildPendingThreePid("email ", it) }
 
         genericButtonItem {
             id("addEmail")
@@ -99,12 +102,12 @@ class ThreePidsSettingsController @Inject constructor(
             title(stringProvider.getString(R.string.settings_phone_numbers))
         }
 
-        msisdn.forEach { buildThreePid("msisdn_", it) }
+        msisdn.forEach { buildThreePid("msisdn ", it) }
 
         // Pending threePids
         pendingThreePids.invoke()
                 ?.filterIsInstance(ThreePid.Msisdn::class.java)
-                ?.forEach { buildPendingThreePid("msisdn_", it) }
+                ?.forEach { buildPendingThreePid("msisdn ", it) }
 
         genericButtonItem {
             id("addMsisdn")
@@ -131,15 +134,20 @@ class ThreePidsSettingsController @Inject constructor(
         genericItem {
             id(idPrefix + threePid.value)
             title(threePid.value)
-            if (threePid is ThreePid.Email) {
+        }
-                description(stringProvider.getString(R.string.account_email_validation_message))
+
+        if (threePid is ThreePid.Email) {
+            settingsInformationItem {
+                id("info" + idPrefix + threePid.value)
+                message(stringProvider.getString(R.string.account_email_validation_message))
+                colorProvider(colorProvider)
             }
-            buttonAction(
+        }
-                    GenericItem.Action(stringProvider.getString(R.string._continue))
+
-                            .apply {
+        settingsContinueCancelItem {
-                                perform = Runnable { interactionListener?.continueThreePid(threePid) }
+            id("cont" + idPrefix + threePid.value)
-                            }
+            continueOnClick { interactionListener?.continueThreePid(threePid) }
-            )
+            cancelOnClick { interactionListener?.cancelThreePid(threePid) }
         }
     }
 }

vector/src/main/java/im/vector/app/features/settings/threepids/ThreePidsSettingsFragment.kt

@@ -125,6 +125,10 @@ class ThreePidsSettingsFragment @Inject constructor(
         viewModel.handle(ThreePidsSettingsAction.ContinueThreePid(threePid))
     }
 
+    override fun cancelThreePid(threePid: ThreePid) {
+        viewModel.handle(ThreePidsSettingsAction.CancelThreePid(threePid))
+    }
+
     override fun deleteThreePid(threePid: ThreePid) {
         AlertDialog.Builder(requireActivity())
                 .setMessage(getString(R.string.settings_remove_three_pid_confirmation_content, threePid.value))

vector/src/main/java/im/vector/app/features/settings/threepids/ThreePidsSettingsViewModel.kt

@@ -23,8 +23,10 @@ import com.airbnb.mvrx.MvRxViewModelFactory
 import com.airbnb.mvrx.ViewModelContext
 import com.squareup.inject.assisted.Assisted
 import com.squareup.inject.assisted.AssistedInject
+import im.vector.app.R
 import im.vector.app.core.extensions.exhaustive
 import im.vector.app.core.platform.VectorViewModel
+import im.vector.app.core.resources.StringProvider
 import kotlinx.coroutines.launch
 import org.matrix.android.sdk.api.MatrixCallback
 import org.matrix.android.sdk.api.auth.data.LoginFlowTypes
@@ -35,7 +37,8 @@ import org.matrix.android.sdk.rx.rx
 
 class ThreePidsSettingsViewModel @AssistedInject constructor(
         @Assisted initialState: ThreePidsSettingsViewState,
-        private val session: Session
+        private val session: Session,
+        private val stringProvider: StringProvider
 ) : VectorViewModel<ThreePidsSettingsViewState, ThreePidsSettingsAction, ThreePidsSettingsViewEvents>(initialState) {
 
     // UIA session
@@ -128,6 +131,7 @@ class ThreePidsSettingsViewModel @AssistedInject constructor(
         when (action) {
             is ThreePidsSettingsAction.AddThreePid      -> handleAddThreePid(action)
             is ThreePidsSettingsAction.ContinueThreePid -> handleContinueThreePid(action)
+            is ThreePidsSettingsAction.CancelThreePid   -> handleCancelThreePid(action)
             is ThreePidsSettingsAction.AccountPassword  -> handleAccountPassword(action)
             is ThreePidsSettingsAction.DeleteThreePid   -> handleDeleteThreePid(action)
         }.exhaustive
@@ -135,8 +139,16 @@ class ThreePidsSettingsViewModel @AssistedInject constructor(
 
     private fun handleAddThreePid(action: ThreePidsSettingsAction.AddThreePid) {
         isLoading(true)
-        viewModelScope.launch {
+
-            session.addThreePid(action.threePid, loadingCallback)
+        withState { state ->
+            val allThreePids = state.threePids.invoke().orEmpty() + state.pendingThreePids.invoke().orEmpty()
+            if (allThreePids.any { it.value == action.threePid.value }) {
+                _viewEvents.post(ThreePidsSettingsViewEvents.Failure(IllegalArgumentException(stringProvider.getString(R.string.auth_email_already_defined))))
+            } else {
+                viewModelScope.launch {
+                    session.addThreePid(action.threePid, loadingCallback)
+                }
+            }
         }
     }
 
@@ -148,6 +160,13 @@ class ThreePidsSettingsViewModel @AssistedInject constructor(
         }
     }
 
+    private fun handleCancelThreePid(action: ThreePidsSettingsAction.CancelThreePid) {
+        isLoading(true)
+        viewModelScope.launch {
+            session.cancelAddingThreePid(action.threePid, loadingCallback)
+        }
+    }
+
     private fun handleAccountPassword(action: ThreePidsSettingsAction.AccountPassword) {
         val safeSession = pendingSession ?: return Unit
                 .also { _viewEvents.post(ThreePidsSettingsViewEvents.Failure(IllegalStateException("No pending session"))) }