From 00eca54ca031ca2a3bbc2d69e7a86b9082a8da86 Mon Sep 17 00:00:00 2001 From: k1rill Date: Thu, 8 Feb 2024 11:19:12 +0300 Subject: [PATCH] fix: addressed PR comments --- .../auth/presentation/signup/SignUpFragment.kt | 4 ++-- .../auth/presentation/signup/SignUpViewModel.kt | 12 ++++++------ .../signup/compose/SocialSignedView.kt | 5 ++++- .../openedx/auth/presentation/sso/OAuthHelper.kt | 6 ++++++ .../presentation/signup/SignUpViewModelTest.kt | 10 +++++----- .../main/java/org/openedx/core/ApiConstants.kt | 6 +++++- .../org/openedx/core/extension/ContinuationExt.kt | 15 ++++++--------- 7 files changed, 34 insertions(+), 24 deletions(-) diff --git a/auth/src/main/java/org/openedx/auth/presentation/signup/SignUpFragment.kt b/auth/src/main/java/org/openedx/auth/presentation/signup/SignUpFragment.kt index 823fb23c3..97bfe45d9 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/signup/SignUpFragment.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/signup/SignUpFragment.kt @@ -65,8 +65,8 @@ class SignUpFragment : Fragment() { ) } }, - onFieldUpdated = { name, value -> - viewModel.updateField(name, value) + onFieldUpdated = { key, value -> + viewModel.updateField(key, value) } ) diff --git a/auth/src/main/java/org/openedx/auth/presentation/signup/SignUpViewModel.kt b/auth/src/main/java/org/openedx/auth/presentation/signup/SignUpViewModel.kt index c8a33f81b..af1b8e094 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/signup/SignUpViewModel.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/signup/SignUpViewModel.kt @@ -96,7 +96,7 @@ class SignUpViewModel( fun register() { analytics.createAccountClickedEvent("") val mapFields = uiState.value.allFields.associate { it.name to it.placeholder } + - mapOf("honor_code" to true.toString()) + mapOf(ApiConstants.HONOR_CODE to true.toString()) val resultMap = mapFields.toMutableMap() uiState.value.allFields.filter { !it.required }.forEach { (k, _) -> if (mapFields[k].isNullOrEmpty()) { @@ -114,9 +114,9 @@ class SignUpViewModel( } else { val socialAuth = uiState.value.socialAuth if (socialAuth?.accessToken != null) { - resultMap["access_token"] = socialAuth.accessToken - resultMap["provider"] = socialAuth.authType.postfix - resultMap["client_id"] = config.getOAuthClientId() + resultMap[ApiConstants.ACCESS_TOKEN] = socialAuth.accessToken + resultMap[ApiConstants.PROVIDER] = socialAuth.authType.postfix + resultMap[ApiConstants.CLIENT_ID] = config.getOAuthClientId() } interactor.register(resultMap.toMap()) analytics.registrationSuccessEvent(socialAuth?.authType?.postfix.orEmpty()) @@ -229,10 +229,10 @@ class SignUpViewModel( } } - fun updateField(name: String, value: String) { + fun updateField(key: String, value: String) { _uiState.update { val updatedFields = uiState.value.allFields.toMutableList().map { field -> - if (field.name == name) { + if (field.name == key) { field.copy(placeholder = value) } else { field diff --git a/auth/src/main/java/org/openedx/auth/presentation/signup/compose/SocialSignedView.kt b/auth/src/main/java/org/openedx/auth/presentation/signup/compose/SocialSignedView.kt index 74a9c2a9e..25a9434d1 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/signup/compose/SocialSignedView.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/signup/compose/SocialSignedView.kt @@ -16,6 +16,7 @@ import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import org.openedx.auth.R import org.openedx.auth.data.model.AuthType +import org.openedx.core.ui.theme.OpenEdXTheme import org.openedx.core.ui.theme.appColors import org.openedx.core.ui.theme.appShapes import org.openedx.core.R as coreR @@ -54,5 +55,7 @@ internal fun SocialSignedView(authType: AuthType) { @Preview(name = "NEXUS_5_Dark", device = Devices.NEXUS_5, uiMode = Configuration.UI_MODE_NIGHT_YES) @Composable private fun PreviewSocialSignedView() { - SocialSignedView(AuthType.GOOGLE) + OpenEdXTheme { + SocialSignedView(AuthType.GOOGLE) + } } diff --git a/auth/src/main/java/org/openedx/auth/presentation/sso/OAuthHelper.kt b/auth/src/main/java/org/openedx/auth/presentation/sso/OAuthHelper.kt index a360c9d13..776df7c46 100644 --- a/auth/src/main/java/org/openedx/auth/presentation/sso/OAuthHelper.kt +++ b/auth/src/main/java/org/openedx/auth/presentation/sso/OAuthHelper.kt @@ -9,6 +9,12 @@ class OAuthHelper( private val googleAuthHelper: GoogleAuthHelper, private val microsoftAuthHelper: MicrosoftAuthHelper, ) { + /** + * SDK integration guides: + * https://developer.android.com/training/sign-in/credential-manager + * https://developers.facebook.com/docs/facebook-login/android/ + * https://github.com/AzureAD/microsoft-authentication-library-for-android + */ internal suspend fun socialAuth(fragment: Fragment, authType: AuthType): SocialAuthResponse? { return when (authType) { AuthType.PASSWORD -> null diff --git a/auth/src/test/java/org/openedx/auth/presentation/signup/SignUpViewModelTest.kt b/auth/src/test/java/org/openedx/auth/presentation/signup/SignUpViewModelTest.kt index 6a42e5b2f..bd048902c 100644 --- a/auth/src/test/java/org/openedx/auth/presentation/signup/SignUpViewModelTest.kt +++ b/auth/src/test/java/org/openedx/auth/presentation/signup/SignUpViewModelTest.kt @@ -198,7 +198,7 @@ class SignUpViewModelTest { coVerify(exactly = 0) { interactor.login(any(), any()) } verify(exactly = 1) { appUpgradeNotifier.notifier } - assertEquals(false, viewModel.uiState.value.validationError) + assertFalse(viewModel.uiState.value.validationError) assertFalse(viewModel.uiState.value.successLogin) assertFalse(viewModel.uiState.value.isButtonLoading) assertEquals(noInternet, (deferred.await() as? UIMessage.SnackBarMessage)?.message) @@ -233,7 +233,7 @@ class SignUpViewModelTest { coVerify(exactly = 0) { interactor.login(any(), any()) } verify(exactly = 1) { appUpgradeNotifier.notifier } - assertEquals(false, viewModel.uiState.value.validationError) + assertFalse(viewModel.uiState.value.validationError) assertFalse(viewModel.uiState.value.successLogin) assertFalse(viewModel.uiState.value.isButtonLoading) assertEquals(somethingWrong, (deferred.await() as? UIMessage.SnackBarMessage)?.message) @@ -282,9 +282,9 @@ class SignUpViewModelTest { verify(exactly = 1) { analytics.registrationSuccessEvent(any()) } verify(exactly = 1) { appUpgradeNotifier.notifier } - assertEquals(false, viewModel.uiState.value.validationError) - assertEquals(false, viewModel.uiState.value.isButtonLoading) - assertEquals(true, viewModel.uiState.value.successLogin) + assertFalse(viewModel.uiState.value.validationError) + assertFalse(viewModel.uiState.value.isButtonLoading) + assertTrue(viewModel.uiState.value.successLogin) } @Test diff --git a/core/src/main/java/org/openedx/core/ApiConstants.kt b/core/src/main/java/org/openedx/core/ApiConstants.kt index 1b5040d5f..558df5434 100644 --- a/core/src/main/java/org/openedx/core/ApiConstants.kt +++ b/core/src/main/java/org/openedx/core/ApiConstants.kt @@ -17,9 +17,13 @@ object ApiConstants { const val TOKEN_TYPE_JWT = "jwt" const val TOKEN_TYPE_REFRESH = "refresh_token" - const val NAME = "name" + const val ACCESS_TOKEN = "access_token" + const val CLIENT_ID = "client_id" const val EMAIL = "email" + const val HONOR_CODE = "honor_code" + const val NAME = "name" const val PASSWORD = "password" + const val PROVIDER = "provider" const val AUTH_TYPE_GOOGLE = "google-oauth2" const val AUTH_TYPE_FB = "facebook" diff --git a/core/src/main/java/org/openedx/core/extension/ContinuationExt.kt b/core/src/main/java/org/openedx/core/extension/ContinuationExt.kt index 21be1458b..8de4ec05b 100644 --- a/core/src/main/java/org/openedx/core/extension/ContinuationExt.kt +++ b/core/src/main/java/org/openedx/core/extension/ContinuationExt.kt @@ -1,15 +1,12 @@ package org.openedx.core.extension import kotlinx.coroutines.CancellableContinuation -import kotlin.coroutines.Continuation import kotlin.coroutines.resume -inline fun Continuation.safeResume(value: T, onExceptionCalled: () -> Unit) { - if (this is CancellableContinuation) { - if (isActive) { - resume(value) - } else { - onExceptionCalled() - } - } else throw Exception("Must use suspendCancellableCoroutine instead of suspendCoroutine") +inline fun CancellableContinuation.safeResume(value: T, onExceptionCalled: () -> Unit) { + if (isActive) { + resume(value) + } else { + onExceptionCalled() + } }