From 6a42e9cb2bf1017ddebf41714cc2f2aa649c3a66 Mon Sep 17 00:00:00 2001 From: Omer Habib Date: Thu, 30 Nov 2023 18:17:35 +0500 Subject: [PATCH] fix: Address PR comments + crash fix --- .../auth/presentation/logistration/Logistration.kt | 8 ++++---- .../auth/presentation/signin/SignInFragment.kt | 5 ++--- .../auth/presentation/signin/SignInViewModel.kt | 5 +++++ .../discovery/presentation/DiscoveryScreenTest.kt | 6 +++--- .../discovery/presentation/DiscoveryFragment.kt | 14 +++++++------- .../discovery/presentation/DiscoveryViewModel.kt | 7 ++++++- 6 files changed, 27 insertions(+), 18 deletions(-) diff --git a/auth/src/main/java/org/openedx/auth/presentation/logistration/Logistration.kt b/auth/src/main/java/org/openedx/auth/presentation/logistration/Logistration.kt index f25d0330c..6591adc9d 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/logistration/Logistration.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/logistration/Logistration.kt @@ -64,7 +64,7 @@ class Logistration : Fragment() { setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed) setContent { OpenEdXTheme { - PreLoginScreen( + LogistrationScreen( onSignInClick = { router.navigateToSignIn(parentFragmentManager) }, @@ -81,7 +81,7 @@ class Logistration : Fragment() { } @Composable -private fun PreLoginScreen( +private fun LogistrationScreen( onSearchClick: (String) -> Unit, onRegisterClick: () -> Unit, onSignInClick: () -> Unit, @@ -196,9 +196,9 @@ private fun PreLoginScreen( @Preview(name = "NEXUS_9_Light", device = Devices.NEXUS_9, uiMode = Configuration.UI_MODE_NIGHT_NO) @Preview(name = "NEXUS_9_Night", device = Devices.NEXUS_9, uiMode = Configuration.UI_MODE_NIGHT_YES) @Composable -private fun SignInScreenPreview() { +private fun LogistrationPreview() { OpenEdXTheme { - PreLoginScreen( + LogistrationScreen( onSearchClick = {}, onSignInClick = {}, onRegisterClick = {} diff --git a/auth/src/main/java/org/openedx/auth/presentation/signin/SignInFragment.kt b/auth/src/main/java/org/openedx/auth/presentation/signin/SignInFragment.kt index 6cb8443aa..0db83051f 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/signin/SignInFragment.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/signin/SignInFragment.kt @@ -64,8 +64,8 @@ import org.openedx.auth.R import org.openedx.auth.presentation.AuthRouter import org.openedx.auth.presentation.ui.LoginTextField import org.openedx.core.AppUpdateState -import org.openedx.core.BuildConfig import org.openedx.core.UIMessage +import org.openedx.core.presentation.global.WhatsNewGlobalManager import org.openedx.core.presentation.global.app_upgrade.AppUpgradeRequiredScreen import org.openedx.core.ui.BackBtn import org.openedx.core.ui.HandleUIMessage @@ -80,7 +80,6 @@ import org.openedx.core.ui.theme.appColors import org.openedx.core.ui.theme.appShapes import org.openedx.core.ui.theme.appTypography import org.openedx.core.ui.windowSizeValue -import org.openedx.core.presentation.global.WhatsNewGlobalManager class SignInFragment : Fragment() { @@ -102,7 +101,7 @@ class SignInFragment : Fragment() { val uiMessage by viewModel.uiMessage.observeAsState() val loginSuccess by viewModel.loginSuccess.observeAsState(initial = false) val appUpgradeEvent by viewModel.appUpgradeEvent.observeAsState(null) - val isLogistrationEnabled = BuildConfig.PRE_LOGIN_EXPERIENCE_ENABLED + val isLogistrationEnabled by viewModel.isLogistrationEnabled.observeAsState(initial = false) if (appUpgradeEvent == null) { LoginScreen( diff --git a/auth/src/main/java/org/openedx/auth/presentation/signin/SignInViewModel.kt b/auth/src/main/java/org/openedx/auth/presentation/signin/SignInViewModel.kt index c1a4d1126..02c79efb5 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/signin/SignInViewModel.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/signin/SignInViewModel.kt @@ -8,6 +8,7 @@ import org.openedx.auth.R import org.openedx.auth.domain.interactor.AuthInteractor import org.openedx.auth.presentation.AuthAnalytics import org.openedx.core.BaseViewModel +import org.openedx.core.BuildConfig import org.openedx.core.SingleEventLiveData import org.openedx.core.UIMessage import org.openedx.core.Validator @@ -44,7 +45,11 @@ class SignInViewModel( val appUpgradeEvent: LiveData get() = _appUpgradeEvent + private val _isLogistrationEnabled = MutableLiveData() + val isLogistrationEnabled: LiveData = _isLogistrationEnabled + init { + _isLogistrationEnabled.value = BuildConfig.PRE_LOGIN_EXPERIENCE_ENABLED collectAppUpgradeEvent() } diff --git a/discovery/src/androidTest/java/org/openedx/discovery/presentation/DiscoveryScreenTest.kt b/discovery/src/androidTest/java/org/openedx/discovery/presentation/DiscoveryScreenTest.kt index b30dbff5c..6a70f334b 100644 --- a/discovery/src/androidTest/java/org/openedx/discovery/presentation/DiscoveryScreenTest.kt +++ b/discovery/src/androidTest/java/org/openedx/discovery/presentation/DiscoveryScreenTest.kt @@ -59,7 +59,7 @@ class DiscoveryScreenTest { canLoadMore = false, refreshing = false, hasInternetConnection = true, - isUserLoggedIn = false, + canShowBackButton = false, appUpgradeParameters = AppUpdateState.AppUpgradeParameters(), onSearchClick = {}, onSwipeRefresh = {}, @@ -94,7 +94,7 @@ class DiscoveryScreenTest { canLoadMore = false, refreshing = false, hasInternetConnection = true, - isUserLoggedIn = false, + canShowBackButton = false, appUpgradeParameters = AppUpdateState.AppUpgradeParameters(), onSearchClick = {}, onSwipeRefresh = {}, @@ -120,7 +120,7 @@ class DiscoveryScreenTest { canLoadMore = true, refreshing = false, hasInternetConnection = true, - isUserLoggedIn = false, + canShowBackButton = false, appUpgradeParameters = AppUpdateState.AppUpgradeParameters(), onSearchClick = {}, onSwipeRefresh = {}, diff --git a/discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryFragment.kt b/discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryFragment.kt index 23b574828..2d2444a92 100644 --- a/discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryFragment.kt +++ b/discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryFragment.kt @@ -30,7 +30,6 @@ import org.koin.android.ext.android.inject import org.koin.androidx.viewmodel.ext.android.viewModel import org.openedx.core.AppUpdateState import org.openedx.core.AppUpdateState.wasUpdateDialogClosed -import org.openedx.core.BuildConfig import org.openedx.core.UIMessage import org.openedx.core.data.storage.CorePreferences import org.openedx.core.domain.model.Course @@ -67,7 +66,8 @@ class DiscoveryFragment : Fragment() { val appUpgradeEvent by viewModel.appUpgradeEvent.observeAsState() val wasUpdateDialogClosed by remember { wasUpdateDialogClosed } val querySearch = arguments?.getString(ARG_SEARCH_QUERY, "") ?: "" - val isUserLoggedIn = corePreferencesManager.user != null + val isLogistrationEnabled by viewModel.isLogistrationEnabled.observeAsState(false) + val canShowBackButton = isLogistrationEnabled && corePreferencesManager.user == null DiscoveryScreen( windowSize = windowSize, @@ -76,7 +76,7 @@ class DiscoveryFragment : Fragment() { canLoadMore = canLoadMore, refreshing = refreshing, hasInternetConnection = viewModel.hasInternetConnection, - isUserLoggedIn = isUserLoggedIn, + canShowBackButton = canShowBackButton, appUpgradeParameters = AppUpdateState.AppUpgradeParameters( appUpgradeEvent = appUpgradeEvent, wasUpdateDialogClosed = wasUpdateDialogClosed, @@ -157,7 +157,7 @@ internal fun DiscoveryScreen( canLoadMore: Boolean, refreshing: Boolean, hasInternetConnection: Boolean, - isUserLoggedIn: Boolean, + canShowBackButton: Boolean, appUpgradeParameters: AppUpdateState.AppUpgradeParameters, onSearchClick: () -> Unit, onSwipeRefresh: () -> Unit, @@ -216,7 +216,7 @@ internal fun DiscoveryScreen( HandleUIMessage(uiMessage = uiMessage, scaffoldState = scaffoldState) - if (BuildConfig.PRE_LOGIN_EXPERIENCE_ENABLED && isUserLoggedIn.not()) { + if (canShowBackButton) { Box( modifier = Modifier .statusBarsPadding() @@ -436,7 +436,7 @@ private fun DiscoveryScreenPreview() { hasInternetConnection = true, appUpgradeParameters = AppUpdateState.AppUpgradeParameters(), onBackClick = {}, - isUserLoggedIn = false + canShowBackButton = false ) } } @@ -472,7 +472,7 @@ private fun DiscoveryScreenTabletPreview() { hasInternetConnection = true, appUpgradeParameters = AppUpdateState.AppUpgradeParameters(), onBackClick = {}, - isUserLoggedIn = false + canShowBackButton = false ) } } diff --git a/discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryViewModel.kt b/discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryViewModel.kt index 7d602972b..806648060 100644 --- a/discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryViewModel.kt +++ b/discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryViewModel.kt @@ -7,6 +7,7 @@ import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.launch import org.openedx.core.BaseViewModel +import org.openedx.core.BuildConfig import org.openedx.core.R import org.openedx.core.SingleEventLiveData import org.openedx.core.UIMessage @@ -46,6 +47,9 @@ class DiscoveryViewModel( val appUpgradeEvent: LiveData get() = _appUpgradeEvent + private val _isLogistrationEnabled = MutableLiveData() + val isLogistrationEnabled: LiveData = _isLogistrationEnabled + val hasInternetConnection: Boolean get() = networkConnection.isOnline() @@ -54,6 +58,7 @@ class DiscoveryViewModel( private var isLoading = false init { + _isLogistrationEnabled.value = BuildConfig.PRE_LOGIN_EXPERIENCE_ENABLED getCoursesList() collectAppUpgradeEvent() } @@ -157,7 +162,7 @@ class DiscoveryViewModel( .collect { event -> when (event) { is AppUpgradeEvent.UpgradeRecommendedEvent -> { - _appUpgradeEvent.value = event + _appUpgradeEvent.value = event } is AppUpgradeEvent.UpgradeRequiredEvent -> {