Fix #3327: Create app layer mechanism for saving checkpoints. (#3402)

* add controller and tests * NIT * added tests for constant * moved enum to a different file * fixed lint * fixed comments * saving * added mech to save checkpoints * lint fix * added module to UI-tests * added exploration storage module to ui tests * nit * added module in missed files * lint fix * fixed saving * added app layer saving mechanism * added app layer saving mechanism * lint fix * fix lint in domain layer * lint fix * nit changes * lint fix * implemented dialog boxes * lint fix * added tests for saving checkpoints * nit changes * lint fix * nit changes * lint fix * lint fix * fixed failing test * imporved approach and added tests * nit changes * nit changes * fixed function names * fixed failing domain tests and some nits * completed saving approach * added more tests and fixed failing tests * checkpointing disabled by default * fix * nit * added more tests * added default value to boolean * improved mechanism to save checkpoints * nit changes * nit * completed app layer implementation * nit * fixed failing test * nit changes * suppressed ExperimantalCoroutinesApi warning * added more tests * nit * nit * nit * nit * nit * nit * nit * removed commit * added more tests * removed visibleForTesting * nit * Revert "removed visibleForTesting" This reverts commit 7963300. * added TODO for the issue * removed git add . * nit * nit * added missing module in develop * nit * nit * reverted changes back in StateDeck.kt * fixed failing tests * fixed failing test * fixed failing tests * added more tests * nit * added tests and exempted ui files * nit * nit * changed approach to stop exploration and nit changes * nit * nit * changed approach to stop exploration and nits * lint fix * nit * nits in comments * nit * applied suggestion in stateDeck * nit * nit * nit * nit * fixed dialog box * disabled checkpointing, it was enabled for testing locally
oppia · Jul 16, 2021 · b5697a0 · b5697a0
1 parent c325fc8
commit b5697a0
Show file tree

Hide file tree

Showing 33 changed files with 1,475 additions and 95 deletions.
diff --git a/app/src/main/java/org/oppia/android/app/home/RouteToExplorationListener.kt b/app/src/main/java/org/oppia/android/app/home/RouteToExplorationListener.kt
@@ -7,6 +7,7 @@ interface RouteToExplorationListener {
     topicId: String,
     storyId: String,
     explorationId: String,
-    backflowScreen: Int?
+    backflowScreen: Int?,
+    isCheckpointingEnabled: Boolean
   )
 }
diff --git a/app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedActivity.kt b/app/src/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedActivity.kt
@@ -42,7 +42,8 @@ class RecentlyPlayedActivity : InjectableAppCompatActivity(), RouteToExploration
     topicId: String,
     storyId: String,
     explorationId: String,
-    backflowScreen: Int?
+    backflowScreen: Int?,
+    isCheckpointingEnabled: Boolean
   ) {
     startActivity(
       ExplorationActivity.createExplorationActivityIntent(
@@ -51,7 +52,8 @@ class RecentlyPlayedActivity : InjectableAppCompatActivity(), RouteToExploration
         topicId,
         storyId,
         explorationId,
-        backflowScreen
+        backflowScreen,
+        isCheckpointingEnabled
       )
     )
   }

diff --git a/...rc/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedFragmentPresenter.kt b/...rc/main/java/org/oppia/android/app/home/recentlyplayed/RecentlyPlayedFragmentPresenter.kt
@@ -242,7 +242,8 @@ class RecentlyPlayedFragmentPresenter @Inject constructor(
               topicId,
               storyId,
               explorationId,
-              /* backflowScreen = */ null
+              /* backflowScreen = */ null,
+              isCheckpointingEnabled = false
             )
             activity.finish()
           }

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
@@ -16,18 +16,16 @@ import org.oppia.android.app.model.State
 import org.oppia.android.app.player.audio.AudioButtonListener
 import org.oppia.android.app.player.state.listener.RouteToHintsAndSolutionListener
 import org.oppia.android.app.player.state.listener.StateKeyboardButtonListener
-import org.oppia.android.app.player.stopplaying.StopExplorationDialogFragment
-import org.oppia.android.app.player.stopplaying.StopStatePlayingSessionListener
+import org.oppia.android.app.player.stopplaying.StopStatePlayingSessionWithSavedProgressListener
 import org.oppia.android.app.topic.conceptcard.ConceptCardListener
 import javax.inject.Inject
 
