Fix #2476, #3312 and #3703: Move hint handler to domain layer (#3659)

* moved hint handler to domain layer

* fixed app layer espresso tests

* fix app layer robolectric tests

* fixed domain layer tests

* Added annotations to test exemptions

* proto lint fix

* fixed hint handler for training sessions

* nit and removed test excemptions

* added hint tests for config change

* fixed test file exemptions

* fixed failing test

* made HintHandler injectable

* fixed ktlint error

* Added tests for hint handler

* nits

* fixed failing build

* fixed failing build

* fix build

* fixed imports

* nits and improved testing

* updated exploration.proto

* removed progress controller from kdoc exemptions

* fix failing test

* moved timer to domain

* fixed build and nits

* nit

* added listener back to test exemptions

* nit fixes and added more tests

* lint fix

* First round: make HintHandler independent.

This also brings HintHandler into an interface + factory pattern. This
isn't the final design since I think we can largely simplify the way
hints work; that'll be my next pass.

This breaks questions & HintHandlerTest; those will require further work
later.

* Simplify HelpIndex in proto.

Move HelpIndex to PendingState to avoid the entire domain case of
handling CompletedState.

* Simplify hints & solutions.

This removes the per-Hint/Solution tracking & completely leans on
HelpIndex for proper hints & solution tracking both in the domain & UI
layers.

Fixes a bunch of other stuff, too, including
QuestionAssessmentProgressController tests.

* Clean up dead code paths & improve handler API.

* fixed test modules and lint

* renamed HintHandlerTest to HintHandlerImplTest

* Add tests for HintHandler.

This introduces some new explorations for making testing HintHandlerImpl
easier.

* Add remaining tests/exemptions for new files.

* Lint fixes.

* Post-merge fixes (including lint fixes).

* Post-merge maven_install fix.

* Revert "Merge branch 'develop' into move-hint-handler-to-domain"

This reverts commit e2fea90, reversing
changes made to 6659858.

* Post-merge Gradle-discovered fixes.

* Revert "Revert "Merge branch 'develop' into move-hint-handler-to-domain""

This reverts commit b1622c0.

* Additional post-merge fixes.

* Address first batch of review comments.

(Clarified some proto fields & remove trailing comma).

* Fix testing module tests & remove unnecessary changes.

* Simplify & fix more reviewer comments.

This simplifies some data pipelining in the UI, and improves HintHandler
documentation in addition to fixing some more reviewer comments.

* Improve documentation to address review comment.

* Rename new module to prod module.

This simplifies the changes & approvals needed in #3705.

* Rename proto field (to address review comment).

Also, fix broken reference error accidentally introduced in an earlier
commit.

* Kotlin lint fixes.

Co-authored-by: yashraj-01 <[email protected]>
Co-authored-by: Ben Henning <[email protected]>

app/src/main/java/org/oppia/android/app/application/ApplicationComponent.kt

@@ -7,7 +7,6 @@ import dagger.Component
 import org.oppia.android.app.activity.ActivityComponent
 import org.oppia.android.app.devoptions.DeveloperOptionsModule
 import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule
-import org.oppia.android.app.player.state.hintsandsolution.HintsAndSolutionConfigModule
 import org.oppia.android.app.shim.IntentFactoryShimModule
 import org.oppia.android.app.shim.ViewBindingShimModule
 import org.oppia.android.app.topic.PracticeTabModule
@@ -24,6 +23,8 @@ import org.oppia.android.domain.classify.rules.numericinput.NumericInputRuleModu
 import org.oppia.android.domain.classify.rules.ratioinput.RatioInputModule
 import org.oppia.android.domain.classify.rules.textinput.TextInputRuleModule
 import org.oppia.android.domain.exploration.lightweightcheckpointing.ExplorationStorageModule
+import org.oppia.android.domain.hintsandsolution.HintsAndSolutionConfigModule
+import org.oppia.android.domain.hintsandsolution.HintsAndSolutionProdModule
 import org.oppia.android.domain.onboarding.ExpirationMetaDataRetrieverModule
 import org.oppia.android.domain.oppialogger.ApplicationStartupListener
 import org.oppia.android.domain.oppialogger.LogStorageModule
@@ -76,10 +77,11 @@ import javax.inject.Singleton
  ExpirationMetaDataRetrieverModule::class, RatioInputModule::class,
  UncaughtExceptionLoggerModule::class, ApplicationStartupListenerModule::class,
  LogUploadWorkerModule::class, WorkManagerConfigurationModule::class,
