diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ee3c5c..88bfddd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,9 @@ The long form, with what was wrong before and how it was found, is in ### Fixed +- Currently Listening lists the episodes you have started. It left out anything marked read, and + opening an episode marks it read, so it usually showed nothing. An episode now leaves the list + once 90% of it has played. - A WordPress post that embeds the file it encloses no longer lists, and downloads, that file twice. Items that already had it twice are folded into one at startup, and the spare copy deleted. diff --git a/docs/history.md b/docs/history.md index 8b16844..fffd002 100644 --- a/docs/history.md +++ b/docs/history.md @@ -6,6 +6,21 @@ reasoning lives. New write-ups go at the top. See [README.md](../README.md) for what the thing is. +## 2026-09-15 — Currently Listening, empty for anyone who opens what they play + +Issue #14: an episode 32 minutes into 41 was missing from Currently Listening, which said nothing +was in progress. The filter was "position past five seconds and unread", on the reasoning that +`markPlayed` marks an episode read at 90%. But opening an item marks it read too, and you open an +episode to play it. In production every one of the twelve episodes with a saved position was read, +so the list was empty for everyone. The browser test for it had set the episode unread before +checking, to cope with an earlier test having opened it, and so tested around exactly this. + +Finished now means the saved position is 90% of the episode's length or more, the same line +`markPlayed` draws, and read plays no part. Three of those twelve episodes had no length in their +feed, and with no length there is no telling finished from started, so they would have stayed +listed for good. The player now sends the length it measured with each saved position, and that +fills in a missing one, never replacing a length the feed gave. + ## 2026-09-15 — davewiner's 922 rows, retired at last davewiner's OPML subscription left config.toml before `retire_group` existed, so nothing ever diff --git a/src/db.rs b/src/db.rs index 22c9882..a4b593d 100644 --- a/src/db.rs +++ b/src/db.rs @@ -702,9 +702,11 @@ pub enum Filter { Unread, Downloaded, Flagged, - /// Started (a saved playback position past the first few seconds) but not finished - /// (`markPlayed` in the UI marks an item read at 90% played, so unread is "not finished" - /// here too). Currently Listening, below Popular, is this filter on every feed at once. + /// Started (a saved playback position past the first few seconds) and short of the 90% + /// where `markPlayed` in the UI calls it finished. Not `read`: opening an item marks it + /// read, so filtering on that hid nearly every episode anyone had started. An episode of + /// unknown length counts as unfinished until the player reports one (`set_position`). + /// Currently Listening, below Popular, is this filter on every feed at once. InProgress, } @@ -730,7 +732,10 @@ impl Filter { "EXISTS (SELECT 1 FROM enclosures x WHERE x.feed_id = e.feed_id AND x.guid = e.guid AND x.path IS NOT NULL)" } - Self::InProgress => "coalesce(s.position, 0) > 5 AND coalesce(s.read, 0) = 0", + Self::InProgress => { + "coalesce(s.position, 0) > 5 + AND (coalesce(e.duration, 0) = 0 OR s.position * 10 < e.duration * 9)" + } } } } @@ -866,13 +871,30 @@ impl Db { } /// Where playback got to, so it resumes there next time -- for this listener only. - pub fn set_position(&self, user_id: i64, feed_id: &str, guid: &str, secs: i64) -> Result<()> { + /// `duration` is the length the player measured. It fills in an episode whose feed gives + /// none, and never replaces one the feed gave: without a length, Currently Listening cannot + /// tell a finished episode from a started one, and would keep it listed for good. + pub fn set_position( + &self, + user_id: i64, + feed_id: &str, + guid: &str, + secs: i64, + duration: Option, + ) -> Result<()> { let conn = self.conn.lock().unwrap(); conn.execute( "INSERT INTO entry_state (user_id, feed_id, guid, position) VALUES (?1, ?2, ?3, ?4) ON CONFLICT(user_id, feed_id, guid) DO UPDATE SET position = excluded.position", rusqlite::params![user_id, feed_id, guid, secs.max(0)], )?; + if let Some(d) = duration.filter(|d| *d > 0) { + conn.execute( + "UPDATE entries SET duration = ?3 + WHERE feed_id = ?1 AND guid = ?2 AND coalesce(duration, 0) = 0", + rusqlite::params![feed_id, guid, d], + )?; + } Ok(()) } @@ -1658,7 +1680,7 @@ mod tests { // Starring and position are just as private. db.set_entry_flag(1, "f", "b", EntryFlag::Flagged, true).unwrap(); - db.set_position(2, "f", "b", 42).unwrap(); + db.set_position(2, "f", "b", 42, None).unwrap(); let order = order_sql("published", "desc"); let page = |user| db.entries_in(user, Some("f"), Filter::All, None, 0, 50, &order).unwrap(); let (ray, sam) = (page(1), page(2)); @@ -1797,13 +1819,14 @@ mod tests { // so plain filtering failed with "Wrong number of parameters passed to query". let db = Db::memory().unwrap(); db.exec_for_test( - "INSERT INTO entries (feed_id, guid, title, description, first_seen) VALUES - ('f','a','Alpha dive','notes one',100), - ('f','b','Beta', 'notes two',200), - ('f','c','Gamma dive','notes three',300), - ('f','d','Delta', 'notes four',400), - ('f','e','Epsilon', 'notes five',500), - ('f','g','Gimel', 'notes six',600); + "INSERT INTO entries (feed_id, guid, title, description, first_seen, duration) VALUES + ('f','a','Alpha dive','notes one', 100,NULL), + ('f','b','Beta', 'notes two', 200,NULL), + ('f','c','Gamma dive','notes three',300,NULL), + ('f','d','Delta', 'notes four', 400,NULL), + ('f','e','Epsilon', 'notes five', 500,45), + ('f','g','Gimel', 'notes six', 600,NULL), + ('f','h','Heth', 'notes seven',700,900); INSERT INTO enclosures (id, feed_id, guid, url, path, state) VALUES (1,'f','b','u1','/tmp/b','done'); -- Read and starred belong to a person now, so say which one. @@ -1811,12 +1834,15 @@ mod tests { INSERT INTO entry_state (user_id, feed_id, guid, read, flagged, position) VALUES (7,'f','b',1,0,0), (7,'f','c',1,1,0), - -- Started and not finished: this is Currently Listening. + -- Started, length unknown: this is Currently Listening. (7,'f','d',0,0,42), - -- Already finished: not Currently Listening, however far it got. + -- 42 of 45 seconds is past the 90% the player calls finished. (7,'f','e',1,0,42), -- Barely touched (opened, closed within seconds): not Currently Listening. - (7,'f','g',0,0,3);", + (7,'f','g',0,0,3), + -- Opened, and so read, but 42 of 900 seconds in: still Currently Listening. + -- Filtering on read hid exactly these (issue #14). + (7,'f','h',1,0,42);", ) .unwrap(); @@ -1832,7 +1858,7 @@ mod tests { assert_eq!(rows.len() as i64, n, "{f:?} with search disagrees"); } - assert_eq!(db.count_in(7, Some("f"), Filter::All, None).unwrap(), 6); + assert_eq!(db.count_in(7, Some("f"), Filter::All, None).unwrap(), 7); assert_eq!(db.count_in(7, Some("f"), Filter::Unread, None).unwrap(), 3); assert_eq!(db.count_in(7, Some("f"), Filter::Downloaded, None).unwrap(), 1); assert_eq!(db.count_in(7, Some("f"), Filter::Flagged, None).unwrap(), 1); @@ -1841,8 +1867,18 @@ mod tests { "search is case-insensitive and covers the description"); // Currently Listening: started, not finished, and not just an accidental tap. - let listening = db.entries_in(7, Some("f"), Filter::InProgress, None, 0, 50, &order_sql("published", "desc")).unwrap(); - assert_eq!(listening.iter().map(|e| e.guid.as_str()).collect::>(), ["d"]); + let order = order_sql("published", "desc"); + let listening = || { + let rows = db.entries_in(7, Some("f"), Filter::InProgress, None, 0, 50, &order).unwrap(); + rows.into_iter().map(|e| e.guid).collect::>() + }; + assert_eq!(listening(), ["h", "d"]); + + // The player's measured length fills in the one d's feed left out, which settles it: + // 42 of 45 seconds is finished. It never replaces a feed's own length. + db.set_position(7, "f", "d", 42, Some(45)).unwrap(); + db.set_position(7, "f", "e", 42, Some(9000)).unwrap(); + assert_eq!(listening(), ["h"]); } #[test] diff --git a/src/web.rs b/src/web.rs index 7f83807..115df5e 100644 --- a/src/web.rs +++ b/src/web.rs @@ -1374,6 +1374,8 @@ async fn media( #[derive(Deserialize)] struct Position { secs: i64, + /// The length the player measured, for an episode whose feed gives none. + duration: Option, } async fn set_position( @@ -1382,7 +1384,7 @@ async fn set_position( user: crate::db::User, Json(body): Json, ) -> Result { - state.ctx.db.set_position(user.id, &feed_id, &guid, body.secs)?; + state.ctx.db.set_position(user.id, &feed_id, &guid, body.secs, body.duration)?; Ok(StatusCode::NO_CONTENT) } diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index d72f2c1..ccf4358 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -156,11 +156,11 @@ test('an item with several enclosures lists them all', async ({ page }) => { }); test('Currently Listening, its own place below Popular, resumes an episode you started', async ({ page }) => { - // Second Episode (900s) is 42 seconds in and unfinished. Which of Test Show's two episodes - // the daemon auto-downloaded is not fixed (see the three-panes test above), so an earlier - // test may have opened -- and so read -- this one already; reset it before relying on it. + // Second Episode (900s) is 42 seconds in and unfinished. An earlier test may have opened it, + // and opening marks it read; it is listed all the same, because read is not finished. This + // test used to set it unread first, which hid exactly the bug in issue #14. await page.evaluate(() => - api('/api/entries/test-show/ui-2/flags', { method: 'POST', body: JSON.stringify({ read: false }) })); + api('/api/entries/test-show/ui-2/flags', { method: 'POST', body: JSON.stringify({ read: true }) })); await page.evaluate(() => api('/api/entries/test-show/ui-2/position', { method: 'POST', body: JSON.stringify({ secs: 42 }) })); @@ -181,9 +181,9 @@ test('Currently Listening, its own place below Popular, resumes an episode you s await expect(page.locator('#ptitle')).toHaveText('Second Episode'); await page.locator('#pclose').click(); - // Finished (read) drops it from the list, however far it got. + // Finished, 90% of the way or more, drops it from the list. await page.evaluate(() => - api('/api/entries/test-show/ui-2/flags', { method: 'POST', body: JSON.stringify({ read: true }) })); + api('/api/entries/test-show/ui-2/position', { method: 'POST', body: JSON.stringify({ secs: 850 }) })); await page.locator('#feedlist .place', { hasText: 'Currently Listening' }).click(); await expect(page.locator('#listening')).not.toContainText('Second Episode', { timeout: 20_000 }); }); diff --git a/web/index.html b/web/index.html index a0a90cf..a6b4eb7 100644 --- a/web/index.html +++ b/web/index.html @@ -1528,9 +1528,12 @@ function savePos(){ if(!player.guid) return; player.saveAt=audio.currentTime; if(player.entry) player.entry.position=Math.floor(audio.currentTime); + // The measured length stands in for one the feed left out: without it Currently Listening + // cannot tell a finished episode from a started one. NaN before metadata, Infinity on a stream. + const duration=isFinite(audio.duration)?Math.floor(audio.duration):null; navigator.sendBeacon?.( `/api/entries/${encodeURIComponent(player.feed)}/${encodeURIComponent(player.guid)}/position`, - new Blob([JSON.stringify({secs:Math.floor(audio.currentTime)})],{type:'application/json'})); + new Blob([JSON.stringify({secs:Math.floor(audio.currentTime),duration})],{type:'application/json'})); } audio.addEventListener('pause',savePos); audio.addEventListener('ended',()=>{savePos();markPlayed();$('#pplay').innerHTML=ICON.play});