Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Settings Sync - Refactor archive settings to be UserSettings #1234

Merged
merged 6 commits into from
Aug 7, 2023
Merged

Settings Sync - Refactor archive settings to be UserSettings #1234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6d044f5
Make auto archive after playing a UserSetting
mchowning Aug 2, 2023
27a6d33
Make auto archive inactive a UserSetting
mchowning Aug 2, 2023
46f5b4a
Make archive starred episodes a UserSetting
mchowning Aug 2, 2023
4b39acb
Fix AutoArchiveTest
mchowning Aug 3, 2023
2767f55
Return to mocking settings in PodcastManagerTest
mchowning Aug 3, 2023
4ac0b16
Update changelog
mchowning Aug 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
Unknown
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this won't be in 7.45, so putting this here in order to create a merge conflict when this gets merged to main. At that point, I'll add this release note under the correct release heading.

-----
* 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:
Expand Down
76 changes: 50 additions & 26 deletions app/src/androidTest/java/au/com/shiftyjelly/pocketcasts/models/db/AutoArchiveTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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())
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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) {}
}
}
32 changes: 22 additions & 10 deletions app/src/androidTest/java/au/com/shiftyjelly/pocketcasts/models/db/PodcastManagerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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) {}
}
}
4 changes: 2 additions & 2 deletions .../java/au/com/shiftyjelly/pocketcasts/podcasts/view/podcast/PodcastAutoArchiveViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
Loading