Fix #4343: Fixes Explorations hints being numbered randomly (#4630)

* fix: explorations hints numbered randomly * remove the nullable property part from OppiaLocale on the toHumanReadable mehtod * change hint name to hint_list_item_number * fix: name hint strng, remove nullanle factor from the toHumanReadableString including removing the .toString() method, compute index property inline, add three tests * fix: revert formating changes, test wit non english number localization, test for values larger than 10,000 * fix: optimise imports * fix: change test to hebrew locale * fix: test toHumanReadableString to take hebrew number locale * fix: test toHumanReadableString to take EgyptArabic number locale * fix: test toHumanReadableString to take EgyptArabic number locale in displayLoacleImpl * fix: remove unused code and verify test success with suugested fix * Fix: revert back to hint_1, to test the breaking tests * Fix: revert back to hint_1, in test_exp_id_2.json file (gradle) * Fix: revert back to hint_6 in textproto * Fix: add hint_6 to translation mappings Co-authored-by: MAZAKPE <[email protected]>
oppia · Oct 10, 2022 · 18814f5 · 18814f5
1 parent d192258
commit 18814f5
Show file tree

Hide file tree

Showing 12 changed files with 86 additions and 12 deletions.
diff --git a/...in/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt b/...in/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt
@@ -206,8 +206,6 @@ class HintsAndSolutionDialogFragmentPresenter @Inject constructor(
       }
     }
 
