diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-11-05 09:11:58 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-11-05 09:11:58 +0000 |
| commit | 5c68a0835e1d8cf47fb613660d3e4c32c203fd68 (patch) | |
| tree | 851117bbdeeb8a7511b32cc220206a886e1db1c9 | |
| parent | ca385bfac567697f050ccaa506f9fb27681750de (diff) | |
One tap to play, and a way to squeeze the next drip episode out
Two things Peter asked for, both about a cadence the app imposed and gave no way around.
**Opening the app and pressing play was two taps.** The library's Up Next row had no play button:
tapping it opened the queue, and the button was there. Two taps for the single thing the app is
for. The row has a play button now and still opens the queue, which is where reordering and
everything else lives.
**A slow drip had no override.** A drip is a cadence you chose so you would not have to think
about it, and most of the time that is the whole point -- but a cadence you cannot override owns
you. Finish Tuesday's episode when the next is due Thursday and there is nothing to listen to and
no way to ask for more.
"Release the next one now" sits under the drip cadence in the show's settings, and only while drip
is on: a show listing everything already has nothing to bring forward. It honours the show's batch
size and its oldest/newest-first choice, because those are the same decision.
One judgement in it, and the opposite choice is defensible. It **brings the next release forward**
rather than granting an extra: the mark moves to now, so the scheduled one is a full interval away
instead of landing on top of this one. Asking for the next episode reads as asking for it sooner.
The clock restarts even when the archive turned out to be empty, so repeatedly asking an exhausted
show cannot build a debt that bursts when the publisher next posts -- the same reasoning
DripSchedule already applies to the scheduled path.
An empty archive says so. "Nothing happened" and "the button is broken" look identical otherwise,
which is the same fault the drip toggle's own banner was written to avoid.
DripSettings' callbacks are bundled into DripActions, because adding the fourth took it past the
parameter limit -- the same shape EpisodeActions and LibraryNav already use.
Checked on the emulator, not just in tests: the play button appears on the Up Next row and starts
the episode, and the release button queued episodes and advanced the drip mark.
1031 tests, detekt 0, lint clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
9 files changed, 278 insertions, 15 deletions
diff --git a/app/src/main/java/org/terst/listen/queue/DripReleaser.kt b/app/src/main/java/org/terst/listen/queue/DripReleaser.kt index e6c7b6b..675eb2a 100644 --- a/app/src/main/java/org/terst/listen/queue/DripReleaser.kt +++ b/app/src/main/java/org/terst/listen/queue/DripReleaser.kt @@ -45,6 +45,42 @@ class DripReleaser( } /** + * Pays out now, whatever the clock says. Returns how many episodes were released. + * + * A drip is a cadence you chose so you would not have to think about it, and most of the + * time that is the whole point. But a cadence with no override owns you: finish Tuesday's + * episode and the next one is due Thursday, and there is nothing to listen to and no way to + * ask. + * + * Brought forward, not added. The mark moves to now, so the scheduled release is a full + * interval away rather than landing on top of this one -- asking for the next episode is + * asking for it sooner, not asking for an extra. + */ + suspend fun releaseNow(subscriptionId: String): Int { + val subscription = subscriptions.getById(subscriptionId) ?: return 0 + // Only a show that is actually dripping. Anything else already lists everything it has. + if (!subscription.dripEnabled) return 0 + + val nowMs = now() + val count = subscription.dripCount.coerceAtLeast(1) + val batch = if (subscription.dripOldestFirst) { + episodes.unreleasedOldestFirst(subscription.id, count) + } else { + episodes.unreleasedNewestFirst(subscription.id, count) + } + + for (episode in batch) { + episodes.markDripReleased(episode.id, nowMs) + queue.append(episode.id) + } + + // Even when the archive had nothing left: the clock still restarts, so repeatedly asking + // an exhausted show does not queue up a debt that bursts when the publisher next posts. + subscriptions.setDripMark(subscription.id, nowMs) + return batch.size + } + + /** * Switches drip on for a show and settles what it already owes you. * * Anything played or already queued is released up front: hiding episodes you have heard diff --git a/app/src/main/java/org/terst/listen/ui/DripSettings.kt b/app/src/main/java/org/terst/listen/ui/DripSettings.kt index 97ba063..19a5e27 100644 --- a/app/src/main/java/org/terst/listen/ui/DripSettings.kt +++ b/app/src/main/java/org/terst/listen/ui/DripSettings.kt @@ -30,9 +30,7 @@ private val DRIP_COUNTS = listOf(1, 2, 3) fun DripSettings( subscription: SubscriptionEntity, busy: Boolean, - onEnable: (intervalMs: Long, count: Int, oldestFirst: Boolean) -> Unit, - onDisable: () -> Unit, - onFetchArchive: () -> Unit, + actions: DripActions, ) { Column( Modifier.fillMaxWidth(), @@ -45,22 +43,25 @@ fun DripSettings( detail = dripSummary(subscription), onCheckedChange = { wanted -> if (wanted) { - onEnable( + actions.onEnable( subscription.dripIntervalMs, subscription.dripCount, subscription.dripOldestFirst, ) } else { - onDisable() + actions.onDisable() } }, ) if (subscription.dripEnabled) { - DripCadence(subscription, onEnable) + DripCadence(subscription, actions.onEnable) + // Only while dripping, because it is the cadence this overrides. A show listing + // everything already has nothing to bring forward. + OutlinedButton(onClick = actions.onReleaseNow, enabled = !busy) { Text("Release the next one now") } } - OutlinedButton(onClick = onFetchArchive, enabled = !busy) { Text("Fetch archive") } + OutlinedButton(onClick = actions.onFetchArchive, enabled = !busy) { Text("Fetch archive") } } } @@ -116,3 +117,12 @@ private fun dripSummary(subscription: SubscriptionEntity): String { val order = if (subscription.dripOldestFirst) "oldest" else "newest" return "${subscription.dripCount} $order first, every $every" } + +/** What the drip controls do, bundled for the same reason [EpisodeActions] is. */ +data class DripActions( + val onEnable: (intervalMs: Long, count: Int, oldestFirst: Boolean) -> Unit, + val onDisable: () -> Unit, + /** Brings the next scheduled release forward to now. */ + val onReleaseNow: () -> Unit, + val onFetchArchive: () -> Unit, +) diff --git a/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt b/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt index 912d697..fea837a 100644 --- a/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt @@ -23,6 +23,7 @@ import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton +import androidx.compose.material.icons.filled.PlayArrow import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Scaffold @@ -186,7 +187,7 @@ internal fun LibraryList( LazyColumn(Modifier.fillMaxSize()) { if (queue.isNotEmpty()) { item { SectionHeading("Up Next \u00b7 ${queue.size}") } - item { UpNextRow(queue.first(), onOpenQueue) } + item { UpNextRow(queue.first(), actionsFor(queue.first()).play, onOpenQueue) } } if (subscriptions.isEmpty()) { item { if (bedtime) EmptyBedtime() else EmptyLibrary() } @@ -228,9 +229,9 @@ private fun SectionHeading(text: String) { * Peter asked for that screen to stay -- "Leave it" -- so this is a way in, not a replacement. */ @Composable -private fun UpNextRow(episode: EpisodeEntity, onOpenQueue: () -> Unit) { +private fun UpNextRow(episode: EpisodeEntity, onPlay: () -> Unit, onOpenQueue: () -> Unit) { Row( - Modifier.fillMaxWidth().clickable(onClick = onOpenQueue).padding(horizontal = 16.dp, vertical = 10.dp), + Modifier.fillMaxWidth().clickable(onClick = onOpenQueue).padding(start = 16.dp, end = 4.dp, top = 10.dp, bottom = 10.dp), verticalAlignment = Alignment.CenterVertically, ) { AsyncImage( @@ -248,6 +249,12 @@ private fun UpNextRow(episode: EpisodeEntity, onOpenQueue: () -> Unit) { overflow = TextOverflow.Ellipsis, modifier = Modifier.weight(1f).padding(start = 12.dp), ) + // The reason this row exists. Opening the app and pressing play was two taps -- the row + // opened the queue and the queue had the button -- which is two too many for the thing + // the app is for. The row still opens the queue, for reordering and everything else. + IconButton(onClick = onPlay) { + Icon(Icons.Default.PlayArrow, contentDescription = "Play ${episode.title}") + } } } diff --git a/app/src/main/java/org/terst/listen/ui/ShowSettingsSheet.kt b/app/src/main/java/org/terst/listen/ui/ShowSettingsSheet.kt index 693fee3..f7780a5 100644 --- a/app/src/main/java/org/terst/listen/ui/ShowSettingsSheet.kt +++ b/app/src/main/java/org/terst/listen/ui/ShowSettingsSheet.kt @@ -71,11 +71,14 @@ fun ShowSettingsSheet( DripSettings( subscription = show, busy = busy, - onEnable = { intervalMs, count, oldestFirst -> - settings.enableDrip(show.id, intervalMs, count, oldestFirst) - }, - onDisable = { settings.disableDrip(show.id) }, - onFetchArchive = { settings.fetchArchive(show.id) }, + actions = DripActions( + onEnable = { intervalMs, count, oldestFirst -> + settings.enableDrip(show.id, intervalMs, count, oldestFirst) + }, + onDisable = { settings.disableDrip(show.id) }, + onReleaseNow = { settings.releaseNextNow(show.id) }, + onFetchArchive = { settings.fetchArchive(show.id) }, + ), ) HorizontalDivider() diff --git a/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt b/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt index 6bdf4c4..221f876 100644 --- a/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt +++ b/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt @@ -71,6 +71,24 @@ class ShowSettingsViewModel(app: Application, private val graph: UiGraph) : Andr } } + /** + * Brings the next drip release forward to now. + * + * A cadence with no override owns you: finish Tuesday's episode when the next is due + * Thursday and there is nothing to listen to and no way to ask. Says plainly when the + * archive is empty, because "nothing happened" and "it did not work" look identical. + */ + fun releaseNextNow(subscriptionId: String) { + viewModelScope.launch { + val released = graph.drip.releaseNow(subscriptionId) + _banner.value = if (released > 0) { + Banner("Queued $released more") + } else { + Banner("Nothing left in this show's archive to release") + } + } + } + fun disableDrip(subscriptionId: String) { viewModelScope.launch { graph.drip.disable(subscriptionId) diff --git a/app/src/test/java/org/terst/listen/feed/FeedFailureTextTest.kt b/app/src/test/java/org/terst/listen/feed/FeedFailureTextTest.kt new file mode 100644 index 0000000..88fe231 --- /dev/null +++ b/app/src/test/java/org/terst/listen/feed/FeedFailureTextTest.kt @@ -0,0 +1,71 @@ +package org.terst.listen.feed + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Test + +/** + * What a failed refresh says, and what it is safe to write down. + * + * The messages are read in two places -- the banner after trying to subscribe, and the error line + * under a show in the library -- and until 2026-11-05 they were whatever the machine said: a dead + * host put the literal word `UnknownHostException` under a podcast's name. + * + * [FeedFailureText.redact] is the other half, and it is a security control rather than tidiness. + * A private feed's URL carries its token in the query string and logcat is readable by anything + * with the permission, so a log line must never carry the whole address. + */ +class FeedFailureTextTest { + + @Test + fun `a feed that has moved reads differently from one having a bad day`() { + assertEquals("That feed is no longer at this address.", FeedFailureText.status(404)) + assertEquals("That feed is no longer at this address.", FeedFailureText.status(410)) + assertEquals("That feed's server is having trouble. It is worth trying later.", FeedFailureText.status(503)) + } + + @Test + fun `a refusal says it was refused rather than naming the code`() { + assertEquals("That feed refused the request.", FeedFailureText.status(401)) + assertEquals("That feed refused the request.", FeedFailureText.status(403)) + } + + @Test + fun `an unmapped status still gets a sentence`() { + val text = FeedFailureText.status(418) + assertFalse("a status code is not for the listener: $text", text.contains("418")) + assertEquals("That feed's server could not provide it.", text) + } + + /** The whole point: the part that identifies the feed, and nothing that authenticates to it. */ + @Test + fun `redacting keeps the host and drops the token`() { + assertEquals( + "https://feeds.example.com", + FeedFailureText.redact("https://feeds.example.com/private.xml?token=s3cret&user=peter"), + ) + } + + /** Some private feeds put credentials in the userinfo, which is worse than a query token. */ + @Test + fun `redacting drops credentials carried in the userinfo`() { + val redacted = FeedFailureText.redact("https://peter:hunter2@feeds.example.com/private.xml") + + assertEquals("https://feeds.example.com", redacted) + assertFalse(redacted.contains("hunter2")) + assertFalse(redacted.contains("peter")) + } + + /** + * Something with no structure cannot be redacted, so it is named rather than printed. This is + * the branch the security review called out: the exception for one of these carries the + * address in its message, so logging the cause would undo the redaction. + */ + @Test + fun `an address that will not parse is described, not quoted`() { + val redacted = FeedFailureText.redact("not a url ?token=s3cret") + + assertEquals("an unparsable address", redacted) + assertFalse(redacted.contains("s3cret")) + } +} 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 28fbc42..2023833 100644 --- a/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt +++ b/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt @@ -1,5 +1,6 @@ package org.terst.listen.playback +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull @@ -420,6 +421,28 @@ class PlaybackAdvancerTest { assertTrue(advancer.start("a") is StartOutcome.Ready) } + /** + * A cancelled start is not a failed one. + * + * The catch that stops a thrown fetch escaping is deliberately broad, which would otherwise + * swallow cancellation too -- so navigating away mid-fetch would report "could not fetch + * audio" and leave a false error on screen. Structured concurrency also relies on + * CancellationException propagating. + */ + @Test + fun `a cancelled fetch stays cancelled rather than becoming a failure`() = runTest { + show() + episode("a") + val cancelling = PlaybackAdvancer( + episodes, subscriptions, queue, + sources = FakeSources(onFetch = { throw CancellationException("navigated away") }), + ) { now } + + val thrown = runCatching { cancelling.startFetchingIfNeeded("a") }.exceptionOrNull() + + assertTrue("got $thrown", thrown is CancellationException) + } + @Test fun `a fetch that throws is reported rather than escaping`() = runTest { show() diff --git a/app/src/test/java/org/terst/listen/queue/DripReleaserTest.kt b/app/src/test/java/org/terst/listen/queue/DripReleaserTest.kt index d5fedf7..757b0d4 100644 --- a/app/src/test/java/org/terst/listen/queue/DripReleaserTest.kt +++ b/app/src/test/java/org/terst/listen/queue/DripReleaserTest.kt @@ -258,4 +258,68 @@ class DripReleaserTest { assertEquals(false, subscriptions.rows.single().dripEnabled) assertNotNull(episodes.rows.single().dripReleasedAtMs) } + + /* + Squeezing the next one out by hand. + + A drip is a cadence somebody chose, and the point of choosing it is not having to think + about it. But a cadence you cannot override is a cadence that owns you: finishing the + current episode on a Tuesday when the next is due Thursday leaves nothing to listen to and + no way to ask. + + So this pays out now whatever the clock says, and then moves the mark so the scheduled one + does not arrive on top of it. Asking for one is asking to bring it forward, not to get an + extra. + */ + @Test + fun `releasing by hand pays out before the interval is up`() = runTest { + show(dripLastReleaseAtMs = now) + episode("a", publishedAtMs = now - 10 * day) + episode("b", publishedAtMs = now - 9 * day) + + assertEquals("nothing is owed yet", 0, releaser.release(subscriptions.rows.single())) + assertEquals(1, releaser.releaseNow("s1")) + assertEquals(listOf("a"), queued()) + } + + /** Brought forward, not added: the next scheduled one is a full interval from the squeeze. */ + @Test + fun `releasing by hand restarts the clock`() = runTest { + show(dripLastReleaseAtMs = now - 10 * day) + episode("a", publishedAtMs = now - 10 * day) + episode("b", publishedAtMs = now - 9 * day) + + releaser.releaseNow("s1") + + assertEquals(now, subscriptions.rows.single().dripLastReleaseAtMs) + } + + @Test + fun `releasing by hand respects the show's count and order`() = runTest { + show(dripCount = 2, dripOldestFirst = false, dripLastReleaseAtMs = now) + episode("a", publishedAtMs = now - 10 * day) + episode("b", publishedAtMs = now - 9 * day) + episode("c", publishedAtMs = now - 8 * day) + + assertEquals(2, releaser.releaseNow("s1")) + assertEquals(listOf("c", "b"), queued()) + } + + /** An archive with nothing left in it says nothing happened, rather than claiming it did. */ + @Test + fun `releasing by hand with nothing left releases nothing`() = runTest { + show(dripLastReleaseAtMs = now) + episode("a", publishedAtMs = now - 10 * day, dripReleasedAtMs = now - day) + + assertEquals(0, releaser.releaseNow("s1")) + } + + /** Only for a show that is actually dripping; anything else has a whole list already. */ + @Test + fun `releasing by hand does nothing for a show that is not dripping`() = runTest { + show(dripEnabled = false) + episode("a", publishedAtMs = now - 10 * day) + + assertEquals(0, releaser.releaseNow("s1")) + } } diff --git a/app/src/test/java/org/terst/listen/ui/ShowSettingsViewModelTest.kt b/app/src/test/java/org/terst/listen/ui/ShowSettingsViewModelTest.kt index 0ebd559..44ca546 100644 --- a/app/src/test/java/org/terst/listen/ui/ShowSettingsViewModelTest.kt +++ b/app/src/test/java/org/terst/listen/ui/ShowSettingsViewModelTest.kt @@ -106,6 +106,37 @@ class ShowSettingsViewModelTest { ) } + /* + Squeezing the next one out. + + A drip is a cadence you chose so you would not have to think about it, and a cadence with no + override owns you: finish Tuesday's episode when the next is due Thursday and there is + nothing to listen to and no way to ask. + */ + @Test + fun `releasing the next one now queues it and says how many`() { + show() + graph.episodes.rows += episodeRow("e1") + graph.episodes.rows += episodeRow("e2") + vm.enableDrip("s1", intervalMs = 1_000, count = 1, oldestFirst = true) + + vm.releaseNextNow("s1") + + assertEquals(Banner("Queued 1 more"), vm.banner.value) + assertEquals(2, graph.queueDao.entries.size) + } + + /** An exhausted archive has to say so, or it is indistinguishable from the button not working. */ + @Test + fun `releasing the next one with nothing left says the archive is empty`() { + show() + vm.enableDrip("s1", intervalMs = 1_000, count = 1, oldestFirst = true) + + vm.releaseNextNow("s1") + + assertEquals(Banner("Nothing left in this show's archive to release"), vm.banner.value) + } + @Test fun `enabling drip on a show with nothing to release says so rather than nothing`() { show() |