- HintsAndSolutionConfigModule::class, FirebaseLogUploaderModule::class,
- NetworkModule::class, PracticeTabModule::class, PlatformParameterModule::class,
- ExplorationStorageModule::class, DeveloperOptionsStarterModule::class,
- DeveloperOptionsModule::class, NetworkConnectionUtilDebugModule::class,
+ HintsAndSolutionConfigModule::class, HintsAndSolutionProdModule::class,
+ FirebaseLogUploaderModule::class, NetworkModule::class, PracticeTabModule::class,
+ PlatformParameterModule::class, ExplorationStorageModule::class,
+ DeveloperOptionsStarterModule::class, DeveloperOptionsModule::class,
+ NetworkConnectionUtilDebugModule::class,
  // TODO(#59): Remove this 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.
  NetworkConnectionDebugUtilModule::class

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

@@ -7,12 +7,14 @@ import android.view.View
 import android.view.ViewGroup
 import org.oppia.android.R
 import org.oppia.android.app.fragment.InjectableDialogFragment
+import org.oppia.android.app.model.HelpIndex
 import org.oppia.android.app.model.State
+import org.oppia.android.util.extensions.getProto
+import org.oppia.android.util.extensions.putProto
 import javax.inject.Inject
 
 private const val CURRENT_EXPANDED_LIST_INDEX_SAVED_KEY =
  "HintsAndSolutionDialogFragment.current_expanded_list_index"
-private const val STATE_SAVED_KEY = "HintsAndSolutionDialogFragment.state"
 private const val HINT_INDEX_SAVED_KEY = "HintsAndSolutionDialogFragment.hint_index"
 private const val IS_HINT_REVEALED_SAVED_KEY = "HintsAndSolutionDialogFragment.is_hint_revealed"
 private const val SOLUTION_INDEX_SAVED_KEY = "HintsAndSolutionDialogFragment.solution_index"
@@ -28,8 +30,6 @@ class HintsAndSolutionDialogFragment :
  @Inject
  lateinit var hintsAndSolutionDialogFragmentPresenter: HintsAndSolutionDialogFragmentPresenter
 
- private lateinit var state: State
-
  private var currentExpandedHintListIndex: Int? = null
 
  private var index: Int? = null
@@ -40,30 +40,29 @@ class HintsAndSolutionDialogFragment :
  companion object {
 
  internal const val ID_ARGUMENT_KEY = "HintsAndSolutionDialogFragment.id"
- internal const val NEW_AVAILABLE_HINT_INDEX_ARGUMENT_KEY =
- "HintsAndSolutionDialogFragment.new_available_hint_index"
- internal const val ALL_HINTS_EXHAUSTED_ARGUMENT_KEY =
- "HintsAndSolutionDialogFragment.all_hints_exhausted"
+ internal const val STATE_KEY = "HintsAndSolutionDialogFragment.state"
+ internal const val HELP_INDEX_KEY = "HintsAndSolutionDialogFragment.help_index"
 
  /**
  * Creates a new instance of a DialogFragment to display hints and solution
+ *
  * @param id Used in ExplorationController/QuestionAssessmentProgressController to get current state data.
- * @param newAvailableHintIndex Index of new available hint.
- * @param allHintsExhausted Boolean set to true when all hints are exhausted.
+ * @param state the [State] being viewed by the learner
+ * @param helpIndex the [HelpIndex] corresponding to the current hints/solution configuration
  * @return [HintsAndSolutionDialogFragment]: DialogFragment
  */
  fun newInstance(
  id: String,
- newAvailableHintIndex: Int,
- allHintsExhausted: Boolean
+ state: State,
+ helpIndex: HelpIndex
  ): HintsAndSolutionDialogFragment {
- val hintsAndSolutionFrag = HintsAndSolutionDialogFragment()
- val args = Bundle()
- args.putString(ID_ARGUMENT_KEY, id)
- args.putInt(NEW_AVAILABLE_HINT_INDEX_ARGUMENT_KEY, newAvailableHintIndex)
- args.putBoolean(ALL_HINTS_EXHAUSTED_ARGUMENT_KEY, allHintsExhausted)
- hintsAndSolutionFrag.arguments = args
- return hintsAndSolutionFrag
+ return HintsAndSolutionDialogFragment().apply {
+  arguments = Bundle().apply {
+  putString(ID_ARGUMENT_KEY, id)
+  putProto(STATE_KEY, state)
+  putProto(HELP_INDEX_KEY, helpIndex)
+  }
+ }
  }
  }
 
@@ -88,7 +87,6 @@ class HintsAndSolutionDialogFragment :
  if (currentExpandedHintListIndex == -1) {
  currentExpandedHintListIndex = null
  }
