Make auto add to up next settings a UserSetting

CHANGELOG.md

@@ -1,3 +1,9 @@
+Unknown
+-----
+*   Bug Fixes:
+    *   Fixed auto archive settings getting lost when switching languages
+        ([#1234](https://github.com/Automattic/pocket-casts-android/pull/1234))
+
 7.45
 -----
 *   Bug Fixes:

app/src/androidTest/java/au/com/shiftyjelly/pocketcasts/models/db/AutoArchiveTest.kt

@@ -1,6 +1,7 @@
 package au.com.shiftyjelly.pocketcasts.models.db
 
 import android.content.Context
+import android.content.SharedPreferences
 import androidx.room.Room
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.platform.app.InstrumentationRegistry
@@ -9,6 +10,9 @@ import au.com.shiftyjelly.pocketcasts.models.entity.Podcast
 import au.com.shiftyjelly.pocketcasts.models.entity.PodcastEpisode
 import au.com.shiftyjelly.pocketcasts.models.type.EpisodePlayingStatus
 import au.com.shiftyjelly.pocketcasts.preferences.Settings
+import au.com.shiftyjelly.pocketcasts.preferences.UserSetting
+import au.com.shiftyjelly.pocketcasts.preferences.model.AutoArchiveAfterPlayingSetting
+import au.com.shiftyjelly.pocketcasts.preferences.model.AutoArchiveInactiveSetting
 import au.com.shiftyjelly.pocketcasts.repositories.download.DownloadManager
 import au.com.shiftyjelly.pocketcasts.repositories.file.FileStorage
 import au.com.shiftyjelly.pocketcasts.repositories.playback.UpNextQueue
@@ -54,11 +58,17 @@ class AutoArchiveTest {
         testDb.close()
     }
 
-    private fun episodeManagerFor(db: AppDatabase, played: Settings.AutoArchiveAfterPlaying, inactive: Settings.AutoArchiveInactive, includeStarred: Boolean = false, excludedPodcasts: List<String> = emptyList()): EpisodeManager {
+    private fun episodeManagerFor(
+        db: AppDatabase,
+        played: AutoArchiveAfterPlayingSetting,
+        inactive: AutoArchiveInactiveSetting,
+        includeStarred: Boolean = false,
+        excludedPodcasts: List<String> = emptyList()
+    ): EpisodeManager {
         val settings = mock<Settings> {
-            on { getAutoArchiveInactive() } doReturn inactive
-            on { getAutoArchiveAfterPlaying() } doReturn played
-            on { getAutoArchiveIncludeStarred() } doReturn includeStarred
+            on { autoArchiveInactive } doReturn MockUserSetting(inactive)
+            on { autoArchiveAfterPlaying } doReturn MockUserSetting(played)
+            on { autoArchiveIncludeStarred } doReturn MockUserSetting(includeStarred)
             on { getAutoArchiveExcludedPodcasts() } doReturn excludedPodcasts
         }
         return EpisodeManagerImpl(settings, fileStorage, downloadManager, context, db, podcastCacheServerManager, userEpisodeManager)
@@ -81,7 +91,7 @@ class AutoArchiveTest {
     @Test
     fun testNever() {
         val uuid = UUID.randomUUID().toString()
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Never)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Never)
         val podcast = Podcast(UUID.randomUUID().toString())
         val podcastManager = podcastManagerThatReturns(podcast)
         val episode = PodcastEpisode(uuid = uuid, podcastUuid = podcast.uuid, isArchived = false, publishedDate = Date())
@@ -95,7 +105,7 @@ class AutoArchiveTest {
 
     @Test
     fun testInactive30Days() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Days30)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Days30)
         val podcastUUID = UUID.randomUUID().toString()
         val podcast = Podcast(uuid = podcastUUID, isSubscribed = true)
         val podcastManager = podcastManagerThatReturns(podcast)
@@ -121,7 +131,7 @@ class AutoArchiveTest {
 
     @Test
     fun testPlayedRecently() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Days30)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Days30)
         val podcastUUID = UUID.randomUUID().toString()
         val podcast = Podcast(uuid = podcastUUID, isSubscribed = true)
         val podcastManager = podcastManagerThatReturns(podcast)
@@ -142,7 +152,7 @@ class AutoArchiveTest {
 
     @Test
     fun testDownloadedRecently() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Days30)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Days30)
         val podcastUUID = UUID.randomUUID().toString()
         val podcast = Podcast(uuid = podcastUUID, isSubscribed = true)
         val podcastManager = podcastManagerThatReturns(podcast)
@@ -163,7 +173,7 @@ class AutoArchiveTest {
 
     @Test
     fun testPlayed24Hours() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Hours24, Settings.AutoArchiveInactive.Never)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Hours24, AutoArchiveInactiveSetting.Never)
         val podcast = Podcast(UUID.randomUUID().toString())
         val podcastManager = podcastManagerThatReturns(podcast)
         val calendar = Calendar.getInstance()
