Merge branch 'develop' into #4419

.github/workflows/code_coverage.yml

@@ -258,10 +258,10 @@ jobs:
   evaluate_code_coverage_reports:
     name: Evaluate Code Coverage Reports
     runs-on: ubuntu-20.04
-    needs: code_coverage_run
+    needs: [ check_unit_tests_completed, code_coverage_run  ]
     # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
     # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
-    if: ${{ !cancelled() }}
+    if: ${{ !cancelled() && needs.check_unit_tests_completed.result == 'success'}}
     env:
       CACHE_DIRECTORY: ~/.bazel_cache
     steps:
@@ -311,12 +311,16 @@ jobs:
   # Reference: https://github.community/t/127354/7.
   check_coverage_results:
     name: Check Code Coverage Results
-    needs: [ compute_changed_files, code_coverage_run, evaluate_code_coverage_reports ]
+    needs: [ check_unit_tests_completed, compute_changed_files, code_coverage_run, evaluate_code_coverage_reports ]
     # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations,
     # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows.
     if: ${{ !cancelled() }}
     runs-on: ubuntu-20.04
     steps:
+      - name: Check unit tests passed
+        if: ${{ needs.check_unit_tests_completed.result != 'success' }}
+        run: exit 1
+
       - name: Check coverages passed
         if: ${{ needs.compute_changed_files.outputs.can_skip_files != 'true' && needs.code_coverage_run.result != 'success' }}
         run: exit 1

.github/workflows/comment_coverage_report.yml

@@ -3,11 +3,11 @@
 name: Comment Coverage Report
 
 # Controls when the action will run. Triggers the workflow  on pull request events
-# (assigned, opened, synchronize, reopened)
+# (opened, synchronize, reopened)
 
 on:
   pull_request_target:
-    types: [assigned, opened, synchronize, reopened]
+    types: [opened, synchronize, reopened]
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}

app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragment.kt

@@ -14,6 +14,7 @@ import org.oppia.android.app.model.HintsAndSolutionDialogFragmentStateBundle
 import org.oppia.android.app.model.ProfileId
 import org.oppia.android.app.model.State
 import org.oppia.android.app.model.WrittenTranslationContext
+import org.oppia.android.app.topic.conceptcard.ConceptCardFragment
 import org.oppia.android.util.extensions.getProto
 import org.oppia.android.util.extensions.putProto
 import javax.inject.Inject
@@ -192,4 +193,12 @@ class HintsAndSolutionDialogFragment :
       isSolutionRevealed
     )
   }
+
+  /**
+   * Delegates the removal of all [ConceptCardFragment] instances
+   * to the [hintsAndSolutionDialogFragmentPresenter].
+   */
+  fun dismissConceptCard() {
+    hintsAndSolutionDialogFragmentPresenter.dismissConceptCard()
+  }
 }

app/src/main/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt

@@ -331,4 +331,9 @@ class HintsAndSolutionDialogFragmentPresenter @Inject constructor(
   override fun onConceptCardLinkClicked(view: View, skillId: String) {
     ConceptCardFragment.bringToFrontOrCreateIfNew(skillId, profileId, fragment.childFragmentManager)
   }
+
+  /** Removes all [ConceptCardFragment] in the given FragmentManager. */
+  fun dismissConceptCard() {
+    ConceptCardFragment.dismissAll(fragment.childFragmentManager)
+  }
 }

app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt

@@ -190,7 +190,9 @@ class ExplorationActivity :
     this.writtenTranslationContext = writtenTranslationContext
   }
 
