From 0dc284f35fa0712f64147fd264a55980b66abcd5 Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Mon, 28 Oct 2024 12:49:36 +0530 Subject: [PATCH 01/13] Fix crash in AudioViewModel by initializing state variables --- .../oppia/android/app/player/audio/AudioViewModel.kt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt index 07037abd873..954be314639 100644 --- a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt @@ -66,9 +66,11 @@ class AudioViewModel @Inject constructor( processPlayStatusLiveData() } - fun setStateAndExplorationId(newState: State, id: String) { - state = newState - explorationId = id + fun setStateAndExplorationId(newState: State?, id: String?) { + if (newState != null && id != null) { + state = newState + explorationId = id + } } fun loadMainContentAudio(allowAutoPlay: Boolean, reloadingContent: Boolean) { @@ -88,6 +90,8 @@ class AudioViewModel @Inject constructor( * @param allowAutoPlay If false, audio is guaranteed not to be autoPlayed. */ private fun loadAudio(contentId: String?, allowAutoPlay: Boolean, reloadingMainContent: Boolean) { + // Check if 'state' is initialized before proceeding. + if (!::state.isInitialized) return val targetContentId = contentId ?: state.content.contentId val voiceoverMapping = state.recordedVoiceoversMap[targetContentId] ?: VoiceoverMapping.getDefaultInstance() From 30ca79653dba6cd474fe71f6daa3154c5547d7f1 Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Wed, 30 Oct 2024 02:35:14 +0530 Subject: [PATCH 02/13] Fix crash in AudioViewModel on rotation by ensuring state initialization --- .../android/app/player/audio/AudioViewModel.kt | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt index 954be314639..53e1f47b2d8 100644 --- a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt @@ -1,5 +1,6 @@ package org.oppia.android.app.player.audio +import android.util.Log import androidx.databinding.ObservableBoolean import androidx.databinding.ObservableField import androidx.lifecycle.LiveData @@ -67,12 +68,15 @@ class AudioViewModel @Inject constructor( } fun setStateAndExplorationId(newState: State?, id: String?) { - if (newState != null && id != null) { - state = newState - explorationId = id + if (newState == null || id == null) { + Log.e("AudioViewModel", "Failed to set state or id - parameters are null") + return } + state = newState + explorationId = id } + fun loadMainContentAudio(allowAutoPlay: Boolean, reloadingContent: Boolean) { hasFeedback = false loadAudio(contentId = null, allowAutoPlay, reloadingContent) @@ -90,8 +94,11 @@ class AudioViewModel @Inject constructor( * @param allowAutoPlay If false, audio is guaranteed not to be autoPlayed. */ private fun loadAudio(contentId: String?, allowAutoPlay: Boolean, reloadingMainContent: Boolean) { - // Check if 'state' is initialized before proceeding. - if (!::state.isInitialized) return + if (!::state.isInitialized || !::explorationId.isInitialized) { + Log.w("AudioViewModel", "Cannot load audio: state or explorationId is not initialized.") + return + } + val targetContentId = contentId ?: state.content.contentId val voiceoverMapping = state.recordedVoiceoversMap[targetContentId] ?: VoiceoverMapping.getDefaultInstance() From 418e0c5d1aded8d37e2c90e80f92e20971ab5a81 Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Fri, 8 Nov 2024 02:29:00 +0530 Subject: [PATCH 03/13] AudioViewModel: resolved changes --- .../app/player/audio/AudioViewModel.kt | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt index 53e1f47b2d8..5ac6e3fa3ce 100644 --- a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt @@ -20,6 +20,7 @@ import org.oppia.android.util.gcsresource.DefaultResourceBucketName import org.oppia.android.util.locale.OppiaLocale import java.util.Locale import javax.inject.Inject +import org.oppia.android.domain.oppialogger.OppiaLogger /** [ObservableViewModel] for audio-player state. */ @FragmentScope @@ -27,7 +28,8 @@ class AudioViewModel @Inject constructor( private val audioPlayerController: AudioPlayerController, @DefaultResourceBucketName private val gcsResource: String, private val machineLocale: OppiaLocale.MachineLocale, - private val resourceHandler: AppLanguageResourceHandler + private val resourceHandler: AppLanguageResourceHandler, + private val oppiaLogger: OppiaLogger, ) : ObservableViewModel() { private lateinit var state: State @@ -67,16 +69,11 @@ class AudioViewModel @Inject constructor( processPlayStatusLiveData() } - fun setStateAndExplorationId(newState: State?, id: String?) { - if (newState == null || id == null) { - Log.e("AudioViewModel", "Failed to set state or id - parameters are null") - return - } + fun setStateAndExplorationId(newState: State, id: String) { state = newState explorationId = id } - fun loadMainContentAudio(allowAutoPlay: Boolean, reloadingContent: Boolean) { hasFeedback = false loadAudio(contentId = null, allowAutoPlay, reloadingContent) @@ -94,12 +91,12 @@ class AudioViewModel @Inject constructor( * @param allowAutoPlay If false, audio is guaranteed not to be autoPlayed. */ private fun loadAudio(contentId: String?, allowAutoPlay: Boolean, reloadingMainContent: Boolean) { - if (!::state.isInitialized || !::explorationId.isInitialized) { - Log.w("AudioViewModel", "Cannot load audio: state or explorationId is not initialized.") - return + val targetContentId = when { + !contentId.isNullOrEmpty() -> contentId + ::state.isInitialized -> state.content.contentId + else -> "" } - val targetContentId = contentId ?: state.content.contentId val voiceoverMapping = state.recordedVoiceoversMap[targetContentId] ?: VoiceoverMapping.getDefaultInstance() From 18452ca65026ef771f26b964c9bd53d3f4ea175d Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Fri, 8 Nov 2024 02:32:01 +0530 Subject: [PATCH 04/13] Remove Unnecessary Lines --- .../java/org/oppia/android/app/player/audio/AudioViewModel.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt index 5ac6e3fa3ce..3bde2aed021 100644 --- a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt @@ -20,7 +20,6 @@ import org.oppia.android.util.gcsresource.DefaultResourceBucketName import org.oppia.android.util.locale.OppiaLocale import java.util.Locale import javax.inject.Inject -import org.oppia.android.domain.oppialogger.OppiaLogger /** [ObservableViewModel] for audio-player state. */ @FragmentScope @@ -28,8 +27,7 @@ class AudioViewModel @Inject constructor( private val audioPlayerController: AudioPlayerController, @DefaultResourceBucketName private val gcsResource: String, private val machineLocale: OppiaLocale.MachineLocale, - private val resourceHandler: AppLanguageResourceHandler, - private val oppiaLogger: OppiaLogger, + private val resourceHandler: AppLanguageResourceHandler ) : ObservableViewModel() { private lateinit var state: State From 9e023f900af101619680751c0615dce5626c0d3a Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Tue, 3 Dec 2024 01:45:54 +0530 Subject: [PATCH 05/13] Correct setStateAndExplorationId --- .../oppia/android/app/player/audio/AudioViewModel.kt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt index 3bde2aed021..4efbfa9591c 100644 --- a/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt +++ b/app/src/main/java/org/oppia/android/app/player/audio/AudioViewModel.kt @@ -1,6 +1,5 @@ package org.oppia.android.app.player.audio -import android.util.Log import androidx.databinding.ObservableBoolean import androidx.databinding.ObservableField import androidx.lifecycle.LiveData @@ -73,6 +72,7 @@ class AudioViewModel @Inject constructor( } fun loadMainContentAudio(allowAutoPlay: Boolean, reloadingContent: Boolean) { + setStateAndExplorationId(state, explorationId) hasFeedback = false loadAudio(contentId = null, allowAutoPlay, reloadingContent) } @@ -91,7 +91,7 @@ class AudioViewModel @Inject constructor( private fun loadAudio(contentId: String?, allowAutoPlay: Boolean, reloadingMainContent: Boolean) { val targetContentId = when { !contentId.isNullOrEmpty() -> contentId - ::state.isInitialized -> state.content.contentId + this::state.isInitialized -> state.content.contentId else -> "" } @@ -112,10 +112,8 @@ class AudioViewModel @Inject constructor( selectedLanguageName.set(locale.getDisplayLanguage(locale)) when { - selectedLanguageCode.isEmpty() && languages.any { - it == defaultLanguage - } -> setAudioLanguageCode(defaultLanguage) - languages.any { it == selectedLanguageCode } -> setAudioLanguageCode(selectedLanguageCode) + selectedLanguageCode.isEmpty() && languages.contains(defaultLanguage) -> setAudioLanguageCode(defaultLanguage) + languages.contains(selectedLanguageCode) -> setAudioLanguageCode(selectedLanguageCode) languages.isNotEmpty() -> { autoPlay = false this.reloadingMainContent = false From 233d71de3c77725257ba57821e3eb1fb8b69869b Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Tue, 3 Dec 2024 15:17:38 +0530 Subject: [PATCH 06/13] Added Test --- .../app/player/audio/AudioFragmentTest.kt | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt index dec40d9a790..8d7477450a2 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/audio/AudioFragmentTest.kt @@ -11,8 +11,10 @@ import androidx.appcompat.app.AppCompatActivity import androidx.test.core.app.ActivityScenario.launch import androidx.test.core.app.ApplicationProvider import androidx.test.espresso.Espresso.onView +import androidx.test.espresso.PerformException import androidx.test.espresso.UiController import androidx.test.espresso.ViewAction +import androidx.test.espresso.ViewInteraction import androidx.test.espresso.action.ViewActions.click import androidx.test.espresso.assertion.ViewAssertions.matches import androidx.test.espresso.matcher.RootMatchers.isDialog @@ -22,9 +24,12 @@ import androidx.test.espresso.matcher.ViewMatchers.isRoot import androidx.test.espresso.matcher.ViewMatchers.withContentDescription import androidx.test.espresso.matcher.ViewMatchers.withId import androidx.test.espresso.matcher.ViewMatchers.withText +import androidx.test.espresso.util.HumanReadables +import androidx.test.espresso.util.TreeIterables import androidx.test.ext.junit.runners.AndroidJUnit4 import com.google.common.truth.Truth.assertThat import dagger.Component +import java.util.concurrent.TimeoutException import org.hamcrest.Description import org.hamcrest.Matcher import org.hamcrest.TypeSafeMatcher @@ -115,6 +120,7 @@ import org.robolectric.annotation.Config import org.robolectric.annotation.LooperMode import javax.inject.Inject import javax.inject.Singleton +import org.hamcrest.CoreMatchers.allOf /** * TODO(#59): Make this test work with Espresso. @@ -219,6 +225,80 @@ class AudioFragmentTest { } } + @Test + fun testAudioFragment_playAudio_configurationChange_checkAudioPaused() { + addMediaInfo() + launch( + createAudioFragmentTestIntent( + internalProfileId + ) + ).use { + testCoroutineDispatchers.runCurrent() + + waitForTheView(allOf(withId(R.id.play_pause_audio_icon), WithNonZeroDimensionsMatcher())) + .perform(click()) + testCoroutineDispatchers.runCurrent() + + onView(withId(R.id.audio_progress_seek_bar)).perform(setProgress(100)) + + onView(isRoot()).perform(orientationLandscape()) + testCoroutineDispatchers.runCurrent() + onView(withId(R.id.play_pause_audio_icon)) + .check(matches(withContentDescription(context.getString(R.string.audio_play_description)))) + } + } + + /** Returns a matcher that matches view based on non-zero width and height. */ + private class WithNonZeroDimensionsMatcher : TypeSafeMatcher() { + + override fun matchesSafely(target: View): Boolean { + val targetWidth = target.width + val targetHeight = target.height + return targetWidth > 0 && targetHeight > 0 + } + + override fun describeTo(description: Description) { + description.appendText("with non-zero width and height") + } + } + + private fun waitForTheView(viewMatcher: Matcher): ViewInteraction { + return onView(isRoot()).perform(waitForMatch(viewMatcher, 30000L)) + } + + private fun waitForMatch(viewMatcher: Matcher, millis: Long): ViewAction { + return object : ViewAction { + override fun getDescription(): String { + return "wait for a specific view with matcher <$viewMatcher> during $millis millis." + } + + override fun getConstraints(): Matcher { + return isRoot() + } + + override fun perform(uiController: UiController?, view: View?) { + checkNotNull(uiController) + uiController.loopMainThreadUntilIdle() + val startTime = System.currentTimeMillis() + val endTime = startTime + millis + + do { + if (TreeIterables.breadthFirstViewTraversal(view).any { viewMatcher.matches(it) }) { + return + } + uiController.loopMainThreadForAtLeast(50) + } while (System.currentTimeMillis() < endTime) + + // Couldn't match in time. + throw PerformException.Builder() + .withActionDescription(description) + .withViewDescription(HumanReadables.describe(view)) + .withCause(TimeoutException()) + .build() + } + } + } + @Test fun testAudioFragment_openFragment_showsFragment() { addMediaInfo() From cde7984890502e3633fb3b3788523fa51a017063 Mon Sep 17 00:00:00 2001 From: TanishMoral11 Date: Tue, 3 Dec 2024 15:30:56 +0530 Subject: [PATCH 07/13] Improving Formatting --- .idea/codeStyles/Project.xml | 4 +- .../app/player/audio/AudioViewModel.kt | 3 +- .../app/player/audio/AudioFragmentTest.kt | 173 ++++++++++-------- 3 files changed, 96 insertions(+), 84 deletions(-) diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml index 66d46e74f0c..2ae2a26048e 100644 --- a/.idea/codeStyles/Project.xml +++ b/.idea/codeStyles/Project.xml @@ -5,8 +5,6 @@