Skip to content

#5307 Adds ForceLoginFallback feature flag to Login and Registration #5325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Feb 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5325.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Adds forceLoginFallback feature flag and usages to FTUE login and registration
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class DebugFeaturesStateFactory @Inject constructor(
label = "FTUE Personalize profile",
key = DebugFeatureKeys.onboardingPersonalize,
factory = VectorFeatures::isOnboardingPersonalizeEnabled
)
),
))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,13 @@ class DebugPrivateSettingsFragment : VectorBaseFragment<FragmentDebugPrivateSett
views.forceDialPadTabDisplay.setOnCheckedChangeListener { _, isChecked ->
viewModel.handle(DebugPrivateSettingsViewActions.SetDialPadVisibility(isChecked))
}
views.forceLoginFallback.setOnCheckedChangeListener { _, isChecked ->
viewModel.handle(DebugPrivateSettingsViewActions.SetForceLoginFallbackEnabled(isChecked))
}
}

override fun invalidate() = withState(viewModel) {
views.forceDialPadTabDisplay.isChecked = it.dialPadVisible
views.forceLoginFallback.isChecked = it.forceLoginFallback
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ import im.vector.app.core.platform.VectorViewModelAction

sealed class DebugPrivateSettingsViewActions : VectorViewModelAction {
data class SetDialPadVisibility(val force: Boolean) : DebugPrivateSettingsViewActions()
data class SetForceLoginFallbackEnabled(val force: Boolean) : DebugPrivateSettingsViewActions()
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,18 @@ class DebugPrivateSettingsViewModel @AssistedInject constructor(

private fun observeVectorDataStore() {
vectorDataStore.forceDialPadDisplayFlow.setOnEach {
copy(
dialPadVisible = it
)
copy(dialPadVisible = it)
}

vectorDataStore.forceLoginFallbackFlow.setOnEach {
copy(forceLoginFallback = it)
}
}

override fun handle(action: DebugPrivateSettingsViewActions) {
when (action) {
is DebugPrivateSettingsViewActions.SetDialPadVisibility -> handleSetDialPadVisibility(action)
is DebugPrivateSettingsViewActions.SetDialPadVisibility -> handleSetDialPadVisibility(action)
is DebugPrivateSettingsViewActions.SetForceLoginFallbackEnabled -> handleSetForceLoginFallbackEnabled(action)
}
}

Expand All @@ -62,4 +65,10 @@ class DebugPrivateSettingsViewModel @AssistedInject constructor(
vectorDataStore.setForceDialPadDisplay(action.force)
}
}

private fun handleSetForceLoginFallbackEnabled(action: DebugPrivateSettingsViewActions.SetForceLoginFallbackEnabled) {
viewModelScope.launch {
vectorDataStore.setForceLoginFallbackFlow(action.force)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ package im.vector.app.features.debug.settings
import com.airbnb.mvrx.MavericksState

data class DebugPrivateSettingsViewState(
val dialPadVisible: Boolean = false
val dialPadVisible: Boolean = false,
val forceLoginFallback: Boolean = false,
) : MavericksState
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
android:layout_height="wrap_content"
android:text="Force DialPad tab display" />

<CheckBox
android:id="@+id/forceLoginFallback"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="Force login and registration fallback" />

</LinearLayout>

</ScrollView>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import im.vector.app.features.login.LoginMode
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.settings.VectorDataStore
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.MatrixPatterns.getDomain
Expand Down Expand Up @@ -78,7 +79,8 @@ class OnboardingViewModel @AssistedInject constructor(
private val stringProvider: StringProvider,
private val homeServerHistoryService: HomeServerHistoryService,
private val vectorFeatures: VectorFeatures,
private val analyticsTracker: AnalyticsTracker
private val analyticsTracker: AnalyticsTracker,
private val vectorDataStore: VectorDataStore,
) : VectorViewModel<OnboardingViewState, OnboardingAction, OnboardingViewEvents>(initialState) {

@AssistedFactory
Expand All @@ -90,6 +92,7 @@ class OnboardingViewModel @AssistedInject constructor(

init {
getKnownCustomHomeServersUrls()
observeDataStore()
}

private fun getKnownCustomHomeServersUrls() {
Expand All @@ -98,6 +101,12 @@ class OnboardingViewModel @AssistedInject constructor(
}
}

private fun observeDataStore() = viewModelScope.launch {
vectorDataStore.forceLoginFallbackFlow.setOnEach { isForceLoginFallbackEnabled ->
copy(isForceLoginFallbackEnabled = isForceLoginFallbackEnabled)
}
}

// Store the last action, to redo it after user has trusted the untrusted certificate
private var lastAction: OnboardingAction? = null
private var currentHomeServerConnectionConfig: HomeServerConnectionConfig? = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ data class OnboardingViewState(
// Supported types for the login. We cannot use a sealed class for LoginType because it is not serializable
@PersistState
val loginModeSupportedTypes: List<String> = emptyList(),
val knownCustomHomeServersUrls: List<String> = emptyList()
val knownCustomHomeServersUrls: List<String> = emptyList(),
val isForceLoginFallbackEnabled: Boolean = false,
) : MavericksState {

fun isLoading(): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ class FtueAuthVariant(
private val popEnterAnim = R.anim.no_anim
private val popExitAnim = R.anim.exit_fade_out

private var isForceLoginFallbackEnabled = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for this private var, as a cache, because the boolean is also in the state.


private val topFragment: Fragment?
get() = supportFragmentManager.findFragmentById(views.loginFragmentContainer.id)

Expand Down Expand Up @@ -109,10 +111,6 @@ class FtueAuthVariant(
}
}

override fun setIsLoading(isLoading: Boolean) {
// do nothing
}

private fun addFirstFragment() {
val splashFragment = when (vectorFeatures.isOnboardingSplashCarouselEnabled()) {
true -> FtueAuthSplashCarouselFragment::class.java
Expand All @@ -121,11 +119,25 @@ class FtueAuthVariant(
activity.addFragment(views.loginFragmentContainer, splashFragment)
}

private fun updateWithState(viewState: OnboardingViewState) {
isForceLoginFallbackEnabled = viewState.isForceLoginFallbackEnabled
views.loginLoading.isVisible = shouldShowLoading(viewState)
}

private fun shouldShowLoading(viewState: OnboardingViewState) =
if (vectorFeatures.isOnboardingPersonalizeEnabled()) {
viewState.isLoading()
} else {
// Keep loading when during success because of the delay when switching to the next Activity
viewState.isLoading() || viewState.isAuthTaskCompleted()
}

override fun setIsLoading(isLoading: Boolean) = Unit

private fun handleOnboardingViewEvents(viewEvents: OnboardingViewEvents) {
when (viewEvents) {
is OnboardingViewEvents.RegistrationFlowResult -> {
// Check that all flows are supported by the application
if (viewEvents.flowResult.missingStages.any { !it.isSupported() }) {
if (registrationShouldFallback(viewEvents)) {
// Display a popup to propose use web fallback
onRegistrationStageNotSupported()
} else {
Expand All @@ -136,11 +148,7 @@ class FtueAuthVariant(
// 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
activity.addFragmentToBackstack(views.loginFragmentContainer,
FtueAuthLoginFragment::class.java,
tag = FRAGMENT_REGISTRATION_STAGE_TAG,
option = commonOption
)
openAuthLoginFragmentWithTag(FRAGMENT_REGISTRATION_STAGE_TAG)
}
}
}
Expand Down Expand Up @@ -228,13 +236,23 @@ class FtueAuthVariant(
}.exhaustive
}

private fun updateWithState(viewState: OnboardingViewState) {
views.loginLoading.isVisible = if (vectorFeatures.isOnboardingPersonalizeEnabled()) {
viewState.isLoading()
} else {
// Keep loading when during success because of the delay when switching to the next Activity
viewState.isLoading() || viewState.isAuthTaskCompleted()
}
private fun registrationShouldFallback(registrationFlowResult: OnboardingViewEvents.RegistrationFlowResult) =
isForceLoginFallbackEnabled || registrationFlowResult.containsUnsupportedRegistrationFlow()

private fun OnboardingViewEvents.RegistrationFlowResult.containsUnsupportedRegistrationFlow() =
flowResult.missingStages.any { !it.isSupported() }

private fun onRegistrationStageNotSupported() {
MaterialAlertDialogBuilder(activity)
.setTitle(R.string.app_name)
.setMessage(activity.getString(R.string.login_registration_not_supported))
.setPositiveButton(R.string.yes) { _, _ ->
activity.addFragmentToBackstack(views.loginFragmentContainer,
FtueAuthWebFragment::class.java,
option = commonOption)
}
.setNegativeButton(R.string.no, null)
.show()
}

private fun onWebLoginError(onWebLoginError: OnboardingViewEvents.OnWebLoginError) {
Expand Down Expand Up @@ -264,64 +282,67 @@ class FtueAuthVariant(
// state.signMode could not be ready yet. So use value from the ViewEvent
when (OnboardingViewEvents.signMode) {
SignMode.Unknown -> error("Sign mode has to be set before calling this method")
SignMode.SignUp -> {
// This is managed by the OnboardingViewEvents
}
SignMode.SignIn -> {
// It depends on the LoginMode
when (state.loginMode) {
LoginMode.Unknown,
is LoginMode.Sso -> error("Developer error")
is LoginMode.SsoAndPassword,
LoginMode.Password -> activity.addFragmentToBackstack(views.loginFragmentContainer,
FtueAuthLoginFragment::class.java,
tag = FRAGMENT_LOGIN_TAG,
option = commonOption)
LoginMode.Unsupported -> onLoginModeNotSupported(state.loginModeSupportedTypes)
}.exhaustive
}
SignMode.SignInWithMatrixId -> activity.addFragmentToBackstack(views.loginFragmentContainer,
FtueAuthLoginFragment::class.java,
tag = FRAGMENT_LOGIN_TAG,
option = commonOption)
SignMode.SignUp -> Unit // This case is processed in handleOnboardingViewEvents
SignMode.SignIn -> handleSignInSelected(state)
SignMode.SignInWithMatrixId -> handleSignInWithMatrixId(state)
}.exhaustive
}

/**
* Handle the SSO redirection here
*/
override fun onNewIntent(intent: Intent?) {
intent?.data
?.let { tryOrNull { it.getQueryParameter("loginToken") } }
?.let { onboardingViewModel.handle(OnboardingAction.LoginWithToken(it)) }
private fun handleSignInSelected(state: OnboardingViewState) {
if (isForceLoginFallbackEnabled) {
onLoginModeNotSupported(state.loginModeSupportedTypes)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real change no. 2

} else {
disambiguateLoginMode(state)
}
}

private fun onRegistrationStageNotSupported() {
MaterialAlertDialogBuilder(activity)
.setTitle(R.string.app_name)
.setMessage(activity.getString(R.string.login_registration_not_supported))
.setPositiveButton(R.string.yes) { _, _ ->
activity.addFragmentToBackstack(views.loginFragmentContainer,
FtueAuthWebFragment::class.java,
option = commonOption)
}
.setNegativeButton(R.string.no, null)
.show()
private fun disambiguateLoginMode(state: OnboardingViewState) = when (state.loginMode) {
LoginMode.Unknown,
is LoginMode.Sso -> error("Developer error")
is LoginMode.SsoAndPassword,
LoginMode.Password -> openAuthLoginFragmentWithTag(FRAGMENT_LOGIN_TAG)
LoginMode.Unsupported -> onLoginModeNotSupported(state.loginModeSupportedTypes)
}

private fun openAuthLoginFragmentWithTag(tag: String) {
activity.addFragmentToBackstack(views.loginFragmentContainer,
FtueAuthLoginFragment::class.java,
tag = tag,
option = commonOption)
}

private fun onLoginModeNotSupported(supportedTypes: List<String>) {
MaterialAlertDialogBuilder(activity)
.setTitle(R.string.app_name)
.setMessage(activity.getString(R.string.login_mode_not_supported, supportedTypes.joinToString { "'$it'" }))
.setPositiveButton(R.string.yes) { _, _ ->
activity.addFragmentToBackstack(views.loginFragmentContainer,
FtueAuthWebFragment::class.java,
option = commonOption)
}
.setPositiveButton(R.string.yes) { _, _ -> openAuthWebFragment() }
.setNegativeButton(R.string.no, null)
.show()
}

private fun handleSignInWithMatrixId(state: OnboardingViewState) {
if (isForceLoginFallbackEnabled) {
onLoginModeNotSupported(state.loginModeSupportedTypes)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real change no. 3

} else {
openAuthLoginFragmentWithTag(FRAGMENT_LOGIN_TAG)
}
}

private fun openAuthWebFragment() {
activity.addFragmentToBackstack(views.loginFragmentContainer,
FtueAuthWebFragment::class.java,
option = commonOption)
}

/**
* Handle the SSO redirection here
*/
override fun onNewIntent(intent: Intent?) {
intent?.data
?.let { tryOrNull { it.getQueryParameter("loginToken") } }
?.let { onboardingViewModel.handle(OnboardingAction.LoginWithToken(it)) }
}

private fun handleRegistrationNavigation(flowResult: FlowResult) {
// Complete all mandatory stages first
val mandatoryStage = flowResult.missingStages.firstOrNull { it.mandatory }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,16 @@ class VectorDataStore @Inject constructor(
settings[forceDialPadDisplay] = force
}
}

private val forceLoginFallback = booleanPreferencesKey("force_login_fallback")
Copy link
Contributor

@ouchadam ouchadam Feb 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected this to be beside the dial pad option within the VectorDataStore (along with the store read/write access below) 🤔

I have an upcoming PR to move the override options out of the VectorDataStore and into a VectorOverrides abstraction

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Soz for the conflict 😅 For visibility sake, because they're pretty minor, we decided that you'd resolve them on your pr


val forceLoginFallbackFlow: Flow<Boolean> = context.dataStore.data.map { preferences ->
preferences[forceLoginFallback].orFalse()
}

suspend fun setForceLoginFallbackFlow(force: Boolean) {
context.dataStore.edit { settings ->
settings[forceLoginFallback] = force
}
}
}