@@ -188,7 +198,7 @@ class AutoArchiveTest {
 
     @Test
     fun testPlayedNotIncludeStarred() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Hours24, Settings.AutoArchiveInactive.Never)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Hours24, AutoArchiveInactiveSetting.Never)
         val podcast = Podcast(UUID.randomUUID().toString())
         val podcastManager = podcastManagerThatReturns(podcast)
         val calendar = Calendar.getInstance()
@@ -213,7 +223,7 @@ class AutoArchiveTest {
 
     @Test
     fun testPlayedIncludeStarred() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Hours24, Settings.AutoArchiveInactive.Never, includeStarred = true)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Hours24, AutoArchiveInactiveSetting.Never, includeStarred = true)
         val podcast = Podcast(UUID.randomUUID().toString())
         val podcastManager = podcastManagerThatReturns(podcast)
         val calendar = Calendar.getInstance()
@@ -238,7 +248,7 @@ class AutoArchiveTest {
 
     @Test
     fun inactiveNotIncludeStarred() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Days30)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Days30)
         val podcastUUID = UUID.randomUUID().toString()
         val podcast = Podcast(uuid = podcastUUID, isSubscribed = true)
         val podcastManager = podcastManagerThatReturns(podcast)
@@ -264,7 +274,7 @@ class AutoArchiveTest {
 
     @Test
     fun inactiveIncludeStarred() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Days30, includeStarred = true)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Days30, includeStarred = true)
         val podcastUUID = UUID.randomUUID().toString()
 
         val podcast = Podcast(uuid = podcastUUID, isSubscribed = true)
@@ -291,7 +301,7 @@ class AutoArchiveTest {
 
     @Test
     fun inactiveArchiveModified() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Weeks1, includeStarred = true)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Weeks1, includeStarred = true)
         val podcastUUID = UUID.randomUUID().toString()
 
         val podcast = Podcast(uuid = podcastUUID, isSubscribed = true)
@@ -328,8 +338,8 @@ class AutoArchiveTest {
 
     @Test
     fun testPlayed24HoursPodcastOverride() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Never)
-        val podcast = Podcast(UUID.randomUUID().toString(), overrideGlobalArchive = true, autoArchiveAfterPlaying = Settings.AutoArchiveAfterPlaying.Hours24.toIndex())
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Never)
+        val podcast = Podcast(UUID.randomUUID().toString(), overrideGlobalArchive = true, autoArchiveAfterPlaying = AutoArchiveAfterPlayingSetting.Hours24.toIndex())
         val podcastManager = podcastManagerThatReturns(podcast)
         val calendar = Calendar.getInstance()
         calendar.add(Calendar.DATE, -2)
@@ -353,9 +363,9 @@ class AutoArchiveTest {
 
     @Test
     fun testInactive30DaysPodcastOverride() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Never)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Never)
         val podcastUUID = UUID.randomUUID().toString()
