From a893f5acdb74c2553ffc7cbbd8991f27f4aa3fed Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 16 May 2022 12:00:53 +0100 Subject: [PATCH 1/6] applying the manual reordering of the sign up stages only when matrix.org is selected - lifts the logic up to the view model --- changelog.d/5783.wip | 1 + .../app/features/onboarding/OnboardingViewModel.kt | 13 ++++++++++++- .../features/onboarding/ftueauth/FtueAuthVariant.kt | 8 +------- 3 files changed, 14 insertions(+), 8 deletions(-) create mode 100644 changelog.d/5783.wip diff --git a/changelog.d/5783.wip b/changelog.d/5783.wip new file mode 100644 index 0000000000..6f44813d81 --- /dev/null +++ b/changelog.d/5783.wip @@ -0,0 +1 @@ +FTUE - Overrides sign up flow ordering for matrix.org only diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index 25ae0327a8..a7468cf95b 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -42,6 +42,7 @@ import im.vector.app.features.login.ReAuthHelper import im.vector.app.features.login.ServerType import im.vector.app.features.login.SignMode import im.vector.app.features.onboarding.StartAuthenticationFlowUseCase.StartAuthenticationResult +import im.vector.app.features.onboarding.ftueauth.FtueMissingRegistrationStagesComparator import kotlinx.coroutines.Job import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.launch @@ -289,9 +290,19 @@ class OnboardingViewModel @AssistedInject constructor( } private fun emitFlowResultViewEvent(flowResult: FlowResult) { - _viewEvents.post(OnboardingViewEvents.RegistrationFlowResult(flowResult, isRegistrationStarted)) + withState { state -> + val orderedResult = when { + state.hasSelectedMatrixOrg() && vectorFeatures.isOnboardingCombinedRegisterEnabled() -> flowResult.overrideOrder() + else -> flowResult + } + _viewEvents.post(OnboardingViewEvents.RegistrationFlowResult(orderedResult, isRegistrationStarted)) + } } + private fun OnboardingViewState.hasSelectedMatrixOrg() = selectedHomeserver.userFacingUrl == matrixOrgUrl + + private fun FlowResult.overrideOrder() = copy(missingStages = missingStages.sortedWith(FtueMissingRegistrationStagesComparator())) + private fun handleRegisterWith(action: OnboardingAction.Register) { reAuthHelper.data = action.password handleRegisterAction( 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 63b3bc0f71..8513845b8e 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 @@ -54,7 +54,6 @@ import im.vector.app.features.onboarding.OnboardingViewState import im.vector.app.features.onboarding.ftueauth.terms.FtueAuthLegacyStyleTermsFragment import im.vector.app.features.onboarding.ftueauth.terms.FtueAuthTermsFragment import im.vector.app.features.onboarding.ftueauth.terms.FtueAuthTermsLegacyStyleFragmentArgument -import org.matrix.android.sdk.api.auth.registration.FlowResult import org.matrix.android.sdk.api.auth.registration.Stage import org.matrix.android.sdk.api.auth.toLocalizedLoginTerms import org.matrix.android.sdk.api.extensions.tryOrNull @@ -240,17 +239,12 @@ class FtueAuthVariant( private fun onRegistrationFlow(viewEvents: OnboardingViewEvents.RegistrationFlowResult) { when { registrationShouldFallback(viewEvents) -> displayFallbackWebDialog() - viewEvents.isRegistrationStarted -> handleRegistrationNavigation(viewEvents.flowResult.orderedStages()) + viewEvents.isRegistrationStarted -> handleRegistrationNavigation(viewEvents.flowResult.missingStages) 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) } From 6c3150edb7e671e522369ed63f16ff20a13dd5eb Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 16 May 2022 17:16:37 +0100 Subject: [PATCH 2/6] using compareTo instead of direct subtraction --- .../ftueauth/FtueMissingRegistrationStagesComparator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 6a6326625e..4d9fe10852 100644 --- 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 @@ -21,7 +21,7 @@ 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) + return (a?.toPriority() ?: 0).compareTo(b?.toPriority() ?: 0) } private fun Stage.toPriority() = when (this) { From 51ffe26a919a8b0e999496d7eb5cb16f8095d413 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 16 May 2022 17:17:15 +0100 Subject: [PATCH 3/6] renaming comparator to give more context to its usage --- .../im/vector/app/features/onboarding/OnboardingViewModel.kt | 4 ++-- ...tor.kt => MatrixOrgMissingRegistrationStagesComparator.kt} | 2 +- ...kt => MatrixOrgMissingRegistrationStagesComparatorTest.kt} | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) rename vector/src/main/java/im/vector/app/features/onboarding/ftueauth/{FtueMissingRegistrationStagesComparator.kt => MatrixOrgMissingRegistrationStagesComparator.kt} (93%) rename vector/src/test/java/im/vector/app/features/onboarding/ftueauth/{FtueMissingRegistrationStagesComparatorTest.kt => MatrixOrgMissingRegistrationStagesComparatorTest.kt} (91%) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index a7468cf95b..94bb36dbbc 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -42,7 +42,7 @@ import im.vector.app.features.login.ReAuthHelper import im.vector.app.features.login.ServerType import im.vector.app.features.login.SignMode import im.vector.app.features.onboarding.StartAuthenticationFlowUseCase.StartAuthenticationResult -import im.vector.app.features.onboarding.ftueauth.FtueMissingRegistrationStagesComparator +import im.vector.app.features.onboarding.ftueauth.MatrixOrgMissingRegistrationStagesComparator import kotlinx.coroutines.Job import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.launch @@ -301,7 +301,7 @@ class OnboardingViewModel @AssistedInject constructor( private fun OnboardingViewState.hasSelectedMatrixOrg() = selectedHomeserver.userFacingUrl == matrixOrgUrl - private fun FlowResult.overrideOrder() = copy(missingStages = missingStages.sortedWith(FtueMissingRegistrationStagesComparator())) + private fun FlowResult.overrideOrder() = copy(missingStages = missingStages.sortedWith(MatrixOrgMissingRegistrationStagesComparator())) private fun handleRegisterWith(action: OnboardingAction.Register) { reAuthHelper.data = action.password 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/MatrixOrgMissingRegistrationStagesComparator.kt similarity index 93% rename from vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueMissingRegistrationStagesComparator.kt rename to vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgMissingRegistrationStagesComparator.kt index 4d9fe10852..894a474a9e 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/FtueMissingRegistrationStagesComparator.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgMissingRegistrationStagesComparator.kt @@ -18,7 +18,7 @@ package im.vector.app.features.onboarding.ftueauth import org.matrix.android.sdk.api.auth.registration.Stage -class FtueMissingRegistrationStagesComparator : Comparator { +class MatrixOrgMissingRegistrationStagesComparator : Comparator { override fun compare(a: Stage?, b: Stage?): Int { return (a?.toPriority() ?: 0).compareTo(b?.toPriority() ?: 0) 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/MatrixOrgMissingRegistrationStagesComparatorTest.kt similarity index 91% rename from vector/src/test/java/im/vector/app/features/onboarding/ftueauth/FtueMissingRegistrationStagesComparatorTest.kt rename to vector/src/test/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgMissingRegistrationStagesComparatorTest.kt index 010cf5de60..1fe8ad6000 100644 --- a/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/FtueMissingRegistrationStagesComparatorTest.kt +++ b/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgMissingRegistrationStagesComparatorTest.kt @@ -25,7 +25,7 @@ import im.vector.app.test.fixtures.anOtherStage import org.amshove.kluent.shouldBeEqualTo import org.junit.Test -class FtueMissingRegistrationStagesComparatorTest { +class MatrixOrgMissingRegistrationStagesComparatorTest { @Test fun `when ordering stages, then prioritizes email`() { @@ -38,7 +38,7 @@ class FtueMissingRegistrationStagesComparatorTest { aTermsStage() ) - val result = input.sortedWith(FtueMissingRegistrationStagesComparator()) + val result = input.sortedWith(MatrixOrgMissingRegistrationStagesComparator()) result shouldBeEqualTo listOf( anEmailStage(), From 444980395eb33c84749d93aede4950f014dfe2ce Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 16 May 2022 17:26:12 +0100 Subject: [PATCH 4/6] inlining single use extension --- .../vector/app/features/onboarding/OnboardingViewModel.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index 94bb36dbbc..8424a5e915 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -292,7 +292,9 @@ class OnboardingViewModel @AssistedInject constructor( private fun emitFlowResultViewEvent(flowResult: FlowResult) { withState { state -> val orderedResult = when { - state.hasSelectedMatrixOrg() && vectorFeatures.isOnboardingCombinedRegisterEnabled() -> flowResult.overrideOrder() + state.hasSelectedMatrixOrg() && vectorFeatures.isOnboardingCombinedRegisterEnabled() -> flowResult.copy( + missingStages = flowResult.missingStages.sortedWith(MatrixOrgMissingRegistrationStagesComparator()) + ) else -> flowResult } _viewEvents.post(OnboardingViewEvents.RegistrationFlowResult(orderedResult, isRegistrationStarted)) @@ -301,8 +303,6 @@ class OnboardingViewModel @AssistedInject constructor( private fun OnboardingViewState.hasSelectedMatrixOrg() = selectedHomeserver.userFacingUrl == matrixOrgUrl - private fun FlowResult.overrideOrder() = copy(missingStages = missingStages.sortedWith(MatrixOrgMissingRegistrationStagesComparator())) - private fun handleRegisterWith(action: OnboardingAction.Register) { reAuthHelper.data = action.password handleRegisterAction( From 1ec99ee89e85ef16823991fd6e00669a84a6131c Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 17 May 2022 10:31:32 +0100 Subject: [PATCH 5/6] renaming comparator to only specify the matrix.org domain --- .../im/vector/app/features/onboarding/OnboardingViewModel.kt | 4 ++-- ...Comparator.kt => MatrixOrgRegistrationStagesComparator.kt} | 2 +- ...orTest.kt => MatrixOrgRegistrationStagesComparatorTest.kt} | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) rename vector/src/main/java/im/vector/app/features/onboarding/ftueauth/{MatrixOrgMissingRegistrationStagesComparator.kt => MatrixOrgRegistrationStagesComparator.kt} (93%) rename vector/src/test/java/im/vector/app/features/onboarding/ftueauth/{MatrixOrgMissingRegistrationStagesComparatorTest.kt => MatrixOrgRegistrationStagesComparatorTest.kt} (91%) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt index 8424a5e915..9fd013ee79 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/OnboardingViewModel.kt @@ -42,7 +42,7 @@ import im.vector.app.features.login.ReAuthHelper import im.vector.app.features.login.ServerType import im.vector.app.features.login.SignMode import im.vector.app.features.onboarding.StartAuthenticationFlowUseCase.StartAuthenticationResult -import im.vector.app.features.onboarding.ftueauth.MatrixOrgMissingRegistrationStagesComparator +import im.vector.app.features.onboarding.ftueauth.MatrixOrgRegistrationStagesComparator import kotlinx.coroutines.Job import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.launch @@ -293,7 +293,7 @@ class OnboardingViewModel @AssistedInject constructor( withState { state -> val orderedResult = when { state.hasSelectedMatrixOrg() && vectorFeatures.isOnboardingCombinedRegisterEnabled() -> flowResult.copy( - missingStages = flowResult.missingStages.sortedWith(MatrixOrgMissingRegistrationStagesComparator()) + missingStages = flowResult.missingStages.sortedWith(MatrixOrgRegistrationStagesComparator()) ) else -> flowResult } diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgMissingRegistrationStagesComparator.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgRegistrationStagesComparator.kt similarity index 93% rename from vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgMissingRegistrationStagesComparator.kt rename to vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgRegistrationStagesComparator.kt index 894a474a9e..fcc70803a0 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgMissingRegistrationStagesComparator.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgRegistrationStagesComparator.kt @@ -18,7 +18,7 @@ package im.vector.app.features.onboarding.ftueauth import org.matrix.android.sdk.api.auth.registration.Stage -class MatrixOrgMissingRegistrationStagesComparator : Comparator { +class MatrixOrgRegistrationStagesComparator : Comparator { override fun compare(a: Stage?, b: Stage?): Int { return (a?.toPriority() ?: 0).compareTo(b?.toPriority() ?: 0) diff --git a/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgMissingRegistrationStagesComparatorTest.kt b/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgRegistrationStagesComparatorTest.kt similarity index 91% rename from vector/src/test/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgMissingRegistrationStagesComparatorTest.kt rename to vector/src/test/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgRegistrationStagesComparatorTest.kt index 1fe8ad6000..08be0ee058 100644 --- a/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgMissingRegistrationStagesComparatorTest.kt +++ b/vector/src/test/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgRegistrationStagesComparatorTest.kt @@ -25,7 +25,7 @@ import im.vector.app.test.fixtures.anOtherStage import org.amshove.kluent.shouldBeEqualTo import org.junit.Test -class MatrixOrgMissingRegistrationStagesComparatorTest { +class MatrixOrgRegistrationStagesComparatorTest { @Test fun `when ordering stages, then prioritizes email`() { @@ -38,7 +38,7 @@ class MatrixOrgMissingRegistrationStagesComparatorTest { aTermsStage() ) - val result = input.sortedWith(MatrixOrgMissingRegistrationStagesComparator()) + val result = input.sortedWith(MatrixOrgRegistrationStagesComparator()) result shouldBeEqualTo listOf( anEmailStage(), From 5f2cb671e2d27361b0e1561035fe1265d4061e6b Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 17 May 2022 10:35:14 +0100 Subject: [PATCH 6/6] making comparator override non null as kotlin guards against this --- .../ftueauth/MatrixOrgRegistrationStagesComparator.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgRegistrationStagesComparator.kt b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgRegistrationStagesComparator.kt index fcc70803a0..527c20987a 100644 --- a/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgRegistrationStagesComparator.kt +++ b/vector/src/main/java/im/vector/app/features/onboarding/ftueauth/MatrixOrgRegistrationStagesComparator.kt @@ -20,8 +20,8 @@ import org.matrix.android.sdk.api.auth.registration.Stage class MatrixOrgRegistrationStagesComparator : Comparator { - override fun compare(a: Stage?, b: Stage?): Int { - return (a?.toPriority() ?: 0).compareTo(b?.toPriority() ?: 0) + override fun compare(a: Stage, b: Stage): Int { + return a.toPriority().compareTo(b.toPriority()) } private fun Stage.toPriority() = when (this) {