Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #4652 : Implemented Learner Progress for Exploration State #5388

Closed
wants to merge 1 commit into from
Closed

Fix #4652 : Implemented Learner Progress for Exploration State #5388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.app.utility.SplitScreenManager
import org.oppia.android.app.utility.lifecycle.LifecycleSafeTimerFactory
import org.oppia.android.databinding.StateFragmentBinding
import org.oppia.android.domain.exploration.ExplorationRetriever
import org.oppia.android.domain.exploration.ExplorationProgressController
import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.domain.survey.SurveyGatingController
Expand All @@ -54,6 +55,7 @@ import org.oppia.android.util.gcsresource.DefaultResourceBucketName
import org.oppia.android.util.parser.html.ExplorationHtmlParserEntityType
import org.oppia.android.util.system.OppiaClock
import javax.inject.Inject
import kotlinx.coroutines.runBlocking

const val STATE_FRAGMENT_PROFILE_ID_ARGUMENT_KEY =
"StateFragmentPresenter.state_fragment_profile_id"
Expand All @@ -71,6 +73,7 @@ class StateFragmentPresenter @Inject constructor(
private val fragment: Fragment,
private val context: Context,
private val lifecycleSafeTimerFactory: LifecycleSafeTimerFactory,
private val explorationRetriever: ExplorationRetriever,
private val explorationProgressController: ExplorationProgressController,
private val storyProgressController: StoryProgressController,
private val oppiaLogger: OppiaLogger,
Expand Down Expand Up @@ -99,6 +102,7 @@ class StateFragmentPresenter @Inject constructor(
private var forceAnnouncedForHintsBar = false

private lateinit var recyclerViewAssembler: StatePlayerRecyclerViewAssembler
private lateinit var explorationStateOrderData: Pair<Int, MutableMap<String, Int>>
private val ephemeralStateLiveData: LiveData<AsyncResult<EphemeralState>> by lazy {
explorationProgressController.getCurrentState().toLiveData()
}
Expand All @@ -124,6 +128,9 @@ class StateFragmentPresenter @Inject constructor(
container,
/* attachToRoot= */ false
)

explorationStateOrderData = fetchExplorationStateOrder(explorationId)

recyclerViewAssembler = createRecyclerViewAssembler(
assemblerBuilderFactory.create(resourceBucketName, entityType, profileId),
binding.congratulationsTextView,
Expand Down Expand Up @@ -294,6 +301,18 @@ class StateFragmentPresenter @Inject constructor(
)
}

private fun fetchExplorationStateOrder(explorationId: String): Pair<Int, MutableMap<String, Int>> {
var explorationStateOrderResult: Pair<Int, MutableMap<String, Int>>
runBlocking {
explorationStateOrderResult = loadExplorationStateOrderAsync(explorationId)
}
return explorationStateOrderResult
}

private suspend fun loadExplorationStateOrderAsync(explorationId: String): Pair<Int, MutableMap<String, Int>> {
return explorationRetriever.loadExplorationPosition(explorationId)
}

private fun processEphemeralStateResult(result: AsyncResult<EphemeralState>) {
when (result) {
is AsyncResult.Failure ->
Expand All @@ -320,6 +339,8 @@ class StateFragmentPresenter @Inject constructor(
currentState = ephemeralState.state
currentStateName = ephemeralState.state.name

updateStateProgress(ephemeralState.state)

showOrHideAudioByState(ephemeralState.state)

val dataPair = recyclerViewAssembler.compute(
Expand All @@ -341,6 +362,27 @@ class StateFragmentPresenter @Inject constructor(
}
}

private fun updateStateProgress(state: State) {
val currentStateName = state.name
val currentPosition = getPositionOfState(currentStateName, explorationStateOrderData)
val totalStates = explorationStateOrderData.first

var stateProgress = calculateStateProgress(currentPosition, totalStates)
viewModel.setStateProgress(stateProgress)
}

private fun getPositionOfState(currentStateName: String?, explorationStateOrderData: Pair<Int, MutableMap<String, Int>>): Int? {
return explorationStateOrderData.second[currentStateName]
}

private fun calculateStateProgress(currentPosition: Int?, totalStates: Int): Int {
var calculatedStateProgress : Double = 0.0
if (currentPosition != null) {
calculatedStateProgress = ((currentPosition.toDouble() / totalStates) * 100)
}
return calculatedStateProgress.toInt()
}

/** Subscribes to the result of requesting to show a hint or solution. */
private fun subscribeToHintSolution(resultDataProvider: DataProvider<Any?>) {
resultDataProvider.toLiveData().observe(
Expand Down
6 changes: 6 additions & 0 deletions app/src/main/java/org/oppia/android/app/player/state/StateViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ class StateViewModel @Inject constructor(
val itemList: ObservableList<StateItemViewModel> = ObservableArrayList()
val rightItemList: ObservableList<StateItemViewModel> = ObservableArrayList()

var stateProgress = ObservableField(0)

val isSplitView = ObservableField(false)
val centerGuidelinePercentage = ObservableField(0.5f)

Expand Down Expand Up @@ -83,6 +85,10 @@ class StateViewModel @Inject constructor(
this.profileId = profileId
}

fun setStateProgress(progress: Int) {
stateProgress.set(progress)
}

fun setAudioBarVisibility(audioBarVisible: Boolean) {
isAudioBarVisible.set(audioBarVisible)
}
Expand Down
20 changes: 18 additions & 2 deletions app/src/main/res/layout/state_fragment.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,22 @@
android:layout_width="match_parent"
android:layout_height="match_parent">

<ProgressBar
android:id="@+id/state_progress_bar"
style="@style/Widget.AppCompat.ProgressBar.Horizontal"
android:layout_width="0dp"
android:layout_height="12dp"
android:max="100"
android:progress="@{viewModel.stateProgress}"
android:progressDrawable="@drawable/progress_bar"
android:layout_marginStart="32dp"
android:layout_marginEnd="32dp"
android:layout_marginTop="12dp"
android:layout_marginBottom="12dp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"/>

<androidx.recyclerview.widget.RecyclerView
android:id="@+id/state_recycler_view"
android:layout_width="0dp"
Expand All @@ -39,7 +55,7 @@
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toStartOf="@id/center_guideline"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
app:layout_constraintTop_toBottomOf="@id/state_progress_bar" />

<androidx.recyclerview.widget.RecyclerView
android:id="@+id/extra_interaction_recycler_view"
Expand All @@ -56,7 +72,7 @@
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@id/center_guideline"
app:layout_constraintTop_toTopOf="parent" />
app:layout_constraintTop_toBottomOf="@id/state_progress_bar" />

<androidx.constraintlayout.widget.Guideline
android:id="@+id/center_guideline"
Expand Down
4 changes: 4 additions & 0 deletions domain/src/main/java/org/oppia/android/domain/exploration/ExplorationRetriever.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@ import org.oppia.android.app.model.Exploration
interface ExplorationRetriever {
/** Loads and returns an exploration for the specified exploration ID, or fails. */
suspend fun loadExploration(explorationId: String): Exploration
/** Loads and returns Pair of Total number of states for the specified exploration ID
* and a Mutable Map of State Names and their position in the exploration list
*/
suspend fun loadExplorationPosition(explorationId: String): Pair<Int, MutableMap<String, Int>>
}
42 changes: 42 additions & 0 deletions domain/src/main/java/org/oppia/android/domain/exploration/ExplorationRetrieverImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ class ExplorationRetrieverImpl @Inject constructor(
}
}

override suspend fun loadExplorationPosition(explorationId: String): Pair<Int, MutableMap<String, Int>> {
val explorationObject = jsonAssetRetriever.loadJsonFromAsset("$explorationId.json")
return parseJsonObjectToCalculatePosition(explorationObject!!)
}

// Returns an exploration given an assetName
private fun loadExplorationFromAsset(explorationObject: JSONObject): Exploration {
val innerExplorationObject = explorationObject.getJSONObject("exploration")
Expand Down Expand Up @@ -68,4 +73,41 @@ class ExplorationRetrieverImpl @Inject constructor(
}
return statesMap
}

private fun parseJsonObjectToCalculatePosition(explorationObject: JSONObject): Pair<Int, MutableMap<String, Int>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seanlip has anyone worked on the in-lesson progress bar on web? I'm wondering if anyone has already come up with an algorithm for computing the linear path through the lesson that we could copy here for parity.

@Rd4dev one potential issue I see with this approach is that it's sort of implicitly assuming that each destination is an equal outcome when making progress. So long as we never let the user go backwards in progress, that helps a bit with making sure the progress bar looks correct, but it may be a bit unpredictable since sometimes states with many outcomes will show "more progress" of completing than those with few.

I think the intended way to implement this is to use a pathfinding algorithm from the starting state to all ending states. There should be only one path (but we'll need some sort of tiebreaker if there is more than one). From there, we only count progress for states who have checkpoints marked (this is a property on Oppia web we don't currently import: https://github.com/oppia/oppia/blob/5ebbb3d369532a22ee1638a810d597ffb9719594/core/domain/state_domain.py#L3615).

@seanlip does the algorithm explained in the above paragraph sound correct to you (in the event there isn't an existing implementation to follow)? Also, is the checkpoint field being set in lessons now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenHenning Yes, we have done a bit of work here. We label some states as checkpoints, and use those checkpoints in the lessons as a measure of progress. The checkpoint states are always located at articulation points of the graph (i.e. if you remove the checkpoint state then it disconnects the graph). Lessons have 2-8 checkpoints and progress is measured by the achievement of checkpoints. More granular progress is not recorded.

The checkpoint field is being set in lessons, and the third paragraph you mention seems like a good approach to follow. It does match what we do on Web. You don't need a tiebreaker, given the "articulation points" property above.

Please let me know if you need more info. Also, I apologize for entirely missing this in my previous comment -- for some reason I didn't make the connection between the progress bar and the existing checkpoints functionality on Web.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach sounds good and we should be able to utilize the new field.

val states = explorationObject.getJSONObject("exploration").getJSONObject("states")
val initialState = explorationObject.getJSONObject("exploration").getString("init_state_name")

val statePosition = mutableMapOf<String, Int>()
var position = 0

// Function to recursively traverse the states and set their positions
fun getStatePosition(stateName: String) {
if (!statePosition.containsKey(stateName)) {
statePosition[stateName] = ++position

val state = states.getJSONObject(stateName)
val interaction = state.getJSONObject("interaction")

if (interaction.has("default_outcome") && !interaction.isNull("default_outcome")) {
val defaultOutcome = interaction.getJSONObject("default_outcome")
val dest = defaultOutcome.getString("dest")
getStatePosition(dest)
}

if (interaction.has("answer_groups") && interaction.getJSONArray("answer_groups").length() > 0) {
val answerGroups = interaction.getJSONArray("answer_groups")
for (i in answerGroups.length() - 1 downTo 0) {
val outcome = answerGroups.getJSONObject(i).getJSONObject("outcome")
val dest = outcome.getString("dest")
getStatePosition(dest)
}
}
}
}

getStatePosition(initialState)
return position to statePosition

}
}
5 changes: 5 additions & 0 deletions ...in/src/main/java/org/oppia/android/domain/exploration/testing/FakeExplorationRetriever.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ class FakeExplorationRetriever @Inject constructor(
return productionImpl.loadExploration(expIdToLoad)
}

override suspend fun loadExplorationPosition(explorationId: String): Pair<Int, MutableMap<String, Int>> {
val expIdToLoad = explorationProxies[explorationId] ?: explorationId
return productionImpl.loadExplorationPosition(expIdToLoad)
}

/**
* Sets the exploration ID that should be loaded in place of [expIdToLoad] on all future calls to
* [loadExploration].
Expand Down
Loading