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.

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

Fix #112: Introduce question progress controller interface #218

Merged
merged 32 commits into from
Nov 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
9594954
Initial check in for the question data controller
vinitamurthi Oct 8, 2019
6ee7aec
Removed progress controller test
vinitamurthi Oct 8, 2019
848f172
Address the renames suggested in #217.
BenHenning Oct 9, 2019
174e2f4
Merge branch 'develop' into introduce-question-progress-controller-in…
BenHenning Oct 9, 2019
4147857
Introduce thoroughly stubbed QuestionAssessmentProgressController +
BenHenning Oct 9, 2019
4596e86
Review changes
vinitamurthi Oct 9, 2019
f5c23c8
Review changes
vinitamurthi Oct 10, 2019
53d367d
Pass data providers to the assessment controller, and set caps in the…
vinitamurthi Oct 11, 2019
86630b6
Fixed merge conflicts
vinitamurthi Oct 11, 2019
a8adf93
Removed unnecessary imports/variables
vinitamurthi Oct 11, 2019
a7a10f3
Add fake question data for stubbed interfaces
vinitamurthi Oct 11, 2019
459e3e9
Remove duplicate questions while fetching in training controller
vinitamurthi Oct 11, 2019
73c49df
Comment explaining the filter function
vinitamurthi Oct 11, 2019
91cf8e6
Improve duplicate checking - check it while filtering instead of afte…
vinitamurthi Oct 11, 2019
54546d7
Add linked skill id values
vinitamurthi Oct 11, 2019
be375fd
Review changes
vinitamurthi Oct 15, 2019
64b8265
add a new module for questions constants
vinitamurthi Oct 18, 2019
75405a2
fix merge conflicts
vinitamurthi Oct 18, 2019
4878d10
Review changes
vinitamurthi Oct 18, 2019
c37f14b
add a test to verify questions were fetched properly
vinitamurthi Oct 18, 2019
d1bd4a6
reformatted code
vinitamurthi Oct 18, 2019
1c5ca42
Merge branch 'question_data' into introduce-question-progress-control…
BenHenning Oct 20, 2019
86df525
Re-add QuestionTrainingController removed in merge.
BenHenning Oct 20, 2019
54340e6
Finalize question progress controller interface.
BenHenning Oct 20, 2019
543b734
Merge branch 'develop' into introduce-question-progress-controller-in…
BenHenning Oct 28, 2019
73372fe
Fix typo in QuestionTrainingController.
BenHenning Oct 28, 2019
8d29dd1
Post-merge fixes and adjustments.
BenHenning Oct 28, 2019
fedf016
Merge branch 'develop' into introduce-question-progress-controller-in…
BenHenning Nov 20, 2019
d0f8487
Post-merge fixes & address reviewer comments.
BenHenning Nov 20, 2019
cb2274f
Merge branch 'introduce-question-progress-controller-interface' of gi…
BenHenning Nov 20, 2019
0c3be01
Remove unneeded files.
BenHenning Nov 20, 2019
b6cd572
Remove legacy code from tests.
BenHenning Nov 20, 2019
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
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(
BenHenning marked this conversation as resolved.
Show resolved Hide resolved
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