Merge pull request #5785 from vector-im/feature/adm/ftue-step-ordering

FTUE - Registration step ordering

changelog.d/5783.wip

@@ -0,0 +1 @@
+Reorders the registration steps to prioritise email, then terms for the FTUE onboarding

vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt

@@ -131,24 +131,7 @@ class FtueAuthVariant(
     private fun handleOnboardingViewEvents(viewEvents: OnboardingViewEvents) {
         when (viewEvents) {
             is OnboardingViewEvents.RegistrationFlowResult                     -> {
-                if (registrationShouldFallback(viewEvents)) {
+                onRegistrationFlow(viewEvents)
-                    // Display a popup to propose use web fallback
-                    onRegistrationStageNotSupported()
-                } else {
-                    if (viewEvents.isRegistrationStarted) {
-                        // Go on with registration flow
-                        handleRegistrationNavigation(viewEvents.flowResult)
-                    } else {
-                        if (vectorFeatures.isOnboardingCombinedRegisterEnabled()) {
-                            openCombinedRegister()
-                        } else {
-                            // First ask for login and password
-                            // I add a tag to indicate that this fragment is a registration stage.
-                            // This way it will be automatically popped in when starting the next registration stage
-                            openAuthLoginFragmentWithTag(FRAGMENT_REGISTRATION_STAGE_TAG)
-                        }
-                    }
-                }
             }
             is OnboardingViewEvents.OutdatedHomeserver                         -> {
                 MaterialAlertDialogBuilder(activity)
@@ -176,25 +159,33 @@ class FtueAuthVariant(
             is OnboardingViewEvents.OnServerSelectionDone                      -> onServerSelectionDone(viewEvents)
             is OnboardingViewEvents.OnSignModeSelected                         -> onSignModeSelected(viewEvents)
             is OnboardingViewEvents.OnLoginFlowRetrieved                       ->
-                activity.addFragmentToBackstack(views.loginFragmentContainer,
+                activity.addFragmentToBackstack(
+                        views.loginFragmentContainer,
                         FtueAuthSignUpSignInSelectionFragment::class.java,
-                        option = commonOption)
+                        option = commonOption
+                )
             is OnboardingViewEvents.OnWebLoginError                            -> onWebLoginError(viewEvents)
             is OnboardingViewEvents.OnForgetPasswordClicked                    ->
-                activity.addFragmentToBackstack(views.loginFragmentContainer,
+                activity.addFragmentToBackstack(
+                        views.loginFragmentContainer,
                         FtueAuthResetPasswordFragment::class.java,
-                        option = commonOption)
+                        option = commonOption
+                )
             is OnboardingViewEvents.OnResetPasswordSendThreePidDone            -> {
                 supportFragmentManager.popBackStack(FRAGMENT_LOGIN_TAG, POP_BACK_STACK_EXCLUSIVE)
-                activity.addFragmentToBackstack(views.loginFragmentContainer,
+                activity.addFragmentToBackstack(
+                        views.loginFragmentContainer,
                         FtueAuthResetPasswordMailConfirmationFragment::class.java,
-                        option = commonOption)
+                        option = commonOption
+                )
             }
             is OnboardingViewEvents.OnResetPasswordMailConfirmationSuccess     -> {
                 supportFragmentManager.popBackStack(FRAGMENT_LOGIN_TAG, POP_BACK_STACK_EXCLUSIVE)
-                activity.addFragmentToBackstack(views.loginFragmentContainer,
+                activity.addFragmentToBackstack(
+                        views.loginFragmentContainer,
                         FtueAuthResetPasswordSuccessFragment::class.java,
-                        option = commonOption)
+                        option = commonOption
+                )
             }
             is OnboardingViewEvents.OnResetPasswordMailConfirmationSuccessDone -> {
                 // Go back to the login fragment
@@ -221,11 +212,13 @@ class FtueAuthVariant(
                 // This is handled by the Fragments
                 Unit
             OnboardingViewEvents.OpenUseCaseSelection                          -> {
-                activity.addFragmentToBackstack(views.loginFragmentContainer,
+                activity.addFragmentToBackstack(
+                        views.loginFragmentContainer,
                         FtueAuthUseCaseFragment::class.java,
-                        option = commonOption)
+                        option = commonOption
+                )
             }
-            OnboardingViewEvents.OpenCombinedRegister                          -> openCombinedRegister()
+            OnboardingViewEvents.OpenCombinedRegister                          -> openStartCombinedRegister()
             is OnboardingViewEvents.OnAccountCreated                           -> onAccountCreated()
             OnboardingViewEvents.OnAccountSignedIn                             -> onAccountSignedIn()
             OnboardingViewEvents.OnChooseDisplayName                           -> onChooseDisplayName()
@@ -244,7 +237,21 @@ class FtueAuthVariant(
         }
     }
 
-    private fun openCombinedRegister() {
+    private fun onRegistrationFlow(viewEvents: OnboardingViewEvents.RegistrationFlowResult) {
+        when {
+            registrationShouldFallback(viewEvents)               -> displayFallbackWebDialog()
+            viewEvents.isRegistrationStarted                     -> handleRegistrationNavigation(viewEvents.flowResult.orderedStages())
+            vectorFeatures.isOnboardingCombinedRegisterEnabled() -> openStartCombinedRegister()
+            else                                                 -> openAuthLoginFragmentWithTag(FRAGMENT_REGISTRATION_STAGE_TAG)
+        }
+    }
+
+    private fun FlowResult.orderedStages() = when {
+        vectorFeatures.isOnboardingCombinedRegisterEnabled() -> missingStages.sortedWith(FtueMissingRegistrationStagesComparator())
+        else                                                 -> missingStages
+    }
+
+    private fun openStartCombinedRegister() {
         addRegistrationStageFragmentToBackstack(FtueAuthCombinedRegisterFragment::class.java)
     }
 
@@ -254,14 +261,16 @@ class FtueAuthVariant(
     private fun OnboardingViewEvents.RegistrationFlowResult.containsUnsupportedRegistrationFlow() =
             flowResult.missingStages.any { !it.isSupported() }
 
-    private fun onRegistrationStageNotSupported() {
+    private fun displayFallbackWebDialog() {
         MaterialAlertDialogBuilder(activity)
                 .setTitle(R.string.app_name)
                 .setMessage(activity.getString(R.string.login_registration_not_supported))
                 .setPositiveButton(R.string.yes) { _, _ ->
-                    activity.addFragmentToBackstack(views.loginFragmentContainer,
+                    activity.addFragmentToBackstack(
+                            views.loginFragmentContainer,
                             FtueAuthWebFragment::class.java,
-                            option = commonOption)
+                            option = commonOption
+                    )
                 }
                 .setNegativeButton(R.string.no, null)
                 .show()
@@ -283,9 +292,11 @@ class FtueAuthVariant(
         when (OnboardingViewEvents.serverType) {
             ServerType.MatrixOrg -> Unit // In this case, we wait for the login flow
             ServerType.EMS,
-            ServerType.Other     -> activity.addFragmentToBackstack(views.loginFragmentContainer,
+            ServerType.Other     -> activity.addFragmentToBackstack(
+                    views.loginFragmentContainer,
                     FtueAuthServerUrlFormFragment::class.java,
-                    option = commonOption)
+                    option = commonOption
+            )
             ServerType.Unknown   -> Unit /* Should not happen */
         }
     }
@@ -317,10 +328,12 @@ class FtueAuthVariant(
     }
 
     private fun openAuthLoginFragmentWithTag(tag: String) {
-        activity.addFragmentToBackstack(views.loginFragmentContainer,
+        activity.addFragmentToBackstack(
+                views.loginFragmentContainer,
                 FtueAuthLoginFragment::class.java,
                 tag = tag,
-                option = commonOption)
+                option = commonOption
+        )
     }
 
     private fun onLoginModeNotSupported(supportedTypes: List<String>) {
@@ -341,9 +354,11 @@ class FtueAuthVariant(
     }
 
     private fun openAuthWebFragment() {
-        activity.addFragmentToBackstack(views.loginFragmentContainer,
+        activity.addFragmentToBackstack(
+                views.loginFragmentContainer,
                 FtueAuthWebFragment::class.java,
-                option = commonOption)
+                option = commonOption
+        )
     }
 
     /**
@@ -355,15 +370,15 @@ class FtueAuthVariant(
                 ?.let { onboardingViewModel.handle(OnboardingAction.LoginWithToken(it)) }
     }
 
-    private fun handleRegistrationNavigation(flowResult: FlowResult) {
+    private fun handleRegistrationNavigation(remainingStages: List<Stage>) {
         // Complete all mandatory stages first
-        val mandatoryStage = flowResult.missingStages.firstOrNull { it.mandatory }
+        val mandatoryStage = remainingStages.firstOrNull { it.mandatory }
 
         if (mandatoryStage != null) {
             doStage(mandatoryStage)
         } else {
             // Consider optional stages
-            val optionalStage = flowResult.missingStages.firstOrNull { !it.mandatory && it !is Stage.Dummy }
+            val optionalStage = remainingStages.firstOrNull { !it.mandatory && it !is Stage.Dummy }
             if (optionalStage == null) {
                 // Should not happen...
             } else {
@@ -437,14 +452,16 @@ class FtueAuthVariant(
     }
 
     private fun onChooseDisplayName() {
-        activity.addFragmentToBackstack(views.loginFragmentContainer,
+        activity.addFragmentToBackstack(
+                views.loginFragmentContainer,
                 FtueAuthChooseDisplayNameFragment::class.java,
                 option = commonOption
         )
     }
 
     private fun onChooseProfilePicture() {
-        activity.addFragmentToBackstack(views.loginFragmentContainer,
+        activity.addFragmentToBackstack(
+                views.loginFragmentContainer,
                 FtueAuthChooseProfilePictureFragment::class.java,
                 option = commonOption
         )

vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueMissingRegistrationStagesComparator.kt

@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.features.onboarding.ftueauth
+
+import org.matrix.android.sdk.api.auth.registration.Stage
+
+class FtueMissingRegistrationStagesComparator : Comparator<Stage> {
+
+    override fun compare(a: Stage?, b: Stage?): Int {
+        return (a?.toPriority() ?: 0) - (b?.toPriority() ?: 0)
+    }
+
+    private fun Stage.toPriority() = when (this) {
+        is Stage.Email     -> 0
+        is Stage.Msisdn    -> 1
+        is Stage.Terms     -> 2
+        is Stage.ReCaptcha -> 3
+        is Stage.Other     -> 4
+        is Stage.Dummy     -> 5
+    }
+}

vector/src/test/java/im/vector/app/features/onboarding/ftueauth/FtueMissingRegistrationStagesComparatorTest.kt

@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.features.onboarding.ftueauth
+
+import im.vector.app.test.fixtures.aDummyStage
+import im.vector.app.test.fixtures.aMsisdnStage
+import im.vector.app.test.fixtures.aRecaptchaStage
+import im.vector.app.test.fixtures.aTermsStage
+import im.vector.app.test.fixtures.anEmailStage
+import im.vector.app.test.fixtures.anOtherStage
+import org.amshove.kluent.shouldBeEqualTo
+import org.junit.Test
+
+class FtueMissingRegistrationStagesComparatorTest {
+
+    @Test
+    fun `when ordering stages, then prioritizes email`() {
+        val input = listOf(
+                aDummyStage(),
+                anOtherStage(),
+                aMsisdnStage(),
+                anEmailStage(),
+                aRecaptchaStage(),
+                aTermsStage()
+        )
+
+        val result = input.sortedWith(FtueMissingRegistrationStagesComparator())
+
+        result shouldBeEqualTo listOf(
+                anEmailStage(),
+                aMsisdnStage(),
+                aTermsStage(),
+                aRecaptchaStage(),
+                anOtherStage(),
+                aDummyStage()
+        )
+    }
+}

vector/src/test/java/im/vector/app/test/fixtures/StageFixtures.kt

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.test.fixtures
+
+import org.matrix.android.sdk.api.auth.registration.Stage
+
+fun aDummyStage() = Stage.Dummy(mandatory = true)
+fun anEmailStage() = Stage.Email(mandatory = true)
+fun aMsisdnStage() = Stage.Msisdn(mandatory = true)
+fun aTermsStage() = Stage.Terms(mandatory = true, policies = emptyMap<String, String>())
+fun aRecaptchaStage() = Stage.ReCaptcha(mandatory = true, publicKey = "any-key")
+fun anOtherStage() = Stage.Other(mandatory = true, type = "raw-type", params = emptyMap<String, String>())