Merge branch 'develop' into fix-todo-checks

oppia · Nov 19, 2024 · 0b8ef62 · 0b8ef62
2 parents e8e2212 + 7955f7f
commit 0b8ef62
Show file tree

Hide file tree

Showing 18 changed files with 190 additions and 81 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -163,6 +163,7 @@
 # Utilities that are primarily used for frontend/UI purposes.
 /utility/src/*/java/org/oppia/android/util/accessibility/ @oppia/android-frontend-reviewers
 /utility/src/*/java/org/oppia/android/util/statusbar/ @oppia/android-frontend-reviewers
+/utility/src/main/java/org/oppia/android/util/enumfilter/ @oppia/android-frontend-reviewers
 /utility/src/*/java/org/oppia/android/util/extensions/ @oppia/android-frontend-reviewers
 /utility/src/*/java/org/oppia/android/util/parser/html @oppia/android-frontend-reviewers
 /utility/src/*/res/**/*.xml @oppia/android-frontend-reviewers

diff --git a/.github/workflows/comment_coverage_report.yml b/.github/workflows/comment_coverage_report.yml
@@ -17,6 +17,8 @@ jobs:
   check_code_coverage_completed:
     name: Check code coverage completed
     runs-on: ubuntu-latest
+    outputs:
+      conclusion: ${{ steps.wait-for-coverage.outputs.run-conclusion }}
     steps:
       - name: Wait for code coverage to complete
         id: wait-for-coverage
@@ -27,6 +29,13 @@ jobs:
           allowed-conclusions: |
             success
             failure
+            action_required
+
+      - name: Conclusion Analysis
+        if: steps.wait-for-coverage.outputs.run-conclusion == 'action_required'
+        run: |
+          echo "::error::First-time contributor workflows require manual approval. After approval, please re-run the comment coverage workflows to post the coverage report."
+          exit 1
 
   comment_coverage_report:
     name: Comment Code Coverage Report
@@ -60,7 +69,7 @@ jobs:
 
             const run = runs[0];
             if(!run) {
-              core.setFailed('Could not find a succesful workflow run for the PR');
+              core.setFailed('Could not find a successful workflow run for the PR');
               return;
             }
 

diff --git a/app/src/main/java/org/oppia/android/app/drawer/ExitProfileDialogFragment.kt b/app/src/main/java/org/oppia/android/app/drawer/ExitProfileDialogFragment.kt
@@ -70,7 +70,6 @@ class ExitProfileDialogFragment : InjectableDialogFragment() {
         dialog.dismiss()
       }
       .setPositiveButton(R.string.home_activity_back_dialog_exit) { _, _ ->
-        // TODO(#3641): Investigate on using finish instead of intent.
         val intent = ProfileChooserActivity.createProfileChooserActivity(activity!!)
         if (!restoreLastCheckedItem) {
           intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)

diff --git a/app/src/main/java/org/oppia/android/app/profile/AddProfileActivity.kt b/app/src/main/java/org/oppia/android/app/profile/AddProfileActivity.kt
@@ -44,10 +44,7 @@ class AddProfileActivity : InjectableAutoLocalizedAppCompatActivity() {
   }
 
   override fun onSupportNavigateUp(): Boolean {
-    // TODO(#3641): Investigate on using finish instead of intent.
-    val intent = Intent(this, ProfileChooserActivity::class.java)
-    intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP)
-    startActivity(intent)
+    finish()
     return false
   }
 

diff --git a/...src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/MarketFitItemsViewModel.kt b/...src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/MarketFitItemsViewModel.kt
@@ -12,6 +12,7 @@ import org.oppia.android.app.survey.PreviousAnswerHandler
 import org.oppia.android.app.survey.SelectedAnswerAvailabilityReceiver
 import org.oppia.android.app.survey.SelectedAnswerHandler
 import org.oppia.android.app.translation.AppLanguageResourceHandler
+import org.oppia.android.util.enumfilter.filterByEnumCondition
 import javax.inject.Inject
 
 /** [SurveyAnswerItemViewModel] for the market fit question options. */
