From e3642c2e2da76ea8d9f7cb78ba8f81ab55d43d46 Mon Sep 17 00:00:00 2001 From: RD Rama Devi <122200035+Rd4dev@users.noreply.github.com> Date: Wed, 11 Dec 2024 01:16:25 +0530 Subject: [PATCH] Fix #1468: Retain Drag and Drop state after an incorrect answer (#5541) ## Explanation Fix #1468 ### This PR includes - Utilized `explorationProgressController` to retrieve the **'wrongAnswerList'** from ephemeralState's pendingState and use it to populate the `DragDropInteractionContentViewModel` list. - When `wrongAnswerList` is available, corresponding content IDs and HTML values are applied. If not, the original `contentIdHtmlMap` and `SubtitledHtml` values are used as a fallback. - The `choiceItems` are set up as LiveData to ensure the UI can react to changes. - The `_choiceItems` are copied to `_originalChoiceItems ` for detecting arrangement box errors during submission when the answer has not changed. - Retains linked-merged / unlinked states based on the `wrongAnswerList` data ## For UI-specific PRs only ### Re-order Retain State https://github.com/user-attachments/assets/f4c3fd72-d96d-4ee8-b82e-af134ea06a2b ### Group / Unlink Retain https://github.com/user-attachments/assets/c90a5fc2-ccf9-4e21-a7cd-31f9ec1bd6c0 ### Espresso Tests ![image](https://github.com/user-attachments/assets/2e3617eb-4d28-4bee-b8a9-f94c2a3b0231) ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). --------- Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com> --- .../DragAndDropSortInteractionViewModel.kt | 118 ++++++++--- .../app/player/state/StateFragmentTest.kt | 193 +++++++++++++++++- 2 files changed, 278 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/DragAndDropSortInteractionViewModel.kt b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/DragAndDropSortInteractionViewModel.kt index 11d87f4bed0..7abeb9fff1f 100644 --- a/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/DragAndDropSortInteractionViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/state/itemviewmodel/DragAndDropSortInteractionViewModel.kt @@ -3,9 +3,13 @@ package org.oppia.android.app.player.state.itemviewmodel import androidx.annotation.StringRes import androidx.databinding.Observable import androidx.databinding.ObservableField +import androidx.lifecycle.LiveData +import androidx.lifecycle.MediatorLiveData +import androidx.lifecycle.Transformations import androidx.recyclerview.widget.RecyclerView import org.oppia.android.R import org.oppia.android.app.model.AnswerErrorCategory +import org.oppia.android.app.model.EphemeralState import org.oppia.android.app.model.Interaction import org.oppia.android.app.model.InteractionObject import org.oppia.android.app.model.ListOfSetsOfHtmlStrings @@ -24,7 +28,10 @@ import org.oppia.android.app.recyclerview.BindableAdapter import org.oppia.android.app.recyclerview.OnDragEndedListener import org.oppia.android.app.recyclerview.OnItemDragListener import org.oppia.android.app.translation.AppLanguageResourceHandler +import org.oppia.android.domain.exploration.ExplorationProgressController import org.oppia.android.domain.translation.TranslationController +import org.oppia.android.util.data.AsyncResult +import org.oppia.android.util.data.DataProviders.Companion.toLiveData import javax.inject.Inject /** Represents the type of errors that can be thrown by drag and drop sort interaction. */ @@ -49,11 +56,13 @@ class DragAndDropSortInteractionViewModel private constructor( private val writtenTranslationContext: WrittenTranslationContext, private val resourceHandler: AppLanguageResourceHandler, private val translationController: TranslationController, - userAnswerState: UserAnswerState + userAnswerState: UserAnswerState, + private val explorationProgressController: ExplorationProgressController ) : StateItemViewModel(ViewType.DRAG_DROP_SORT_INTERACTION), InteractionAnswerHandler, OnItemDragListener, OnDragEndedListener { + private val allowMultipleItemsInSamePosition: Boolean by lazy { interaction.customizationArgsMap["allowMultipleItemsInSamePosition"]?.boolValue ?: false } @@ -72,19 +81,18 @@ class DragAndDropSortInteractionViewModel private constructor( subtitledHtml.contentId to translatedHtml } - private var answerErrorCetegory: AnswerErrorCategory = AnswerErrorCategory.NO_ERROR + private var answerErrorCategory: AnswerErrorCategory = AnswerErrorCategory.NO_ERROR - private val _originalChoiceItems: MutableList = + private var _originalChoiceItems: MutableList = computeOriginalChoiceItems(contentIdHtmlMap, choiceSubtitledHtmls, this, resourceHandler) - private val _choiceItems = computeSelectedChoiceItems( - contentIdHtmlMap, - choiceSubtitledHtmls, - this, - resourceHandler, - userAnswerState - ) - val choiceItems: List = _choiceItems + lateinit var choiceItems: LiveData> + private var _choiceItems: MutableList = + computeSelectedChoiceItems( + contentIdHtmlMap, + this, + resourceHandler + ) private var pendingAnswerError: String? = null private val isAnswerAvailable = ObservableField(false) @@ -170,7 +178,7 @@ class DragAndDropSortInteractionViewModel private constructor( * updates the error string based on the specified error category. */ override fun checkPendingAnswerError(category: AnswerErrorCategory): String? { - answerErrorCetegory = category + answerErrorCategory = category pendingAnswerError = when (category) { AnswerErrorCategory.REAL_TIME -> null AnswerErrorCategory.SUBMIT_TIME -> @@ -252,7 +260,8 @@ class DragAndDropSortInteractionViewModel private constructor( /** Implementation of [StateItemViewModel.InteractionItemFactory] for this view model. */ class FactoryImpl @Inject constructor( private val resourceHandler: AppLanguageResourceHandler, - private val translationController: TranslationController + private val translationController: TranslationController, + private val explorationProgressController: ExplorationProgressController ) : InteractionItemFactory { override fun create( entityId: String, @@ -275,7 +284,8 @@ class DragAndDropSortInteractionViewModel private constructor( writtenTranslationContext, resourceHandler, translationController, - userAnswerState + userAnswerState, + explorationProgressController ) } } @@ -283,7 +293,7 @@ class DragAndDropSortInteractionViewModel private constructor( override fun getUserAnswerState(): UserAnswerState { if (_choiceItems == _originalChoiceItems) { return UserAnswerState.newBuilder().apply { - this.answerErrorCategory = answerErrorCetegory + this.answerErrorCategory = answerErrorCategory }.build() } return UserAnswerState.newBuilder().apply { @@ -292,7 +302,7 @@ class DragAndDropSortInteractionViewModel private constructor( ListOfSetsOfTranslatableHtmlContentIds.newBuilder().apply { addAllContentIdLists(htmlContentIds) }.build() - answerErrorCategory = answerErrorCetegory + answerErrorCategory = answerErrorCategory }.build() } @@ -324,25 +334,69 @@ class DragAndDropSortInteractionViewModel private constructor( private fun computeSelectedChoiceItems( contentIdHtmlMap: Map, - choiceStrings: List, dragAndDropSortInteractionViewModel: DragAndDropSortInteractionViewModel, - resourceHandler: AppLanguageResourceHandler, - userAnswerState: UserAnswerState + resourceHandler: AppLanguageResourceHandler ): MutableList { - return if (userAnswerState.listOfSetsOfTranslatableHtmlContentIds.contentIdListsCount == 0) { - _originalChoiceItems.toMutableList() - } else { - userAnswerState.listOfSetsOfTranslatableHtmlContentIds.contentIdListsList - .mapIndexed { index, contentId -> - DragDropInteractionContentViewModel( - contentIdHtmlMap = contentIdHtmlMap, - htmlContent = contentId, - itemIndex = index, - listSize = choiceStrings.size, - dragAndDropSortInteractionViewModel = dragAndDropSortInteractionViewModel, - resourceHandler = resourceHandler + val explorationEphemeralStateLiveData = MediatorLiveData>().apply { + value = AsyncResult.Pending() + addSource(explorationProgressController.getCurrentState().toLiveData()) { result -> + value = result + } + } + + choiceItems = Transformations.map(explorationEphemeralStateLiveData) { result -> + when (result) { + is AsyncResult.Failure, is AsyncResult.Pending -> { + _originalChoiceItems + } + is AsyncResult.Success -> { + _choiceItems = processEphemeralStateResult( + result.value, + contentIdHtmlMap, + dragAndDropSortInteractionViewModel, + resourceHandler ) - }.toMutableList() + _originalChoiceItems = _choiceItems.toMutableList() + _choiceItems + } + else -> _originalChoiceItems + } + } + + return _choiceItems.takeIf { !it.isNullOrEmpty() } + ?: _originalChoiceItems.toMutableList() + } + + private fun processEphemeralStateResult( + state: EphemeralState, + contentIdHtmlMap: Map, + dragAndDropSortInteractionViewModel: DragAndDropSortInteractionViewModel, + resourceHandler: AppLanguageResourceHandler + ): MutableList { + val wrongAnswerList = state.pendingState.wrongAnswerList + return if (wrongAnswerList.isNotEmpty()) { + val latestWrongAnswerContentIdList = wrongAnswerList.last() + .userAnswer.answer.listOfSetsOfTranslatableHtmlContentIds.contentIdListsList + latestWrongAnswerContentIdList.mapIndexed { index, setOfTranslatableHtmlContentIds -> + DragDropInteractionContentViewModel( + contentIdHtmlMap = contentIdHtmlMap, + htmlContent = SetOfTranslatableHtmlContentIds.newBuilder().apply { + for (contentIds in setOfTranslatableHtmlContentIds.contentIdsList) { + addContentIds( + TranslatableHtmlContentId.newBuilder().apply { + contentId = contentIds.contentId + } + ) + } + }.build(), + itemIndex = index, + listSize = latestWrongAnswerContentIdList.size, + dragAndDropSortInteractionViewModel = dragAndDropSortInteractionViewModel, + resourceHandler = resourceHandler + ) + }.toMutableList() + } else { + _originalChoiceItems.toMutableList() } } } diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt index afe8a45c5b1..b9aa97a1a1e 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/state/StateFragmentTest.kt @@ -1138,7 +1138,7 @@ class StateFragmentTest { @Test @RunOn(TestPlatform.ESPRESSO) // TODO(#1612): Enable for Robolectric. - fun testStateFragment_loadDragDropExp_mergeItems_dargAndDrop_retainStateOnConfigurationChange() { + fun testStateFragment_loadDragDropExp_mergeItems_dragAndDrop_retainStateOnConfigurationChange() { setUpTestWithLanguageSwitchingFeatureOff() launchForExploration(TEST_EXPLORATION_ID_4, shouldSavePartialProgress = false).use { startPlayingExploration() @@ -1326,6 +1326,197 @@ class StateFragmentTest { } } + @Test + @RunOn(TestPlatform.ESPRESSO) // TODO(#1612): Enable for Robolectric. + fun testStateFragment_loadDragDropExp_wrongAnswer_retainsLatestState() { + setUpTestWithLanguageSwitchingFeatureOff() + launchForExploration(TEST_EXPLORATION_ID_2, shouldSavePartialProgress = false).use { + startPlayingExploration() + playThroughPrototypeState1() + playThroughPrototypeState2() + playThroughPrototypeState3() + playThroughPrototypeState4() + playThroughPrototypeState5() + playThroughPrototypeState6() + playThroughPrototypeState7() + playThroughPrototypeState8() + + // Drag and drop interaction without grouping. + // Ninth state: Drag Drop Sort. Wrong answer: Move 1st item to 2nd position. + dragAndDropItem(fromPosition = 0, toPosition = 1) + clickSubmitAnswerButton() + + scrollToViewType(DRAG_DROP_SORT_INTERACTION) + onView( + atPositionOnView( + recyclerViewId = R.id.drag_drop_interaction_recycler_view, + position = 0, + targetViewId = R.id.drag_drop_content_text_view + ) + ).check(matches(withText("3/5"))) + onView( + atPositionOnView( + recyclerViewId = R.id.drag_drop_interaction_recycler_view, + position = 1, + targetViewId = R.id.drag_drop_content_text_view + ) + ).check(matches(withText("0.35"))) + } + } + + @Test + @RunOn(TestPlatform.ESPRESSO) // TODO(#1612): Enable for Robolectric. + fun testStateFragment_loadDragDropExp_wrongAnswer_unArrangedRetainState_causeSubmitTimeError() { + setUpTestWithLanguageSwitchingFeatureOff() + launchForExploration(TEST_EXPLORATION_ID_2, shouldSavePartialProgress = false).use { + startPlayingExploration() + playThroughPrototypeState1() + playThroughPrototypeState2() + playThroughPrototypeState3() + playThroughPrototypeState4() + playThroughPrototypeState5() + playThroughPrototypeState6() + playThroughPrototypeState7() + playThroughPrototypeState8() + + // Drag and drop interaction without grouping. + // Ninth state: Drag Drop Sort. Wrong answer: Move 1st item to 2nd position. + dragAndDropItem(fromPosition = 0, toPosition = 1) + clickSubmitAnswerButton() + + scrollToViewType(DRAG_DROP_SORT_INTERACTION) + onView( + atPositionOnView( + recyclerViewId = R.id.drag_drop_interaction_recycler_view, + position = 0, + targetViewId = R.id.drag_drop_content_text_view + ) + ).check(matches(withText("3/5"))) + clickSubmitAnswerButton() + + onView(withId(R.id.drag_drop_interaction_error)).check( + matches(withText(R.string.drag_and_drop_interaction_empty_input)) + ) + } + } + + @Test + fun testStateFragment_loadDragDropExp_mergeFirstTwoItems_wrongAnswer_retainsLatestStateCount() { + setUpTestWithLanguageSwitchingFeatureOff() + launchForExploration(TEST_EXPLORATION_ID_4, shouldSavePartialProgress = false).use { + startPlayingExploration() + + mergeDragAndDropItems(position = 0) + clickSubmitAnswerButton() + + scrollToViewType(DRAG_DROP_SORT_INTERACTION) + onView(withId(R.id.drag_drop_interaction_recycler_view)).check(matches(hasChildCount(3))) + onView( + atPositionOnView( + recyclerViewId = R.id.drag_drop_interaction_recycler_view, + position = 0, + targetViewId = R.id.drag_drop_item_recyclerview + ) + ).check(matches(hasChildCount(2))) + } + } + + @Test + fun testStateFragment_loadDragDropExp_mergeFirstTwoItems_wrongAnswer_retainsLatestStateText() { + setUpTestWithLanguageSwitchingFeatureOff() + launchForExploration(TEST_EXPLORATION_ID_4, shouldSavePartialProgress = false).use { + startPlayingExploration() + + mergeDragAndDropItems(position = 0) + clickSubmitAnswerButton() + + scrollToViewType(DRAG_DROP_SORT_INTERACTION) + onView( + atPositionOnView( + recyclerViewId = R.id.drag_drop_item_recyclerview, + position = 0, + targetViewId = R.id.drag_drop_content_text_view + ) + ).check(matches(withText("a camera at the store"))) + onView( + atPositionOnView( + recyclerViewId = R.id.drag_drop_item_recyclerview, + position = 1, + targetViewId = R.id.drag_drop_content_text_view + ) + ).check(matches(withText("I bought"))) + } + } + + @Test + fun testStateFragment_loadDragDropExp_mergeUnlinkFirstTwoItems_wrongAnswer_retainsLatestState() { + setUpTestWithLanguageSwitchingFeatureOff() + launchForExploration(TEST_EXPLORATION_ID_4, shouldSavePartialProgress = false).use { + startPlayingExploration() + + mergeDragAndDropItems(position = 0) + unlinkDragAndDropItems(position = 0) + clickSubmitAnswerButton() + + scrollToViewType(DRAG_DROP_SORT_INTERACTION) + onView(withId(R.id.drag_drop_interaction_recycler_view)).check(matches(hasChildCount(4))) + onView( + atPositionOnView( + recyclerViewId = R.id.drag_drop_interaction_recycler_view, + position = 0, + targetViewId = R.id.drag_drop_item_recyclerview + ) + ).check(matches(hasChildCount(1))) + } + } + + @Test + @RunOn(TestPlatform.ESPRESSO) // TODO(#1612): Enable for Robolectric. + fun testStateFragment_loadDragDropExp_mergeItems_dragItem_wrongAnswer_retainsLatestState() { + setUpTestWithLanguageSwitchingFeatureOff() + launchForExploration(TEST_EXPLORATION_ID_4, shouldSavePartialProgress = false).use { + startPlayingExploration() + + mergeDragAndDropItems(position = 0) + dragAndDropItem(fromPosition = 0, toPosition = 2) + clickSubmitAnswerButton() + + scrollToViewType(DRAG_DROP_SORT_INTERACTION) + onView( + atPositionOnView( + recyclerViewId = R.id.drag_drop_interaction_recycler_view, + position = 2, + targetViewId = R.id.drag_drop_content_text_view + ) + ).check(matches(withText("a camera at the store"))) + } + } + + @Test + fun testStateFragment_loadDragDropExp_mergeItems_unArrangedRetainState_causeSubmitTimeError() { + setUpTestWithLanguageSwitchingFeatureOff() + launchForExploration(TEST_EXPLORATION_ID_4, shouldSavePartialProgress = false).use { + startPlayingExploration() + + mergeDragAndDropItems(position = 0) + clickSubmitAnswerButton() + + scrollToViewType(DRAG_DROP_SORT_INTERACTION) + onView( + atPositionOnView( + recyclerViewId = R.id.drag_drop_item_recyclerview, + position = 0, + targetViewId = R.id.drag_drop_content_text_view + ) + ).check(matches(withText("a camera at the store"))) + clickSubmitAnswerButton() + + onView(withId(R.id.drag_drop_interaction_error)).check( + matches(withText(R.string.drag_and_drop_interaction_empty_input)) + ) + } + } + @Test @RunOn(TestPlatform.ESPRESSO) // TODO(#1612): Enable for Robolectric. fun testStateFragment_loadDragDropExp_mergeFirstTwoItems_dragItem_worksCorrectly() {