fix: addressed PR comments

openedx · Feb 8, 2024 · 00eca54 · 00eca54
1 parent db1d00a
commit 00eca54
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,15 +1,12 @@
 package org.openedx.core.extension
 
 import kotlinx.coroutines.CancellableContinuation
-import kotlin.coroutines.Continuation
 import kotlin.coroutines.resume
 
-inline fun <T> Continuation<T>.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 <T> CancellableContinuation<T>.safeResume(value: T, onExceptionCalled: () -> Unit) {
+    if (isActive) {
+        resume(value)
+    } else {
+        onExceptionCalled()
+    }
 }