-  override fun dismissConceptCard() = explorationActivityPresenter.dismissConceptCard()
+  override fun dismissConceptCard() {
+    getHintsAndSolution()?.dismissConceptCard()
+  }
 
   override fun requestVoiceOverIconSpotlight(numberOfLogins: Int) {
     explorationActivityPresenter.requestVoiceOverIconSpotlight(numberOfLogins)

app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt

@@ -334,10 +334,6 @@ class ExplorationActivityPresenter @Inject constructor(
     showDialogFragmentBasedOnCurrentCheckpointState()
   }
 
-  fun dismissConceptCard() {
-    getExplorationFragment()?.dismissConceptCard()
-  }
-
   private fun updateToolbarTitle(explorationId: String) {
     subscribeToExploration(
       explorationDataController.getExplorationById(profileId, explorationId).toLiveData()

app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt

@@ -84,7 +84,5 @@ class ExplorationFragment : InjectableFragment() {
     explorationFragmentPresenter.viewSolution()
   }
 
-  fun dismissConceptCard() = explorationFragmentPresenter.dismissConceptCard()
-
   fun getExplorationCheckpointState() = explorationFragmentPresenter.getExplorationCheckpointState()
 }

app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt

@@ -141,8 +141,6 @@ class ExplorationFragmentPresenter @Inject constructor(
     getStateFragment()?.viewSolution()
   }
 
-  fun dismissConceptCard() = getStateFragment()?.dismissConceptCard()
-
   fun getExplorationCheckpointState() = getStateFragment()?.getExplorationCheckpointState()
 
   private fun getStateFragment(): StateFragment? {

app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt

@@ -162,8 +162,6 @@ class StateFragment :
     stateFragmentPresenter.viewSolution()
   }
 
-  fun dismissConceptCard() = stateFragmentPresenter.dismissConceptCard()
-
   fun getExplorationCheckpointState() = stateFragmentPresenter.getExplorationCheckpointState()
 
   override fun onSaveInstanceState(outState: Bundle) {

app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt

@@ -38,7 +38,6 @@ import org.oppia.android.app.player.state.listener.RouteToHintsAndSolutionListen
 import org.oppia.android.app.player.stopplaying.StopStatePlayingSessionWithSavedProgressListener
 import org.oppia.android.app.survey.SurveyWelcomeDialogFragment
 import org.oppia.android.app.survey.TAG_SURVEY_WELCOME_DIALOG
-import org.oppia.android.app.topic.conceptcard.ConceptCardFragment
 import org.oppia.android.app.translation.AppLanguageResourceHandler
 import org.oppia.android.app.utility.SplitScreenManager
 import org.oppia.android.app.utility.lifecycle.LifecycleSafeTimerFactory
@@ -428,10 +427,6 @@ class StateFragmentPresenter @Inject constructor(
     subscribeToAnswerOutcome(explorationProgressController.submitAnswer(answer).toLiveData())
   }
 
-  fun dismissConceptCard() {
-    ConceptCardFragment.dismissAll(fragment.childFragmentManager)
-  }
-
   private fun moveToNextState() {
     stateViewModel.setCanSubmitAnswer(canSubmitAnswer = false)
     explorationProgressController.moveToNextState().toLiveData().observe(

app/src/main/java/org/oppia/android/app/testing/ConceptCardFragmentTestActivity.kt

@@ -29,7 +29,7 @@ class ConceptCardFragmentTestActivity :
   }
 
   override fun dismissConceptCard() {
-    getConceptCardFragment()?.dismiss()
+    ConceptCardFragment.dismissAll(supportFragmentManager)
   }
 
   private fun getConceptCardFragment(): ConceptCardFragment? {

app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerActivityPresenter.kt

@@ -274,7 +274,9 @@ class QuestionPlayerActivityPresenter @Inject constructor(
     getHintsAndSolutionDialogFragment()?.dismiss()
   }
 
-  fun dismissConceptCard() = getQuestionPlayerFragment()?.dismissConceptCard()
+  fun dismissConceptCard() {
+    getHintsAndSolutionDialogFragment()?.dismissConceptCard()
+  }
 
   private fun getHintsAndSolutionDialogFragment(): HintsAndSolutionDialogFragment? {
     return activity.supportFragmentManager.findFragmentByTag(

app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragment.kt

@@ -106,8 +106,6 @@ class QuestionPlayerFragment :
     questionPlayerFragmentPresenter.revealSolution()
   }
 
-  fun dismissConceptCard() = questionPlayerFragmentPresenter.dismissConceptCard()
-
   companion object {
 
     /** Arguments key for [QuestionPlayerFragment]. */

app/src/main/java/org/oppia/android/app/topic/questionplayer/QuestionPlayerFragmentPresenter.kt

@@ -27,7 +27,6 @@ import org.oppia.android.app.player.state.StatePlayerRecyclerViewAssembler
 import org.oppia.android.app.player.state.listener.RouteToHintsAndSolutionListener
 import org.oppia.android.app.player.stopplaying.RestartPlayingSessionListener
 import org.oppia.android.app.player.stopplaying.StopStatePlayingSessionListener
-import org.oppia.android.app.topic.conceptcard.ConceptCardFragment
 import org.oppia.android.app.utility.FontScaleConfigurationUtil
 import org.oppia.android.app.utility.SplitScreenManager
 import org.oppia.android.databinding.QuestionPlayerFragmentBinding
@@ -124,10 +123,6 @@ class QuestionPlayerFragmentPresenter @Inject constructor(
     subscribeToHintSolution(questionAssessmentProgressController.submitSolutionIsRevealed())
   }
 
-  fun dismissConceptCard() {
-    ConceptCardFragment.dismissAll(fragment.childFragmentManager)
-  }
-
   private fun retrieveArguments(): QuestionPlayerFragmentArguments {
     return fragment.requireArguments().getProto(
       QuestionPlayerFragment.ARGUMENTS_KEY, QuestionPlayerFragmentArguments.getDefaultInstance()

app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt

@@ -3,7 +3,9 @@ package org.oppia.android.app.player.exploration
 import android.app.Application
 import android.content.Context
 import android.content.Intent
+import android.text.Spannable
 import android.text.TextUtils
+import android.text.style.ClickableSpan
 import android.view.View
 import android.widget.TextView
 import androidx.appcompat.app.AppCompatActivity
@@ -48,6 +50,7 @@ import org.hamcrest.CoreMatchers.containsString
 import org.hamcrest.Description
 import org.hamcrest.Matcher
 import org.hamcrest.Matchers.not
+import org.hamcrest.TypeSafeMatcher
 import org.junit.After
 import org.junit.Before
 import org.junit.Ignore
@@ -2558,6 +2561,92 @@ class ExplorationActivityTest {
     }
   }
 
+  @Test
+  @RunOn(TestPlatform.ROBOLECTRIC) // TODO(#3858): Enable for Espresso.
+  fun testExpActivity_openConceptCard_selectNavigationUp_conceptCardCloses() {
+    markAllSpotlightsSeen()
+    launch<ExplorationActivity>(
+      createExplorationActivityIntent(
+        internalProfileId,
+        TEST_CLASSROOM_ID_0,
+        TEST_TOPIC_ID_0,
+        TEST_STORY_ID_0,
+        TEST_EXPLORATION_ID_2,
+        shouldSavePartialProgress = false
+      )
+    ).use {
+      explorationDataController.startPlayingNewExploration(
+        internalProfileId,
+        TEST_CLASSROOM_ID_0,
+        TEST_TOPIC_ID_0,
+        TEST_STORY_ID_0,
+        TEST_EXPLORATION_ID_2
+      )
+      testCoroutineDispatchers.runCurrent()
+      clickContinueButton()
+      // Submit two incorrect answers.
+      submitFractionAnswer(answerText = "1/3")
+      submitFractionAnswer(answerText = "1/4")
+
+      // Reveal the hint.
+      openHintsAndSolutionsDialog()
+      pressRevealHintButton(hintPosition = 0)
+
+      onView(withId(R.id.hints_and_solution_summary))
+        .inRoot(isDialog())
+        .perform(openClickableSpan("test_skill_id_1 concept card"))
+
+      testCoroutineDispatchers.runCurrent()
+
+      onView(withText("Concept Card")).inRoot(isDialog()).check(matches(isDisplayed()))
+      onView(withText("Another important skill")).inRoot(isDialog()).check(matches(isDisplayed()))
+      onView(withId(R.id.concept_card_toolbar)).check(matches(isDisplayed()))
+
+      onView(withContentDescription(R.string.navigate_up)).perform(click())
+
+      testCoroutineDispatchers.runCurrent()
+      onView(withId(R.id.concept_card_toolbar)).check(doesNotExist())
+    }
+    explorationDataController.stopPlayingExploration(isCompletion = false)
+  }
+
+  private fun openClickableSpan(text: String): ViewAction {
+    return object : ViewAction {
+      override fun getDescription(): String = "openClickableSpan"
+
+      override fun getConstraints(): Matcher<View> = hasClickableSpanWithText(text)
+
+      override fun perform(uiController: UiController?, view: View?) {
+        // The view shouldn't be null if the constraints are being met.
+        (view as? TextView)?.getClickableSpans()?.findMatchingTextOrNull(text)?.onClick(view)
+      }
+    }
+  }
+
+  private fun List<Pair<String, ClickableSpan>>.findMatchingTextOrNull(text: String) =
+    find { text in it.first }?.second
+
+  private fun TextView.getClickableSpans(): List<Pair<String, ClickableSpan>> {
+    val viewText = text
+    return (viewText as Spannable).getSpans(
+      /* start= */ 0, /* end= */ text.length, ClickableSpan::class.java
+    ).map {
+      viewText.subSequence(viewText.getSpanStart(it), viewText.getSpanEnd(it)).toString() to it
+    }
+  }
+
+  private fun hasClickableSpanWithText(text: String): Matcher<View> {
+    return object : TypeSafeMatcher<View>(TextView::class.java) {
+      override fun describeTo(description: Description?) {
+        description?.appendText("has ClickableSpan with text")?.appendValue(text)
+      }
+
+      override fun matchesSafely(item: View?): Boolean {
+        return (item as? TextView)?.getClickableSpans()?.findMatchingTextOrNull(text) != null
+      }
+    }
+  }
+
   private fun markSpotlightSeen(feature: Spotlight.FeatureCase) {
     val profileId = ProfileId.newBuilder().setInternalId(internalProfileId).build()
     spotlightStateController.markSpotlightViewed(profileId, feature)

scripts/src/java/org/oppia/android/scripts/coverage/reporter/BUILD.bazel

@@ -15,5 +15,6 @@ kt_jvm_library(
         "//scripts/src/java/org/oppia/android/scripts/common:bazel_client",
         "//scripts/src/java/org/oppia/android/scripts/proto:coverage_java_proto",
         "//scripts/src/java/org/oppia/android/scripts/proto:script_exemptions_java_proto",
+        "//third_party:com_google_guava_guava",
     ],
 )

scripts/src/java/org/oppia/android/scripts/coverage/reporter/CoverageReporter.kt

@@ -1,5 +1,6 @@
 package org.oppia.android.scripts.coverage.reporter
 
+import com.google.common.html.HtmlEscapers
 import org.oppia.android.scripts.proto.Coverage
 import org.oppia.android.scripts.proto.CoverageReport
 import org.oppia.android.scripts.proto.CoverageReportContainer
@@ -277,7 +278,7 @@ class CoverageReporter(
                 """
                   <tr>
                     <td class="line-number-row">${lineNumber.toString().padStart(4, ' ')}</td>
-                    <td class="$lineClass">$line</td>
+                    <td class="$lineClass">${HtmlEscapers.htmlEscaper().escape(line)}</td>
                   </tr>
                 """.trimIndent()
               )