Skip to content

Commit

Permalink
Show a dialog when preloading assets for offline support.
Browse files Browse the repository at this point in the history
This does a bunch of refactoring & hacky workarounds to keep asset
priming support fully isolated (just a few files now need to be deleted
to clean things up). This also:
1. Fixes actual asset downloading (the GCS asset path templates have
changed, so priming didn't actually work anymore).
2. Disables audio file caching since we can't play audio when offline,
anyway (there's a dialog that prevents this in-player).
3. Fixes #1340 and #1341 by accounting for error cases when trying to
play audio. It turns out that playing audio crashes if you didn't have
internet access when going into an exploration (the no connectivity
dialog only appears if you lose connectivity within a lesson). There's
also some issues with existing view model code that wrongly assumed
audio couldn't be in a failure state at that point. This has been fixed.
4. Moves the list of topics to cache to be in the same position as the
flag enabling/disabling this functionality.

The download experience isn't perfect, but it's meant to be a helper for
user study coordinators so that they know when lessons can be brought
offline.

Note that the UI aspects of this change are really hacky. This is by
design--I didn't want to overcomplicate the solution, and I wanted to
keep the priming changes fully isolated to make future cleanup easier.

Finally, no new tests were added. I clarified in StateFragmentTest that
the edge cases fixed in this PR need to have corresponding tests, but
I'm actually not sure offhand how to test that audio's playing. I think
this will require additional work. I prefer to push this off to #388,
but I will follow up with tests if anyone wants to push back on this.
  • Loading branch information
BenHenning committed Aug 13, 2020
1 parent 8b6734d commit 48e01a8
Show file tree
Hide file tree
Showing 14 changed files with 472 additions and 241 deletions.
26 changes: 13 additions & 13 deletions app/src/main/java/org/oppia/app/application/ApplicationComponent.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import org.oppia.domain.classify.rules.numericinput.NumericInputRuleModule
import org.oppia.domain.classify.rules.textinput.TextInputRuleModule
import org.oppia.domain.oppialogger.LogStorageModule
import org.oppia.domain.question.QuestionModule
import org.oppia.domain.topic.PrimeTopicAssetsControllerModule
import org.oppia.util.accessibility.AccessibilityModule
import org.oppia.util.caching.CachingModule
import org.oppia.util.gcsresource.GcsResourceModule
Expand All @@ -29,22 +30,21 @@ import org.oppia.util.threading.DispatcherModule
import javax.inject.Provider
import javax.inject.Singleton

/** Root Dagger component for the application. All application-scoped modules should be included in this component. */
/**
* Root Dagger component for the application. All application-scoped modules should be included in
* this component.
*/
@Singleton
@Component(
modules = [
ApplicationModule::class, DispatcherModule::class,
NetworkModule::class, LoggerModule::class,
ContinueModule::class, FractionInputModule::class,
ItemSelectionInputModule::class, MultipleChoiceInputModule::class,
NumberWithUnitsRuleModule::class, NumericInputRuleModule::class,
TextInputRuleModule::class, DragDropSortInputModule::class,
InteractionsModule::class, GcsResourceModule::class,
GlideImageLoaderModule::class, ImageParsingModule::class,
HtmlParserEntityTypeModule::class, CachingModule::class,
QuestionModule::class, LogReportingModule::class,
AccessibilityModule::class, ImageClickInputModule::class,
LogStorageModule::class
ApplicationModule::class, DispatcherModule::class, NetworkModule::class, LoggerModule::class,
ContinueModule::class, FractionInputModule::class, ItemSelectionInputModule::class,
MultipleChoiceInputModule::class, NumberWithUnitsRuleModule::class,
NumericInputRuleModule::class, TextInputRuleModule::class, DragDropSortInputModule::class,
InteractionsModule::class, GcsResourceModule::class, GlideImageLoaderModule::class,
ImageParsingModule::class, HtmlParserEntityTypeModule::class, CachingModule::class,
QuestionModule::class, LogReportingModule::class, AccessibilityModule::class,
ImageClickInputModule::class, LogStorageModule::class, PrimeTopicAssetsControllerModule::class
]
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.oppia.app.model.CellularDataPreference
import org.oppia.app.model.Profile
import org.oppia.app.model.ProfileId
import org.oppia.app.model.State
import org.oppia.app.player.audio.AudioViewModel.UiAudioPlayStatus
import org.oppia.app.viewmodel.ViewModelProvider
import org.oppia.domain.audio.CellularAudioDialogController
import org.oppia.domain.profile.ProfileManagementController
Expand Down Expand Up @@ -95,7 +96,7 @@ class AudioFragmentPresenter @Inject constructor(
viewModel.playStatusLiveData.observe(
fragment,
Observer {
prepared = it != AudioViewModel.UiAudioPlayStatus.LOADING
prepared = it != UiAudioPlayStatus.LOADING && it != UiAudioPlayStatus.FAILED
binding.sbAudioProgress.isEnabled = prepared
}
)
Expand Down
34 changes: 18 additions & 16 deletions app/src/main/java/org/oppia/app/player/audio/AudioViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,14 @@ class AudioViewModel @Inject constructor(
}

private fun processDurationResultLiveData(playProgressResult: AsyncResult<PlayProgress>): Int {
if (playProgressResult.isPending()) {
if (!playProgressResult.isSuccess()) {
return 0
}
return playProgressResult.getOrThrow().duration
}

private fun processPositionResultLiveData(playProgressResult: AsyncResult<PlayProgress>): Int {
if (playProgressResult.isPending()) {
if (!playProgressResult.isSuccess()) {
return 0
}
return playProgressResult.getOrThrow().position
Expand All @@ -159,20 +159,22 @@ class AudioViewModel @Inject constructor(
private fun processPlayStatusResultLiveData(
playProgressResult: AsyncResult<PlayProgress>
): UiAudioPlayStatus {
if (playProgressResult.isPending()) return UiAudioPlayStatus.LOADING
if (playProgressResult.isFailure()) return UiAudioPlayStatus.FAILED
return when (playProgressResult.getOrThrow().type) {
PlayStatus.PREPARED -> {
if (autoPlay) audioPlayerController.play()
autoPlay = false
UiAudioPlayStatus.PREPARED
}
PlayStatus.PLAYING -> UiAudioPlayStatus.PLAYING
PlayStatus.PAUSED -> UiAudioPlayStatus.PAUSED
PlayStatus.COMPLETED -> {
if (hasFeedback) loadAudio(null, false)
hasFeedback = false
UiAudioPlayStatus.COMPLETED
return when {
playProgressResult.isPending() -> UiAudioPlayStatus.LOADING
playProgressResult.isFailure() -> UiAudioPlayStatus.FAILED
else -> when (playProgressResult.getOrThrow().type) {
PlayStatus.PREPARED -> {
if (autoPlay) audioPlayerController.play()
autoPlay = false
UiAudioPlayStatus.PREPARED
}
PlayStatus.PLAYING -> UiAudioPlayStatus.PLAYING
PlayStatus.PAUSED -> UiAudioPlayStatus.PAUSED
PlayStatus.COMPLETED -> {
if (hasFeedback) loadAudio(null, false)
hasFeedback = false
UiAudioPlayStatus.COMPLETED
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import org.oppia.app.model.OnboardingFlow
import org.oppia.app.onboarding.OnboardingActivity
import org.oppia.app.profile.ProfileActivity
import org.oppia.domain.onboarding.OnboardingFlowController
import org.oppia.domain.topic.PrimeTopicAssetsController
import org.oppia.util.data.AsyncResult
import org.oppia.util.logging.ConsoleLogger
import javax.inject.Inject
Expand All @@ -20,7 +21,8 @@ import javax.inject.Inject
class SplashActivityPresenter @Inject constructor(
private val activity: AppCompatActivity,
private val logger: ConsoleLogger,
private val onboardingFlowController: OnboardingFlowController
private val onboardingFlowController: OnboardingFlowController,
private val primeTopicAssetsController: PrimeTopicAssetsController
) {

fun handleOnCreate() {
Expand All @@ -29,6 +31,8 @@ class SplashActivityPresenter @Inject constructor(
WindowManager.LayoutParams.FLAG_FULLSCREEN,
WindowManager.LayoutParams.FLAG_FULLSCREEN
)
// Initiate download support before any additional processing begins.
primeTopicAssetsController.downloadAssets(R.style.AlertDialogTheme)
subscribeToOnboardingFlow()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class StateFragmentTest {
// 10. Testing interactions with custom Oppia tags (including images) render correctly (when manually inspected) and are correctly functional.
// 11. Update the tests to work properly on Robolectric (requires idling resource + replacing the dispatchers to leverage a coordinated test dispatcher library).
// 12. Add tests for hints & solutions.
// 13. Add tests for audio states.
// 13. Add tests for audio states, including: audio playing & having an error, or no-network connectivity scenarios. See the PR introducing this comment & #1340 / #1341 for context.
// TODO(#56): Add support for testing that previous/next button states are properly retained on config changes.

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.oppia.domain.topic

// TODO(#169): Remove this controller & download priming once downloads are properly supported.

/**
* Controller for conditionally pre-priming assets to enable full download support. Whether
* downloading is enabled is gated by the [org.oppia.util.caching.CacheAssetsLocally] annotation.
*/
interface PrimeTopicAssetsController {
/**
* Initiates asset downloading in the background. UI affordances will be shown before and after
* priming, if it's enabled (otherwise nothing will show).
*
* @param dialogStyleResId the resource ID for the alert dialog style used for the dialog-based UI
* affordances
*/
fun downloadAssets(dialogStyleResId: Int)
}
Loading

0 comments on commit 48e01a8

Please sign in to comment.