Fix #5069: Add a "hint/solution viewed" event to complement the exist…

…ing "offered"/"unlocked" events. (#5298)  ## Explanation Fixes #5069: Added a new event called view existing for hints and solution. This event logs every time user views a hint or solution, except for the first time. Existing event log access_hint and access_solution renamed to reveal_hint and reveal_solution. ### Hints view log [hint.webm](https://github.com/oppia/oppia-android/assets/76042077/c7ab6b74-f5a9-46d9-a8fa-cf1c38fc9985) ### Solution view log [solution.webm](https://github.com/oppia/oppia-android/assets/76042077/e868e261-ef99-4742-a938-6a15a4a51723)  ## Essential Checklist  - [ ] 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: ...".) - [ ] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [ ] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [ ] 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)). - [ ] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [ ] 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  If your PR includes UI-related changes, then: - Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes - For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see [RTL guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines)) - Add a video showing the full UX flow with a screen reader enabled (see [accessibility guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide)) - For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included - Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing --------- Co-authored-by: Adhiambo Peres <[email protected]>
oppia · Jul 1, 2024 · b5354f9 · b5354f9
1 parent d8f7635
commit b5354f9
Show file tree

Hide file tree

Showing 21 changed files with 745 additions and 104 deletions.
diff --git a/...in/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt b/...in/java/org/oppia/android/app/hintsandsolution/HintsAndSolutionDialogFragmentPresenter.kt
@@ -185,11 +185,15 @@ class HintsAndSolutionDialogFragmentPresenter @Inject constructor(
     binding.expandableHintHeader.setOnClickListener {
       if (hintViewModel.isHintRevealed.get()) {
         expandOrCollapseItem(position)
+        if (position in expandedItemIndexes)
+        (fragment.requireActivity() as? ViewHintListener)?.viewHint(hintIndex = position)
       }
     }
     binding.expandHintListIcon.setOnClickListener {
       if (hintViewModel.isHintRevealed.get()) {
         expandOrCollapseItem(position)
+        if (position in expandedItemIndexes)
+        (fragment.requireActivity() as? ViewHintListener)?.viewHint(hintIndex = position)
       }
     }
 
@@ -262,11 +266,15 @@ class HintsAndSolutionDialogFragmentPresenter @Inject constructor(
     binding.expandableSolutionHeader.setOnClickListener {
       if (solutionViewModel.isSolutionRevealed.get()) {
         expandOrCollapseItem(position)
+        if (position in expandedItemIndexes)
+        (fragment.requireActivity() as? ViewSolutionInterface)?.viewSolution()
       }
     }
     binding.expandSolutionListIcon.setOnClickListener {
       if (solutionViewModel.isSolutionRevealed.get()) {
         expandOrCollapseItem(position)
+        if (position in expandedItemIndexes)
+        (fragment.requireActivity() as? ViewSolutionInterface)?.viewSolution()
       }
     }
 

diff --git a/app/src/main/java/org/oppia/android/app/hintsandsolution/ViewHintListener.kt b/app/src/main/java/org/oppia/android/app/hintsandsolution/ViewHintListener.kt
@@ -0,0 +1,10 @@
+package org.oppia.android.app.hintsandsolution
+
+/** Callback listener for when the user wishes to view a hint. */
+interface ViewHintListener {
+  /**
+   * Called when the user indicates they want to view the hint corresponding to the specified
+   * index.
+   */
+  fun viewHint(hintIndex: Int)
+}
diff --git a/app/src/main/java/org/oppia/android/app/hintsandsolution/ViewSolutionInterface.kt b/app/src/main/java/org/oppia/android/app/hintsandsolution/ViewSolutionInterface.kt
@@ -0,0 +1,9 @@
+package org.oppia.android.app.hintsandsolution
+
+/** Interface to check the preference regarding alert for [HintsAndSolutionDialogFragment]. */
+interface ViewSolutionInterface {
+  /**
+   * Called when the user indicates they want to view the solution.
+   */
+  fun viewSolution()
+}
diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivity.kt
@@ -9,6 +9,8 @@ import org.oppia.android.app.hintsandsolution.HintsAndSolutionDialogFragment
 import org.oppia.android.app.hintsandsolution.HintsAndSolutionListener
 import org.oppia.android.app.hintsandsolution.RevealHintListener
 import org.oppia.android.app.hintsandsolution.RevealSolutionInterface
+import org.oppia.android.app.hintsandsolution.ViewHintListener
+import org.oppia.android.app.hintsandsolution.ViewSolutionInterface
 import org.oppia.android.app.model.ExplorationActivityParams
 import org.oppia.android.app.model.HelpIndex
 import org.oppia.android.app.model.ProfileId
@@ -37,7 +39,9 @@ class ExplorationActivity :
   HintsAndSolutionListener,
   RouteToHintsAndSolutionListener,
   RevealHintListener,
+  ViewHintListener,
   RevealSolutionInterface,
+  ViewSolutionInterface,
   DefaultFontSizeStateListener,
   HintsAndSolutionExplorationManagerListener,
   ConceptCardListener,
@@ -188,4 +192,12 @@ class ExplorationActivity :
   override fun requestVoiceOverIconSpotlight(numberOfLogins: Int) {
     explorationActivityPresenter.requestVoiceOverIconSpotlight(numberOfLogins)
   }
+
+  override fun viewHint(hintIndex: Int) {
+    explorationActivityPresenter.viewHint(hintIndex)
+  }
+
+  override fun viewSolution() {
+    explorationActivityPresenter.viewSolution()
+  }
 }
diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt
@@ -401,6 +401,14 @@ class ExplorationActivityPresenter @Inject constructor(
     explorationFragment.revealHint(hintIndex)
   }
 
+  fun viewHint(hintIndex: Int) {
+    val explorationFragment =
+      activity.supportFragmentManager.findFragmentByTag(
+        TAG_EXPLORATION_FRAGMENT
+      ) as ExplorationFragment
+    explorationFragment.viewHint(hintIndex)
+  }
+
   fun revealSolution() {
     val explorationFragment =
       activity.supportFragmentManager.findFragmentByTag(
@@ -409,6 +417,14 @@ class ExplorationActivityPresenter @Inject constructor(
     explorationFragment.revealSolution()
   }
 
+  fun viewSolution() {
+    val explorationFragment =
+      activity.supportFragmentManager.findFragmentByTag(
+        TAG_EXPLORATION_FRAGMENT
+      ) as ExplorationFragment
+    explorationFragment.viewSolution()
+  }
+
   private fun showProgressDatabaseFullDialogFragment() {
     val previousFragment = activity.supportFragmentManager.findFragmentByTag(
       TAG_PROGRESS_DATABASE_FULL_DIALOG

diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragment.kt
@@ -72,9 +72,15 @@ class ExplorationFragment : InjectableFragment() {
     explorationFragmentPresenter.revealHint(hintIndex)
   }
 
+  fun viewHint(hintIndex: Int) {
+    explorationFragmentPresenter.viewHint(hintIndex)
+  }
   fun revealSolution() {
     explorationFragmentPresenter.revealSolution()
   }
+  fun viewSolution() {
+    explorationFragmentPresenter.viewSolution()
+  }
 
   fun dismissConceptCard() = explorationFragmentPresenter.dismissConceptCard()
 

diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationFragmentPresenter.kt
@@ -129,11 +129,18 @@ class ExplorationFragmentPresenter @Inject constructor(
   fun revealHint(hintIndex: Int) {
     getStateFragment()?.revealHint(hintIndex)
   }
+  fun viewHint(hintIndex: Int) {
+    getStateFragment()?.viewHint(hintIndex)
+  }
 
   fun revealSolution() {
     getStateFragment()?.revealSolution()
   }
 
+  fun viewSolution() {
+    getStateFragment()?.viewSolution()
+  }
+
   fun dismissConceptCard() = getStateFragment()?.dismissConceptCard()
 
   fun getExplorationCheckpointState() = getStateFragment()?.getExplorationCheckpointState()

diff --git a/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt b/app/src/main/java/org/oppia/android/app/player/state/StateFragment.kt
@@ -141,8 +141,16 @@ class StateFragment :
     stateFragmentPresenter.revealHint(hintIndex)
   }
 
+  fun viewHint(hintIndex: Int) {
+    stateFragmentPresenter.viewHint(hintIndex)
+  }
+
   fun revealSolution() = stateFragmentPresenter.revealSolution()
 
+  fun viewSolution() {
+    stateFragmentPresenter.viewSolution()
+  }
+
   fun dismissConceptCard() = stateFragmentPresenter.dismissConceptCard()
 
   fun getExplorationCheckpointState() = stateFragmentPresenter.getExplorationCheckpointState()

diff --git a/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt b/app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt
@@ -268,10 +268,18 @@ class StateFragmentPresenter @Inject constructor(
     subscribeToHintSolution(explorationProgressController.submitHintIsRevealed(hintIndex))
   }
 
+  fun viewHint(hintIndex: Int) {
+    explorationProgressController.submitHintIsViewed(hintIndex)
+  }
+
   fun revealSolution() {
     subscribeToHintSolution(explorationProgressController.submitSolutionIsRevealed())
   }
 
+  fun viewSolution() {
+    explorationProgressController.submitSolutionIsViewed()
+  }
+
   private fun getAudioFragment(): Fragment? {
     return fragment.childFragmentManager.findFragmentByTag(TAG_AUDIO_FRAGMENT)
   }

diff --git a/domain/src/main/java/org/oppia/android/domain/exploration/ExplorationProgressController.kt b/domain/src/main/java/org/oppia/android/domain/exploration/ExplorationProgressController.kt
@@ -61,8 +61,12 @@ private const val SUBMIT_ANSWER_RESULT_PROVIDER_ID =
   "ExplorationProgressController.submit_answer_result"
 private const val SUBMIT_HINT_REVEALED_RESULT_PROVIDER_ID =
   "ExplorationProgressController.submit_hint_revealed_result"
+private const val SUBMIT_HINT_VIEWED_RESULT_PROVIDER_ID =
+  "ExplorationProgressController.submit_hint_revealed_result"
 private const val SUBMIT_SOLUTION_REVEALED_RESULT_PROVIDER_ID =
   "ExplorationProgressController.submit_solution_revealed_result"
+private const val SUBMIT_SOLUTION_VIEWED_RESULT_PROVIDER_ID =
+  "ExplorationProgressController.submit_solution_revealed_result"
 private const val MOVE_TO_PREVIOUS_STATE_RESULT_PROVIDER_ID =
   "ExplorationProgressController.move_to_previous_state_result"
 private const val MOVE_TO_NEXT_STATE_RESULT_PROVIDER_ID =
@@ -275,6 +279,24 @@ class ExplorationProgressController @Inject constructor(
     return submitResultFlow.convertToSessionProvider(SUBMIT_HINT_REVEALED_RESULT_PROVIDER_ID)
   }
 
+  /**
+   * Notifies the controller that the user has viewed a hint.
+   *
+   * @param hintIndex index of the hint that is being viewed
+   *
+   * @return a [DataProvider] that indicates success/failure of the operation (the actual payload of
+   *     the result isn't relevant)
+   */
+
+  fun submitHintIsViewed(hintIndex: Int): DataProvider<Any?> {
+    val submitResultFlow = createAsyncResultStateFlow<Any?>()
+    val message = ControllerMessage.LogHintIsViewed(hintIndex, activeSessionId, submitResultFlow)
+    sendCommandForOperation(message) {
+      "Failed to schedule command for viewing hint: $hintIndex."
+    }
+    return submitResultFlow.convertToSessionProvider(SUBMIT_HINT_VIEWED_RESULT_PROVIDER_ID)
+  }
+
   /**
    * Notifies the controller that the user has revealed the solution to the current state.
    *
@@ -291,6 +313,18 @@ class ExplorationProgressController @Inject constructor(
     return submitResultFlow.convertToSessionProvider(SUBMIT_SOLUTION_REVEALED_RESULT_PROVIDER_ID)
   }
 
+  /**
+   * Notifies the controller that the user has viewed the answer.
+   * @return a [DataProvider] that indicates success/failure of the operation (the actual payload of
+   *     the result isn't relevant)
+   */
+  fun submitSolutionIsViewed(): DataProvider<Any?> {
+    val submitResultFlow = createAsyncResultStateFlow<Any?>()
+    val message = ControllerMessage.LogSolutionIsViewed(activeSessionId, submitResultFlow)
+    sendCommandForOperation(message) { "Failed to schedule command for viewing the solution." }
+    return submitResultFlow.convertToSessionProvider(SUBMIT_SOLUTION_VIEWED_RESULT_PROVIDER_ID)
+  }
+
   /**
    * Navigates to the previous state in the graph. If the learner is currently on the initial state,
    * this method will throw an exception. Calling code is responsible for ensuring this method is
@@ -419,7 +453,6 @@ class ExplorationProgressController @Inject constructor(
   @OptIn(ObsoleteCoroutinesApi::class)
   private fun createControllerCommandActor(): SendChannel<ControllerMessage<*>> {
     lateinit var controllerState: ControllerState
-
     // Use an unlimited capacity buffer so that commands can be sent asynchronously without blocking
     // the main thread or scheduling an extra coroutine.
     @Suppress("JoinDeclarationAndAssignment") // Warning is incorrect in this case.
@@ -491,8 +524,14 @@ class ExplorationProgressController @Inject constructor(
             is ControllerMessage.HintIsRevealed -> {
               controllerState.submitHintIsRevealedImpl(message.callbackFlow, message.hintIndex)
             }
+            is ControllerMessage.LogHintIsViewed ->
+              controllerState.logViewedHintImpl(
+                activeSessionId, message.hintIndex, message.callbackFlow
+              )
             is ControllerMessage.SolutionIsRevealed ->
               controllerState.submitSolutionIsRevealedImpl(message.callbackFlow)
+            is ControllerMessage.LogSolutionIsViewed ->
+              controllerState.logViewedSolutionImpl(activeSessionId, message.callbackFlow)
             is ControllerMessage.MoveToPreviousState ->
               controllerState.moveToPreviousStateImpl(message.callbackFlow)
             is ControllerMessage.MoveToNextState ->
@@ -790,6 +829,43 @@ class ExplorationProgressController @Inject constructor(
     }
   }
 
+  private suspend fun ControllerState.logViewedHintImpl(
+    sessionId: String,
+    hintIndex: Int,
+    submitLogHintViewedResultFlow: MutableStateFlow<AsyncResult<Any?>>
+  ) {
+    tryOperation(submitLogHintViewedResultFlow) {
+      check(explorationProgress.playStage != NOT_PLAYING) {
+        "Cannot log hint viewed if an exploration is not being played."
+      }
+      check(explorationProgress.playStage != LOADING_EXPLORATION) {
+        "Cannot log hint viewed if an exploration is being loaded."
+      }
+      check(explorationProgress.playStage != SUBMITTING_ANSWER) {
+        "Cannot log hint viewed if an answer submission is pending."
+      }
+      maybeLogViewedHint(sessionId, hintIndex)
+    }
+  }
+
+  private suspend fun ControllerState.logViewedSolutionImpl(
+    sessionId: String,
+    submitLogSolutionViewedResultFlow: MutableStateFlow<AsyncResult<Any?>>
+  ) {
+    tryOperation(submitLogSolutionViewedResultFlow) {
+      check(explorationProgress.playStage != NOT_PLAYING) {
+        "Cannot log solution viewed if an exploration is not being played."
+      }
+      check(explorationProgress.playStage != LOADING_EXPLORATION) {
+        "Cannot log solution viewed while the exploration is being loaded."
+      }
+      check(explorationProgress.playStage != SUBMITTING_ANSWER) {
+        "Cannot log solution viewed if an answer submission is pending."
+      }
+      maybeLogViewedSolution(sessionId)
+    }
+  }
+
   private fun ControllerState.maybeLogUpdatedHelpIndex(
     helpIndex: HelpIndex,
     activeSessionId: String
@@ -800,6 +876,25 @@ class ExplorationProgressController @Inject constructor(
     }
   }
 
+  private fun ControllerState.maybeLogViewedHint(
+    activeSessionId: String,
+    hintIndex: Int
+  ) {
+    // Only log if the current session is active.
+    if (sessionId == activeSessionId) {
+      stateAnalyticsLogger?.logViewHint(hintIndex)
+    }
+  }
+
+  private fun ControllerState.maybeLogViewedSolution(
+    activeSessionId: String
+  ) {
+    // Only log if the current session is active.
+    if (sessionId == activeSessionId) {
+      stateAnalyticsLogger?.logViewSolution()
+    }
+  }
+
   private suspend fun <T> ControllerState.tryOperation(
     resultFlow: MutableStateFlow<AsyncResult<T>>,
     recomputeState: Boolean = true,
@@ -1216,12 +1311,12 @@ class ExplorationProgressController @Inject constructor(
           NEXT_AVAILABLE_HINT_INDEX ->
             stateAnalyticsLogger?.logHintUnlocked(newHelpIndex.nextAvailableHintIndex)
           LATEST_REVEALED_HINT_INDEX ->
-            stateAnalyticsLogger?.logViewHint(newHelpIndex.latestRevealedHintIndex)
+            stateAnalyticsLogger?.logRevealHint(newHelpIndex.latestRevealedHintIndex)
           SHOW_SOLUTION -> stateAnalyticsLogger?.logSolutionUnlocked()
           EVERYTHING_REVEALED -> when (helpIndex.indexTypeCase) {
-            SHOW_SOLUTION -> stateAnalyticsLogger?.logViewSolution()
+            SHOW_SOLUTION -> stateAnalyticsLogger?.logRevealSolution()
             NEXT_AVAILABLE_HINT_INDEX -> // No solution, so revealing the hint ends available help.
-              stateAnalyticsLogger?.logViewHint(helpIndex.nextAvailableHintIndex)
+              stateAnalyticsLogger?.logRevealHint(helpIndex.nextAvailableHintIndex)
             // Nothing to do in these cases.
             LATEST_REVEALED_HINT_INDEX, EVERYTHING_REVEALED, INDEXTYPE_NOT_SET, null -> {}
           }
@@ -1349,6 +1444,31 @@ class ExplorationProgressController @Inject constructor(
       override val callbackFlow: MutableStateFlow<AsyncResult<Any?>>? = null
     ) : ControllerMessage<Any?>()
 
+    /**
+     * [ControllerMessage] to log cases when the user has viewed a hint for the current session.
+     *
+     * Specific measures are taken to ensure that the handler for this message does not log the
+     * change if the current active session has changed (since that's generally indicative of an
+     * error--hints can't continue to change after the session has ended).
+     */
+    data class LogHintIsViewed(
+      val hintIndex: Int,
+      override val sessionId: String,
+      override val callbackFlow: MutableStateFlow<AsyncResult<Any?>>
+    ) : ControllerMessage<Any?>()
+
+    /**
+     * [ControllerMessage] to log cases when the user has viewed the solution for the current
+     * session.
+     *
+     * Specific measures are taken to ensure that the handler for this message does not log the
+     * change if the current active session has changed.
+     */
+    data class LogSolutionIsViewed(
+      override val sessionId: String,
+      override val callbackFlow: MutableStateFlow<AsyncResult<Any?>>
+    ) : ControllerMessage<Any?>()
+
     /**
      * [ControllerMessage] to ensure a successfully saved checkpoint is reflected in other parts of
      * the app (e.g. that an exploration is considered 'in-progress' in such circumstances).