- state = State.parseFrom(savedInstanceState.getByteArray(STATE_SAVED_KEY)!!)
  index = savedInstanceState.getInt(HINT_INDEX_SAVED_KEY, -1)
  if (index == -1) index = null
  isHintRevealed = savedInstanceState.getBoolean(IS_HINT_REVEALED_SAVED_KEY, false)
@@ -104,23 +102,17 @@ class HintsAndSolutionDialogFragment :
  checkNotNull(
  args.getString(ID_ARGUMENT_KEY)
  ) { "Expected id to be passed to HintsAndSolutionDialogFragment" }
- val newAvailableHintIndex =
- checkNotNull(
- args.getInt(NEW_AVAILABLE_HINT_INDEX_ARGUMENT_KEY)
- ) { "Expected hint index to be passed to HintsAndSolutionDialogFragment" }
- val allHintsExhausted =
- checkNotNull(
- args.getBoolean(ALL_HINTS_EXHAUSTED_ARGUMENT_KEY)
- ) { "Expected if hints exhausted to be passed to HintsAndSolutionDialogFragment" }
+
+ val state = args.getProto(STATE_KEY, State.getDefaultInstance())
+ val helpIndex = args.getProto(HELP_INDEX_KEY, HelpIndex.getDefaultInstance())
 
  return hintsAndSolutionDialogFragmentPresenter.handleCreateView(
  inflater,
  container,
  state,
+ helpIndex,
  id,
  currentExpandedHintListIndex,
- newAvailableHintIndex,
- allHintsExhausted,
  this as ExpandedHintListIndexListener,
  index,
  isHintRevealed,
@@ -151,7 +143,6 @@ class HintsAndSolutionDialogFragment :
  if (isSolutionRevealed != null) {
  outState.putBoolean(IS_SOLUTION_REVEALED_SAVED_KEY, isSolutionRevealed!!)
  }
- outState.putByteArray(STATE_SAVED_KEY, state.toByteArray())
  }
 
  override fun onExpandListIconClicked(index: Int?) {
@@ -177,8 +168,4 @@ class HintsAndSolutionDialogFragment :
  isSolutionRevealed
  )
  }
-
- fun loadState(state: State) {
- this.state = state
- }
 }

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

@@ -6,6 +6,11 @@ import android.view.ViewGroup
 import androidx.fragment.app.Fragment
 import org.oppia.android.R
 import org.oppia.android.app.fragment.FragmentScope
+import org.oppia.android.app.model.HelpIndex
+import org.oppia.android.app.model.HelpIndex.IndexTypeCase.EVERYTHING_REVEALED
+import org.oppia.android.app.model.HelpIndex.IndexTypeCase.LATEST_REVEALED_HINT_INDEX
+import org.oppia.android.app.model.HelpIndex.IndexTypeCase.NEXT_AVAILABLE_HINT_INDEX
+import org.oppia.android.app.model.HelpIndex.IndexTypeCase.SHOW_SOLUTION
 import org.oppia.android.app.model.State
 import org.oppia.android.app.recyclerview.BindableAdapter
 import org.oppia.android.app.viewmodel.ViewModelProvider
@@ -16,6 +21,7 @@ import org.oppia.android.databinding.SolutionSummaryBinding
 import org.oppia.android.util.gcsresource.DefaultResourceBucketName
 import org.oppia.android.util.parser.html.ExplorationHtmlParserEntityType
 import org.oppia.android.util.parser.html.HtmlParser
+import java.lang.IllegalStateException
 import java.util.Locale
 import javax.inject.Inject
 
