From a985f18b3882ff9e3b5aa27d5bc890e02fee3a22 Mon Sep 17 00:00:00 2001 From: Farhan Arshad Date: Mon, 1 Jul 2024 13:43:58 +0500 Subject: [PATCH] fix: address PR comments - Code improvements --- app/build.gradle | 4 +- .../openedx/core/domain/model/AppConfig.kt | 2 +- .../core/domain/model/CourseStructure.kt | 1 + .../core/domain/model/iap/PurchaseFlowData.kt | 2 - .../presentation/dialog/IAPDialogFragment.kt | 2 + .../core/presentation/iap/IAPUIState.kt | 2 + .../java/org/openedx/core/ui/theme/Shape.kt | 2 +- .../container/CourseContainerFragment.kt | 2 +- .../presentation/DashboardListFragment.kt | 78 +++++++++++-------- .../presentation/settings/SettingsFragment.kt | 14 +++- .../presentation/settings/SettingsScreenUI.kt | 16 +--- 11 files changed, 70 insertions(+), 55 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 445794b82..f949e477f 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -32,8 +32,8 @@ android { applicationId appId minSdk 24 targetSdk 34 - versionCode 6 - versionName "6.0.0" + versionCode 1 + versionName "1.0.0" resourceConfigurations += ["en", "uk"] diff --git a/core/src/main/java/org/openedx/core/domain/model/AppConfig.kt b/core/src/main/java/org/openedx/core/domain/model/AppConfig.kt index bdf9277d4..17ef4a5c5 100644 --- a/core/src/main/java/org/openedx/core/domain/model/AppConfig.kt +++ b/core/src/main/java/org/openedx/core/domain/model/AppConfig.kt @@ -15,7 +15,7 @@ data class CourseDatesCalendarSync( val isDeepLinkEnabled: Boolean, ) : Serializable -class IAPConfig( +data class IAPConfig( val isEnabled: Boolean = false, val productPrefix: String? = null, val disableVersions: List = listOf() diff --git a/core/src/main/java/org/openedx/core/domain/model/CourseStructure.kt b/core/src/main/java/org/openedx/core/domain/model/CourseStructure.kt index c92ad52bf..b98543361 100644 --- a/core/src/main/java/org/openedx/core/domain/model/CourseStructure.kt +++ b/core/src/main/java/org/openedx/core/domain/model/CourseStructure.kt @@ -28,6 +28,7 @@ data class CourseStructure( val isUpgradeable: Boolean get() = enrollmentDetails.isAuditMode && isStarted && + courseAccessDetails.coursewareAccess?.hasAccess == true && enrollmentDetails.isUpgradeDeadlinePassed.not() && productInfo != null } diff --git a/core/src/main/java/org/openedx/core/domain/model/iap/PurchaseFlowData.kt b/core/src/main/java/org/openedx/core/domain/model/iap/PurchaseFlowData.kt index fd91e87cd..b452e4445 100644 --- a/core/src/main/java/org/openedx/core/domain/model/iap/PurchaseFlowData.kt +++ b/core/src/main/java/org/openedx/core/domain/model/iap/PurchaseFlowData.kt @@ -1,7 +1,6 @@ package org.openedx.core.domain.model.iap import org.openedx.core.domain.ProductInfo -import org.openedx.core.presentation.iap.IAPFlow data class PurchaseFlowData( val screenName: String? = null, @@ -10,7 +9,6 @@ data class PurchaseFlowData( val isSelfPaced: Boolean? = null, val componentId: String? = null, val productInfo: ProductInfo? = null, - val flow: String = IAPFlow.USER_INITIATED.value() ) { var currencyCode: String = "" var price: Double = 0.0 diff --git a/core/src/main/java/org/openedx/core/presentation/dialog/IAPDialogFragment.kt b/core/src/main/java/org/openedx/core/presentation/dialog/IAPDialogFragment.kt index d66918daf..387fc176a 100644 --- a/core/src/main/java/org/openedx/core/presentation/dialog/IAPDialogFragment.kt +++ b/core/src/main/java/org/openedx/core/presentation/dialog/IAPDialogFragment.kt @@ -284,6 +284,8 @@ class IAPDialogFragment : DialogFragment() { } companion object { + const val TAG = "IAPDialogFragment" + private const val ARG_IAP_FLOW = "iap_flow" private const val ARG_SCREEN_NAME = "SCREEN_NAME" private const val ARG_COURSE_ID = "course_id" diff --git a/core/src/main/java/org/openedx/core/presentation/iap/IAPUIState.kt b/core/src/main/java/org/openedx/core/presentation/iap/IAPUIState.kt index 44e7a64e4..c92645119 100644 --- a/core/src/main/java/org/openedx/core/presentation/iap/IAPUIState.kt +++ b/core/src/main/java/org/openedx/core/presentation/iap/IAPUIState.kt @@ -29,6 +29,7 @@ enum class IAPFlow(val value: String) { } enum class IAPAction(val action: String) { + ACTION_USER_INITIATED("user_initiated"), ACTION_GET_HELP("get_help"), ACTION_CLOSE("close"), ACTION_RELOAD_PRICE("reload_price"), @@ -37,6 +38,7 @@ enum class IAPAction(val action: String) { ACTION_UNFULFILLED("Unfulfilled"), ACTION_RESTORE("restore"), ACTION_ERROR_CLOSE("error_close"), + ACTION_COMPLETION("completion") } enum class IAPRequestType(val request: String) { diff --git a/core/src/main/java/org/openedx/core/ui/theme/Shape.kt b/core/src/main/java/org/openedx/core/ui/theme/Shape.kt index c45ecaf0a..eed4d481d 100644 --- a/core/src/main/java/org/openedx/core/ui/theme/Shape.kt +++ b/core/src/main/java/org/openedx/core/ui/theme/Shape.kt @@ -21,4 +21,4 @@ data class AppShapes( val MaterialTheme.appShapes: AppShapes @Composable @ReadOnlyComposable - get() =LocalShapes.current + get() = LocalShapes.current diff --git a/course/src/main/java/org/openedx/course/presentation/container/CourseContainerFragment.kt b/course/src/main/java/org/openedx/course/presentation/container/CourseContainerFragment.kt index 5c42342f7..68c2c33ca 100644 --- a/course/src/main/java/org/openedx/course/presentation/container/CourseContainerFragment.kt +++ b/course/src/main/java/org/openedx/course/presentation/container/CourseContainerFragment.kt @@ -391,7 +391,7 @@ fun CourseDashboard( productInfo = viewModel.courseStructure?.productInfo!! ).show( fragmentManager, - IAPDialogFragment::class.simpleName + IAPDialogFragment.TAG ) } } diff --git a/dashboard/src/main/java/org/openedx/dashboard/presentation/DashboardListFragment.kt b/dashboard/src/main/java/org/openedx/dashboard/presentation/DashboardListFragment.kt index 0f9d4eaf0..3c3e92953 100644 --- a/dashboard/src/main/java/org/openedx/dashboard/presentation/DashboardListFragment.kt +++ b/dashboard/src/main/java/org/openedx/dashboard/presentation/DashboardListFragment.kt @@ -69,7 +69,6 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.fragment.app.Fragment -import androidx.fragment.app.FragmentManager import coil.compose.AsyncImage import coil.request.ImageRequest import org.koin.android.ext.android.inject @@ -147,7 +146,6 @@ class DashboardListFragment : Fragment() { iapUiState = iapUiState, canLoadMore = canLoadMore, refreshing = refreshing, - fragmentManager = requireActivity().supportFragmentManager, hasInternetConnection = viewModel.hasInternetConnection, onReloadClick = { viewModel.getCourses() @@ -173,8 +171,35 @@ class DashboardListFragment : Fragment() { AppUpdateState.openPlayMarket(requireContext()) }, ), - onIAPAction = { action, iapException -> + onIAPAction = { action, course, iapException -> when (action) { + IAPAction.ACTION_USER_INITIATED -> { + if (course != null) { + IAPDialogFragment.newInstance( + iapFlow = IAPFlow.USER_INITIATED, + screenName = IAPAnalyticsScreen.COURSE_ENROLLMENT.screenName, + courseId = course.course.id, + courseName = course.course.name, + isSelfPaced = course.course.isSelfPaced, + productInfo = course.productInfo!! + ).show( + requireActivity().supportFragmentManager, + IAPDialogFragment.TAG + ) + } + } + + IAPAction.ACTION_COMPLETION -> { + IAPDialogFragment.newInstance( + IAPFlow.SILENT, + IAPAnalyticsScreen.COURSE_ENROLLMENT.screenName + ).show( + requireActivity().supportFragmentManager, + IAPDialogFragment.TAG + ) + viewModel.clearIAPState() + } + IAPAction.ACTION_UNFULFILLED -> { viewModel.detectUnfulfilledPurchase() } @@ -213,13 +238,12 @@ internal fun DashboardListView( iapUiState: IAPUIState?, canLoadMore: Boolean, refreshing: Boolean, - fragmentManager: FragmentManager, hasInternetConnection: Boolean, onReloadClick: () -> Unit, onSwipeRefresh: () -> Unit, paginationCallback: () -> Unit, onItemClick: (EnrolledCourse) -> Unit, - onIAPAction: (IAPAction, IAPException?) -> Unit, + onIAPAction: (IAPAction, EnrolledCourse?, IAPException?) -> Unit, appUpgradeParameters: AppUpdateState.AppUpgradeParameters, ) { val scaffoldState = rememberScaffoldState() @@ -329,16 +353,10 @@ internal fun DashboardListView( bottom = 16.dp ) ) { - IAPDialogFragment.newInstance( - iapFlow = IAPFlow.USER_INITIATED, - screenName = IAPAnalyticsScreen.COURSE_ENROLLMENT.screenName, - courseId = course.course.id, - courseName = course.course.name, - isSelfPaced = course.course.isSelfPaced, - productInfo = course.productInfo!! - ).show( - fragmentManager, - IAPDialogFragment::class.simpleName + onIAPAction( + IAPAction.ACTION_USER_INITIATED, + course, + null ) } } @@ -364,7 +382,7 @@ internal fun DashboardListView( LaunchedEffect(state.courses) { if (state.courses.isNotEmpty()) { - onIAPAction(IAPAction.ACTION_UNFULFILLED, null) + onIAPAction(IAPAction.ACTION_UNFULFILLED, null, null) } } } @@ -424,16 +442,9 @@ internal fun DashboardListView( when (iapUiState) { is IAPUIState.PurchasesFulfillmentCompleted -> { PurchasesFulfillmentCompletedDialog(onConfirm = { - onIAPAction(IAPAction.ACTION_CLOSE, null) - IAPDialogFragment.newInstance( - IAPFlow.SILENT, - IAPAnalyticsScreen.COURSE_ENROLLMENT.screenName - ).show( - fragmentManager, - IAPDialogFragment::class.simpleName - ) + onIAPAction(IAPAction.ACTION_COMPLETION, null, null) }, onDismiss = { - onIAPAction(IAPAction.ACTION_CLOSE, null) + onIAPAction(IAPAction.ACTION_CLOSE, null, null) }) } @@ -442,11 +453,18 @@ internal fun DashboardListView( title = stringResource(id = CoreR.string.iap_error_title), description = stringResource(id = CoreR.string.iap_course_not_fullfilled), confirmText = stringResource(id = CoreR.string.core_cancel), - onConfirm = { onIAPAction(IAPAction.ACTION_ERROR_CLOSE, null) }, + onConfirm = { + onIAPAction( + IAPAction.ACTION_ERROR_CLOSE, + null, + null + ) + }, dismissText = stringResource(id = CoreR.string.iap_get_help), onDismiss = { onIAPAction( IAPAction.ACTION_GET_HELP, + null, iapUiState.iapException ) } @@ -618,8 +636,6 @@ private fun CourseItemPreview() { } } -object MockFragmentManager : FragmentManager() - @Preview(uiMode = UI_MODE_NIGHT_NO) @Preview(uiMode = UI_MODE_NIGHT_YES) @Composable @@ -642,13 +658,12 @@ private fun DashboardListViewPreview() { iapUiState = null, canLoadMore = false, refreshing = false, - fragmentManager = MockFragmentManager, hasInternetConnection = true, onReloadClick = {}, onSwipeRefresh = {}, paginationCallback = {}, onItemClick = {}, - onIAPAction = { _, _ -> }, + onIAPAction = { _, _, _ -> }, appUpgradeParameters = AppUpdateState.AppUpgradeParameters() ) } @@ -676,13 +691,12 @@ private fun DashboardListViewTabletPreview() { iapUiState = null, canLoadMore = false, refreshing = false, - fragmentManager = MockFragmentManager, hasInternetConnection = true, onReloadClick = {}, onSwipeRefresh = {}, paginationCallback = {}, onItemClick = {}, - onIAPAction = { _, _ -> }, + onIAPAction = { _, _, _ -> }, appUpgradeParameters = AppUpdateState.AppUpgradeParameters() ) } diff --git a/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsFragment.kt b/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsFragment.kt index bd2f1bae4..a374c2ff6 100644 --- a/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsFragment.kt +++ b/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsFragment.kt @@ -10,7 +10,10 @@ import androidx.compose.ui.platform.ComposeView import androidx.compose.ui.platform.ViewCompositionStrategy import androidx.fragment.app.Fragment import org.koin.androidx.viewmodel.ext.android.viewModel +import org.openedx.core.presentation.IAPAnalyticsScreen +import org.openedx.core.presentation.dialog.IAPDialogFragment import org.openedx.core.presentation.iap.IAPAction +import org.openedx.core.presentation.iap.IAPFlow import org.openedx.core.ui.rememberWindowSize import org.openedx.core.ui.theme.OpenEdXTheme @@ -36,7 +39,6 @@ class SettingsFragment : Fragment() { windowSize = windowSize, uiState = uiState, iapUiState = iapUiState, - fragmentManager = requireActivity().supportFragmentManager, appUpgradeEvent = appUpgradeEvent, onBackClick = { requireActivity().supportFragmentManager.popBackStack() @@ -127,6 +129,16 @@ class SettingsFragment : Fragment() { ?.let { viewModel.showFeedbackScreen(requireActivity(), it) } } + IAPAction.ACTION_RESTORE -> { + IAPDialogFragment.newInstance( + IAPFlow.RESTORE, + IAPAnalyticsScreen.PROFILE.screenName + ).show( + requireActivity().supportFragmentManager, + IAPDialogFragment.TAG + ) + } + else -> {} } } diff --git a/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsScreenUI.kt b/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsScreenUI.kt index 775c77b4b..c05899151 100644 --- a/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsScreenUI.kt +++ b/profile/src/main/java/org/openedx/profile/presentation/settings/SettingsScreenUI.kt @@ -50,15 +50,11 @@ import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.compose.ui.window.Dialog -import androidx.fragment.app.FragmentManager import org.openedx.core.R import org.openedx.core.domain.model.AgreementUrls import org.openedx.core.exception.iap.IAPException -import org.openedx.core.presentation.IAPAnalyticsScreen -import org.openedx.core.presentation.dialog.IAPDialogFragment import org.openedx.core.presentation.global.AppData import org.openedx.core.presentation.iap.IAPAction -import org.openedx.core.presentation.iap.IAPFlow import org.openedx.core.presentation.iap.IAPLoaderType import org.openedx.core.presentation.iap.IAPUIState import org.openedx.core.system.notifier.AppUpgradeEvent @@ -87,7 +83,6 @@ internal fun SettingsScreen( windowSize: WindowSize, uiState: SettingsUIState, iapUiState: IAPUIState?, - fragmentManager: FragmentManager, appUpgradeEvent: AppUpgradeEvent?, onBackClick: () -> Unit, onAction: (SettingsScreenAction) -> Unit, @@ -241,13 +236,7 @@ internal fun SettingsScreen( } iapUiState is IAPUIState.PurchasesFulfillmentCompleted -> { - IAPDialogFragment.newInstance( - IAPFlow.RESTORE, - IAPAnalyticsScreen.PROFILE.screenName - ).show( - fragmentManager, - IAPDialogFragment::class.simpleName - ) + onIAPAction(IAPAction.ACTION_RESTORE, null) } iapUiState is IAPUIState.Error -> { @@ -770,8 +759,6 @@ private fun LogoutDialogPreview() { LogoutDialog({}, {}) } -object MockFragmentManager : FragmentManager() - @Preview @Composable private fun SettingsScreenPreview() { @@ -784,7 +771,6 @@ private fun SettingsScreenPreview() { onBackClick = {}, onAction = {}, onIAPAction = { _, _ -> }, - fragmentManager = MockFragmentManager, ) } }