Skip to content

Commit

Permalink
Fix part of #4044: Prepare for supporting math expressions (math util…
Browse files Browse the repository at this point in the history
…ity refactor) (#4046)

## Explanation
Fix part of #4044
Originally copied from #2173 when it was in proof-of-concept form

This PR introduces a new math.proto and math utility package for upcoming protos & utilities coming in later PRs as part of the broader math expressions project. This includes moving some pure math protos & utilities to the new package, but not quite everything can be moved yet due to resource dependencies for RatioExtensions and the ratio/fraction parsers. These could be split up and partly moved to the new package in a future PR, but it's not a prerequisite for any of the math expressions work.

This PR also moves most of fraction parsing to utility and extracts just the UI-specific portion (i.e. accessing app strings). Consequently, there are now two test suites to verify the fraction parsing functionality. StringExtensions needed to be moved out of app due to it being needed in FractionParser. This move is needed for a downstream PR that will utilize the fraction parser in tests.

The new test file exemption changes are just updates since there are moved files.

## Essential Checklist
- [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
- [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation.
- [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
N/A refactor with no side effects to affect user flows.

Commit history:

* Copy proto-based changes from #2173.

* Introduce math.proto & refactor math extensions.

Much of this is copied from #2173.

* Migrate tests & remove unneeded prefix.

* Add needed newline.

* Some needed Fraction changes.

* Lint fix.

* Fix broken test post-refactor.

* Post-merge fix.

* Add regex check, docs, and resolve TODOs.

This also changes regex handling in the check to be more generic for
better flexibility when matching files.

* Lint fix.

* Fix failing static checks.

* Move StringExtensions & fraction parsing.

This splits fraction parsing between UI & utility components.

* Add missing KDocs.

* Fix broken build.

* Fix broken build post-merge.

* Post-merge fix.

* More post-merge fixes.
  • Loading branch information
BenHenning authored Mar 26, 2022
1 parent edcfe94 commit 240bdb6
Show file tree
Hide file tree
Showing 44 changed files with 824 additions and 613 deletions.
3 changes: 2 additions & 1 deletion app/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ VIEW_MODELS_WITH_RESOURCE_IMPORTS = [
"src/main/java/org/oppia/android/app/onboarding/OnboardingViewModel.kt",
"src/main/java/org/oppia/android/app/ongoingtopiclist/OngoingTopicItemViewModel.kt",
"src/main/java/org/oppia/android/app/options/TextSizeItemViewModel.kt",
"src/main/java/org/oppia/android/app/parser/StringToFractionParser.kt",
"src/main/java/org/oppia/android/app/parser/FractionParsingUiError.kt",
"src/main/java/org/oppia/android/app/parser/StringToNumberParser.kt",
"src/main/java/org/oppia/android/app/parser/StringToRatioParser.kt",
"src/main/java/org/oppia/android/app/player/state/itemviewmodel/DragDropInteractionContentViewModel.kt",
Expand Down Expand Up @@ -658,6 +658,7 @@ kt_android_library(
"//utility/src/main/java/org/oppia/android/util/extensions:context_extensions",
"//utility/src/main/java/org/oppia/android/util/logging/firebase:debug_event_logger",
"//utility/src/main/java/org/oppia/android/util/logging/firebase:debug_module",
"//utility/src/main/java/org/oppia/android/util/math:fraction_parser",
# TODO(#59): Remove 'debug_util_module' once we completely migrate to Bazel from Gradle as
# we can then directly exclude debug files from the build and thus won't be requiring this module.
"//utility/src/main/java/org/oppia/android/util/networking:debug_util_module",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import org.oppia.android.app.utility.KeyboardHelper.Companion.showSoftKeyboard
// background="@drawable/edit_text_background"
// maxLength="200".

// TODO(#4135): Add a dedicated test suite for this class.

/** The custom EditText class for fraction input interaction view. */
class FractionInputInteractionView @JvmOverloads constructor(
context: Context,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package org.oppia.android.app.parser

import androidx.annotation.StringRes
import org.oppia.android.R
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.util.math.FractionParser.FractionParsingError

/** Enum to store the errors of [FractionInputInteractionView]. */
enum class FractionParsingUiError(@StringRes private var error: Int?) {
/** Corresponds to [FractionParsingError.VALID]. */
VALID(error = null),

/** Corresponds to [FractionParsingError.INVALID_CHARS]. */
INVALID_CHARS(error = R.string.fraction_error_invalid_chars),

/** Corresponds to [FractionParsingError.INVALID_FORMAT]. */
INVALID_FORMAT(error = R.string.fraction_error_invalid_format),

/** Corresponds to [FractionParsingError.DIVISION_BY_ZERO]. */
DIVISION_BY_ZERO(error = R.string.fraction_error_divide_by_zero),

/** Corresponds to [FractionParsingError.NUMBER_TOO_LONG]. */
NUMBER_TOO_LONG(error = R.string.fraction_error_larger_than_seven_digits);

/**
* Returns the string corresponding to this error's string resources, or null if there is none.
*/
fun getErrorMessageFromStringRes(resourceHandler: AppLanguageResourceHandler): String? =
error?.let(resourceHandler::getStringInLocale)

companion object {
/**
* Returns the [FractionParsingUiError] corresponding to the specified [FractionParsingError].
*/
fun createFromParsingError(parsingError: FractionParsingError): FractionParsingUiError {
return when (parsingError) {
FractionParsingError.VALID -> VALID
FractionParsingError.INVALID_CHARS -> INVALID_CHARS
FractionParsingError.INVALID_FORMAT -> INVALID_FORMAT
FractionParsingError.DIVISION_BY_ZERO -> DIVISION_BY_ZERO
FractionParsingError.NUMBER_TOO_LONG -> NUMBER_TOO_LONG
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package org.oppia.android.app.parser
import androidx.annotation.StringRes
import org.oppia.android.R
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.domain.util.normalizeWhitespace
import org.oppia.android.util.extensions.normalizeWhitespace

/**
* This class contains methods that help to parse string to number, check realtime and submit time
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import androidx.annotation.StringRes
import org.oppia.android.R
import org.oppia.android.app.model.RatioExpression
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.domain.util.normalizeWhitespace
import org.oppia.android.domain.util.removeWhitespace
import org.oppia.android.util.extensions.normalizeWhitespace
import org.oppia.android.util.extensions.removeWhitespace

/**
* Utility for parsing [RatioExpression]s from strings and validating strings can be parsed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ import org.oppia.android.app.model.Interaction
import org.oppia.android.app.model.InteractionObject
import org.oppia.android.app.model.UserAnswer
import org.oppia.android.app.model.WrittenTranslationContext
import org.oppia.android.app.parser.StringToFractionParser
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.translation.AppLanguageResourceHandler
import org.oppia.android.domain.translation.TranslationController
import org.oppia.android.util.math.FractionParser

/** [StateItemViewModel] for the fraction input interaction. */
class FractionInteractionViewModel(
Expand All @@ -32,7 +33,7 @@ class FractionInteractionViewModel(
var errorMessage = ObservableField<String>("")

val hintText: CharSequence = deriveHintText(interaction)
private val stringToFractionParser: StringToFractionParser = StringToFractionParser()
private val fractionParser = FractionParser()

init {
val callback: Observable.OnPropertyChangedCallback =
Expand All @@ -52,7 +53,7 @@ class FractionInteractionViewModel(
if (answerText.isNotEmpty()) {
val answerTextString = answerText.toString()
answer = InteractionObject.newBuilder().apply {
fraction = stringToFractionParser.parseFractionFromString(answerTextString)
fraction = fractionParser.parseFractionFromString(answerTextString)
}.build()
plainAnswer = answerTextString
this.writtenTranslationContext = this@FractionInteractionViewModel.writtenTranslationContext
Expand All @@ -63,14 +64,18 @@ class FractionInteractionViewModel(
override fun checkPendingAnswerError(category: AnswerErrorCategory): String? {
if (answerText.isNotEmpty()) {
when (category) {
AnswerErrorCategory.REAL_TIME ->
AnswerErrorCategory.REAL_TIME -> {
pendingAnswerError =
stringToFractionParser.getRealTimeAnswerError(answerText.toString())
.getErrorMessageFromStringRes(resourceHandler)
AnswerErrorCategory.SUBMIT_TIME ->
FractionParsingUiError.createFromParsingError(
fractionParser.getRealTimeAnswerError(answerText.toString())
).getErrorMessageFromStringRes(resourceHandler)
}
AnswerErrorCategory.SUBMIT_TIME -> {
pendingAnswerError =
stringToFractionParser.getSubmitTimeError(answerText.toString())
.getErrorMessageFromStringRes(resourceHandler)
FractionParsingUiError.createFromParsingError(
fractionParser.getSubmitTimeError(answerText.toString())
).getErrorMessageFromStringRes(resourceHandler)
}
}
errorMessage.set(pendingAnswerError)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import org.oppia.android.app.player.state.answerhandling.InteractionAnswerHandle
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.app.utility.toAccessibleAnswerString
import org.oppia.android.domain.translation.TranslationController
import org.oppia.android.domain.util.toAnswerString
import org.oppia.android.util.math.toAnswerString

/** [StateItemViewModel] for the ratio expression input interaction. */
class RatioExpressionInputInteractionViewModel(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ class InputInteractionViewTestActivityTest {
ApplicationProvider.getApplicationContext<TestApplication>().inject(this)
}

// TODO(#4135): Move fraction input tests to a dedicated test suite.

@Test
fun testFractionInput_withNoInput_hasCorrectPendingAnswerType() {
val activityScenario = ActivityScenario.launch(
Expand Down
Loading

0 comments on commit 240bdb6

Please sign in to comment.