@@ -39,6 +45,7 @@ class HintsAndSolutionDialogFragmentPresenter @Inject constructor(
  private lateinit var expandedHintListIndexListener: ExpandedHintListIndexListener
  private lateinit var binding: HintsAndSolutionFragmentBinding
  private lateinit var state: State
+ private lateinit var helpIndex: HelpIndex
  private lateinit var itemList: List<HintsAndSolutionItemViewModel>
  private lateinit var bindingAdapter: BindableAdapter<HintsAndSolutionItemViewModel>
 
@@ -54,17 +61,15 @@ class HintsAndSolutionDialogFragmentPresenter @Inject constructor(
  inflater: LayoutInflater,
  container: ViewGroup?,
  state: State,
+ helpIndex: HelpIndex,
  id: String?,
  currentExpandedHintListIndex: Int?,
- newAvailableHintIndex: Int,
- allHintsExhausted: Boolean,
  expandedHintListIndexListener: ExpandedHintListIndexListener,
  index: Int?,
  isHintRevealed: Boolean?,
  solutionIndex: Int?,
  isSolutionRevealed: Boolean?
  ): View? {
-
  binding =
  HintsAndSolutionFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false)
  this.currentExpandedHintListIndex = currentExpandedHintListIndex
@@ -86,25 +91,52 @@ class HintsAndSolutionDialogFragmentPresenter @Inject constructor(
  }
 
  this.state = state
- // The newAvailableHintIndex received here is coming from state player but in this implementation
- // hints/solutions are shown on every even index and on every odd index we show a divider.
- // Therefore multiplying the original index by 2.
+ this.helpIndex = helpIndex
+ // The newAvailableHintIndex received here is coming from state player but in this
+ // implementation hints/solutions are shown on every even index and on every odd index we show a
+ // divider. The relative index therefore needs to be doubled to account for the divider.
+ val newAvailableHintIndex = computeNewAvailableHintIndex(helpIndex)
  viewModel.newAvailableHintIndex.set(
  newAvailableHintIndex * RECYCLERVIEW_INDEX_CORRECTION_MULTIPLIER
  )
- viewModel.allHintsExhausted.set(allHintsExhausted)
+ viewModel.allHintsExhausted.set(computeWhetherAllHintsAreExhausted(helpIndex))
  viewModel.explorationId.set(id)
 
  loadHintsAndSolution(state)
 
  return binding.root
  }
 
