diff options
Diffstat (limited to 'app/src')
6 files changed, 71 insertions, 18 deletions
diff --git a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt index 181e853..cf691e0 100644 --- a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt +++ b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt @@ -197,4 +197,29 @@ class MigrationTest { assertEquals(2, cursor.getInt(0)) } } + + /** + * Ad-chapter skipping was shipped off and Peter asked for it on after living with it, so + * existing shows have to move rather than only new ones picking up the new default. + */ + @Test + fun migrates5To6TurningAdChapterSkippingOn() { + helper.createDatabase(name, 5).use { db -> + db.execSQL( + """INSERT INTO subscriptions + (id, feedUrl, kind, title, addedAtMs, skipIntroMs, skipOutroMs, dripEnabled, + dripIntervalMs, dripCount, dripOldestFirst, skipAdChapters) + VALUES ('s1', 'https://example.com/feed.xml', 'PODCAST', 'A Show', 1000, 0, 0, + 0, 86400000, 1, 1, 0)""" + ) + } + + val db = helper.runMigrationsAndValidate(name, 6, true, AppDatabase.MIGRATION_5_6) + + db.query("SELECT skipAdChapters, title FROM subscriptions WHERE id = 's1'").use { cursor -> + cursor.moveToFirst() + assertEquals(1, cursor.getInt(0)) + assertEquals("the show itself must survive the flip", "A Show", cursor.getString(1)) + } + } } diff --git a/app/src/main/java/org/terst/listen/data/AppDatabase.kt b/app/src/main/java/org/terst/listen/data/AppDatabase.kt index fe73ce0..4ff9f50 100644 --- a/app/src/main/java/org/terst/listen/data/AppDatabase.kt +++ b/app/src/main/java/org/terst/listen/data/AppDatabase.kt @@ -25,7 +25,7 @@ class Converters { QueueEntity::class, RuleEntity::class, ], - version = 5, + version = 6, exportSchema = true, ) @TypeConverters(Converters::class) @@ -141,6 +141,24 @@ abstract class AppDatabase : RoomDatabase() { override fun onCreate(db: SupportSQLiteDatabase) = seedDefaultFilters(db) } + /** + * Turns ad-chapter skipping on, which Peter asked for after living with it. + * + * It shipped off-by-default out of caution, since it is the one part of the skip + * feature that removes audio nobody has heard. In practice it only ever acts on + * chapters a publisher explicitly marked as not-content, and doing that automatically + * is the point. + * + * Only the stored rows move. The column's SQL default stays 0 because nothing inserts + * a subscription without it -- Room always writes every column -- so changing it would + * mean recreating the table for a value that is never read. + */ + val MIGRATION_5_6 = object : Migration(5, 6) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("UPDATE subscriptions SET skipAdChapters = 1") + } + } + fun get(context: Context): AppDatabase = instance ?: synchronized(this) { instance ?: build(context.applicationContext).also { instance = it } } @@ -149,7 +167,9 @@ abstract class AppDatabase : RoomDatabase() { Room.databaseBuilder(context, AppDatabase::class.java, "listen.db") // Listed one by one rather than spread from an array: every migration here is a // deliberate addition, and the list is the record of them. - .addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5) + .addMigrations( + MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5, MIGRATION_5_6, + ) .addCallback(SEED_DEFAULT_FILTERS) // No fallbackToDestructiveMigration. A listener's playback positions and play // history are not reproducible from anywhere, so a missing migration must fail diff --git a/app/src/main/java/org/terst/listen/data/Entities.kt b/app/src/main/java/org/terst/listen/data/Entities.kt index ed80c4b..547a4ad 100644 --- a/app/src/main/java/org/terst/listen/data/Entities.kt +++ b/app/src/main/java/org/terst/listen/data/Entities.kt @@ -88,11 +88,15 @@ data class SubscriptionEntity( /** * Whether to pass over chapters the publisher marked `toc: false`. * - * Off by default and opt-in per show. Acting on someone else's metadata to remove audio a - * listener has not heard is the one part of this feature that can eat an episode, so it is - * never on unless asked for. + * On by default, at Peter's request after living with it off. Only ever acts on sections a + * publisher explicitly marked as not part of the episode -- never guessed from a title -- + * and doing that without being asked each time is the point of the feature. + * + * The SQL default stays 0 while the Kotlin default is true: Room writes every column on + * insert, so the SQL one is only consulted by an ALTER TABLE that has already happened. + * Changing it would mean recreating the table for a value nothing reads. */ - @ColumnInfo(defaultValue = "0") val skipAdChapters: Boolean = false, + @ColumnInfo(defaultValue = "0") val skipAdChapters: Boolean = true, ) /** One a day, the cadence a drip subscription is for. */ diff --git a/app/src/main/java/org/terst/listen/ui/SkipSettings.kt b/app/src/main/java/org/terst/listen/ui/SkipSettings.kt index 7c6146c..106e3fd 100644 --- a/app/src/main/java/org/terst/listen/ui/SkipSettings.kt +++ b/app/src/main/java/org/terst/listen/ui/SkipSettings.kt @@ -66,9 +66,9 @@ fun SkipSettings( Text("Skip marked breaks", Modifier.padding(start = 12.dp)) } Text( - "Only where the show publishes chapters and marks a section as not part of the " + - "episode. Nothing is guessed from chapter titles, so a show discussing its own " + - "sponsors keeps playing.", + "On by default. Only acts where the show publishes chapters and marks a section " + + "as not part of the episode -- nothing is guessed from chapter titles, so a " + + "show discussing its own sponsors keeps playing.", style = MaterialTheme.typography.bodySmall, ) } diff --git a/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt b/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt index e22be1e..171114d 100644 --- a/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt +++ b/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt @@ -49,6 +49,7 @@ class PlaybackAdvancerTest { skipIntroMs: Long = 0, playbackSpeed: Float? = null, kind: SubscriptionKind = SubscriptionKind.PODCAST, + skipAdChapters: Boolean = true, ): SubscriptionEntity { val entity = SubscriptionEntity( id = id, @@ -58,6 +59,7 @@ class PlaybackAdvancerTest { addedAtMs = 0, skipIntroMs = skipIntroMs, playbackSpeed = playbackSpeed, + skipAdChapters = skipAdChapters, ) subscriptions.insert(entity) return entity @@ -356,7 +358,8 @@ class PlaybackAdvancerTest { ), ) { now } - show(id = "s1") + // Explicitly off, rather than leaning on a default that has since flipped to on. + show(id = "s1", skipAdChapters = false) episode("a") assertEquals(emptyList<Chapter>(), fetching.contextFor("a").chapters) assertEquals(0, asked) diff --git a/app/src/test/java/org/terst/listen/playback/PlayheadPolicyTest.kt b/app/src/test/java/org/terst/listen/playback/PlayheadPolicyTest.kt index 1488559..d0a49f7 100644 --- a/app/src/test/java/org/terst/listen/playback/PlayheadPolicyTest.kt +++ b/app/src/test/java/org/terst/listen/playback/PlayheadPolicyTest.kt @@ -69,22 +69,23 @@ class PlayheadPolicyTest { } @Test - fun `inside a marked break, on a show that opted in, it seeks past it`() { + fun `inside a marked break it seeks past it`() { assertEquals( TickAction.SeekTo(3 * minute), - decide( - show = show.copy(skipAdChapters = true), - chapters = chapters, - positionMs = 2 * minute + 5_000, - ), + decide(show = show, chapters = chapters, positionMs = 2 * minute + 5_000), ) } + /** Opted out explicitly: skipping is on by default, so this is the deliberate case. */ @Test - fun `a show that did not opt in keeps playing the break`() { + fun `a show that opted out keeps playing the break`() { assertEquals( TickAction.Nothing, - decide(show = show, chapters = chapters, positionMs = 2 * minute + 5_000), + decide( + show = show.copy(skipAdChapters = false), + chapters = chapters, + positionMs = 2 * minute + 5_000, + ), ) } |