-        val podcast = Podcast(uuid = podcastUUID, isSubscribed = true, overrideGlobalArchive = true, autoArchiveInactive = Settings.AutoArchiveInactive.Days30.toIndex())
+        val podcast = Podcast(uuid = podcastUUID, isSubscribed = true, overrideGlobalArchive = true, autoArchiveInactive = AutoArchiveInactiveSetting.Days30.toIndex())
         val podcastManager = podcastManagerThatReturns(podcast)
         val calendar = Calendar.getInstance()
         calendar.add(Calendar.DATE, -31)
@@ -379,9 +389,9 @@ class AutoArchiveTest {
 
     @Test
     fun testInactive24HoursAddedRecentlyPodcastOverride() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Never)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Never)
         val podcastUUID = UUID.randomUUID().toString()
-        val podcast = Podcast(uuid = podcastUUID, isSubscribed = true, overrideGlobalArchive = true, autoArchiveInactive = Settings.AutoArchiveInactive.Hours24.toIndex())
+        val podcast = Podcast(uuid = podcastUUID, isSubscribed = true, overrideGlobalArchive = true, autoArchiveInactive = AutoArchiveInactiveSetting.Hours24.toIndex())
         val podcastManager = podcastManagerThatReturns(podcast)
         val calendar = Calendar.getInstance()
         calendar.add(Calendar.HOUR, -30)
@@ -405,9 +415,9 @@ class AutoArchiveTest {
 
     @Test
     fun testInactive2DaysAndAfterPlayingPodcastOverride() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.AfterPlaying, Settings.AutoArchiveInactive.Weeks2)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.AfterPlaying, AutoArchiveInactiveSetting.Weeks2)
         val podcastUUID = UUID.randomUUID().toString()
-        val podcast = Podcast(uuid = podcastUUID, isSubscribed = true, overrideGlobalArchive = true, autoArchiveInactive = Settings.AutoArchiveInactive.Days2.toIndex(), autoArchiveAfterPlaying = Settings.AutoArchiveAfterPlaying.AfterPlaying.toIndex())
+        val podcast = Podcast(uuid = podcastUUID, isSubscribed = true, overrideGlobalArchive = true, autoArchiveInactive = AutoArchiveInactiveSetting.Days2.toIndex(), autoArchiveAfterPlaying = AutoArchiveAfterPlayingSetting.AfterPlaying.toIndex())
         val podcastManager = podcastManagerThatReturns(podcast)
         val calendar = Calendar.getInstance()
         calendar.set(2019, 0, 24, 11, 30)
@@ -431,7 +441,7 @@ class AutoArchiveTest {
 
     @Test
     fun testEpisodeLimit() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Never)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Never)
         val podcast = Podcast(UUID.randomUUID().toString(), autoArchiveEpisodeLimit = 1, overrideGlobalArchive = true)
         val podcastManager = podcastManagerThatReturns(podcast)
         val calendar = Calendar.getInstance()
@@ -456,7 +466,7 @@ class AutoArchiveTest {
 
     @Test
     fun testEpisodeLimitIgnoresManualUnarchiveInCount() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Never)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Never)
         val podcast = Podcast(UUID.randomUUID().toString(), autoArchiveEpisodeLimit = 0, overrideGlobalArchive = true)
         val podcastManager = podcastManagerThatReturns(podcast)
         val calendar = Calendar.getInstance()
@@ -481,7 +491,7 @@ class AutoArchiveTest {
 
     @Test
     fun testEpisodeLimitRespectsIgnoreGlobal() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Never)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Never)
         val podcast = Podcast(UUID.randomUUID().toString(), autoArchiveEpisodeLimit = 0, overrideGlobalArchive = false)
         val podcastManager = podcastManagerThatReturns(podcast)
         val calendar = Calendar.getInstance()