+ private fun computeNewAvailableHintIndex(helpIndex: HelpIndex): Int {
+ return when (helpIndex.indexTypeCase) {
+ NEXT_AVAILABLE_HINT_INDEX -> helpIndex.nextAvailableHintIndex
+ LATEST_REVEALED_HINT_INDEX -> helpIndex.latestRevealedHintIndex
+ SHOW_SOLUTION, EVERYTHING_REVEALED -> {
+ // 1 is subtracted from the hint count because hints are indexed from 0.
+ state.interaction.hintCount - 1
+ }
+ else ->
+ throw IllegalStateException(
+ "Encountered invalid type for showing hints: ${helpIndex.indexTypeCase}"
+ )
+ }
+ }
+
+ private fun computeWhetherAllHintsAreExhausted(helpIndex: HelpIndex): Boolean {
+ return when (helpIndex.indexTypeCase) {
+ NEXT_AVAILABLE_HINT_INDEX, LATEST_REVEALED_HINT_INDEX -> false
+ SHOW_SOLUTION, EVERYTHING_REVEALED -> true
+ else ->
+ throw IllegalStateException(
+ "Encountered invalid type for showing hints: ${helpIndex.indexTypeCase}"
+ )
+ }
+ }
+
  private fun loadHintsAndSolution(state: State) {
  // Check if hints are available for this state.
- if (state.interaction.hintList.size != 0) {
- viewModel.setHintsList(state.interaction.hintList)
- viewModel.setSolution(state.interaction.solution)
+ if (state.interaction.hintList.isNotEmpty()) {
+ viewModel.initialize(helpIndex, state.interaction.hintList, state.interaction.solution)
 
  itemList = viewModel.processHintList()
 
@@ -203,7 +235,6 @@ class HintsAndSolutionDialogFragmentPresenter @Inject constructor(
  hintsViewModel.isHintRevealed.set(true)
  expandedHintListIndexListener.onRevealHintClicked(position, /* isHintRevealed= */ true)
  (fragment.requireActivity() as? RevealHintListener)?.revealHint(
- saveUserChoice = true,
  hintIndex = position / RECYCLERVIEW_INDEX_CORRECTION_MULTIPLIER
  )
  val previousIndex: Int? = currentExpandedHintListIndex

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

@@ -3,8 +3,11 @@ package org.oppia.android.app.hintsandsolution
 import androidx.databinding.ObservableField
 import androidx.lifecycle.ViewModel
 import org.oppia.android.app.fragment.FragmentScope
+import org.oppia.android.app.model.HelpIndex
 import org.oppia.android.app.model.Hint
 import org.oppia.android.app.model.Solution
+import org.oppia.android.domain.hintsandsolution.isHintRevealed
+import org.oppia.android.domain.hintsandsolution.isSolutionRevealed
 import javax.inject.Inject
 
 /**
@@ -29,21 +32,21 @@ class HintsViewModel @Inject constructor() : HintsAndSolutionItemViewModel() {
 
  private lateinit var hintList: List<Hint>
  private lateinit var solution: Solution
+ private lateinit var helpIndex: HelpIndex
  val itemList: MutableList<HintsAndSolutionItemViewModel> = ArrayList()
 
- fun setHintsList(hintList: List<Hint>) {
+ /** Initializes the view model to display hints and a solution. */
+ fun initialize(helpIndex: HelpIndex, hintList: List<Hint>, solution: Solution) {
+ this.helpIndex = helpIndex
  this.hintList = hintList
- }
-
- fun setSolution(solution: Solution) {
  this.solution = solution
  }
 
  fun processHintList(): List<HintsAndSolutionItemViewModel> {
  itemList.clear()
  for (index in hintList.indices) {
  if (itemList.isEmpty()) {
- addHintToList(hintList[index])
+ addHintToList(index, hintList[index])
  } else if (itemList.size > 1) {
  val isLastHintRevealed =
  (itemList[itemList.size - RECYCLERVIEW_INDEX_CORRECTION_MULTIPLIER] as HintsViewModel)
@@ -53,7 +56,7 @@ class HintsViewModel @Inject constructor() : HintsAndSolutionItemViewModel() {
  if (isLastHintRevealed &&
  index <= availableHintIndex / RECYCLERVIEW_INDEX_CORRECTION_MULTIPLIER
  ) {
- addHintToList(hintList[index])
+ addHintToList(index, hintList[index])
  } else {
  break
  }
@@ -76,11 +79,11 @@ class HintsViewModel @Inject constructor() : HintsAndSolutionItemViewModel() {
  return itemList
  }
 
- private fun addHintToList(hint: Hint) {
+ private fun addHintToList(hintIndex: Int, hint: Hint) {
  val hintsViewModel = HintsViewModel()
  hintsViewModel.title.set(hint.hintContent.contentId)
  hintsViewModel.hintsAndSolutionSummary.set(hint.hintContent.html)
- hintsViewModel.isHintRevealed.set(hint.hintIsRevealed)
+ hintsViewModel.isHintRevealed.set(helpIndex.isHintRevealed(hintIndex, hintList))
  itemList.add(hintsViewModel)
  addDividerItem()
  }
@@ -94,7 +97,7 @@ class HintsViewModel @Inject constructor() : HintsAndSolutionItemViewModel() {
  solutionViewModel.wholeNumber.set(solution.correctAnswer.wholeNumber)
  solutionViewModel.isNegative.set(solution.correctAnswer.isNegative)
  solutionViewModel.solutionSummary.set(solution.explanation.html)
- solutionViewModel.isSolutionRevealed.set(solution.solutionIsRevealed)
+ solutionViewModel.isSolutionRevealed.set(helpIndex.isSolutionRevealed())
  itemList.add(solutionViewModel)
  addDividerItem()
  }

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

@@ -1,10 +1,10 @@
 package org.oppia.android.app.hintsandsolution
 
-/** Interface to check the preference regarding alert for [HintsAndSolutionDialogFragment]. */
+/** Callback listener for when the user wishes to reveal a hint. */
 interface RevealHintListener {
  /**
- * If saveUserChoice is true, show solution and save preference do not show dialog again.
- * If saveUserChoice is false, show solution and do not save preference and show this dialog next time too.
+ * Called when the user indicates they want to reveal the hint corresponding to the specified
+ * index.
  */
- fun revealHint(saveUserChoice: Boolean, hintIndex: Int)
+ fun revealHint(hintIndex: Int)
 }

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

@@ -11,6 +11,7 @@ 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.model.HelpIndex
 import org.oppia.android.app.model.ReadingTextSize
 import org.oppia.android.app.model.State
 import org.oppia.android.app.player.audio.AudioButtonListener
@@ -143,8 +144,8 @@ class ExplorationActivity :
  explorationActivityPresenter.onKeyboardAction(actionCode)
  }
 
- override fun revealHint(saveUserChoice: Boolean, hintIndex: Int) {
- explorationActivityPresenter.revealHint(saveUserChoice, hintIndex)
+ override fun revealHint(hintIndex: Int) {
+ explorationActivityPresenter.revealHint(hintIndex)
  }
 
  override fun revealSolution() = explorationActivityPresenter.revealSolution()
@@ -157,16 +158,14 @@ class ExplorationActivity :
 
  override fun routeToHintsAndSolution(
  explorationId: String,
- newAvailableHintIndex: Int,
- allHintsExhausted: Boolean
+ helpIndex: HelpIndex
  ) {
  if (getHintsAndSolution() == null) {
  val hintsAndSolutionDialogFragment = HintsAndSolutionDialogFragment.newInstance(
  explorationId,
- newAvailableHintIndex,
- allHintsExhausted
+ state,
+ helpIndex
  )
- hintsAndSolutionDialogFragment.loadState(state)
  hintsAndSolutionDialogFragment.showNow(supportFragmentManager, TAG_HINTS_AND_SOLUTION_DIALOG)
  }
  }