Skip to content

Commit

Permalink
Fixes part of oppia#4938: Use TranslationController as the source of …
Browse files Browse the repository at this point in the history
…truth for the audio language setting (oppia#5487)

## Explanation
Fixes part of oppia#4938

This primarily updates ``ProfileManagementController`` to use
``TranslationController`` as the source of truth for audio language
(rather than using the audio language property stored within the
``Profile`` proto). This has some noteworthy advantages:
- It allows for proper translation setting fallback behaviors to be
enabled for the audio language setting without needing to migrate UI
code over to ``TranslationController``.
- It isolates changes to the domain layer (with one exception: reading
the audio language setting now uses a new getter in
``ProfileManagementController`` rather than reading the profile
directly).

Some peripheral changes were also needed as part of this:
- A bunch of tests needed to be disabled in Gradle now that different
options UIs (including for reading text) may depend on
``TranslationController`` (which is only fully configured in Bazel
builds).
- The ``Profile`` proto was updated to remove its audio language
setting. This means that existing users will revert back to whichever
default ``TranslationController`` decides for them (most likely English,
but it depends on several factors). This is considered a reasonable
regression since most users are unlikely to depend on the automatic
audio language setting, and the app is currently in a beta state so
regressions like this should be expected.
- French and Chinese were removed from the list of ``AudioLanguage``s
since they are not currently supported by the Oppia Android app (per
``OppiaLanguage`` which, plus the configured supported language
textproto files, determine for which languages the app is guaranteeing
support).
- ``ProfileManagementControllerTest`` was updated to have much more
thorough testing around audio language.
- ``TranslationController`` was updated to use a
``PersistentCacheStore`` backing for audio and written translation
languages (in addition to app language which was already supported).
- As part of the previous change,
``TranslationController.updateAppLanguage()`` and its related tests were
updated to verify the _previous_ language is returned, not the current
(for consistency with voiceover and written translations).

Long-term, ``AudioLanguage`` should be removed (along with its
corresponding functionality in ``ProfileManagementController``) in favor
of using ``TranslationController`` and ``OppiaLocale`` as the bases for
managing language functionality in the UI layer.

Note that the Gradle version of the app will have increased degraded
functionality in the options menu due to no supported language
configuration being included in the build for that app (see the
corresponding comment thread in this PR for more context). This is
considered a reasonable medium-term gap as developers ought to be using
the Bazel build of the app, anyway, as the Gradle version has
significant functional limitations for all aspects of language selection
and management (due to the lack of language configuration).

## 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)).

## For UI-specific PRs only

Options screen (without changes):


![image](https://github.com/user-attachments/assets/065caa9f-5815-42a5-98ec-278186fa8dcd)

Options screen (with changes):


![image](https://github.com/user-attachments/assets/f4ece283-bf0e-4490-a1bb-57e77bb7a834)

I also verified setting a profile audio setting (Portuguese) on a
``develop`` branch build and then upgrading to a build from this branch.
There are no crashes or stability issues, and (per my device setup) the
audio language does revert back to English as expected.

I also verified that, like before, the audio setting persists across app
instances when changed and is distinct between multiple profiles.
  • Loading branch information
BenHenning authored and adhiamboperes committed Aug 24, 2024
1 parent 1c6c9f7 commit 02eadc1
Show file tree
Hide file tree
Showing 13 changed files with 341 additions and 168 deletions.
3 changes: 2 additions & 1 deletion app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ def filesToExclude = [
'**/*AppLanguageLocaleHandlerTest*.kt',
'**/*AppLanguageResourceHandlerTest*.kt',
'**/*AppLanguageWatcherMixinTest*.kt',
'**/*ActivityLanguageLocaleHandlerTest*.kt'
'**/*ActivityLanguageLocaleHandlerTest*.kt',
'**/*OptionsFragmentTest*.kt', // Excludes 2 tests.
]
_excludeSourceFiles(filesToExclude)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import androidx.databinding.ObservableField
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.Transformations
import androidx.lifecycle.ViewModel
import org.oppia.android.app.fragment.FragmentScope
import org.oppia.android.app.model.AudioLanguage
import org.oppia.android.app.model.OppiaLanguage
import org.oppia.android.app.model.Profile
import org.oppia.android.app.model.ProfileId
Expand All @@ -20,7 +20,8 @@ import org.oppia.android.util.data.DataProviders.Companion.combineWith
import org.oppia.android.util.data.DataProviders.Companion.toLiveData
import javax.inject.Inject

/** [ViewModel] for [OptionsFragment]. */
private const val OPTIONS_ITEM_VIEW_MODEL_APP_AUDIO_LANGUAGE_PROVIDER_ID =
"OPTIONS_ITEM_VIEW_MODEL_APP_AUDIO_LANGUAGE_PROVIDER_ID"
private const val OPTIONS_ITEM_VIEW_MODEL_LIST_PROVIDER_ID =
"OPTIONS_ITEM_VIEW_MODEL_LIST_PROVIDER_ID"

Expand Down Expand Up @@ -67,11 +68,14 @@ class OptionControlsViewModel @Inject constructor(
}

private fun createOptionsItemViewModelProvider(): DataProvider<List<OptionsItemViewModel>> {
val appAudioLangProvider =
translationController.getAppLanguage(profileId).combineWith(
profileManagementController.getAudioLanguage(profileId),
OPTIONS_ITEM_VIEW_MODEL_APP_AUDIO_LANGUAGE_PROVIDER_ID
) { appLanguage, audioLanguage -> appLanguage to audioLanguage }
return profileManagementController.getProfile(profileId).combineWith(
translationController.getAppLanguage(profileId),
OPTIONS_ITEM_VIEW_MODEL_LIST_PROVIDER_ID,
::processViewModelList
)
appAudioLangProvider, OPTIONS_ITEM_VIEW_MODEL_LIST_PROVIDER_ID
) { profile, (appLang, audioLang) -> processViewModelList(profile, appLang, audioLang) }
}

private fun processViewModelListsResult(
Expand All @@ -93,12 +97,13 @@ class OptionControlsViewModel @Inject constructor(

private fun processViewModelList(
profile: Profile,
oppiaLanguage: OppiaLanguage
appLanguage: OppiaLanguage,
audioLanguage: AudioLanguage
): List<OptionsItemViewModel> {
return listOfNotNull(
createReadingTextSizeViewModel(profile),
createAppLanguageViewModel(oppiaLanguage),
createAudioLanguageViewModel(profile)
createAppLanguageViewModel(appLanguage),
createAudioLanguageViewModel(audioLanguage)
)
}

Expand All @@ -117,12 +122,14 @@ class OptionControlsViewModel @Inject constructor(
)
}

private fun createAudioLanguageViewModel(profile: Profile): OptionsAudioLanguageViewModel {
private fun createAudioLanguageViewModel(
audioLanguage: AudioLanguage
): OptionsAudioLanguageViewModel {
return OptionsAudioLanguageViewModel(
routeToAudioLanguageListListener,
loadAudioLanguageListListener,
profile.audioLanguage,
resourceHandler.computeLocalizedDisplayName(profile.audioLanguage)
audioLanguage,
resourceHandler.computeLocalizedDisplayName(audioLanguage)
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import org.oppia.android.R
import org.oppia.android.app.fragment.FragmentScope
import org.oppia.android.app.model.AudioLanguage
import org.oppia.android.app.model.CellularDataPreference
import org.oppia.android.app.model.Profile
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.Spotlight
import org.oppia.android.app.model.State
Expand Down Expand Up @@ -147,15 +146,15 @@ class AudioFragmentPresenter @Inject constructor(
) as? SpotlightManager
}

private fun getProfileData(): LiveData<String> {
private fun retrieveAudioLanguageCode(): LiveData<String> {
return Transformations.map(
profileManagementController.getProfile(profileId).toLiveData(),
::processGetProfileResult
profileManagementController.getAudioLanguage(profileId).toLiveData(),
::processAudioLanguageResult
)
}

private fun subscribeToAudioLanguageLiveData() {
getProfileData().observe(
retrieveAudioLanguageCode().observe(
activity,
Observer<String> { result ->
audioViewModel.selectedLanguageCode = result
Expand All @@ -165,11 +164,9 @@ class AudioFragmentPresenter @Inject constructor(
}

/** Gets language code by [AudioLanguage]. */
private fun getAudioLanguage(audioLanguage: AudioLanguage): String {
private fun computeLanguageCode(audioLanguage: AudioLanguage): String {
return when (audioLanguage) {
AudioLanguage.HINDI_AUDIO_LANGUAGE -> "hi"
AudioLanguage.FRENCH_AUDIO_LANGUAGE -> "fr"
AudioLanguage.CHINESE_AUDIO_LANGUAGE -> "zh"
AudioLanguage.BRAZILIAN_PORTUGUESE_LANGUAGE -> "pt"
AudioLanguage.ARABIC_LANGUAGE -> "ar"
AudioLanguage.NIGERIAN_PIDGIN_LANGUAGE -> "pcm"
Expand All @@ -178,16 +175,16 @@ class AudioFragmentPresenter @Inject constructor(
}
}

private fun processGetProfileResult(profileResult: AsyncResult<Profile>): String {
val profile = when (profileResult) {
private fun processAudioLanguageResult(languageResult: AsyncResult<AudioLanguage>): String {
val audioLanguage = when (languageResult) {
is AsyncResult.Failure -> {
oppiaLogger.e("AudioFragment", "Failed to retrieve profile", profileResult.error)
Profile.getDefaultInstance()
oppiaLogger.e("AudioFragment", "Failed to retrieve audio language", languageResult.error)
AudioLanguage.AUDIO_LANGUAGE_UNSPECIFIED
}
is AsyncResult.Pending -> Profile.getDefaultInstance()
is AsyncResult.Success -> profileResult.value
is AsyncResult.Pending -> AudioLanguage.AUDIO_LANGUAGE_UNSPECIFIED
is AsyncResult.Success -> languageResult.value
}
return getAudioLanguage(profile.audioLanguage)
return computeLanguageCode(audioLanguage)
}

/** Sets selected language code in presenter and ViewModel. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ class LanguageDialogFragment : InjectableDialogFragment() {
for (languageCode in languageCodeArrayList) {
val audioLanguage = when (machineLocale.run { languageCode.toMachineLowerCase() }) {
"hi" -> AudioLanguage.HINDI_AUDIO_LANGUAGE
"fr" -> AudioLanguage.FRENCH_AUDIO_LANGUAGE
"zh" -> AudioLanguage.CHINESE_AUDIO_LANGUAGE
"pt", "pt-br" -> AudioLanguage.BRAZILIAN_PORTUGUESE_LANGUAGE
"ar" -> AudioLanguage.ARABIC_LANGUAGE
"pcm" -> AudioLanguage.NIGERIAN_PIDGIN_LANGUAGE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,6 @@ class AppLanguageResourceHandler @Inject constructor(
fun computeLocalizedDisplayName(audioLanguage: AudioLanguage): String {
return when (audioLanguage) {
AudioLanguage.HINDI_AUDIO_LANGUAGE -> getLocalizedDisplayName("hi")
AudioLanguage.FRENCH_AUDIO_LANGUAGE -> getLocalizedDisplayName("fr")
AudioLanguage.CHINESE_AUDIO_LANGUAGE -> getLocalizedDisplayName("zh")
AudioLanguage.BRAZILIAN_PORTUGUESE_LANGUAGE -> getLocalizedDisplayName("pt", "BR")
AudioLanguage.ARABIC_LANGUAGE -> getLocalizedDisplayName("ar", "EG")
AudioLanguage.NIGERIAN_PIDGIN_LANGUAGE ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ import javax.inject.Singleton
class AudioLanguageFragmentTest {
private companion object {
private const val ENGLISH_BUTTON_INDEX = 0
private const val PORTUGUESE_BUTTON_INDEX = 4
private const val ARABIC_BUTTON_INDEX = 5
private const val NIGERIAN_PIDGIN_BUTTON_INDEX = 6
private const val PORTUGUESE_BUTTON_INDEX = 2
private const val ARABIC_BUTTON_INDEX = 3
private const val NIGERIAN_PIDGIN_BUTTON_INDEX = 4
}

@get:Rule val initializeDefaultLocaleRule = InitializeDefaultLocaleRule()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ import org.oppia.android.domain.platformparameter.PlatformParameterModule
import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModule
import org.oppia.android.domain.question.QuestionModule
import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule
import org.oppia.android.testing.BuildEnvironment
import org.oppia.android.testing.OppiaTestRule
import org.oppia.android.testing.RunOn
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.firebase.TestAuthenticationModule
import org.oppia.android.testing.junit.InitializeDefaultLocaleRule
Expand Down Expand Up @@ -180,8 +182,10 @@ class ReadingTextSizeFragmentTest {
}
}

// Requires language configurations.
@Test
@Config(qualifiers = "sw600dp")
@RunOn(buildEnvironments = [BuildEnvironment.BAZEL])
fun testTextSize_changeTextSizeToMedium_mediumItemIsSelected() {
launch<OptionsActivity>(createOptionActivityIntent(0, true)).use {
testCoroutineDispatchers.runCurrent()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,6 @@ class AppLanguageResourceHandlerTest {
// TODO(#3793): Remove this once OppiaLanguage is used as the source of truth.
@Test
@Iteration("hi", "lang=HINDI_AUDIO_LANGUAGE", "expectedDisplayText=हिन्दी")
@Iteration("fr", "lang=FRENCH_AUDIO_LANGUAGE", "expectedDisplayText=Français")
@Iteration("zh", "lang=CHINESE_AUDIO_LANGUAGE", "expectedDisplayText=中文")
@Iteration("pr-pt", "lang=BRAZILIAN_PORTUGUESE_LANGUAGE", "expectedDisplayText=Português")
@Iteration("ar", "lang=ARABIC_LANGUAGE", "expectedDisplayText=العربية")
@Iteration("pcm", "lang=NIGERIAN_PIDGIN_LANGUAGE", "expectedDisplayText=Naijá")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import android.provider.MediaStore
import androidx.exifinterface.media.ExifInterface
import kotlinx.coroutines.Deferred
import org.oppia.android.app.model.AudioLanguage
import org.oppia.android.app.model.AudioTranslationLanguageSelection
import org.oppia.android.app.model.DeviceSettings
import org.oppia.android.app.model.OppiaLanguage
import org.oppia.android.app.model.Profile
import org.oppia.android.app.model.ProfileAvatar
import org.oppia.android.app.model.ProfileDatabase
Expand All @@ -22,6 +24,7 @@ import org.oppia.android.domain.oppialogger.LoggingIdentifierController
import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.domain.oppialogger.analytics.LearnerAnalyticsLogger
import org.oppia.android.domain.oppialogger.exceptions.ExceptionsController
import org.oppia.android.domain.translation.TranslationController
import org.oppia.android.util.data.AsyncResult
import org.oppia.android.util.data.DataProvider
import org.oppia.android.util.data.DataProviders
Expand Down Expand Up @@ -64,8 +67,8 @@ private const val SET_CURRENT_PROFILE_ID_PROVIDER_ID = "set_current_profile_id_p
private const val UPDATE_READING_TEXT_SIZE_PROVIDER_ID =
"update_reading_text_size_provider_id"
private const val UPDATE_APP_LANGUAGE_PROVIDER_ID = "update_app_language_provider_id"
private const val UPDATE_AUDIO_LANGUAGE_PROVIDER_ID =
"update_audio_language_provider_id"
private const val GET_AUDIO_LANGUAGE_PROVIDER_ID = "get_audio_language_provider_id"
private const val UPDATE_AUDIO_LANGUAGE_PROVIDER_ID = "update_audio_language_provider_id"
private const val UPDATE_LEARNER_ID_PROVIDER_ID = "update_learner_id_provider_id"
private const val SET_SURVEY_LAST_SHOWN_TIMESTAMP_PROVIDER_ID =
"record_survey_last_shown_timestamp_provider_id"
Expand Down Expand Up @@ -93,7 +96,8 @@ class ProfileManagementController @Inject constructor(
private val enableLearnerStudyAnalytics: PlatformParameterValue<Boolean>,
@EnableLoggingLearnerStudyIds
private val enableLoggingLearnerStudyIds: PlatformParameterValue<Boolean>,
private val profileNameValidator: ProfileNameValidator
private val profileNameValidator: ProfileNameValidator,
private val translationController: TranslationController
) {
private var currentProfileId: Int = DEFAULT_LOGGED_OUT_INTERNAL_PROFILE_ID
private val profileDataStore =
Expand Down Expand Up @@ -275,7 +279,6 @@ class ProfileManagementController @Inject constructor(
dateCreatedTimestampMs = oppiaClock.getCurrentTimeMs()
this.isAdmin = isAdmin
readingTextSize = ReadingTextSize.MEDIUM_TEXT_SIZE
audioLanguage = AudioLanguage.ENGLISH_AUDIO_LANGUAGE
numberOfLogins = 0

if (enableLoggingLearnerStudyIds.value) {
Expand Down Expand Up @@ -628,34 +631,52 @@ class ProfileManagementController @Inject constructor(
}
}

/**
* Returns the current audio language configured for the specified profile ID, as possibly set by
* [updateAudioLanguage].
*
* The return [DataProvider] will automatically update for subsequent calls to
* [updateAudioLanguage] for this [profileId].
*/
fun getAudioLanguage(profileId: ProfileId): DataProvider<AudioLanguage> {
return translationController.getAudioTranslationContentLanguage(
profileId
).transform(GET_AUDIO_LANGUAGE_PROVIDER_ID) { oppiaLanguage ->
when (oppiaLanguage) {
OppiaLanguage.UNRECOGNIZED, OppiaLanguage.LANGUAGE_UNSPECIFIED, OppiaLanguage.HINGLISH,
OppiaLanguage.PORTUGUESE, OppiaLanguage.SWAHILI -> AudioLanguage.AUDIO_LANGUAGE_UNSPECIFIED
OppiaLanguage.ARABIC -> AudioLanguage.ARABIC_LANGUAGE
OppiaLanguage.ENGLISH -> AudioLanguage.ENGLISH_AUDIO_LANGUAGE
OppiaLanguage.HINDI -> AudioLanguage.HINDI_AUDIO_LANGUAGE
OppiaLanguage.BRAZILIAN_PORTUGUESE -> AudioLanguage.BRAZILIAN_PORTUGUESE_LANGUAGE
OppiaLanguage.NIGERIAN_PIDGIN -> AudioLanguage.NIGERIAN_PIDGIN_LANGUAGE
}
}
}

/**
* Updates the audio language of the profile.
*
* @param profileId the ID corresponding to the profile being updated.
* @param audioLanguage New audio language for the profile being updated.
* @return a [DataProvider] that indicates the success/failure of this update operation.
* @param profileId the ID corresponding to the profile being updated
* @param audioLanguage New audio language for the profile being updated
* @return a [DataProvider] that indicates the success/failure of this update operation
*/
fun updateAudioLanguage(profileId: ProfileId, audioLanguage: AudioLanguage): DataProvider<Any?> {
val deferred = profileDataStore.storeDataWithCustomChannelAsync(
updateInMemoryCache = true
) {
val profile =
it.profilesMap[profileId.internalId] ?: return@storeDataWithCustomChannelAsync Pair(
it,
ProfileActionStatus.PROFILE_NOT_FOUND
)
val updatedProfile = profile.toBuilder().setAudioLanguage(audioLanguage).build()
val profileDatabaseBuilder = it.toBuilder().putProfiles(
profileId.internalId,
updatedProfile
)
Pair(profileDatabaseBuilder.build(), ProfileActionStatus.SUCCESS)
}
return dataProviders.createInMemoryDataProviderAsync(
UPDATE_AUDIO_LANGUAGE_PROVIDER_ID
) {
return@createInMemoryDataProviderAsync getDeferredResult(profileId, null, deferred)
}
val audioSelection = AudioTranslationLanguageSelection.newBuilder().apply {
this.selectedLanguage = when (audioLanguage) {
AudioLanguage.UNRECOGNIZED, AudioLanguage.AUDIO_LANGUAGE_UNSPECIFIED,
AudioLanguage.NO_AUDIO -> OppiaLanguage.LANGUAGE_UNSPECIFIED
AudioLanguage.ENGLISH_AUDIO_LANGUAGE -> OppiaLanguage.ENGLISH
AudioLanguage.HINDI_AUDIO_LANGUAGE -> OppiaLanguage.HINDI
AudioLanguage.BRAZILIAN_PORTUGUESE_LANGUAGE -> OppiaLanguage.BRAZILIAN_PORTUGUESE
AudioLanguage.ARABIC_LANGUAGE -> OppiaLanguage.ARABIC
AudioLanguage.NIGERIAN_PIDGIN_LANGUAGE -> OppiaLanguage.NIGERIAN_PIDGIN
}
}.build()
// The transformation is needed to reinterpreted the result of the update to 'Any?'.
return translationController.updateAudioTranslationContentLanguage(
profileId, audioSelection
).transform(UPDATE_AUDIO_LANGUAGE_PROVIDER_ID) { value -> value }
}

/**
Expand Down
Loading

0 comments on commit 02eadc1

Please sign in to comment.