Skip to content

Commit

Permalink
Fix #112: Introduce question progress controller interface (#218)
Browse files Browse the repository at this point in the history
* Initial check in for the question data controller

* Removed progress controller test

* Address the renames suggested in #217.

* Introduce thoroughly stubbed QuestionAssessmentProgressController +
empty test suite.

* Review changes

* Review changes

* Pass data providers to the assessment controller, and set caps in the training controller

* Removed unnecessary imports/variables

* Add fake question data for stubbed interfaces

* Remove duplicate questions while fetching in training controller

* Comment explaining the filter function

* Improve duplicate checking - check it while filtering instead of after filtering

* Add linked skill id values

* Review changes

* add a new module for questions constants

* Review changes

* add a test to verify questions were fetched properly

* reformatted code

* Re-add QuestionTrainingController removed in merge.

* Finalize question progress controller interface.

* Fix typo in QuestionTrainingController.

* Post-merge fixes and adjustments.

* Post-merge fixes & address reviewer comments.

* Remove unneeded files.

* Remove legacy code from tests.
  • Loading branch information
BenHenning authored Nov 20, 2019
1 parent b8100ad commit 5be79f2
Show file tree
Hide file tree
Showing 6 changed files with 491 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,21 +149,13 @@ class ExplorationProgressController @Inject constructor(
}
}

