Code review

Infomaniak · Aug 30, 2024 · 9829680 · 9829680
1 parent 77a9a54
commit 9829680
Show file tree

Hide file tree

Showing 10 changed files with 54 additions and 46 deletions.
diff --git a/app/build.gradle.kts b/app/build.gradle.kts
@@ -83,7 +83,6 @@ dependencies {
     implementation(libs.androidx.adaptive.layout)
     implementation(libs.androidx.adaptive.navigation)
 
-    implementation(libs.swisstransfer.database)
     implementation(libs.swisstransfer.core)
 
     // Compose preview tools

diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/MainActivity.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/MainActivity.kt
@@ -22,23 +22,15 @@ import android.os.Bundle
 import androidx.activity.ComponentActivity
 import androidx.activity.compose.setContent
 import androidx.activity.enableEdgeToEdge
-import androidx.lifecycle.lifecycleScope
-import com.infomaniak.multiplatform_swisstransfer.SwissTransferInjection
 import com.infomaniak.swisstransfer.ui.screen.main.MainScreen
 import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme
 import dagger.hilt.android.AndroidEntryPoint
-import kotlinx.coroutines.launch
-import javax.inject.Inject
 
 @AndroidEntryPoint
 class MainActivity : ComponentActivity() {
 
-    @Inject
-    lateinit var swissTransferInjection: SwissTransferInjection
-
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
-        lifecycleScope.launch { swissTransferInjection.loadDefaultAccount() }
         enableEdgeToEdge()
         setContent {
             SwissTransferTheme {

diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/MainApplication.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/MainApplication.kt
@@ -19,8 +19,21 @@
 package com.infomaniak.swisstransfer.ui
 
 import android.app.Application
+import androidx.lifecycle.DefaultLifecycleObserver
+import androidx.lifecycle.LifecycleOwner
+import androidx.lifecycle.lifecycleScope
+import com.infomaniak.multiplatform_swisstransfer.SwissTransferInjection
 import dagger.hilt.android.HiltAndroidApp
+import kotlinx.coroutines.launch
+import javax.inject.Inject
 
 @HiltAndroidApp
-class MainApplication : Application() {
+class MainApplication : Application(), DefaultLifecycleObserver {
+
+    @Inject
+    lateinit var swissTransferInjection: SwissTransferInjection
+
+    override fun onStart(owner: LifecycleOwner) {
+        owner.lifecycleScope.launch { swissTransferInjection.loadDefaultAccount() }
+    }
 }
diff --git a/...java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt b/...java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsDownloadsLimitScreen.kt
@@ -58,10 +58,10 @@ fun SettingsDownloadsLimitScreen(
         ) {
             SettingTitle(titleRes = R.string.settingsDownloadsLimitTitle)
 
-            var selectedItem by rememberSaveable { mutableIntStateOf(DownloadLimit.entries.indexOf(downloadLimit)) }
-            SingleSelectOptions(DownloadsLimitOption.entries, { selectedItem }, {
-                selectedItem = it
-                val selectedDownloadLimit = DownloadLimit.entries[it]
+            var selectedItem by rememberSaveable { mutableIntStateOf(downloadLimit.ordinal) }
+            SingleSelectOptions(DownloadsLimitOption.entries, { selectedItem }, { position ->
+                selectedItem = position
+                val selectedDownloadLimit = DownloadLimit.entries[position]
                 onDownloadLimitChange(selectedDownloadLimit)
             })
         }

diff --git a/.../java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt b/.../java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsEmailLanguageScreen.kt
@@ -59,10 +59,10 @@ fun SettingsEmailLanguageScreen(
         ) {
             SettingTitle(titleRes = R.string.settingsEmailLanguageTitle)
 
-            var selectedItem by rememberSaveable { mutableIntStateOf(EmailLanguage.entries.indexOf(emailLanguage)) }
-            SingleSelectOptions(EmailLanguageOption.entries, { selectedItem }, {
-                selectedItem = it
-                val selectedEmailLanguage = EmailLanguage.entries[it]
+            var selectedItem by rememberSaveable { mutableIntStateOf(emailLanguage.ordinal) }
+            SingleSelectOptions(EmailLanguageOption.entries, { selectedItem }, { position ->
+                selectedItem = position
+                val selectedEmailLanguage = EmailLanguage.entries[position]
                 onEmailLanguageChange(selectedEmailLanguage)
             })
         }

diff --git a/...c/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsScreenWrapper.kt b/...c/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsScreenWrapper.kt
@@ -40,10 +40,6 @@ import androidx.compose.ui.res.stringResource
 import androidx.hilt.navigation.compose.hiltViewModel
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import com.infomaniak.multiplatform_swisstransfer.common.interfaces.appSettings.AppSettings
-import com.infomaniak.multiplatform_swisstransfer.common.models.DownloadLimit
-import com.infomaniak.multiplatform_swisstransfer.common.models.EmailLanguage
-import com.infomaniak.multiplatform_swisstransfer.common.models.Theme
-import com.infomaniak.multiplatform_swisstransfer.common.models.ValidityPeriod
 import com.infomaniak.swisstransfer.R
 import com.infomaniak.swisstransfer.extensions.goToPlayStore
 import com.infomaniak.swisstransfer.extensions.openAppNotificationSettings
@@ -61,16 +57,18 @@ fun SettingsScreenWrapper(
     settingsViewModel: SettingsViewModel = hiltViewModel<SettingsViewModel>(),
 ) {
     val appSettings by settingsViewModel.appSettingsFlow.collectAsStateWithLifecycle(null)
-    TwoPaneScaffold<SettingsOptionScreens>(
-        windowAdaptiveInfo,
-        listPane = { ListPane(this, appSettings) },
-        detailPane = { DetailPane(settingsViewModel, this, appSettings) }
-    )
+    appSettings?.let { safeAppSettings ->
+        TwoPaneScaffold<SettingsOptionScreens>(
+            windowAdaptiveInfo,
+            listPane = { ListPane(this, safeAppSettings) },
+            detailPane = { DetailPane(settingsViewModel, this, safeAppSettings) }
+        )
+    }
 }
 
 @OptIn(ExperimentalMaterial3AdaptiveApi::class)
 @Composable
-private fun ListPane(navigator: ThreePaneScaffoldNavigator<SettingsOptionScreens>, appSettings: AppSettings?) {
+private fun ListPane(navigator: ThreePaneScaffoldNavigator<SettingsOptionScreens>, appSettings: AppSettings) {
     val context = LocalContext.current
     val aboutURL = stringResource(R.string.urlAbout)
     val userReportURL = stringResource(R.string.urlUserReportAndroid)
@@ -98,7 +96,7 @@ private fun ListPane(navigator: ThreePaneScaffoldNavigator<SettingsOptionScreens
 private fun DetailPane(
     settingsViewModel: SettingsViewModel,
     navigator: ThreePaneScaffoldNavigator<SettingsOptionScreens>,
-    appSettings: AppSettings?
+    appSettings: AppSettings
 ) {
     var lastSelectedScreen by rememberSaveable { mutableStateOf<SettingsOptionScreens?>(null) }
 
@@ -110,24 +108,24 @@ private fun DetailPane(
 
     when (destination) {
         THEME -> {
-            SettingsThemeScreen(appSettings?.theme ?: Theme.SYSTEM, navigateBack) {
+            SettingsThemeScreen(appSettings.theme, navigateBack) {
                 settingsViewModel.setTheme(it)
             }
         }
         VALIDITY_PERIOD -> {
-            val validityPeriod = appSettings?.validityPeriod ?: ValidityPeriod.THIRTY
+            val validityPeriod = appSettings.validityPeriod
             SettingsValidityPeriodScreen(validityPeriod, navigateBack) {
                 settingsViewModel.setValidityPeriod(it)
             }
         }
         DOWNLOAD_LIMIT -> {
-            val downloadLimit = appSettings?.downloadLimit ?: DownloadLimit.TWOHUNDREDFIFTY
+            val downloadLimit = appSettings.downloadLimit
             SettingsDownloadsLimitScreen(downloadLimit, navigateBack) {
                 settingsViewModel.setDownloadLimit(it)
             }
         }
         EMAIL_LANGUAGE -> {
-            val emailLanguage = appSettings?.emailLanguage ?: EmailLanguage.FRENCH
+            val emailLanguage = appSettings.emailLanguage
             SettingsEmailLanguageScreen(emailLanguage, navigateBack) {
                 settingsViewModel.setEmailLanguage(it)
             }

diff --git a/...src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt b/...src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsThemeScreen.kt
@@ -47,7 +47,11 @@ import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme
 import com.infomaniak.swisstransfer.ui.utils.PreviewMobile
 
 @Composable
-fun SettingsThemeScreen(theme: Theme, navigateBack: (() -> Unit)?, onThemeUpdate: (Theme) -> Unit) {
+fun SettingsThemeScreen(
+    theme: Theme,
+    navigateBack: (() -> Unit)?,
+    onThemeUpdate: (Theme) -> Unit
+) {
     Scaffold(topBar = {
         val canDisplayBackButton = navigateBack?.let { TopAppBarButton.backButton(navigateBack) }
         SwissTransferTobAppBar(R.string.settingsOptionTheme, navigationMenu = canDisplayBackButton)
@@ -59,10 +63,10 @@ fun SettingsThemeScreen(theme: Theme, navigateBack: (() -> Unit)?, onThemeUpdate
         ) {
             SettingTitle(titleRes = R.string.settingsThemeTitle)
 
-            var selectedItem by rememberSaveable { mutableIntStateOf(Theme.entries.indexOf(theme)) }
-            SingleSelectOptions(ThemeOption.entries, { selectedItem }, {
-                selectedItem = it
-                val selectedTheme = Theme.entries[it]
+            var selectedItem by rememberSaveable { mutableIntStateOf(theme.ordinal) }
+            SingleSelectOptions(ThemeOption.entries, { selectedItem }, { position ->
+                selectedItem = position
+                val selectedTheme = Theme.entries[position]
                 onThemeUpdate(selectedTheme)
             })
         }

diff --git a/...ettings/SettingsDurationValidityScreen.kt → .../settings/SettingsValidityPeriodScreen.kt b/...ettings/SettingsDurationValidityScreen.kt → .../settings/SettingsValidityPeriodScreen.kt
@@ -44,7 +44,7 @@ import com.infomaniak.swisstransfer.ui.utils.PreviewMobile
 
 @Composable
 fun SettingsValidityPeriodScreen(
-    validityPeriod: ValidityPeriod? = ValidityPeriod.THIRTY,
+    validityPeriod: ValidityPeriod,
     navigateBack: (() -> Unit)?,
     onValidityPeriodChange: (ValidityPeriod) -> Unit
 ) {
@@ -59,10 +59,10 @@ fun SettingsValidityPeriodScreen(
         ) {
             SettingTitle(titleRes = R.string.settingsValidityPeriodTitle)
 
-            var selectedItem by rememberSaveable { mutableIntStateOf(ValidityPeriod.entries.indexOf(validityPeriod)) }
-            SingleSelectOptions(ValidityPeriodOption.entries, { selectedItem }, {
-                selectedItem = it
-                val selectedValidityPeriod = ValidityPeriod.entries[it]
+            var selectedItem by rememberSaveable { mutableIntStateOf(validityPeriod.ordinal) }
+            SingleSelectOptions(ValidityPeriodOption.entries, { selectedItem }, { position ->
+                selectedItem = position
+                val selectedValidityPeriod = ValidityPeriod.entries[position]
                 onValidityPeriodChange(selectedValidityPeriod)
             })
         }
@@ -86,6 +86,7 @@ private fun SettingsThemeScreenPreview() {
     SwissTransferTheme {
         Surface {
             SettingsValidityPeriodScreen(
+                validityPeriod = ValidityPeriod.THIRTY,
                 navigateBack = {},
                 onValidityPeriodChange = {}
             )

diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/main/settings/SettingsViewModel.kt
@@ -36,8 +36,7 @@ class SettingsViewModel @Inject constructor(
     private val swissTransferInjection: SwissTransferInjection,
     @IoDispatcher private val ioDispatcher: CoroutineDispatcher,
 ) : ViewModel() {
-    private val appSettingsManager
-        inline get() = swissTransferInjection.appSettingsManager
+    private val appSettingsManager inline get() = swissTransferInjection.appSettingsManager
 
     val appSettingsFlow = appSettingsManager.appSettings
 

diff --git a/settings.gradle.kts b/settings.gradle.kts
@@ -17,7 +17,9 @@ dependencyResolutionManagement {
     @Suppress("UnstableApiUsage")
     repositories {
         google()
-        mavenLocal()
+        if (gradle.startParameter.taskNames.any { it.contains("Debug") }) {
+            mavenLocal()
+        }
         mavenCentral()
         maven { url = uri("https://jitpack.io") }
     }