-private const val TAG_STOP_EXPLORATION_DIALOG = "STOP_EXPLORATION_DIALOG"
 const val TAG_HINTS_AND_SOLUTION_DIALOG = "HINTS_AND_SOLUTION_DIALOG"
 
 /** The starting point for exploration. */
 class ExplorationActivity :
   InjectableAppCompatActivity(),
-  StopStatePlayingSessionListener,
+  StopStatePlayingSessionWithSavedProgressListener,
   StateKeyboardButtonListener,
   AudioButtonListener,
   HintsAndSolutionListener,
@@ -46,6 +44,7 @@ class ExplorationActivity :
   private lateinit var explorationId: String
   private lateinit var state: State
   private var backflowScreen: Int? = null
+  private var isCheckpointingEnabled: Boolean = false
 
   override fun onCreate(savedInstanceState: Bundle?) {
     super.onCreate(savedInstanceState)
@@ -55,13 +54,16 @@ class ExplorationActivity :
     storyId = intent.getStringExtra(EXPLORATION_ACTIVITY_STORY_ID_ARGUMENT_KEY)
     explorationId = intent.getStringExtra(EXPLORATION_ACTIVITY_EXPLORATION_ID_ARGUMENT_KEY)
     backflowScreen = intent.getIntExtra(EXPLORATION_ACTIVITY_BACKFLOW_SCREEN_KEY, -1)
+    isCheckpointingEnabled =
+      intent.getBooleanExtra(EXPLORATION_ACTIVITY_IS_CHECKPOINTING_ENABLED_KEY, false)
     explorationActivityPresenter.handleOnCreate(
       this,
       internalProfileId,
       topicId,
       storyId,
       explorationId,
-      backflowScreen
+      backflowScreen,
+      isCheckpointingEnabled
     )
   }
 
@@ -77,40 +79,39 @@ class ExplorationActivity :
       "ExplorationActivity.exploration_id"
     const val EXPLORATION_ACTIVITY_BACKFLOW_SCREEN_KEY =
       "ExplorationActivity.backflow_screen"
+    const val EXPLORATION_ACTIVITY_IS_CHECKPOINTING_ENABLED_KEY =
+      "ExplorationActivity.is_checkpointing_enabled_key"
 
     fun createExplorationActivityIntent(
       context: Context,
       profileId: Int,
       topicId: String,
       storyId: String,
       explorationId: String,
-      backflowScreen: Int?
+      backflowScreen: Int?,
+      isCheckpointingEnabled: Boolean
     ): Intent {
       val intent = Intent(context, ExplorationActivity::class.java)
       intent.putExtra(EXPLORATION_ACTIVITY_PROFILE_ID_ARGUMENT_KEY, profileId)
       intent.putExtra(EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY, topicId)
       intent.putExtra(EXPLORATION_ACTIVITY_STORY_ID_ARGUMENT_KEY, storyId)
       intent.putExtra(EXPLORATION_ACTIVITY_EXPLORATION_ID_ARGUMENT_KEY, explorationId)
       intent.putExtra(EXPLORATION_ACTIVITY_BACKFLOW_SCREEN_KEY, backflowScreen)
+      intent.putExtra(EXPLORATION_ACTIVITY_IS_CHECKPOINTING_ENABLED_KEY, isCheckpointingEnabled)
       return intent
     }
   }
 
   override fun onBackPressed() {
-    showStopExplorationDialogFragment()
+    explorationActivityPresenter.backButtonPressed()
   }
 
