Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues around configuration changes #3

Open
wants to merge 1 commit into
base: handle-configuration-change-using-onSavedInstance
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class ExplorationActivityPresenter @Inject constructor(

fun loadExplorationFragment(readingTextSize: ReadingTextSize) {
if (getExplorationFragment() == null) {
activity.supportFragmentManager.beginTransaction().replace(
activity.supportFragmentManager.beginTransaction().add(
R.id.exploration_fragment_placeholder,
ExplorationFragment.newInstance(
profileId, topicId, storyId, explorationId, readingTextSize
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ class StateFragmentPresenter @Inject constructor(

fun handleAnswerReadyForSubmission(answer: UserAnswer) {
// An interaction has indicated that an answer is ready for submission.
handleSubmitAnswer(answer)
handleSubmitAnswer(answer, canSubmitAnswer = true)
}

fun onContinueButtonClicked() {
Expand Down Expand Up @@ -224,9 +224,7 @@ class StateFragmentPresenter @Inject constructor(

fun handleKeyboardAction() {
hideKeyboard()
if (viewModel.getCanSubmitAnswer().get() == true) {
handleSubmitAnswer(viewModel.getPendingAnswer(recyclerViewAssembler::getPendingAnswerHandler))
}
handleSubmitAnswer(viewModel.getPendingAnswer(recyclerViewAssembler::getPendingAnswerHandler))
}

fun onHintAvailable(helpIndex: HelpIndex, isCurrentStatePendingState: Boolean) {
Expand Down Expand Up @@ -425,8 +423,15 @@ class StateFragmentPresenter @Inject constructor(
}
}

private fun handleSubmitAnswer(answer: UserAnswer) {
subscribeToAnswerOutcome(explorationProgressController.submitAnswer(answer).toLiveData())
private fun handleSubmitAnswer(
answer: UserAnswer,
canSubmitAnswer: Boolean = viewModel.getCanSubmitAnswer().get() ?: false
) {
// This check seems to avoid a crash on configuration change when attempting to resubmit answers
// after encountering a submit-time error, but it's also more correct to keep it.
if (canSubmitAnswer) {
subscribeToAnswerOutcome(explorationProgressController.submitAnswer(answer).toLiveData())
}
}

fun dismissConceptCard() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,10 @@ class StatePlayerRecyclerViewAssembler private constructor(
hasPreviousButton,
isSplitView.get()!!,
writtenTranslationContext
)
).also {
// Ensure that potential errors are re-detected in cases of configuration changes.
(it as? InteractionAnswerHandler)?.checkPendingAnswerError(rawUserAnswer.lastErrorCategory)
}
}

private fun addContentItem(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import androidx.databinding.ObservableField
import androidx.databinding.ObservableList
import androidx.lifecycle.ViewModel
import org.oppia.android.app.fragment.FragmentScope
import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.RawUserAnswer
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.android.app.player.state.itemviewmodel.StateItemViewModel
import org.oppia.android.app.viewmodel.ObservableArrayList
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.oppia.android.app.player.state.answerhandling

import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.RawUserAnswer
import org.oppia.android.app.model.UserAnswer

Expand Down Expand Up @@ -44,11 +45,3 @@ interface InteractionAnswerHandler {
interface InteractionAnswerReceiver {
fun onAnswerReadyForSubmission(answer: UserAnswer)
}

/** Categories of errors that can be inferred from a pending answer. */
enum class AnswerErrorCategory {
/** Corresponds to errors that may be found while the user is trying to input an answer. */
REAL_TIME,
/** Corresponds to errors that may be found only when a user tries to submit an answer. */
SUBMIT_TIME
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.oppia.android.app.player.state.itemviewmodel
import androidx.databinding.Observable
import androidx.databinding.ObservableField
import androidx.recyclerview.widget.RecyclerView
import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.InteractionObject
import org.oppia.android.app.model.ListOfSetsOfHtmlStrings
Expand Down Expand Up @@ -143,6 +144,7 @@ class DragAndDropSortInteractionViewModel private constructor(
ListOfSetsOfTranslatableHtmlContentIds.newBuilder().apply {
addAllContentIdLists(htmlContentIds)
}.build()
lastErrorCategory = AnswerErrorCategory.NO_ERROR
}.build()

/** Returns an HTML list containing all of the HTML string elements as items in the list. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import android.text.TextWatcher
import androidx.databinding.Observable
import androidx.databinding.ObservableField
import org.oppia.android.R
import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.InteractionObject
import org.oppia.android.app.model.RawUserAnswer
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.parser.FractionParsingUiError
import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver
Expand All @@ -32,11 +32,12 @@ class FractionInteractionViewModel private constructor(
private val translationController: TranslationController
) : StateItemViewModel(ViewType.FRACTION_INPUT_INTERACTION), InteractionAnswerHandler {
private var pendingAnswerError: String? = null
var answerText: CharSequence = rawUserAnswer.textualAnswer ?: ""
var answerText: CharSequence = rawUserAnswer.textualAnswer
var isAnswerAvailable = ObservableField<Boolean>(false)
var errorMessage = ObservableField<String>("")
val hintText: CharSequence = deriveHintText(interaction)
private val fractionParser = FractionParser()
private var currentErrorCategory = AnswerErrorCategory.NO_ERROR

init {
val callback: Observable.OnPropertyChangedCallback =
Expand All @@ -50,7 +51,6 @@ class FractionInteractionViewModel private constructor(
}
errorMessage.addOnPropertyChangedCallback(callback)
isAnswerAvailable.addOnPropertyChangedCallback(callback)
checkPendingAnswerError(AnswerErrorCategory.REAL_TIME)
}

override fun getPendingAnswer(): UserAnswer = UserAnswer.newBuilder().apply {
Expand All @@ -67,20 +67,23 @@ class FractionInteractionViewModel private constructor(
/** It checks the pending error for the current fraction input, and correspondingly updates the error string based on the specified error category. */
override fun checkPendingAnswerError(category: AnswerErrorCategory): String? {
if (answerText.isNotEmpty()) {
when (category) {
pendingAnswerError = when (category) {
AnswerErrorCategory.REAL_TIME -> {
pendingAnswerError =
FractionParsingUiError.createFromParsingError(
fractionParser.getRealTimeAnswerError(answerText.toString())
).getErrorMessageFromStringRes(resourceHandler)
FractionParsingUiError.createFromParsingError(
fractionParser.getRealTimeAnswerError(answerText.toString())
).getErrorMessageFromStringRes(resourceHandler)
}
AnswerErrorCategory.SUBMIT_TIME -> {
pendingAnswerError =
FractionParsingUiError.createFromParsingError(
fractionParser.getSubmitTimeError(answerText.toString())
).getErrorMessageFromStringRes(resourceHandler)
FractionParsingUiError.createFromParsingError(
fractionParser.getSubmitTimeError(answerText.toString())
).getErrorMessageFromStringRes(resourceHandler)
}
AnswerErrorCategory.ANSWER_ERROR_CATEGORY_UNSPECIFIED, AnswerErrorCategory.UNRECOGNIZED,
AnswerErrorCategory.NO_ERROR -> null
}
currentErrorCategory = if (pendingAnswerError == null) {
AnswerErrorCategory.NO_ERROR
} else category
errorMessage.set(pendingAnswerError)
}
return pendingAnswerError
Expand All @@ -89,6 +92,7 @@ class FractionInteractionViewModel private constructor(
override fun getRawUserAnswer(): RawUserAnswer = RawUserAnswer.newBuilder().apply {
if (answerText.isNotEmpty()) {
textualAnswer = answerText.toString()
lastErrorCategory = currentErrorCategory
}
}.build()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.oppia.android.app.player.state.itemviewmodel
import androidx.databinding.Observable
import androidx.databinding.ObservableField
import org.oppia.android.R
import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.ClickOnImage
import org.oppia.android.app.model.ImageWithRegions.LabeledRegion
import org.oppia.android.app.model.Interaction
Expand Down Expand Up @@ -89,6 +90,7 @@ class ImageRegionSelectionInteractionViewModel private constructor(
if (answerText.isNotEmpty()) {
imageRegionSelection = selectableRegions.find { it.label == answerText.toString() }
}
lastErrorCategory = AnswerErrorCategory.NO_ERROR
}.build()

private fun parseClickOnImage(answerTextString: String): ClickOnImage {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import androidx.annotation.StringRes
import androidx.databinding.Observable
import androidx.databinding.ObservableField
import org.oppia.android.R
import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.InteractionObject
import org.oppia.android.app.model.MathEquation
Expand All @@ -14,7 +15,6 @@ import org.oppia.android.app.model.OppiaLanguage
import org.oppia.android.app.model.RawUserAnswer
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver
Expand Down Expand Up @@ -74,7 +74,7 @@ class MathExpressionInteractionsViewModel private constructor(
* Defines the current answer text being entered by the learner. This is expected to be directly
* bound to the corresponding edit text.
*/
var answerText: CharSequence = "" // TODO(#4708): Need to retain state for submit time error.
var answerText: CharSequence = rawUserAnswer.textualAnswer

/**
* Defines whether an answer is currently available to parse. This is expected to be directly
Expand All @@ -91,6 +91,8 @@ class MathExpressionInteractionsViewModel private constructor(
/** Specifies the text to show in the answer box when no text is entered. */
val hintText: CharSequence = deriveHintText(interaction)

private var currentErrorCategory = AnswerErrorCategory.NO_ERROR

private val allowedVariables = retrieveAllowedVariables(interaction)
private val useFractionsForDivision =
interaction.customizationArgsMap["useFractionForDivision"]?.boolValue ?: false
Expand Down Expand Up @@ -146,6 +148,7 @@ class MathExpressionInteractionsViewModel private constructor(
if (answerText.isNotEmpty()) {
textualAnswer = answerText.toString()
}
lastErrorCategory = currentErrorCategory
}.build()

override fun checkPendingAnswerError(category: AnswerErrorCategory): String? {
Expand All @@ -158,7 +161,12 @@ class MathExpressionInteractionsViewModel private constructor(
answerText.toString(), allowedVariables, resourceHandler
)
}
AnswerErrorCategory.ANSWER_ERROR_CATEGORY_UNSPECIFIED, AnswerErrorCategory.UNRECOGNIZED,
AnswerErrorCategory.NO_ERROR -> null
}
currentErrorCategory = if (pendingAnswerError == null) {
AnswerErrorCategory.NO_ERROR
} else category
errorMessage.set(pendingAnswerError)
}
return pendingAnswerError
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import android.text.Editable
import android.text.TextWatcher
import androidx.databinding.Observable
import androidx.databinding.ObservableField
import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.InteractionObject
import org.oppia.android.app.model.RawUserAnswer
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.parser.StringToNumberParser
import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver
Expand All @@ -26,11 +26,12 @@ class NumericInputViewModel private constructor(
private val writtenTranslationContext: WrittenTranslationContext,
private val resourceHandler: AppLanguageResourceHandler
) : StateItemViewModel(ViewType.NUMERIC_INPUT_INTERACTION), InteractionAnswerHandler {
var answerText: CharSequence = rawUserAnswer.textualAnswer ?: ""
var answerText: CharSequence = rawUserAnswer.textualAnswer
private var pendingAnswerError: String? = null
val errorMessage = ObservableField<String>("")
var isAnswerAvailable = ObservableField<Boolean>(false)
private val stringToNumberParser: StringToNumberParser = StringToNumberParser()
private var currentErrorCategory = AnswerErrorCategory.NO_ERROR

init {
val callback: Observable.OnPropertyChangedCallback =
Expand All @@ -45,7 +46,6 @@ class NumericInputViewModel private constructor(

errorMessage.addOnPropertyChangedCallback(callback)
isAnswerAvailable.addOnPropertyChangedCallback(callback)
checkPendingAnswerError(AnswerErrorCategory.REAL_TIME)
}

/** It checks the pending error for the current numeric input, and correspondingly updates the error string based on the specified error category. */
Expand All @@ -58,8 +58,13 @@ class NumericInputViewModel private constructor(
AnswerErrorCategory.SUBMIT_TIME ->
stringToNumberParser.getSubmitTimeError(answerText.toString())
.getErrorMessageFromStringRes(resourceHandler)
AnswerErrorCategory.ANSWER_ERROR_CATEGORY_UNSPECIFIED, AnswerErrorCategory.UNRECOGNIZED,
AnswerErrorCategory.NO_ERROR -> null
}
}
currentErrorCategory = if (pendingAnswerError == null) {
AnswerErrorCategory.NO_ERROR
} else category
errorMessage.set(pendingAnswerError)
return pendingAnswerError
}
Expand Down Expand Up @@ -98,6 +103,7 @@ class NumericInputViewModel private constructor(
if (answerText.isNotEmpty()) {
textualAnswer = answerText.toString()
}
lastErrorCategory = currentErrorCategory
}.build()

/** Implementation of [StateItemViewModel.InteractionItemFactory] for this view model. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import android.text.TextWatcher
import androidx.databinding.Observable
import androidx.databinding.ObservableField
import org.oppia.android.R
import org.oppia.android.app.model.AnswerErrorCategory
import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.InteractionObject
import org.oppia.android.app.model.RawUserAnswer
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.parser.StringToRatioParser
import org.oppia.android.app.player.state.answerhandling.AnswerErrorCategory
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerErrorOrAvailabilityCheckReceiver
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandler
import org.oppia.android.app.player.state.answerhandling.InteractionAnswerReceiver
Expand All @@ -33,9 +33,10 @@ class RatioExpressionInputInteractionViewModel private constructor(
private val translationController: TranslationController
) : StateItemViewModel(ViewType.RATIO_EXPRESSION_INPUT_INTERACTION), InteractionAnswerHandler {
private var pendingAnswerError: String? = null
var answerText: CharSequence = rawUserAnswer.textualAnswer ?: ""
var answerText: CharSequence = rawUserAnswer.textualAnswer
var isAnswerAvailable = ObservableField<Boolean>(false)
var errorMessage = ObservableField<String>("")
private var currentErrorCategory = AnswerErrorCategory.NO_ERROR

val hintText: CharSequence = deriveHintText(interaction)
private val stringToRatioParser: StringToRatioParser = StringToRatioParser()
Expand All @@ -55,7 +56,6 @@ class RatioExpressionInputInteractionViewModel private constructor(

errorMessage.addOnPropertyChangedCallback(callback)
isAnswerAvailable.addOnPropertyChangedCallback(callback)
checkPendingAnswerError(AnswerErrorCategory.REAL_TIME)
}

override fun getPendingAnswer(): UserAnswer = UserAnswer.newBuilder().apply {
Expand All @@ -75,23 +75,29 @@ class RatioExpressionInputInteractionViewModel private constructor(
if (answerText.isNotEmpty()) {
textualAnswer = answerText.toString()
}
lastErrorCategory = currentErrorCategory
}.build()

/** It checks the pending error for the current ratio input, and correspondingly updates the error string based on the specified error category. */
override fun checkPendingAnswerError(category: AnswerErrorCategory): String? {
if (answerText.isNotEmpty()) {
when (category) {
AnswerErrorCategory.REAL_TIME ->
pendingAnswerError =
stringToRatioParser.getRealTimeAnswerError(answerText.toString())
.getErrorMessageFromStringRes(resourceHandler)
AnswerErrorCategory.SUBMIT_TIME ->
pendingAnswerError =
stringToRatioParser.getSubmitTimeError(
answerText.toString(),
numberOfTerms = numberOfTerms
).getErrorMessageFromStringRes(resourceHandler)
pendingAnswerError = when (category) {
AnswerErrorCategory.REAL_TIME -> {
stringToRatioParser.getRealTimeAnswerError(answerText.toString())
.getErrorMessageFromStringRes(resourceHandler)
}
AnswerErrorCategory.SUBMIT_TIME -> {
stringToRatioParser.getSubmitTimeError(
answerText.toString(),
numberOfTerms = numberOfTerms
).getErrorMessageFromStringRes(resourceHandler)
}
AnswerErrorCategory.ANSWER_ERROR_CATEGORY_UNSPECIFIED, AnswerErrorCategory.UNRECOGNIZED,
AnswerErrorCategory.NO_ERROR -> null
}
currentErrorCategory = if (pendingAnswerError == null) {
AnswerErrorCategory.NO_ERROR
} else category
errorMessage.set(pendingAnswerError)
}
return pendingAnswerError
Expand Down
Loading