-    binding.hintTitle.text =
-      resourceHandler.capitalizeForHumans(hintsViewModel.title.get()!!.replace("_", " "))
     binding.hintsAndSolutionSummary.text =
       htmlParserFactory.create(
         resourceBucketName,

diff --git a/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsViewModel.kt b/app/src/main/java/org/oppia/android/app/hintsandsolution/HintsViewModel.kt
@@ -88,7 +88,12 @@ class HintsViewModel @Inject constructor(
 
   private fun addHintToList(hintIndex: Int, hint: Hint) {
     val hintsViewModel = HintsViewModel(resourceHandler, translationController)
-    hintsViewModel.title.set(hint.hintContent.contentId)
+    hintsViewModel.title.set(
+      resourceHandler.getStringInLocaleWithWrapping(
+        R.string.hint_list_item_number,
+        resourceHandler.toHumanReadableString(hintIndex + 1)
+      )
+    )
     val hintContentHtml =
       translationController.extractString(hint.hintContent, writtenTranslationContext)
     hintsViewModel.hintsAndSolutionSummary.set(hintContentHtml)

diff --git a/app/src/main/java/org/oppia/android/app/translation/AppLanguageResourceHandler.kt b/app/src/main/java/org/oppia/android/app/translation/AppLanguageResourceHandler.kt
@@ -123,6 +123,10 @@ class AppLanguageResourceHandler @Inject constructor(
   /** See [OppiaLocale.DisplayLocale.formatDouble] for specific behavior. */
   fun formatDouble(value: Double): String = getDisplayLocale().formatDouble(value)
 
+  /** See [OppiaLocale.DisplayLocale.toHumanReadableString] for specific behavior. */
+  fun toHumanReadableString(number: Int): String =
+    getDisplayLocale().toHumanReadableString(number)
+
   /** See [OppiaLocale.DisplayLocale.computeDateString]. */
   fun computeDateString(timestampMillis: Long): String =
     getDisplayLocale().computeDateString(timestampMillis)

diff --git a/app/src/main/res/layout/hints_summary.xml b/app/src/main/res/layout/hints_summary.xml
@@ -56,6 +56,7 @@
             android:layout_marginTop="20dp"
             android:layout_marginBottom="16dp"
             android:fontFamily="sans-serif-medium"
+            android:text="@{viewModel.title}"
             android:textColor="@color/component_color_hints_and_solutions_fragment_text_color"
             android:textSize="20sp"
             app:layout_constraintBottom_toBottomOf="parent"

diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
@@ -437,6 +437,7 @@
   <string name="new_hint_available">New hint available</string>
   <string name="no_new_hint_available">No new hint available</string>
   <string name="show_hints_and_solution">Show hints and solution</string>
+  <string name="hint_list_item_number">Hint %s</string>
   <string name="hints_andSolution_close_icon_description">Navigate up</string>
   <string name="hints_toolbar_title">Hints</string>
   <string name="show_solution">Show</string>

diff --git a/app/src/test/java/org/oppia/android/app/player/state/StateFragmentLocalTest.kt b/app/src/test/java/org/oppia/android/app/player/state/StateFragmentLocalTest.kt
@@ -1278,6 +1278,32 @@ class StateFragmentLocalTest {
     }
   }
 
+  @Test
+  @DefineAppLanguageLocaleContext(
+    oppiaLanguageEnumId = ENGLISH_VALUE,
+    appStringIetfTag = "en",
+    appStringAndroidLanguageId = ""
+  )
+  fun testStateFragment_englishLocale_defaultContentLang_hint_titlesAreCorrectInEnglish() {
+    // Ensure the system locale matches the initial locale context.
+    forceDefaultLocale(Locale.ENGLISH)
+    launchForExploration(TEST_EXPLORATION_ID_2).use {
+      startPlayingExploration()
+      clickContinueButton()
+      // Submit two incorrect answers.
+      submitFractionAnswer(answerText = "1/3")
+      submitFractionAnswer(answerText = "1/4")
+
+      // Reveal the hint.
+      openHintsAndSolutionsDialog()
+      pressRevealHintButton(hintPosition = 0)
+
+      // The hint title text should be in English with the correct number
+      onView(withId(R.id.hint_title))
+        .check(matches(withText(containsString("Hint 1"))))
+    }
+  }
+
   @Test
   @DefineAppLanguageLocaleContext(
     oppiaLanguageEnumId = ENGLISH_VALUE,

diff --git a/app/src/test/java/org/oppia/android/app/translation/AppLanguageResourceHandlerTest.kt b/app/src/test/java/org/oppia/android/app/translation/AppLanguageResourceHandlerTest.kt
@@ -466,6 +466,16 @@ class AppLanguageResourceHandlerTest {
     assertThat(formattedString).containsMatch("[,.]")
   }
 
+  @Test
+  fun testToHumanReadableString_forInt_returnsStringWithExactNumberInEnglish() {
+    updateAppLanguageTo(OppiaLanguage.ENGLISH)
+    val handler = retrieveAppLanguageResourceHandler()
+
+    val formattedString = handler.toHumanReadableString(1)
+
+    assertThat(formattedString).contains("1")
+  }
+
   @Test
   fun testComputeDateString_forFixedTime_returnMonthDayYearParts() {
     updateAppLanguageTo(OppiaLanguage.ENGLISH)

diff --git a/domain/src/main/assets/test_exp_id_2.json b/domain/src/main/assets/test_exp_id_2.json
@@ -443,7 +443,7 @@
           },
           "hints": [{
             "hint_content": {
-              "content_id": "hint_1",
+              "content_id": "hint_6",
               "html": "<p>Remember that two halves, when added together, make one whole.</p>"
             }
           }],
@@ -465,7 +465,7 @@
         "recorded_voiceovers": {
           "voiceovers_mapping": {
             "feedback_1": {},
-            "hint_1": {},
+            "hint_6": {},
             "solution": {},
             "content": {},
             "default_outcome": {}
@@ -485,7 +485,7 @@
                 "needs_update": false
               }
             },
-            "hint_1": {
+            "hint_6": {
               "pt": {
                 "data_format": "html",
                 "translation": {"translation" : "<p> Lembre-se de que duas metades, quando adicionadas, faça um todo. </p>"},

diff --git a/domain/src/main/assets/test_exp_id_2.textproto b/domain/src/main/assets/test_exp_id_2.textproto
@@ -526,7 +526,7 @@ states {
       }
     }
     recorded_voiceovers {
-      key: "hint_1"
+      key: "hint_6"
       value {
       }
     }
@@ -567,7 +567,7 @@ states {
       }
     }
     written_translations {
-      key: "hint_1"
+      key: "hint_6"
       value {
         translation_mapping {
           key: "pt"
@@ -689,7 +689,7 @@ states {
       hint {
         hint_content {
           html: "<p>Remember that two halves, when added together, make one whole.</p>"
-          content_id: "hint_1"
+          content_id: "hint_6"
         }
       }
       default_outcome {

diff --git a/utility/src/main/java/org/oppia/android/util/locale/DisplayLocaleImpl.kt b/utility/src/main/java/org/oppia/android/util/locale/DisplayLocaleImpl.kt
@@ -50,7 +50,7 @@ class DisplayLocaleImpl(
 
   override fun formatDouble(value: Double): String = numberFormat.format(value)
 
-  override fun toHumanReadableString(number: Int): String? = numberFormat.format(number)
+  override fun toHumanReadableString(number: Int): String = numberFormat.format(number)
 
   override fun computeDateString(timestampMillis: Long): String =
     dateFormat.format(Date(timestampMillis))

diff --git a/utility/src/main/java/org/oppia/android/util/locale/OppiaLocale.kt b/utility/src/main/java/org/oppia/android/util/locale/OppiaLocale.kt
@@ -227,7 +227,7 @@ sealed class OppiaLocale {
     abstract fun formatDouble(value: Double): String
 
     /** Returns a human-readable representation of [number]. */
-    abstract fun toHumanReadableString(number: Int): String?
+    abstract fun toHumanReadableString(number: Int): String
 
     /**
      * Returns a locally formatted date string representing the specified Unix timestamp.

diff --git a/utility/src/test/java/org/oppia/android/util/locale/DisplayLocaleImplTest.kt b/utility/src/test/java/org/oppia/android/util/locale/DisplayLocaleImplTest.kt
@@ -48,7 +48,8 @@ class DisplayLocaleImplTest {
   @Inject
   lateinit var wrapperChecker: TestOppiaBidiFormatter.Checker
 
-  @Inject lateinit var context: Context
+  @Inject
+  lateinit var context: Context
 
   @Before
   fun setUp() {
@@ -159,6 +160,34 @@ class DisplayLocaleImplTest {
     assertThat(formattedString).containsMatch("[,.]")
   }
 
+  @Test
+  fun testToHumanReadableString_forInt_returnsStringWithExactNumberInEnglishLocale() {
+    val impl = createDisplayLocaleImpl(US_ENGLISH_CONTEXT)
+
+    val formattedString = impl.toHumanReadableString(1)
+
+    assertThat(formattedString).contains("1")
+  }
+
+  @Test
+  fun testToHumanReadableString_forInt_returnsStringWithExactNumberInEgyptArabicLocale() {
+    val impl = createDisplayLocaleImpl(EGYPT_ARABIC_CONTEXT)
+
+    val localizedNumber = impl.toHumanReadableString(1)
+
+    assertThat(localizedNumber).isEqualTo("١")
+  }
+
+  @Test
+  fun testToHumanReadableString_forLargeInt_returnsStringWithPeriodsOrCommas() {
+    val impl = createDisplayLocaleImpl(US_ENGLISH_CONTEXT)
+
+    val formattedString = impl.toHumanReadableString(10000)
+
+    // Depending on formatting, commas and/or periods are used for large doubles.
+    assertThat(formattedString).containsMatch("[,.]")
+  }
+
   @Test
   fun testComputeDateString_forFixedTime_returnMonthDayYearParts() {
     val impl = createDisplayLocaleImpl(US_ENGLISH_CONTEXT)