From 726ed6d0dcd7a8851d509d0e055b1ae1f4108f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Zolnai?= Date: Thu, 21 Nov 2024 11:52:21 +0100 Subject: [PATCH] PR comment: Make AccountLinkedViewModel separate --- .../main/kotlin/nl/eduid/graphs/MainGraph.kt | 3 +- .../accountlinked/AccountLinkedScreen.kt | 5 +- .../accountlinked/AccountLinkedViewModel.kt | 127 ++++++++++++++++++ .../nl/eduid/screens/accountlinked/UiState.kt | 10 ++ .../screens/personalinfo/PersonalInfoData.kt | 34 +++++ .../personalinfo/PersonalInfoViewModel.kt | 75 +---------- 6 files changed, 175 insertions(+), 79 deletions(-) create mode 100644 app/src/main/kotlin/nl/eduid/screens/accountlinked/AccountLinkedViewModel.kt create mode 100644 app/src/main/kotlin/nl/eduid/screens/accountlinked/UiState.kt diff --git a/app/src/main/kotlin/nl/eduid/graphs/MainGraph.kt b/app/src/main/kotlin/nl/eduid/graphs/MainGraph.kt index 3849216..6f1973e 100644 --- a/app/src/main/kotlin/nl/eduid/graphs/MainGraph.kt +++ b/app/src/main/kotlin/nl/eduid/graphs/MainGraph.kt @@ -14,6 +14,7 @@ import androidx.navigation.navDeepLink import nl.eduid.graphs.RequestEduIdLinkSent.LOGIN_REASON import nl.eduid.graphs.RequestEduIdLinkSent.reasonArg import nl.eduid.screens.accountlinked.AccountLinkedScreen +import nl.eduid.screens.accountlinked.AccountLinkedViewModel import nl.eduid.screens.accountlinked.ResultAccountLinked import nl.eduid.screens.biometric.EnableBiometricScreen import nl.eduid.screens.biometric.EnableBiometricViewModel @@ -364,7 +365,7 @@ fun MainGraph( val fullUri = deepLinkIntent?.data ?: Uri.EMPTY val result = ResultAccountLinked.fromRedirectUrl(fullUri) - val viewModel = hiltViewModel(entry) + val viewModel = hiltViewModel(entry) AccountLinkedScreen( viewModel = viewModel, result = result, diff --git a/app/src/main/kotlin/nl/eduid/screens/accountlinked/AccountLinkedScreen.kt b/app/src/main/kotlin/nl/eduid/screens/accountlinked/AccountLinkedScreen.kt index 1d00251..a5897da 100644 --- a/app/src/main/kotlin/nl/eduid/screens/accountlinked/AccountLinkedScreen.kt +++ b/app/src/main/kotlin/nl/eduid/screens/accountlinked/AccountLinkedScreen.kt @@ -53,7 +53,7 @@ import java.util.Locale @Composable fun AccountLinkedScreen( - viewModel: PersonalInfoViewModel, + viewModel: AccountLinkedViewModel, result: ResultAccountLinked, continueToHome: () -> Unit, continueToPersonalInfo: () -> Unit @@ -66,9 +66,6 @@ fun AccountLinkedScreen( continueToPersonalInfo() } } - LaunchedEffect(viewModel) { - viewModel.refreshPersonalInfo() - } Column( modifier = Modifier .fillMaxSize() diff --git a/app/src/main/kotlin/nl/eduid/screens/accountlinked/AccountLinkedViewModel.kt b/app/src/main/kotlin/nl/eduid/screens/accountlinked/AccountLinkedViewModel.kt new file mode 100644 index 0000000..5b56baf --- /dev/null +++ b/app/src/main/kotlin/nl/eduid/screens/accountlinked/AccountLinkedViewModel.kt @@ -0,0 +1,127 @@ +package nl.eduid.screens.accountlinked + +import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import nl.eduid.ErrorData +import nl.eduid.R +import nl.eduid.di.assist.DataAssistant +import nl.eduid.di.assist.SaveableResult +import nl.eduid.di.assist.toErrorData +import nl.eduid.graphs.AccountLinked +import nl.eduid.screens.personalinfo.PersonalInfo +import javax.inject.Inject + +@HiltViewModel +class AccountLinkedViewModel @Inject constructor( + savedStateHandle: SavedStateHandle, + private val assistant: DataAssistant +) : ViewModel() { + private val _accountLinked: MutableStateFlow = MutableStateFlow(false) + private val _errorData: MutableStateFlow = MutableStateFlow(null) + private val _isProcessing: MutableStateFlow = MutableStateFlow(false) + + val uiState = assistant.observableDetails.map { + when (it) { + is SaveableResult.Success -> { + val personalInfo = PersonalInfo.fromUserDetails(it.data, assistant) + if (it.saveError != null) { + _errorData.emit(it.saveError.toErrorData()) + } + UiState( + isLoading = false, + personalInfo = personalInfo + ) + } + + is SaveableResult.LoadError -> { + _errorData.emit(it.exception.toErrorData()) + UiState(isLoading = false) + } + + null -> { + _errorData.emit( + ErrorData( + titleId = R.string.ResponseErrors_UnauthorizedTitle_COPY, + messageId = R.string.ResponseErrors_PersonalDetailsRetrieveError_COPY + ) + ) + UiState(isLoading = false) + } + } + }.stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(3_000), + initialValue = UiState(isLoading = true), + ) + + val isRegistrationFlow: Boolean = savedStateHandle.get(AccountLinked.isRegistrationFlowArg) ?: false + + val accountLinked = _accountLinked.stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(3_000), + initialValue = false, + ) + val errorData = _errorData.stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(3_000), + initialValue = null, + ) + val isProcessing = _isProcessing.stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(3_000), + initialValue = false, + ) + + init { + viewModelScope.launch { + refreshPersonalInfo() + } + } + + fun clearErrorData() = _errorData.update { null } + + fun findLinkedAccount(personalInfo: PersonalInfo?, institutionId: String?): PersonalInfo.InstitutionAccount? { + val completeList = (personalInfo?.linkedInternalAccounts ?: listOf()) + (personalInfo?.linkedExternalAccounts ?: listOf()) + completeList.forEach { + if (it.institution == institutionId || it.subjectId == institutionId) { + return it + } + } + return completeList.firstOrNull() + } + + fun isFirstLinkedAccount(personalInfo: PersonalInfo): Boolean { + return personalInfo.linkedInternalAccounts.size + personalInfo.linkedExternalAccounts.size < 2 + } + + fun preferLinkedAccount(linkedAccount: PersonalInfo.InstitutionAccount) { + _isProcessing.update { true } + viewModelScope.launch { + if (assistant.preferLinkedAccount(linkedAccount.updateRequest)) { + _accountLinked.update { true } + } else { + _errorData.update { + ErrorData( + titleId = R.string.ExternalAccountLinkingError_Title_COPY, + messageId = R.string.ExternalAccountLinkingError_Subtitle_COPY + ) + } + } + _isProcessing.update { false } + } + } + + fun refreshPersonalInfo() { + viewModelScope.launch { + assistant.refreshDetails() + } + } +} \ No newline at end of file diff --git a/app/src/main/kotlin/nl/eduid/screens/accountlinked/UiState.kt b/app/src/main/kotlin/nl/eduid/screens/accountlinked/UiState.kt new file mode 100644 index 0000000..363fd30 --- /dev/null +++ b/app/src/main/kotlin/nl/eduid/screens/accountlinked/UiState.kt @@ -0,0 +1,10 @@ +package nl.eduid.screens.accountlinked + +import androidx.compose.runtime.Stable +import nl.eduid.screens.personalinfo.PersonalInfo + +@Stable +data class UiState( + val personalInfo: PersonalInfo = PersonalInfo(), + val isLoading: Boolean = false +) \ No newline at end of file diff --git a/app/src/main/kotlin/nl/eduid/screens/personalinfo/PersonalInfoData.kt b/app/src/main/kotlin/nl/eduid/screens/personalinfo/PersonalInfoData.kt index 23c16ee..6858af4 100644 --- a/app/src/main/kotlin/nl/eduid/screens/personalinfo/PersonalInfoData.kt +++ b/app/src/main/kotlin/nl/eduid/screens/personalinfo/PersonalInfoData.kt @@ -1,11 +1,15 @@ package nl.eduid.screens.personalinfo +import android.app.Person import androidx.compose.runtime.Stable import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.toImmutableList +import nl.eduid.di.assist.DataAssistant import nl.eduid.di.model.ConfirmedName import nl.eduid.di.model.LinkedAccountUpdateRequest import nl.eduid.di.model.SelfAssertedName +import nl.eduid.di.model.UserDetails +import nl.eduid.di.model.mapToPersonalInfo import java.time.LocalDate @Stable @@ -22,6 +26,7 @@ data class PersonalInfo( ) { val isVerified = linkedInternalAccounts.isNotEmpty() || linkedExternalAccounts.isNotEmpty() + data class InstitutionAccount( val subjectId: String, val role: String?, @@ -37,6 +42,35 @@ data class PersonalInfo( ) companion object { + suspend fun fromUserDetails(userDetails: UserDetails, assistant: DataAssistant): PersonalInfo { + var personalInfo = userDetails.mapToPersonalInfo() + val nameMap = mutableMapOf() + for (account in userDetails.linkedAccounts) { + val mappedName = assistant.getInstitutionName(account.schacHomeOrganization) + mappedName?.let { + //If name found, add to list of mapped names + nameMap[account.schacHomeOrganization] = mappedName + //Get name provider from FIRST linked account + if (account.schacHomeOrganization == userDetails.linkedAccounts.firstOrNull()?.schacHomeOrganization) { + personalInfo = personalInfo.copy( + nameProvider = nameMap[account.schacHomeOrganization] + ?: personalInfo.nameProvider + ) + } + //Update UI data to include mapped institution names + personalInfo = + personalInfo.copy(linkedInternalAccounts = personalInfo.linkedInternalAccounts.map { institution -> + institution.copy( + roleProvider = nameMap[institution.roleProvider] + ?: institution.roleProvider + ) + }.toImmutableList()) + } + } + return personalInfo + + } + fun demoData(): PersonalInfo { return PersonalInfo( name = "R. van Hamersdonksveer", diff --git a/app/src/main/kotlin/nl/eduid/screens/personalinfo/PersonalInfoViewModel.kt b/app/src/main/kotlin/nl/eduid/screens/personalinfo/PersonalInfoViewModel.kt index f6d9854..8f70517 100644 --- a/app/src/main/kotlin/nl/eduid/screens/personalinfo/PersonalInfoViewModel.kt +++ b/app/src/main/kotlin/nl/eduid/screens/personalinfo/PersonalInfoViewModel.kt @@ -34,15 +34,12 @@ class PersonalInfoViewModel @Inject constructor( ) : ViewModel() { private val _errorData: MutableStateFlow = MutableStateFlow(null) private val _isProcessing: MutableStateFlow = MutableStateFlow(false) - private val _accountLinked: MutableStateFlow = MutableStateFlow(false) private val _linkUrl: MutableStateFlow = MutableStateFlow(null) - val isRegistrationFlow: Boolean = savedStateHandle.get(AccountLinked.isRegistrationFlowArg) ?: false - val uiState = assistant.observableDetails.map { when (it) { is SaveableResult.Success -> { - val personalInfo = mapUserDetailsToPersonalInfo(it.data) + val personalInfo = PersonalInfo.fromUserDetails(it.data, assistant) if (it.saveError != null) { _errorData.emit(it.saveError.toErrorData()) } @@ -116,11 +113,6 @@ class PersonalInfoViewModel @Inject constructor( started = SharingStarted.WhileSubscribed(3_000), initialValue = false, ) - val accountLinked = _accountLinked.stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(3_000), - initialValue = false, - ) val linkUrl = _linkUrl.stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(3_000), @@ -133,34 +125,6 @@ class PersonalInfoViewModel @Inject constructor( val identityVerificationEnabled = runtimeBehavior.isFeatureEnabled(FeatureFlag.ENABLE_IDENTITY_VERIFICATION) - private suspend fun mapUserDetailsToPersonalInfo(userDetails: UserDetails): PersonalInfo { - var personalInfo = userDetails.mapToPersonalInfo() - val nameMap = mutableMapOf() - for (account in userDetails.linkedAccounts) { - val mappedName = assistant.getInstitutionName(account.schacHomeOrganization) - mappedName?.let { - //If name found, add to list of mapped names - nameMap[account.schacHomeOrganization] = mappedName - //Get name provider from FIRST linked account - if (account.schacHomeOrganization == userDetails.linkedAccounts.firstOrNull()?.schacHomeOrganization) { - personalInfo = personalInfo.copy( - nameProvider = nameMap[account.schacHomeOrganization] - ?: personalInfo.nameProvider - ) - } - //Update UI data to include mapped institution names - personalInfo = - personalInfo.copy(linkedInternalAccounts = personalInfo.linkedInternalAccounts.map { institution -> - institution.copy( - roleProvider = nameMap[institution.roleProvider] - ?: institution.roleProvider - ) - }.toImmutableList()) - } - } - return personalInfo - } - fun clearErrorData() = _errorData.update { null } fun requestLinkUrl() = viewModelScope.launch { @@ -184,41 +148,4 @@ class PersonalInfoViewModel @Inject constructor( intent.data = Uri.parse(url) return intent } - - fun findLinkedAccount(personalInfo: PersonalInfo?, institutionId: String?): PersonalInfo.InstitutionAccount? { - val completeList = (personalInfo?.linkedInternalAccounts ?: listOf()) + (personalInfo?.linkedExternalAccounts ?: listOf()) - completeList.forEach { - if (it.institution == institutionId || it.subjectId == institutionId) { - return it - } - } - return completeList.firstOrNull() - } - - fun isFirstLinkedAccount(personalInfo: PersonalInfo): Boolean { - return personalInfo.linkedInternalAccounts.size + personalInfo.linkedExternalAccounts.size < 2 - } - - fun preferLinkedAccount(linkedAccount: PersonalInfo.InstitutionAccount) { - _isProcessing.update { true } - viewModelScope.launch { - if (assistant.preferLinkedAccount(linkedAccount.updateRequest)) { - _accountLinked.update { true } - } else { - _errorData.update { - ErrorData( - titleId = R.string.ExternalAccountLinkingError_Title_COPY, - messageId = R.string.ExternalAccountLinkingError_Subtitle_COPY - ) - } - } - _isProcessing.update { false } - } - } - - fun refreshPersonalInfo() { - viewModelScope.launch { - assistant.refreshDetails() - } - } }