Skip to content

Commit

Permalink
Fix progress display hiding way before registration navigation.
Browse files Browse the repository at this point in the history
Fixes #13850
Closes #13898

Co-authored-by: Sagar <sagar.0dev@gmail.com>
  • Loading branch information
cody-signal and Sagar0-0 authored Jan 15, 2025
1 parent 93604f5 commit e9d80f4
Show file tree
Hide file tree
Showing 13 changed files with 78 additions and 127 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ internal class ConfirmSvrPinFragment : BaseSvrPinFragment<ConfirmSvrPinViewModel
SaveAnimation.NONE -> confirm.cancelSpinning()
SaveAnimation.LOADING -> confirm.setSpinning()
SaveAnimation.SUCCESS -> {
confirm.cancelSpinning()
requireActivity().setResult(Activity.RESULT_OK)
closeNavGraphBranch()
RegistrationUtil.maybeMarkRegistrationComplete()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ class RegistrationActivity : BaseActivity() {
if (!needsProfile && !needsPin) {
sharedViewModel.completeRegistration()
}
sharedViewModel.setInProgress(false)

val startIntent = MainActivity.clearTop(this).apply {
if (needsPin) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,8 @@ class RegistrationViewModel : ViewModel() {
private val password = Util.getSecret(18)

private val coroutineExceptionHandler = CoroutineExceptionHandler { _, exception ->
Log.w(TAG, "CoroutineExceptionHandler invoked.", exception)
store.update {
it.copy(
networkError = exception,
inProgress = false
)
}
Log.w(TAG, "CoroutineExceptionHandler invoked!")
handleGenericError(exception)
}

val uiState = store.asLiveData()
Expand Down Expand Up @@ -238,8 +233,7 @@ class RegistrationViewModel : ViewModel() {
store.update {
it.copy(
canSkipSms = true,
isReRegister = true,
inProgress = false
isReRegister = true
)
}
return
Expand All @@ -263,8 +257,7 @@ class RegistrationViewModel : ViewModel() {
isReRegister = true,
canSkipSms = true,
svr2AuthCredentials = svrCredentialsResult.svr2Credentials,
svr3AuthCredentials = svrCredentialsResult.svr3Credentials,
inProgress = false
svr3AuthCredentials = svrCredentialsResult.svr3Credentials
)
}
return@launch
Expand Down Expand Up @@ -315,7 +308,7 @@ class RegistrationViewModel : ViewModel() {

if (e164 == null) {
Log.w(TAG, "Phone number was null after confirmation.")
onErrorOccurred()
setInProgress(false)
return
}

Expand Down Expand Up @@ -404,8 +397,7 @@ class RegistrationViewModel : ViewModel() {
nextVerificationAttempt = RegistrationRepository.deriveTimestamp(networkResult.headers, networkResult.body.nextVerificationAttempt),
allowedToRequestCode = networkResult.body.allowedToRequestCode,
challengesRequested = Challenge.parse(networkResult.body.requestedInformation),
verified = networkResult.body.verified,
inProgress = false
verified = networkResult.body.verified
)
}
},
Expand Down Expand Up @@ -451,12 +443,6 @@ class RegistrationViewModel : ViewModel() {
val session = getOrCreateValidSession(context) ?: return@launch bail { Log.i(TAG, "Could not create valid session for submitting a push challenge token.") }

if (!Challenge.parse(session.body.requestedInformation).contains(Challenge.PUSH)) {
Log.d(TAG, "Push submission no longer necessary, bailing.")
store.update {
it.copy(
inProgress = false
)
}
return@launch bail { Log.i(TAG, "Push challenge token no longer needed, bailing.") }
}

Expand Down Expand Up @@ -486,8 +472,7 @@ class RegistrationViewModel : ViewModel() {
nextSmsTimestamp = sessionResult.nextSmsTimestamp,
nextCallTimestamp = sessionResult.nextCallTimestamp,
isAllowedToRequestCode = sessionResult.allowedToRequestCode,
challengesRequested = emptyList(),
inProgress = false
challengesRequested = emptyList()
)
}
return true
Expand All @@ -498,8 +483,7 @@ class RegistrationViewModel : ViewModel() {
store.update {
it.copy(
registrationCheckpoint = RegistrationCheckpoint.CHALLENGE_RECEIVED,
challengesRequested = sessionResult.challenges,
inProgress = false
challengesRequested = sessionResult.challenges
)
}
return false
Expand Down Expand Up @@ -534,9 +518,10 @@ class RegistrationViewModel : ViewModel() {

is AlreadyVerified -> Log.i(TAG, "Received AlreadyVerified", sessionResult.getCause())
}
setInProgress(false)

store.update {
it.copy(
inProgress = false,
sessionStateError = sessionResult
)
}
Expand All @@ -548,6 +533,7 @@ class RegistrationViewModel : ViewModel() {
*/
private suspend fun handleRegistrationResult(context: Context, registrationData: RegistrationData, registrationResult: RegisterAccountResult, reglockEnabled: Boolean): Boolean {
Log.v(TAG, "handleRegistrationResult()")
var stayInProgress = false
when (registrationResult) {
is RegisterAccountResult.Success -> {
Log.i(TAG, "Register account result: Success! Registration lock: $reglockEnabled")
Expand All @@ -567,6 +553,7 @@ class RegistrationViewModel : ViewModel() {

is RegisterAccountResult.RegistrationLocked -> {
Log.i(TAG, "Account is registration locked!", registrationResult.getCause())
stayInProgress = true
}

is RegisterAccountResult.SvrWrongPin -> {
Expand All @@ -582,9 +569,9 @@ class RegistrationViewModel : ViewModel() {
is RegisterAccountResult.ValidationError,
is RegisterAccountResult.UnknownError -> Log.i(TAG, "Received error when trying to register!", registrationResult.getCause())
}
setInProgress(false)
store.update {
it.copy(
inProgress = stayInProgress,
registerAccountError = registrationResult
)
}
Expand Down Expand Up @@ -636,7 +623,6 @@ class RegistrationViewModel : ViewModel() {
updateSvrTriesRemaining(0)
setUserSkippedReRegisterFlow(true)
}
setInProgress(false)
}
return
}
Expand All @@ -647,19 +633,17 @@ class RegistrationViewModel : ViewModel() {
Log.d(TAG, "Found recovery password, attempting to re-register.")
viewModelScope.launch(context = coroutineExceptionHandler) {
verifyReRegisterInternal(context, pin, SignalStore.svr.masterKey)
setInProgress(false)
}
} else {
Log.d(TAG, "Entered PIN did not match local PIN hash.")
wrongPinHandler()
setInProgress(false)
}
return
}

Log.w(TAG, "Could not get credentials to skip SMS registration, aborting!")
store.update {
it.copy(canSkipSms = false, inProgress = false)
it.copy(canSkipSms = false)
}
}

Expand Down Expand Up @@ -857,8 +841,7 @@ class RegistrationViewModel : ViewModel() {

store.update {
it.copy(
registrationCheckpoint = RegistrationCheckpoint.LOCAL_REGISTRATION_COMPLETE,
inProgress = false
registrationCheckpoint = RegistrationCheckpoint.LOCAL_REGISTRATION_COMPLETE
)
}
}
Expand Down Expand Up @@ -898,14 +881,6 @@ class RegistrationViewModel : ViewModel() {
return RegistrationData(code, e164, password, RegistrationRepository.getRegistrationId(), RegistrationRepository.getProfileKey(e164), currentState.fcmToken, RegistrationRepository.getPniRegistrationId(), recoveryPassword)
}

/**
* This is a generic error UI handler that re-enables the UI so that the user can recover from errors.
* Do not forget to log any errors when calling this method!
*/
private fun onErrorOccurred() {
setInProgress(false)
}

/**
* Used for early returns in order to end the in-progress visual state, as well as print a log message explaining what happened.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ class EnterCodeFragment : LoggingFragment(R.layout.fragment_registration_enter_c
object : AssertedSuccessListener<Boolean>() {
override fun onSuccess(result: Boolean?) {
findNavController().safeNavigate(EnterCodeFragmentDirections.actionRequireKbsLockPin(timeRemaining))
sharedViewModel.setInProgress(false)
}
}
)
Expand Down Expand Up @@ -265,6 +266,7 @@ class EnterCodeFragment : LoggingFragment(R.layout.fragment_registration_enter_c
private fun popBackStack() {
sharedViewModel.setRegistrationCheckpoint(RegistrationCheckpoint.PUSH_NETWORK_AUDITED)
NavHostFragment.findNavController(this).popBackStack()
sharedViewModel.setInProgress(false)
}

@Subscribe(threadMode = ThreadMode.MAIN)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import com.google.android.material.textfield.TextInputEditText
import com.google.i18n.phonenumbers.NumberParseException
import com.google.i18n.phonenumbers.PhoneNumberUtil
import com.google.i18n.phonenumbers.Phonenumber.PhoneNumber
import org.signal.core.util.ThreadUtil
import org.signal.core.util.isNotNullOrBlank
import org.signal.core.util.logging.Log
import org.thoughtcrime.securesms.LoggingFragment
Expand Down Expand Up @@ -114,7 +115,7 @@ class EnterPhoneNumberFragment : LoggingFragment(R.layout.fragment_registration_

sharedViewModel.uiState.observe(viewLifecycleOwner) { sharedState ->
presentRegisterButton(sharedState)
presentProgressBar(sharedState.inProgress, sharedState.isReRegister)
updateEnabledControls(sharedState.inProgress, sharedState.isReRegister)

sharedState.networkError?.let {
presentNetworkError(it)
Expand Down Expand Up @@ -400,6 +401,7 @@ class EnterPhoneNumberFragment : LoggingFragment(R.layout.fragment_registration_

private fun presentRegistrationLocked(timeRemaining: Long) {
findNavController().safeNavigate(EnterPhoneNumberFragmentDirections.actionPhoneNumberRegistrationLock(timeRemaining))
sharedViewModel.setInProgress(false)
}

private fun presentRateLimitedDialog() {
Expand All @@ -408,10 +410,12 @@ class EnterPhoneNumberFragment : LoggingFragment(R.layout.fragment_registration_

private fun presentAccountLocked() {
findNavController().safeNavigate(EnterPhoneNumberFragmentDirections.actionPhoneNumberAccountLocked())
ThreadUtil.postToMain { sharedViewModel.setInProgress(false) }
}

private fun moveToCaptcha() {
findNavController().safeNavigate(EnterPhoneNumberFragmentDirections.actionRequestCaptcha())
ThreadUtil.postToMain { sharedViewModel.setInProgress(false) }
}

private fun presentRemoteErrorDialog(message: String, positiveButtonListener: DialogInterface.OnClickListener? = null) {
Expand Down Expand Up @@ -491,12 +495,7 @@ class EnterPhoneNumberFragment : LoggingFragment(R.layout.fragment_registration_
}
}

private fun presentProgressBar(showProgress: Boolean, isReRegister: Boolean) {
if (showProgress) {
binding.registerButton.setSpinning()
} else {
binding.registerButton.cancelSpinning()
}
private fun updateEnabledControls(showProgress: Boolean, isReRegister: Boolean) {
binding.countryCode.isEnabled = !showProgress
binding.number.isEnabled = !showProgress
binding.cancelButton.visible = !showProgress && isReRegister
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import org.thoughtcrime.securesms.registration.ui.RegistrationViewModel
import org.thoughtcrime.securesms.util.CommunicationActions
import org.thoughtcrime.securesms.util.SupportEmailUtil
import org.thoughtcrime.securesms.util.ViewUtil
import org.thoughtcrime.securesms.util.livedata.LiveDataUtil
import org.thoughtcrime.securesms.util.navigation.safeNavigate

class ReRegisterWithPinFragment : LoggingFragment(R.layout.fragment_registration_pin_restore_entry_v2) {
Expand Down Expand Up @@ -76,21 +77,24 @@ class ReRegisterWithPinFragment : LoggingFragment(R.layout.fragment_registration

binding.pinRestoreKeyboardToggle.setIconResource(getPinEntryKeyboardType().other.iconResource)

registrationViewModel.uiState.observe(viewLifecycleOwner, ::updateViewState)
LiveDataUtil
.combineLatest(registrationViewModel.uiState, reRegisterViewModel.uiState) { reg, rereg -> reg to rereg }
.observe(viewLifecycleOwner) { (registrationState, reRegisterState) -> updateViewState(registrationState, reRegisterState) }
}

private fun updateViewState(state: RegistrationState) {
private fun updateViewState(state: RegistrationState, reRegisterState: ReRegisterWithPinState) {
if (state.networkError != null) {
genericErrorDialog()
registrationViewModel.networkErrorShown()
} else if (!state.canSkipSms) {
findNavController().safeNavigate(ReRegisterWithPinFragmentDirections.actionReRegisterWithPinFragmentToEnterPhoneNumberFragment())
registrationViewModel.setInProgress(false)
} else if (state.isRegistrationLockEnabled && state.svrTriesRemaining == 0) {
Log.w(TAG, "Unable to continue skip flow, KBS is locked")
onAccountLocked()
} else {
presentProgress(state.inProgress)
presentTriesRemaining(state.svrTriesRemaining)
presentTriesRemaining(reRegisterState, state.svrTriesRemaining)
}

state.registerAccountError?.let { error ->
Expand Down Expand Up @@ -131,14 +135,15 @@ class ReRegisterWithPinFragment : LoggingFragment(R.layout.fragment_registration
context = requireContext(),
pin = pin,
wrongPinHandler = {
registrationViewModel.setInProgress(false)
reRegisterViewModel.markIncorrectGuess()
}
)
}

private fun presentTriesRemaining(triesRemaining: Int) {
if (reRegisterViewModel.hasIncorrectGuess) {
if (triesRemaining == 1 && !reRegisterViewModel.isLocalVerification) {
private fun presentTriesRemaining(reRegisterState: ReRegisterWithPinState, triesRemaining: Int) {
if (reRegisterState.hasIncorrectGuess) {
if (triesRemaining == 1 && !reRegisterState.isLocalVerification) {
MaterialAlertDialogBuilder(requireContext())
.setTitle(R.string.PinRestoreEntryFragment_incorrect_pin)
.setMessage(resources.getQuantityString(R.plurals.PinRestoreEntryFragment_you_have_d_attempt_remaining, triesRemaining, triesRemaining))
Expand All @@ -155,7 +160,7 @@ class ReRegisterWithPinFragment : LoggingFragment(R.layout.fragment_registration
} else {
if (triesRemaining == 1) {
binding.pinRestoreForgotPin.visibility = View.VISIBLE
if (!reRegisterViewModel.isLocalVerification) {
if (!reRegisterState.isLocalVerification) {
MaterialAlertDialogBuilder(requireContext())
.setMessage(resources.getQuantityString(R.plurals.PinRestoreEntryFragment_you_have_d_attempt_remaining, triesRemaining, triesRemaining))
.setPositiveButton(android.R.string.ok, null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,17 @@
package org.thoughtcrime.securesms.registration.ui.reregisterwithpin

import androidx.lifecycle.ViewModel
import androidx.lifecycle.asLiveData
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.update
import org.signal.core.util.logging.Log

class ReRegisterWithPinViewModel : ViewModel() {
companion object {
private val TAG = Log.tag(ReRegisterWithPinViewModel::class.java)
}

private val store = MutableStateFlow(ReRegisterWithPinState())

val uiState = store.asLiveData()

val isLocalVerification: Boolean
get() = store.value.isLocalVerification
val hasIncorrectGuess: Boolean
get() = store.value.hasIncorrectGuess

fun markAsRemoteVerification() {
store.update {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ class RegistrationActivity : BaseActivity() {
if (!needsProfile && !needsPin) {
sharedViewModel.completeRegistration()
}
sharedViewModel.setInProgress(false)

val startIntent = MainActivity.clearTop(this)

Expand Down
Loading

0 comments on commit e9d80f4

Please sign in to comment.