@@ -98,8 +99,12 @@ class MarketFitItemsViewModel @Inject constructor(
   private fun getMarketFitOptions(): ObservableList<MultipleChoiceOptionContentViewModel> {
     val appName = resourceHandler.getStringInLocale(R.string.app_name)
     val observableList = ObservableArrayList<MultipleChoiceOptionContentViewModel>()
-    observableList += MarketFitAnswer.values()
-      .filter { it.isValid() }
+    val filteredmarketFitAnswer = filterByEnumCondition(
+      MarketFitAnswer.values().toList(),
+      { marketFitAnswer -> marketFitAnswer },
+      { marketFitAnswer -> marketFitAnswer.isValid() }
+    )
+    observableList += filteredmarketFitAnswer
       .mapIndexed { index, marketFitAnswer ->
         when (marketFitAnswer) {
           MarketFitAnswer.VERY_DISAPPOINTED -> MultipleChoiceOptionContentViewModel(

diff --git a/app/src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/UserTypeItemsViewModel.kt b/app/src/main/java/org/oppia/android/app/survey/surveyitemviewmodel/UserTypeItemsViewModel.kt
@@ -12,6 +12,7 @@ import org.oppia.android.app.survey.SelectedAnswerAvailabilityReceiver
 import org.oppia.android.app.survey.SelectedAnswerHandler
 import org.oppia.android.app.translation.AppLanguageResourceHandler
 import org.oppia.android.app.viewmodel.ObservableArrayList
+import org.oppia.android.util.enumfilter.filterByEnumCondition
 import javax.inject.Inject
 
 /** [SurveyAnswerItemViewModel] for providing the type of user question options. */
@@ -97,46 +98,36 @@ class UserTypeItemsViewModel @Inject constructor(
 
   private fun getUserTypeOptions(): ObservableArrayList<MultipleChoiceOptionContentViewModel> {
     val observableList = ObservableArrayList<MultipleChoiceOptionContentViewModel>()
-    observableList += UserTypeAnswer.values()
-      .filter { it.isValid() }
-      .mapIndexed { index, userTypeOption ->
-        when (userTypeOption) {
-          UserTypeAnswer.LEARNER ->
-            MultipleChoiceOptionContentViewModel(
-              resourceHandler.getStringInLocale(
-                R.string.user_type_answer_learner
-              ),
-              index,
-              this
-            )
-          UserTypeAnswer.TEACHER -> MultipleChoiceOptionContentViewModel(
-            resourceHandler.getStringInLocale(
-              R.string.user_type_answer_teacher
-            ),
-            index,
-            this
-          )
-
-          UserTypeAnswer.PARENT ->
-            MultipleChoiceOptionContentViewModel(
-              resourceHandler.getStringInLocale(
-                R.string.user_type_answer_parent
-              ),
-              index,
-              this
-            )
-
-          UserTypeAnswer.OTHER ->
-            MultipleChoiceOptionContentViewModel(
-              resourceHandler.getStringInLocale(
-                R.string.user_type_answer_other
-              ),
-              index,
-              this
-            )
-          else -> throw IllegalStateException("Invalid UserTypeAnswer")
-        }
+    val filteredUserTypes = filterByEnumCondition(
+      UserTypeAnswer.values().toList(),
+      { userTypeAnswer -> userTypeAnswer },
+      { userTypeAnswer -> userTypeAnswer.isValid() }
+    )
+    observableList += filteredUserTypes.mapIndexed { index, userTypeOption ->
+      when (userTypeOption) {
+        UserTypeAnswer.LEARNER -> MultipleChoiceOptionContentViewModel(
+          resourceHandler.getStringInLocale(R.string.user_type_answer_learner),
+          index,
+          this
+        )
+        UserTypeAnswer.TEACHER -> MultipleChoiceOptionContentViewModel(
+          resourceHandler.getStringInLocale(R.string.user_type_answer_teacher),
+          index,
+          this
+        )
+        UserTypeAnswer.PARENT -> MultipleChoiceOptionContentViewModel(
+          resourceHandler.getStringInLocale(R.string.user_type_answer_parent),
+          index,
+          this
+        )
+        UserTypeAnswer.OTHER -> MultipleChoiceOptionContentViewModel(
+          resourceHandler.getStringInLocale(R.string.user_type_answer_other),
+          index,
+          this
+        )
+        else -> throw IllegalStateException("Invalid UserTypeAnswer")
       }
+    }
     return observableList
   }
 

diff --git a/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt
@@ -30,6 +30,7 @@ import org.oppia.android.domain.oppialogger.OppiaLogger
 import org.oppia.android.util.accessibility.AccessibilityService
 import org.oppia.android.util.data.AsyncResult
 import org.oppia.android.util.data.DataProviders.Companion.toLiveData
+import org.oppia.android.util.enumfilter.filterByEnumCondition
 import javax.inject.Inject
 
 /** The presenter for [TopicLessonsFragment]. */
@@ -161,18 +162,18 @@ class TopicLessonsFragmentPresenter @Inject constructor(
 
     val chapterSummaries = storySummaryViewModel
       .storySummary.chapterList
-    val completedChapterCount =
-      chapterSummaries.map(ChapterSummary::getChapterPlayState)
-        .filter {
-          it == ChapterPlayState.COMPLETED
-        }
-        .size
+    val completedChapterCount = filterByEnumCondition(
+      chapterSummaries.map(ChapterSummary::getChapterPlayState),
+      { it },
+      { it == ChapterPlayState.COMPLETED }
+    ).size
+
     val inProgressChapterCount =
-      chapterSummaries.map(ChapterSummary::getChapterPlayState)
-        .filter {
-          it == ChapterPlayState.IN_PROGRESS_SAVED
-        }
-        .size
+      filterByEnumCondition(
+        chapterSummaries.map(ChapterSummary::getChapterPlayState),
+        { it },
+        { it == ChapterPlayState.IN_PROGRESS_SAVED }
+      ).size
 
     val storyPercentage: Int =
       (completedChapterCount * 100) / storySummaryViewModel.storySummary.chapterCount

diff --git a/app/src/sharedTest/java/org/oppia/android/app/profile/AddProfileActivityTest.kt b/app/src/sharedTest/java/org/oppia/android/app/profile/AddProfileActivityTest.kt
@@ -13,6 +13,7 @@ import androidx.appcompat.app.AppCompatActivity
 import androidx.test.core.app.ActivityScenario.launch
 import androidx.test.core.app.ApplicationProvider
 import androidx.test.espresso.Espresso.onView
+import androidx.test.espresso.action.ViewActions
 import androidx.test.espresso.action.ViewActions.click
 import androidx.test.espresso.action.ViewActions.closeSoftKeyboard
 import androidx.test.espresso.action.ViewActions.pressImeActionButton
@@ -1748,6 +1749,15 @@ class AddProfileActivityTest {
     assertThat(currentScreenName).isEqualTo(ScreenName.ADD_PROFILE_ACTIVITY)
   }
 
+  @Test
+  fun testAddProfileActivity_onBackPressed_finishActivity() {
+    val scenario = launch(AddProfileActivity::class.java)
+    onView(isRoot()).perform(ViewActions.pressBack())
+    testCoroutineDispatchers.runCurrent()
+    scenario.onActivity { activity ->
+      assertThat(activity.isFinishing).isTrue()
+    }
+  }
   private fun createAddProfileActivityIntent(): Intent {
     return AddProfileActivity.createAddProfileActivityIntent(
       ApplicationProvider.getApplicationContext(),

diff --git a/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel b/domain/src/main/java/org/oppia/android/domain/oppialogger/analytics/BUILD.bazel
@@ -49,6 +49,7 @@ kt_android_library(
         "//domain/src/main/java/org/oppia/android/domain/oppialogger:prod_module",
         "//model/src/main/proto:performance_metrics_event_logger_java_proto_lite",
         "//utility/src/main/java/org/oppia/android/util/data:data_provider",
+        "//utility/src/main/java/org/oppia/android/util/enumfilter:enum_filter_util",
         "//utility/src/main/java/org/oppia/android/util/logging:console_logger",
         "//utility/src/main/java/org/oppia/android/util/logging:exception_logger",
         "//utility/src/main/java/org/oppia/android/util/logging/performancemetrics:performance_metrics_assessor",

diff --git a/.../main/java/org/oppia/android/domain/oppialogger/analytics/PerformanceMetricsController.kt b/.../main/java/org/oppia/android/domain/oppialogger/analytics/PerformanceMetricsController.kt
@@ -7,6 +7,7 @@ import org.oppia.android.app.model.ScreenName
 import org.oppia.android.data.persistence.PersistentCacheStore
 import org.oppia.android.domain.oppialogger.PerformanceMetricsLogStorageCacheSize
 import org.oppia.android.util.data.DataProvider
+import org.oppia.android.util.enumfilter.filterByEnumCondition
 import org.oppia.android.util.logging.ConsoleLogger
 import org.oppia.android.util.logging.ExceptionLogger
 import org.oppia.android.util.logging.performancemetrics.PerformanceMetricsAssessor
@@ -128,9 +129,11 @@ class PerformanceMetricsController @Inject constructor(
    * priority is returned.
    */
   private fun getLeastRecentMetricLogIndex(oppiaMetricLogs: OppiaMetricLogs): Int? =
-    oppiaMetricLogs.oppiaMetricLogList.withIndex()
-      .filter { it.value.priority == Priority.LOW_PRIORITY }
-      .minByOrNull { it.value.timestampMillis }?.index
+    filterByEnumCondition(
+      oppiaMetricLogs.oppiaMetricLogList.withIndex().toList(),
+      { it.value.priority },
+      { it == Priority.LOW_PRIORITY }
+    ).minByOrNull { it.value.timestampMillis }?.index
       ?: getLeastRecentMediumPriorityEventIndex(oppiaMetricLogs)
 
   /**
@@ -142,9 +145,11 @@ class PerformanceMetricsController @Inject constructor(
    * priority is returned.
    */
   private fun getLeastRecentMediumPriorityEventIndex(oppiaMetricLogs: OppiaMetricLogs): Int? =
-    oppiaMetricLogs.oppiaMetricLogList.withIndex()
-      .filter { it.value.priority == Priority.MEDIUM_PRIORITY }
-      .minByOrNull { it.value.timestampMillis }?.index
+    filterByEnumCondition(
+      oppiaMetricLogs.oppiaMetricLogList.withIndex().toList(),
+      { it.value.priority },
+      { it == Priority.MEDIUM_PRIORITY }
+    ).minByOrNull { it.value.timestampMillis }?.index
       ?: getLeastRecentGeneralEventIndex(oppiaMetricLogs)
 
   /** Returns the index of the least recent event regardless of their priority. */

diff --git a/scripts/assets/test_file_exemptions.textproto b/scripts/assets/test_file_exemptions.textproto
@@ -4310,6 +4310,10 @@ test_file_exemption {
   exempted_file_path: "utility/src/main/java/org/oppia/android/util/extensions/ContextExtensions.kt"
   test_file_not_required: true
 }
+test_file_exemption {
+  exempted_file_path: "utility/src/main/java/org/oppia/android/util/enumfilter/EnumFilterUtil.kt"
+  test_file_not_required: true
+}
 test_file_exemption {
   exempted_file_path: "utility/src/main/java/org/oppia/android/util/extensions/StringExtensions.kt"
   source_file_is_incompatible_with_code_coverage: true

diff --git a/utility/src/main/java/org/oppia/android/util/enumfilter/BUILD.bazel b/utility/src/main/java/org/oppia/android/util/enumfilter/BUILD.bazel
@@ -0,0 +1,13 @@
+"""
+General purpose utility for filtering enums.
+"""
+
+load("@io_bazel_rules_kotlin//kotlin:android.bzl", "kt_android_library")
+
+kt_android_library(
+    name = "enum_filter_util",
+    srcs = [
+        "EnumFilterUtil.kt",
+    ],
+    visibility = ["//:oppia_api_visibility"],
+)
diff --git a/utility/src/main/java/org/oppia/android/util/enumfilter/EnumFilterUtil.kt b/utility/src/main/java/org/oppia/android/util/enumfilter/EnumFilterUtil.kt
@@ -0,0 +1,20 @@
+package org.oppia.android.util.enumfilter
+
+/**
+ * Filters a collection based on a condition applied to an enum property of each element.
+ *
+ * @param E the type of enum values.
+ * @param T the type of elements in the collection.
+ * @param collection the collection of elements to filter.
+ * @param enumExtractor a function that extracts the enum value from each element.
+ * @param condition a predicate function that determines if an enum value should be included in the result.
+ * @return a list of elements from the collection that satisfy the condition when their enum property is evaluated.
+ */
+
+inline fun <E : Enum<E>, T> filterByEnumCondition(
+  collection: Collection<T>,
+  enumExtractor: (T) -> E,
+  condition: (E) -> Boolean
+): List<T> {
+  return collection.filter { condition(enumExtractor(it)) }
+}
diff --git a/utility/src/main/java/org/oppia/android/util/logging/ConsoleLogger.kt b/utility/src/main/java/org/oppia/android/util/logging/ConsoleLogger.kt
@@ -11,6 +11,8 @@ import org.oppia.android.app.model.EventLog.ConsoleLoggerContext
 import org.oppia.android.util.locale.OppiaLocale
 import org.oppia.android.util.threading.BlockingDispatcher
 import java.io.File
+import java.io.FileWriter
+import java.io.PrintWriter
 import javax.inject.Inject
 import javax.inject.Singleton
 
@@ -33,6 +35,8 @@ class ConsoleLogger @Inject constructor(
    */
   val logErrorMessagesFlow: SharedFlow<ConsoleLoggerContext> = _logErrorMessagesFlow
 
+  private var printWriter: PrintWriter? = null
+
   /** Logs a verbose message with the specified tag. */
   fun v(tag: String, msg: String) {
     writeLog(LogLevel.VERBOSE, tag, msg)
@@ -73,12 +77,12 @@ class ConsoleLogger @Inject constructor(
     writeError(LogLevel.WARNING, tag, msg, tr)
   }
 
-  /** Logs a error message with the specified tag. */
+  /** Logs an error message with the specified tag. */
   fun e(tag: String, msg: String) {
     writeLog(LogLevel.ERROR, tag, msg)
   }
 
-  /** Logs a error message with the specified tag, message and exception. */
+  /** Logs an error message with the specified tag, message and exception. */
   fun e(tag: String, msg: String, tr: Throwable?) {
     writeError(LogLevel.ERROR, tag, msg, tr)
   }
@@ -109,7 +113,7 @@ class ConsoleLogger @Inject constructor(
     }
 
     // Add the log to the error message flow so it can be logged to firebase.
-    CoroutineScope(blockingDispatcher).launch {
+    blockingScope.launch {
       // Only log error messages to firebase.
       if (logLevel == LogLevel.ERROR) {
         _logErrorMessagesFlow.emit(
@@ -124,10 +128,17 @@ class ConsoleLogger @Inject constructor(
   }
 
   /**
-   * Writes the specified text line to file in a background thread to ensure that saving messages don't block the main
-   * thread. A blocking dispatcher is used to ensure messages are written in order.
+   * Writes the specified text line to file in a background thread to ensure that saving messages
+   * doesn't block the main thread. A blocking dispatcher is used to ensure messages are written
+   * in order.
    */
   private fun logToFileInBackground(text: String) {
-    blockingScope.launch { logDirectory.printWriter().use { out -> out.println(text) } }
+    blockingScope.launch {
+      if (printWriter == null) {
+        printWriter = PrintWriter(FileWriter(logDirectory, true)) // Open in append mode.
+      }
+      printWriter?.println(text)
+      printWriter?.flush()
+    }
   }
 }