diff --git a/changelog.d/5783.wip b/changelog.d/5783.wip new file mode 100644 index 0000000000..e306c0f217 --- /dev/null +++ b/changelog.d/5783.wip @@ -0,0 +1 @@ +Reorders the registration steps to prioritise email, then terms for the FTUE onboarding diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt index c96a2fbf42..2fe9a82fbd 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueAuthVariant.kt +++ b/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)) { - // 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) - } - } - } + onRegistrationFlow(viewEvents) } 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) { @@ -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) { // 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 ) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueMissingRegistrationStagesComparator.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueMissingRegistrationStagesComparator.kt new file mode 100644 index 0000000000..6a6326625e --- /dev/null +++ b/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 { + + 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 + } +} diff --git a/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/FtueMissingRegistrationStagesComparatorTest.kt b/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/FtueMissingRegistrationStagesComparatorTest.kt new file mode 100644 index 0000000000..010cf5de60 --- /dev/null +++ b/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() + ) + } +} diff --git a/vector/src/test/java/im/vector/app/test/fixtures/StageFixtures.kt b/vector/src/test/java/im/vector/app/test/fixtures/StageFixtures.kt new file mode 100644 index 0000000000..e06993523c --- /dev/null +++ b/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()) +fun aRecaptchaStage() = Stage.ReCaptcha(mandatory = true, publicKey = "any-key") +fun anOtherStage() = Stage.Other(mandatory = true, type = "raw-type", params = emptyMap())