-  private fun showStopExplorationDialogFragment() {
-    val previousFragment = supportFragmentManager.findFragmentByTag(TAG_STOP_EXPLORATION_DIALOG)
-    if (previousFragment != null) {
-      supportFragmentManager.beginTransaction().remove(previousFragment).commitNow()
-    }
-    val dialogFragment = StopExplorationDialogFragment.newInstance()
-    dialogFragment.showNow(supportFragmentManager, TAG_STOP_EXPLORATION_DIALOG)
+  override fun deleteCurrentProgressAndStopSession() {
+    explorationActivityPresenter.deleteCurrentProgressAndStopExploration()
   }
 
-  override fun stopSession() {
-    explorationActivityPresenter.stopExploration()
+  override fun deleteOldestProgressAndStopSession() {
+    explorationActivityPresenter.deleteOldestSavedProgressAndStopExploration()
   }
 
   override fun onCreateOptionsMenu(menu: Menu?): Boolean {

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
@@ -14,9 +14,13 @@ import androidx.lifecycle.Transformations
 import org.oppia.android.R
 import org.oppia.android.app.activity.ActivityScope
 import org.oppia.android.app.help.HelpActivity
+import org.oppia.android.app.model.CheckpointState
 import org.oppia.android.app.model.Exploration
+import org.oppia.android.app.model.ProfileId
 import org.oppia.android.app.model.ReadingTextSize
 import org.oppia.android.app.options.OptionsActivity
+import org.oppia.android.app.player.stopplaying.ProgressDatabaseFullDialogFragment
+import org.oppia.android.app.player.stopplaying.UnsavedExplorationDialogFragment
 import org.oppia.android.app.topic.TopicActivity
 import org.oppia.android.app.utility.FontScaleConfigurationUtil
 import org.oppia.android.app.viewmodel.ViewModelProvider
@@ -27,8 +31,11 @@ import org.oppia.android.util.data.AsyncResult
 import org.oppia.android.util.data.DataProviders.Companion.toLiveData
 import javax.inject.Inject
 
-const val TAG_EXPLORATION_FRAGMENT = "TAG_EXPLORATION_FRAGMENT"
-const val TAG_EXPLORATION_MANAGER_FRAGMENT = "TAG_EXPLORATION_MANAGER_FRAGMENT"
+private const val TAG_UNSAVED_EXPLORATION_DIALOG = "UNSAVED_EXPLORATION_DIALOG"
+private const val TAG_STOP_EXPLORATION_DIALOG = "STOP_EXPLORATION_DIALOG"
+private const val TAG_PROGRESS_DATABASE_FULL_DIALOG = "PROGRESS_DATABASE_FULL_DIALOG"
+private const val TAG_EXPLORATION_FRAGMENT = "TAG_EXPLORATION_FRAGMENT"
+private const val TAG_EXPLORATION_MANAGER_FRAGMENT = "TAG_EXPLORATION_MANAGER_FRAGMENT"
 const val TAG_HINTS_AND_SOLUTION_EXPLORATION_MANAGER = "HINTS_AND_SOLUTION_EXPLORATION_MANAGER"
 
 /** The Presenter for [ExplorationActivity]. */
@@ -49,6 +56,11 @@ class ExplorationActivityPresenter @Inject constructor(
   private lateinit var context: Context
   private var backflowScreen: Int? = null
 
+  private var isCheckpointingEnabled: Boolean = false
+
+  private lateinit var oldestCheckpointExplorationId: String
+  private lateinit var oldestCheckpointExplorationTitle: String
+
   enum class ParentActivityForExploration(val value: Int) {
     BACKFLOW_SCREEN_LESSONS(0),
     BACKFLOW_SCREEN_STORY(1);
@@ -64,7 +76,8 @@ class ExplorationActivityPresenter @Inject constructor(
     topicId: String,
     storyId: String,
     explorationId: String,
-    backflowScreen: Int?
+    backflowScreen: Int?,
+    isCheckpointingEnabled: Boolean
   ) {
     val binding = DataBindingUtil.setContentView<ExplorationActivityBinding>(
       activity,
@@ -98,6 +111,11 @@ class ExplorationActivityPresenter @Inject constructor(
     this.explorationId = explorationId
     this.context = context
     this.backflowScreen = backflowScreen
+    this.isCheckpointingEnabled = isCheckpointingEnabled
+
+    // Retrieve oldest saved checkpoint details.
+    subscribeToOldestSavedExplorationDetails()
+
     if (getExplorationManagerFragment() == null) {
       val explorationManagerFragment = ExplorationManagerFragment()
       val args = Bundle()
@@ -195,6 +213,29 @@ class ExplorationActivityPresenter @Inject constructor(
     ) as HintsAndSolutionExplorationManagerFragment?
   }
 
+  /** Deletes the saved progress for the current exploration and then stops the exploration. */
+  fun deleteCurrentProgressAndStopExploration() {
+    explorationDataController.deleteExplorationProgressById(
+      ProfileId.newBuilder().setInternalId(internalProfileId).build(),
+      explorationId
+    )
+    stopExploration()
+  }
+
+  /** Deletes the oldest saved checkpoint and then stops the exploration. */
+  fun deleteOldestSavedProgressAndStopExploration() {
+    // If oldestCheckpointExplorationId is not initialized, it means that there was an error while
+    // retrieving the oldest saved checkpoint details. In this case, the exploration is exited
+    // without deleting the any checkpoints.
+    oldestCheckpointExplorationId.let {
+      explorationDataController.deleteExplorationProgressById(
+        ProfileId.newBuilder().setInternalId(internalProfileId).build(),
+        oldestCheckpointExplorationId
+      )
+    }
+    stopExploration()
+  }
+
   fun stopExploration() {
     fontScaleConfigurationUtil.adjustFontScale(activity, ReadingTextSize.MEDIUM_TEXT_SIZE.name)
     explorationDataController.stopPlayingExploration()
@@ -227,6 +268,25 @@ class ExplorationActivityPresenter @Inject constructor(
     }
   }
 
+  /**
+   * Shows an appropriate dialog box or exits the exploration directly without showing any dialog
+   * box when back button is pressed. This function shows [UnsavedExplorationDialogFragment] if
+   * checkpointing is not enabled otherwise it either exits the exploration or shows
+   * [ProgressDatabaseFullDialogFragment] depending upon the state of the saved checkpoint for the
+   * current exploration.
+   */
+  fun backButtonPressed() {
+    // If checkpointing is not enabled, show StopExplorationDialogFragment to exit the exploration,
+    // this is expected to happen if the exploration is marked as completed.
+    if (!isCheckpointingEnabled) {
+      showUnsavedExplorationDialogFragment()
+      return
+    }
+    // If checkpointing is enabled, get the current checkpoint state to show an appropriate dialog
+    // fragment.
+    showDialogFragmentBasedOnCurrentCheckpointState()
+  }
+
   fun dismissConceptCard() {
     getExplorationFragment()?.dismissConceptCard()
   }
@@ -299,4 +359,107 @@ class ExplorationActivityPresenter @Inject constructor(
       ) as ExplorationFragment
     explorationFragment.revealSolution()
   }
+
+  private fun showProgressDatabaseFullDialogFragment() {
+    val previousFragment = activity.supportFragmentManager.findFragmentByTag(
+      TAG_PROGRESS_DATABASE_FULL_DIALOG
+    )
+    if (previousFragment != null) {
+      activity.supportFragmentManager.beginTransaction().remove(previousFragment).commitNow()
+    }
+    // If any one of oldestCheckpointExplorationId or oldestCheckpointExplorationTitle is not
+    // initialized, it means that there was an error while retrieving the oldest saved checkpoint
+    // details. In that case the exploration will be exited without deleting the any checkpoints.
+    if (
+      !::oldestCheckpointExplorationId.isInitialized ||
+      !::oldestCheckpointExplorationTitle.isInitialized
+    ) {
+      stopExploration()
+      return
+    }
+
+    val dialogFragment =
+      ProgressDatabaseFullDialogFragment.newInstance(oldestCheckpointExplorationTitle)
+    dialogFragment.showNow(
+      activity.supportFragmentManager,
+      TAG_PROGRESS_DATABASE_FULL_DIALOG
+    )
+  }
+
+  private fun showUnsavedExplorationDialogFragment() {
+    val previousFragment =
+      activity.supportFragmentManager.findFragmentByTag(TAG_UNSAVED_EXPLORATION_DIALOG)
+    if (previousFragment != null) {
+      activity.supportFragmentManager.beginTransaction().remove(previousFragment).commitNow()
+    }
+    val dialogFragment = UnsavedExplorationDialogFragment.newInstance()
+    dialogFragment.showNow(
+      activity.supportFragmentManager,
+      TAG_UNSAVED_EXPLORATION_DIALOG
+    )
+  }
+
+  /**
+   * Listens to the result of [ExplorationDataController.getOldestExplorationDetailsDataProvider].
+   *
+   * If the result is success it updates the value of the variables oldestCheckpointExplorationId
+   * and oldestCheckpointExplorationTitle. If the result fails, it does not initializes the
+   * variables oldestCheckpointExplorationId and oldestCheckpointExplorationTitle with any value.
+   *
+   * Since this function is kicked off before any other save operation, therefore it is expected
+   * to complete before any following save operation completes.
+   *
+   * If operations fails or this function does not get enough time to complete, user is not blocked
+   * instead the flow of the application proceeds as if the checkpoints were not found. In that case,
+   * the variables oldestCheckpointExplorationId and oldestCheckpointExplorationTitle are not
+   * initialized and they remain uninitialized.
+   */
+  private fun subscribeToOldestSavedExplorationDetails() {
+    explorationDataController.getOldestExplorationDetailsDataProvider(
+      ProfileId.newBuilder().setInternalId(internalProfileId).build()
+    ).toLiveData().observe(
+      activity,
+      Observer {
+        if (it.isSuccess()) {
+          oldestCheckpointExplorationId = it.getOrThrow().explorationId
+          oldestCheckpointExplorationTitle = it.getOrThrow().explorationTitle
+        } else if (it.isFailure()) {
+          oppiaLogger.e(
+            "ExplorationActivity",
+            "Failed to retrieve oldest saved checkpoint details.",
+            it.getErrorOrNull()
+          )
+        }
+      }
+    )
+  }
+
+  /**
+   * Checks the checkpointState for the current exploration and shows an appropriate dialog
+   * fragment.
+   *
+   * If the checkpointState is equal to CHECKPOINT_SAVED_DATABASE_NOT_EXCEEDED_LIMIT,
+   * exploration will be stopped without showing any dialogFragment. If the checkpointState is equal
+   * to CHECKPOINT_SAVED_DATABASE_EXCEEDED_LIMIT, [ProgressDatabaseFullDialogFragment] will be
+   * displayed to the user. Otherwise, the dialog fragment [UnsavedExplorationDialogFragment] will
+   * be displayed to the user.
+   */
+  private fun showDialogFragmentBasedOnCurrentCheckpointState() {
+    val checkpointState = getExplorationFragment()?.getExplorationCheckpointState()
+
+    // Show UnsavedExplorationDialogFragment if checkpoint state could not be retrieved.
+    if (checkpointState == null) {
+      showUnsavedExplorationDialogFragment()
+    } else {
+      when (checkpointState) {
+        CheckpointState.CHECKPOINT_SAVED_DATABASE_NOT_EXCEEDED_LIMIT -> {
+          stopExploration()
+        }
+        CheckpointState.CHECKPOINT_SAVED_DATABASE_EXCEEDED_LIMIT -> {
+          showProgressDatabaseFullDialogFragment()
+        }
+        else -> showUnsavedExplorationDialogFragment()
+      }
+    }
+  }
 }
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
@@ -32,7 +32,7 @@ import org.oppia.android.app.player.state.ConfettiConfig.LARGE_CONFETTI_BURST
 import org.oppia.android.app.player.state.ConfettiConfig.MEDIUM_CONFETTI_BURST
 import org.oppia.android.app.player.state.ConfettiConfig.MINI_CONFETTI_BURST
 import org.oppia.android.app.player.state.listener.RouteToHintsAndSolutionListener
-import org.oppia.android.app.player.stopplaying.StopStatePlayingSessionListener
+import org.oppia.android.app.player.stopplaying.StopStatePlayingSessionWithSavedProgressListener
 import org.oppia.android.app.topic.conceptcard.ConceptCardFragment.Companion.CONCEPT_CARD_DIALOG_FRAGMENT_TAG
 import org.oppia.android.app.utility.SplitScreenManager
 import org.oppia.android.app.viewmodel.ViewModelProvider
@@ -179,7 +179,8 @@ class StateFragmentPresenter @Inject constructor(
   fun onReturnToTopicButtonClicked() {
     hideKeyboard()
     markExplorationCompleted()
-    (activity as StopStatePlayingSessionListener).stopSession()
+    (activity as StopStatePlayingSessionWithSavedProgressListener)
+      .deleteCurrentProgressAndStopSession()
   }
 
   private fun showOrHideAudioByState(state: State) {