From 3db1aaeb794d6f1ec7c9176b210abdf2f39652db Mon Sep 17 00:00:00 2001 From: Rajat Talesra Date: Wed, 9 Oct 2019 14:39:24 +0530 Subject: [PATCH] Introduce load exploration part 1 (#213) * Introduce first pass interface for ExplorationProgressController. * Fill in the stubbed logic for ExplorationProgressController. Still no tests to verify correctness. Also, added a method to facilitate notifying of DataProvider changes on the UI thread. * Fix lateinit issue in ExplorationProgressController due to wrongly ordered initialization. * Fix a variaty of issues in the exp progress controller, properly hook it up to the data controller, and start adding tests. * Created a separate ExplorationRetriever, hooked up AnswerClassificationController, and attempted to make ExplorationProgressController thread-safe. The thread-safety led to significant interface changes in the progress controller, and led to discovering some issues with the mediator live data approach to interop coroutines and LiveData. This locking mechanism will need to change since the optimal solution requires resolving #90. * Change the locking mechanism for ExplorationProgressController to work with the current MediatorLiveData implementation (see #90 for more context). Fix existing progress controller tests and add a few more. All current progress controller tests are passing. * Finish tests for ExplorationProgressController and add test classification support for the second test exploration (about_oppia). * ObservableViewModel * Load exploration and open ExplorationActivity * Test case for load exploration * Connection with StateFragment * Nit changes self review * Refactor home * Resolve merge conflicts * Test case updated --- .../java/org/oppia/app/home/HomeActivity.kt | 7 ++- .../oppia/app/home/HomeFragmentPresenter.kt | 32 ++++++++++++- .../app/home/RouteToExplorationListener.kt | 6 +++ .../player/exploration/ExplorationActivity.kt | 16 ++++++- .../player/state/StateFragmentPresenter.kt | 35 +++++++++++++- .../main/res/layout/exploration_fragment.xml | 9 ++-- app/src/main/res/layout/home_activity.xml | 12 ++--- app/src/main/res/layout/home_fragment.xml | 17 +++++-- .../org/oppia/app/home/HomeActivityTest.kt | 48 +++++++++++++------ 9 files changed, 148 insertions(+), 34 deletions(-) create mode 100755 app/src/main/java/org/oppia/app/home/RouteToExplorationListener.kt diff --git a/app/src/main/java/org/oppia/app/home/HomeActivity.kt b/app/src/main/java/org/oppia/app/home/HomeActivity.kt index c3a8d1f699f..149eaf1e3c3 100644 --- a/app/src/main/java/org/oppia/app/home/HomeActivity.kt +++ b/app/src/main/java/org/oppia/app/home/HomeActivity.kt @@ -2,10 +2,11 @@ package org.oppia.app.home import android.os.Bundle import org.oppia.app.activity.InjectableAppCompatActivity +import org.oppia.app.player.exploration.ExplorationActivity import javax.inject.Inject /** The central activity for all users entering the app. */ -class HomeActivity : InjectableAppCompatActivity() { +class HomeActivity : InjectableAppCompatActivity(), RouteToExplorationListener { @Inject lateinit var homeActivityPresenter: HomeActivityPresenter override fun onCreate(savedInstanceState: Bundle?) { @@ -13,4 +14,8 @@ class HomeActivity : InjectableAppCompatActivity() { activityComponent.inject(this) homeActivityPresenter.handleOnCreate() } + + override fun routeToExploration(explorationId: String) { + startActivity(ExplorationActivity.createExplorationActivityIntent(this, explorationId)) + } } diff --git a/app/src/main/java/org/oppia/app/home/HomeFragmentPresenter.kt b/app/src/main/java/org/oppia/app/home/HomeFragmentPresenter.kt index 688f86a1123..cac6ffe695a 100644 --- a/app/src/main/java/org/oppia/app/home/HomeFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/app/home/HomeFragmentPresenter.kt @@ -3,26 +3,41 @@ package org.oppia.app.home import android.view.LayoutInflater import android.view.View import android.view.ViewGroup +import androidx.appcompat.app.AppCompatActivity import androidx.fragment.app.Fragment +import androidx.lifecycle.Observer import org.oppia.app.databinding.HomeFragmentBinding import org.oppia.app.fragment.FragmentScope import org.oppia.app.viewmodel.ViewModelProvider import org.oppia.domain.UserAppHistoryController +import org.oppia.domain.exploration.ExplorationDataController +import org.oppia.domain.exploration.TEST_EXPLORATION_ID_5 +import org.oppia.util.data.AsyncResult +import org.oppia.util.logging.Logger import javax.inject.Inject +private const val EXPLORATION_ID = TEST_EXPLORATION_ID_5 + /** The controller for [HomeFragment]. */ @FragmentScope class HomeFragmentPresenter @Inject constructor( + activity: AppCompatActivity, private val fragment: Fragment, private val viewModelProvider: ViewModelProvider, - private val userAppHistoryController: UserAppHistoryController + private val userAppHistoryController: UserAppHistoryController, + private val explorationDataController: ExplorationDataController, + private val logger: Logger ) { + + private val routeToExplorationListener = activity as RouteToExplorationListener + fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? { val binding = HomeFragmentBinding.inflate(inflater, container, /* attachToRoot= */ false) // NB: Both the view model and lifecycle owner must be set in order to correctly bind LiveData elements to // data-bound view models. binding.let { it.viewModel = getUserAppHistoryViewModel() + it.presenter = this it.lifecycleOwner = fragment } @@ -34,4 +49,19 @@ class HomeFragmentPresenter @Inject constructor( private fun getUserAppHistoryViewModel(): UserAppHistoryViewModel { return viewModelProvider.getForFragment(fragment, UserAppHistoryViewModel::class.java) } + + fun playExplorationButton(v: View) { + explorationDataController.startPlayingExploration( + EXPLORATION_ID + ).observe(fragment, Observer> { result -> + when { + result.isPending() -> logger.d("HomeFragment", "Loading exploration") + result.isFailure() -> logger.e("HomeFragment", "Failed to load exploration", result.getErrorOrNull()!!) + else -> { + logger.d("HomeFragment", "Successfully loaded exploration") + routeToExplorationListener.routeToExploration(EXPLORATION_ID) + } + } + }) + } } diff --git a/app/src/main/java/org/oppia/app/home/RouteToExplorationListener.kt b/app/src/main/java/org/oppia/app/home/RouteToExplorationListener.kt new file mode 100755 index 00000000000..06eb553ebd7 --- /dev/null +++ b/app/src/main/java/org/oppia/app/home/RouteToExplorationListener.kt @@ -0,0 +1,6 @@ +package org.oppia.app.home + +/** Listener for when an activity should route to a exploration. */ +interface RouteToExplorationListener { + fun routeToExploration(explorationId: String) +} diff --git a/app/src/main/java/org/oppia/app/player/exploration/ExplorationActivity.kt b/app/src/main/java/org/oppia/app/player/exploration/ExplorationActivity.kt index 5df1bd8884c..caf6374f3ed 100755 --- a/app/src/main/java/org/oppia/app/player/exploration/ExplorationActivity.kt +++ b/app/src/main/java/org/oppia/app/player/exploration/ExplorationActivity.kt @@ -1,16 +1,30 @@ package org.oppia.app.player.exploration +import android.content.Context +import android.content.Intent import android.os.Bundle import org.oppia.app.activity.InjectableAppCompatActivity import javax.inject.Inject +const val EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY = "ExplorationActivity.exploration_id" + /** The starting point for exploration. */ class ExplorationActivity : InjectableAppCompatActivity() { - @Inject lateinit var explorationActivityPresenter: ExplorationActivityPresenter + @Inject + lateinit var explorationActivityPresenter: ExplorationActivityPresenter override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) activityComponent.inject(this) explorationActivityPresenter.handleOnCreate() } + + companion object { + /** Returns a new [Intent] to route to [ExplorationActivity] for a specified topic ID. */ + fun createExplorationActivityIntent(context: Context, explorationId: String): Intent { + val intent = Intent(context, ExplorationActivity::class.java) + intent.putExtra(EXPLORATION_ACTIVITY_TOPIC_ID_ARGUMENT_KEY, explorationId) + return intent + } + } } diff --git a/app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt b/app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt index f5a2d663fd0..45a30923e1a 100755 --- a/app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt +++ b/app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt @@ -4,14 +4,19 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import androidx.fragment.app.Fragment +import androidx.lifecycle.LiveData import androidx.lifecycle.Observer +import androidx.lifecycle.Transformations import org.oppia.app.databinding.StateFragmentBinding import org.oppia.app.fragment.FragmentScope import org.oppia.app.model.CellularDataPreference +import org.oppia.app.model.EphemeralState import org.oppia.app.player.audio.CellularDataDialogFragment import org.oppia.app.viewmodel.ViewModelProvider import org.oppia.domain.audio.CellularDialogController +import org.oppia.domain.exploration.ExplorationProgressController import org.oppia.util.data.AsyncResult +import org.oppia.util.logging.Logger import javax.inject.Inject private const val TAG_CELLULAR_DATA_DIALOG = "CELLULAR_DATA_DIALOG" @@ -21,7 +26,9 @@ private const val TAG_CELLULAR_DATA_DIALOG = "CELLULAR_DATA_DIALOG" class StateFragmentPresenter @Inject constructor( private val fragment: Fragment, private val cellularDialogController: CellularDialogController, - private val viewModelProvider: ViewModelProvider + private val viewModelProvider: ViewModelProvider, + private val explorationProgressController: ExplorationProgressController, + private val logger: Logger ) { private var showCellularDataDialog = true @@ -29,7 +36,7 @@ class StateFragmentPresenter @Inject constructor( fun handleCreateView(inflater: LayoutInflater, container: ViewGroup?): View? { cellularDialogController.getCellularDataPreference() - .observe(fragment, Observer>{ + .observe(fragment, Observer> { if (it.isSuccess()) { val prefs = it.getOrDefault(CellularDataPreference.getDefaultInstance()) showCellularDataDialog = !(prefs.hideDialog) @@ -42,6 +49,9 @@ class StateFragmentPresenter @Inject constructor( it.stateFragment = fragment as StateFragment it.viewModel = getStateViewModel() } + + subscribeToCurrentState() + return binding.root } @@ -81,4 +91,25 @@ class StateFragmentPresenter @Inject constructor( fun setAudioFragmentVisible(isVisible: Boolean) { getStateViewModel().setAudioFragmentVisible(isVisible) } + + private fun subscribeToCurrentState() { + ephemeralStateLiveData.observe(fragment, Observer { result -> + logger.d("StateFragment", "getCurrentState: ${result.state.name}") + }) + } + + private val ephemeralStateLiveData: LiveData by lazy { + getEphemeralState() + } + + private fun getEphemeralState(): LiveData { + return Transformations.map(explorationProgressController.getCurrentState(), ::processCurrentState) + } + + private fun processCurrentState(ephemeralStateResult: AsyncResult): EphemeralState { + if (ephemeralStateResult.isFailure()) { + logger.e("StateFragment", "Failed to retrieve ephemeral state", ephemeralStateResult.getErrorOrNull()!!) + } + return ephemeralStateResult.getOrDefault(EphemeralState.getDefaultInstance()) + } } diff --git a/app/src/main/res/layout/exploration_fragment.xml b/app/src/main/res/layout/exploration_fragment.xml index ce663113c6c..231f8162b38 100755 --- a/app/src/main/res/layout/exploration_fragment.xml +++ b/app/src/main/res/layout/exploration_fragment.xml @@ -6,14 +6,15 @@ android:layout_width="match_parent" android:layout_height="match_parent"> - + diff --git a/app/src/main/res/layout/home_activity.xml b/app/src/main/res/layout/home_activity.xml index 86fdf2ff0a0..7b1a54c0c3d 100644 --- a/app/src/main/res/layout/home_activity.xml +++ b/app/src/main/res/layout/home_activity.xml @@ -1,8 +1,8 @@ +xmlns:android="http://schemas.android.com/apk/res/android" +xmlns:tools="http://schemas.android.com/tools" +android:id="@+id/home_fragment_placeholder" +android:layout_width="match_parent" +android:layout_height="match_parent" +tools:context=".home.HomeActivity" /> diff --git a/app/src/main/res/layout/home_fragment.xml b/app/src/main/res/layout/home_fragment.xml index 45e60f50205..a7d2e051f24 100644 --- a/app/src/main/res/layout/home_fragment.xml +++ b/app/src/main/res/layout/home_fragment.xml @@ -2,18 +2,18 @@ - + - - - +