/**
* Navigates to the previous state in the stack. If the learner is currently on the initial state, this method will
* throw an exception. Calling code is responsible to make sure that this method is not called when it's not possible
* to navigate to a previous card.
*
* This method cannot be called until an exploration has started and [getCurrentState] returns a non-pending result or
* an exception will be thrown.
*/
/**
* 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 only called when it's possible to
* navigate backward.
*
* @return a one-time [LiveData] indicating whether the movement to the previous state was successful, or a failure if
* state navigation was attempted at an invalid time in the state graph (e.g. if currently vieiwng the initial
* state navigation was attempted at an invalid time in the state graph (e.g. if currently viewing the initial
* state of the exploration). It's recommended that calling code only listen to this result for failures, and
* instead rely on [getCurrentState] for observing a successful transition to another state.
*/
Expand Down Expand Up @@ -228,9 +220,6 @@ class ExplorationProgressController @Inject constructor(
* corresponds to a a terminal state, then the learner has completed the exploration. Note that [moveToPreviousState]
* and [moveToNextState] will automatically update observers of this live data when the next state is navigated to.
*
* Note that the returned [LiveData] is always the same object no matter when this method is called, except
* potentially when a new exploration is started.
*
* This [LiveData] may initially be pending while the exploration object is loaded. It may also switch from a
* completed to a pending result during transient operations like submitting an answer via [submitAnswer]. Calling
* code should be made resilient to this by caching the current state object to display since it may disappear
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,174 @@
package org.oppia.domain.question

import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import org.oppia.app.model.AnsweredQuestionOutcome
import org.oppia.app.model.EphemeralQuestion
import org.oppia.app.model.EphemeralState
import org.oppia.app.model.InteractionObject
import org.oppia.app.model.PendingState
import org.oppia.app.model.Question
import org.oppia.app.model.SubtitledHtml
import org.oppia.util.data.AsyncResult
import org.oppia.util.data.DataProvider
import org.oppia.util.data.DataProviders
import javax.inject.Inject
import javax.inject.Singleton

private const val EPHEMERAL_QUESTION_DATA_PROVIDER_ID = "EphemeralQuestionDataProvider"
private const val EMPTY_QUESTIONS_LIST_DATA_PROVIDER_ID = "EmptyQuestionsListDataProvider"

/**
* Controller that tracks and reports the learner's ephemeral/non-persisted progress through a question training
* Controller that tracks and reports the learner's ephemeral/non-persisted progress through a practice training
* session. Note that this controller only supports one active training session at a time.
*
* The current training session session is started via the question training controller.
* The current training session is started via the question training controller.
*
* This class is thread-safe, but the order of applied operations is arbitrary. Calling code should take care to ensure
* that uses of this class do not specifically depend on ordering.
*/
@Singleton
class QuestionAssessmentProgressController @Inject constructor(
) {
fun beginQuestionTrainingSession(questionsList: DataProvider<List<Question>>) {
class QuestionAssessmentProgressController @Inject constructor(private val dataProviders: DataProviders) {
private var inProgressQuestionsListDataProvider: DataProvider<List<Question>> = createEmptyQuestionsListDataProvider()
private var playing: Boolean = false
private val ephemeralQuestionDataSource: DataProvider<EphemeralQuestion> by lazy {
dataProviders.transformAsync(
EPHEMERAL_QUESTION_DATA_PROVIDER_ID, inProgressQuestionsListDataProvider, this::computeEphemeralQuestionStateAsync
)
}

internal fun beginQuestionTrainingSession(questionsListDataProvider: DataProvider<List<Question>>) {
check(!playing) { "Cannot start a new training session until the previous one is completed" }
inProgressQuestionsListDataProvider = questionsListDataProvider
playing = true
}

internal fun finishQuestionTrainingSession() {
check(playing) { "Cannot stop a new training session which wasn't started" }
playing = false
inProgressQuestionsListDataProvider = createEmptyQuestionsListDataProvider()
}

/**
* Submits an answer to the current question and returns how the UI should respond to this answer. The returned
* [LiveData] will only have at most two results posted: a pending result, and then a completed success/failure
* result. Failures in this case represent a failure of the app (possibly due to networking conditions). The app
* should report this error in a consumable way to the user so that they may take action on it. No additional values
* will be reported to the [LiveData]. Each call to this method returns a new, distinct, [LiveData] object that must
* be observed. Note also that the returned [LiveData] is not guaranteed to begin with a pending state.
*
* If the app undergoes a configuration change, calling code should rely on the [LiveData] from [getCurrentQuestion]
* to know whether a current answer is pending. That [LiveData] will have its state changed to pending during answer
* submission and until answer resolution.
*
* Submitting an answer should result in the learner staying in the current question or moving to a new question in
* the training session. Note that once a correct answer is processed, the current state reported to
* [getCurrentQuestion] will change from a pending question to a completed question since the learner completed that
* question card. The learner can then proceed from the current completed question to the next pending question using
* [moveToNextQuestion].
*
* This method cannot be called until a training session has started and [getCurrentQuestion] returns a non-pending
* result or the result will fail. Calling code must also take care not to allow users to submit an answer while a
* previous answer is pending. That scenario will also result in a failed answer submission.
*
* No assumptions should be made about the completion order of the returned [LiveData] vs. the [LiveData] from
* [getCurrentQuestion]. Also note that the returned [LiveData] will only have a single value and not be reused after
* that point.
*/
fun submitAnswer(answer: InteractionObject): LiveData<AsyncResult<AnsweredQuestionOutcome>> {
val outcome = AnsweredQuestionOutcome.newBuilder()
.setFeedback(SubtitledHtml.newBuilder().setHtml("Response to answer: $answer"))
.setIsCorrectAnswer(true)
.build()
return MutableLiveData(AsyncResult.success(outcome))
}

/**
* Navigates to the previous question already answered. If the learner is currently on the first question, this method
* will throw an exception. Calling code is responsible for ensuring this method is only called when it's possible to
* navigate backward.
*
* @return a one-time [LiveData] indicating whether the movement to the previous question was successful, or a failure
* if question navigation was attempted at an invalid time (such as when viewing the first question). It's
* recommended that calling code only listen to this result for failures, and instead rely on [getCurrentQuestion]
* for observing a successful transition to another state.
*/
fun moveToPreviousQuestion(): LiveData<AsyncResult<Any?>> {
check(playing) { "Cannot move to the previous question unless an active training session is ongoing" }
return MutableLiveData(AsyncResult.success<Any?>(null))
}

/**
* Navigates to the next question in the assessment. This method is only valid if the current [EphemeralQuestion]
* reported by [getCurrentQuestion] is a completed question. Calling code is responsible for ensuring this method is
* only called when it's possible to navigate forward.
*
* Note that if the current question is pending, the user needs to submit a correct answer via [submitAnswer] before
* forward navigation can occur.
*
* @return a one-time [LiveData] indicating whether the movement to the next question was successful, or a failure if
* question navigation was attempted at an invalid time (such as if the current question is pending or terminal).
* It's recommended that calling code only listen to this result for failures, and instead rely on
* [getCurrentQuestion] for observing a successful transition to another question.
*/
fun moveToNextQuestion(): LiveData<AsyncResult<Any?>> {
check(playing) { "Cannot move to the next question unless an active training session is ongoing" }
return MutableLiveData(AsyncResult.success<Any?>(null))
}

/**
* Returns a [LiveData] monitoring the current [EphemeralQuestion] the learner is currently viewing. If this state
* corresponds to a a terminal state, then the learner has completed the training session. Note that
* [moveToPreviousQuestion] and [moveToNextQuestion] will automatically update observers of this live data when the
* next question is navigated to.
*
* This [LiveData] may switch from a completed to a pending result during transient operations like submitting an
* answer via [submitAnswer]. Calling code should be made resilient to this by caching the current question object to
* display since it may disappear temporarily during answer submission. Calling code should persist this state object
* across configuration changes if needed since it cannot rely on this [LiveData] for immediate UI reconstitution
* after configuration changes.
*
* The underlying question returned by this function can only be changed by calls to [moveToNextQuestion] and
* [moveToPreviousQuestion], or the question training controller if another question session begins. UI code can be
* confident only calls from the UI layer will trigger changes here to ensure atomicity between receiving and making
* question state changes.
*
* This method is safe to be called before a training session has started. If there is no ongoing session, it should
* return a pending state.
*/
fun getCurrentQuestion(): LiveData<AsyncResult<EphemeralQuestion>> {
return dataProviders.convertToLiveData(ephemeralQuestionDataSource)
}

@Suppress("RedundantSuspendModifier") // 'suspend' expected by DataProviders.
private suspend fun computeEphemeralQuestionStateAsync(
questionsList: List<Question>
): AsyncResult<EphemeralQuestion> {
if (!playing) {
return AsyncResult.pending()
}
return try {
AsyncResult.success(computeEphemeralQuestionState(questionsList))
} catch (e: Exception) {
AsyncResult.failed(e)
}
}

private fun computeEphemeralQuestionState(questionsList: List<Question>): EphemeralQuestion {
check(questionsList.isNotEmpty()) { "Cannot start a training session with zero questions." }
val currentQuestion = questionsList.first()
return EphemeralQuestion.newBuilder()
.setEphemeralState(EphemeralState.newBuilder()
.setState(currentQuestion.questionState)
.setPendingState(PendingState.getDefaultInstance()))
.setCurrentQuestionIndex(0)
.setTotalQuestionCount(questionsList.size)
.setInitialTotalQuestionCount(questionsList.size)
.build()
}

fun finishQuestionTrainingSession() {
/** Returns a temporary [DataProvider] that always provides an empty list of [Question]s. */
private fun createEmptyQuestionsListDataProvider(): DataProvider<List<Question>> {
return dataProviders.createInMemoryDataProvider(EMPTY_QUESTIONS_LIST_DATA_PROVIDER_ID) { listOf<Question>() }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import javax.inject.Inject
import javax.inject.Singleton
import kotlin.random.Random

const val TRAINING_QUESTIONS_PROVIDER = "TrainingQuestionsProvider"
private const val TRAINING_QUESTIONS_PROVIDER = "TrainingQuestionsProvider"
private const val RETRIEVE_QUESTIONS_RESULT_DATA_PROVIDER = "RetrieveQuestionsResultsProvider"

/** Controller for retrieving a set of questions. */
@Singleton
Expand All @@ -35,13 +36,17 @@ class QuestionTrainingController @Inject constructor(
* @return a one-time [LiveData] to observe whether initiating the play request succeeded.
* The training session may still fail to load, but this provides early-failure detection.
*/
fun startQuestionTrainingSession(skillIdsList: List<String>): LiveData<AsyncResult<List<Question>>> {
fun startQuestionTrainingSession(skillIdsList: List<String>): LiveData<AsyncResult<Any>> {
return try {
val retrieveQuestionsDataProvider = retrieveQuestionsForSkillIds(skillIdsList)
questionAssessmentProgressController.beginQuestionTrainingSession(
retrieveQuestionsDataProvider
)
dataProviders.convertToLiveData(retrieveQuestionsDataProvider)
// Convert the data provider type to 'Any' via a transformation.
val erasedDataProvider: DataProvider<Any> = dataProviders.transform(
RETRIEVE_QUESTIONS_RESULT_DATA_PROVIDER, retrieveQuestionsDataProvider
) { it }
dataProviders.convertToLiveData(erasedDataProvider)
} catch (e: Exception) {
MutableLiveData(AsyncResult.failed(e))
}
Expand All @@ -50,10 +55,14 @@ class QuestionTrainingController @Inject constructor(
private fun retrieveQuestionsForSkillIds(skillIdsList: List<String>): DataProvider<List<Question>> {
val questionsDataProvider = topicController.retrieveQuestionsForSkillIds(skillIdsList)
return dataProviders.transform(TRAINING_QUESTIONS_PROVIDER, questionsDataProvider) {
getFilteredQuestionsForTraining(
skillIdsList, it.shuffled(random),
questionCountPerSession / skillIdsList.size
)
if (skillIdsList.isEmpty()) {
listOf()
} else {
getFilteredQuestionsForTraining(
skillIdsList, it.shuffled(random),
questionCountPerSession / skillIdsList.size
)
}
}
}

Expand Down
Loading

0 comments on commit 5be79f2

Please sign in to comment.