@@ -501,7 +511,7 @@ class AutoArchiveTest {
 
     @Test
     fun testAddingInactiveEpisodeToUpNext() {
-        val episodeManager = episodeManagerFor(testDb, Settings.AutoArchiveAfterPlaying.Never, Settings.AutoArchiveInactive.Weeks1, includeStarred = true)
+        val episodeManager = episodeManagerFor(testDb, AutoArchiveAfterPlayingSetting.Never, AutoArchiveInactiveSetting.Weeks1, includeStarred = true)
         val upNext = upNextQueueFor(testDb, episodeManager)
 
         val podcastUUID = UUID.randomUUID().toString()
@@ -539,4 +549,18 @@ class AutoArchiveTest {
         val updatedNewEpisodeInUpNextAfterInactive = episodeDao.findByUuid(newUUID)!!
         assertTrue("Episode should not be archived as it was added to up next after being inactive", !updatedNewEpisodeInUpNextAfterInactive.isArchived)
     }
+
+    // This manual mock is needed to avoid problems when accessing a lazily initialized UserSetting::flow
+    // from a mocked Settings class
+    private class MockUserSetting<T>(
+        private val initialValue: T,
+        sharedPrefKey: String = "a_shared_pref_key",
+        sharedPrefs: SharedPreferences = mock(),
+    ) : UserSetting<T>(
+        sharedPrefKey = sharedPrefKey,
+        sharedPrefs = sharedPrefs,
+    ) {
+        override fun get(): T = initialValue
+        override fun persist(value: T, commit: Boolean) {}
+    }
 }

app/src/androidTest/java/au/com/shiftyjelly/pocketcasts/models/db/PodcastManagerTest.kt

@@ -1,12 +1,14 @@
 package au.com.shiftyjelly.pocketcasts.models.db
 
-import androidx.preference.PreferenceManager
+import android.content.SharedPreferences
 import androidx.room.Room
 import androidx.test.internal.runner.junit4.AndroidJUnit4ClassRunner
 import androidx.test.platform.app.InstrumentationRegistry
 import au.com.shiftyjelly.pocketcasts.models.db.dao.PodcastDao
 import au.com.shiftyjelly.pocketcasts.models.entity.Podcast
-import au.com.shiftyjelly.pocketcasts.preferences.SettingsImpl
+import au.com.shiftyjelly.pocketcasts.models.to.PodcastGrouping
+import au.com.shiftyjelly.pocketcasts.preferences.Settings
+import au.com.shiftyjelly.pocketcasts.preferences.UserSetting
 import au.com.shiftyjelly.pocketcasts.repositories.playback.PlaybackManager
 import au.com.shiftyjelly.pocketcasts.repositories.podcast.EpisodeManager
 import au.com.shiftyjelly.pocketcasts.repositories.podcast.PlaylistManager
@@ -18,7 +20,6 @@ import au.com.shiftyjelly.pocketcasts.servers.cdn.StaticServerManager
 import au.com.shiftyjelly.pocketcasts.servers.podcast.PodcastCacheServerManager
 import au.com.shiftyjelly.pocketcasts.servers.refresh.RefreshServerManager
 import au.com.shiftyjelly.pocketcasts.utils.Optional
-import com.squareup.moshi.Moshi
 import io.reactivex.Single
 import org.junit.After
 import org.junit.Assert.assertTrue
@@ -45,13 +46,10 @@ class PodcastManagerTest {
         val episodeManager = mock<EpisodeManager>()
         val playlistManager = mock<PlaylistManager>()
 
-        val settings = SettingsImpl(
-            sharedPreferences = PreferenceManager.getDefaultSharedPreferences(context),
-            privatePreferences = PreferenceManager.getDefaultSharedPreferences(context),
-            context = context,
-            firebaseRemoteConfig = mock(),
-            moshi = Moshi.Builder().build(),
-        )
+        val settings = mock<Settings> {
+            on { podcastGroupingDefault } doReturn MockUserSetting(PodcastGrouping.None)
+            on { showArchivedDefault } doReturn MockUserSetting(false)
+        }
 
         val syncManagerSignedOut = mock<SyncManager> {
             on { isLoggedIn() } doReturn false
@@ -114,4 +112,18 @@ class PodcastManagerTest {
         val daoPodcast = podcastDao.findByUuid(uuid)
         assertTrue("Podcast should be unsubscribed", daoPodcast?.isSubscribed == false)
     }
+
+    // This manual mock is needed to avoid problems when accessing a lazily initialized UserSetting::flow
+    // from a mocked Settings class
+    private class MockUserSetting<T>(
+        private val initialValue: T,
+        sharedPrefKey: String = "a_shared_pref_key",
+        sharedPrefs: SharedPreferences = mock(),
+    ) : UserSetting<T>(
+        sharedPrefKey = sharedPrefKey,
+        sharedPrefs = sharedPrefs,
+    ) {
+        override fun get(): T = initialValue
+        override fun persist(value: T, commit: Boolean) {}
+    }
 }

modules/features/podcasts/src/main/java/au/com/shiftyjelly/pocketcasts/podcasts/view/podcast/PodcastAutoArchiveViewModel.kt

@@ -39,8 +39,8 @@ class PodcastAutoArchiveViewModel @Inject constructor(
         launch {
             val podcast = podcast.value ?: return@launch
             podcast.overrideGlobalArchive = checked
-            podcast.autoArchiveAfterPlaying = settings.getAutoArchiveAfterPlaying().toIndex()
-            podcast.autoArchiveInactive = settings.getAutoArchiveInactive().toIndex()
+            podcast.autoArchiveAfterPlaying = settings.autoArchiveAfterPlaying.flow.value.toIndex()
+            podcast.autoArchiveInactive = settings.autoArchiveInactive.flow.value.toIndex()
             podcastManager.updatePodcast(podcast)
         }
     }

modules/features/podcasts/src/main/java/au/com/shiftyjelly/pocketcasts/podcasts/view/podcast/PodcastSettingsFragment.kt

@@ -19,7 +19,7 @@ import au.com.shiftyjelly.pocketcasts.models.entity.Podcast
 import au.com.shiftyjelly.pocketcasts.models.type.EpisodeStatusEnum
 import au.com.shiftyjelly.pocketcasts.podcasts.R
 import au.com.shiftyjelly.pocketcasts.podcasts.viewmodel.PodcastSettingsViewModel
-import au.com.shiftyjelly.pocketcasts.preferences.Settings
+import au.com.shiftyjelly.pocketcasts.preferences.model.AutoAddUpNextLimitBehaviour
 import au.com.shiftyjelly.pocketcasts.repositories.podcast.PodcastManager
 import au.com.shiftyjelly.pocketcasts.settings.AutoAddSettingsFragment
 import au.com.shiftyjelly.pocketcasts.ui.extensions.getThemeColor
@@ -189,8 +189,8 @@ class PodcastSettingsFragment : BasePreferenceFragment(), CoroutineScope, Filter
 
         viewModel.globalSettings.observe(viewLifecycleOwner) {
             val summary = when (it.second) {
-                Settings.AutoAddUpNextLimitBehaviour.ONLY_ADD_TO_TOP, null -> getString(LR.string.settings_auto_up_next_limit_reached_top_summary, it.first)
-                Settings.AutoAddUpNextLimitBehaviour.STOP_ADDING -> getString(LR.string.settings_auto_up_next_limit_reached_stop_summary, it.first)
+                AutoAddUpNextLimitBehaviour.ONLY_ADD_TO_TOP -> getString(LR.string.settings_auto_up_next_limit_reached_top_summary, it.first)
+                AutoAddUpNextLimitBehaviour.STOP_ADDING -> getString(LR.string.settings_auto_up_next_limit_reached_stop_summary, it.first)
             }
 
             preferenceAddToUpNextGlobal?.summary = getString(LR.string.podcast_settings_up_next_episode_limit, it.first) + "\n\n" + summary