From c99e17bd8037889282ac38fe7fba48e46d42b70c Mon Sep 17 00:00:00 2001 From: rays Date: Fri, 18 Sep 2026 17:51:38 +0000 Subject: [PATCH] A pinned item goes to the top of its list order_sql takes pinned_first, which puts coalesce(s.flagged, 0) DESC ahead of the chosen sort, so pins lead every list in whatever order is asked for and on every page of it. Not when sorting by the pin column itself, where the direction is the point, and not for Currently Listening. Pinning now asks for the list again so the row moves at once, instead of redrawing it where it stood. Closes #35. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 ++++++ src/db.rs | 29 ++++++++++++++++++++++------- src/web.rs | 2 ++ tests/ui/app.spec.js | 17 +++++++++++++++++ web/src/items.ts | 4 +++- 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 482d32f..be53d5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- A pinned item sits at the top of its list, above everything else in whatever order you sort + by, and moves there the moment you pin it. Sorting by the pin column itself still goes both + ways, and Currently Listening keeps its own order. + ## [0.7.0] - 2026-09-18 ### Added diff --git a/src/db.rs b/src/db.rs index ac34db7..e891c22 100644 --- a/src/db.rs +++ b/src/db.rs @@ -694,7 +694,10 @@ fn scope_sql(feed_id: Option<&str>, user_param: u8) -> String { /// /// ponytail: file type and size look at the item's first and largest file. The row shows the file /// it summarises, which is almost always that one; sort by that one if they ever disagree. -pub fn order_sql(col: &str, dir: &str) -> String { +/// `pinned_first` puts your pinned items above the rest, each part in the order asked for, so a +/// pin keeps something at the top of its list (issue #35). Not when sorting by the pin itself, +/// where the direction chosen is the point. +pub fn order_sql(col: &str, dir: &str, pinned_first: bool) -> String { let expr = match col { "kept" => "coalesce(s.flagged, 0)", "title" => "lower(coalesce(e.title, ''))", @@ -704,7 +707,8 @@ pub fn order_sql(col: &str, dir: &str) -> String { _ => "coalesce(e.published, e.first_seen)", }; let dir = if dir == "asc" { "ASC" } else { "DESC" }; - format!("{expr} {dir}, coalesce(e.published, e.first_seen) DESC, e.rowid DESC") + let pins = if pinned_first && col != "kept" { "coalesce(s.flagged, 0) DESC, " } else { "" }; + format!("{pins}{expr} {dir}, coalesce(e.published, e.first_seen) DESC, e.rowid DESC") } /// Which slice of a feed the UI is asking for. @@ -1663,7 +1667,7 @@ mod tests { ) .unwrap(); let order = |col: &str, dir: &str| -> Vec { - db.entries_in(1, None, Filter::All, None, 0, 50, &order_sql(col, dir)) + db.entries_in(1, None, Filter::All, None, 0, 50, &order_sql(col, dir, false)) .unwrap() .into_iter() .map(|e| e.guid) @@ -1677,9 +1681,20 @@ mod tests { assert_eq!(order("published", "desc"), ["c", "b", "a"]); db.set_entry_flag(1, "f", "a", EntryFlag::Flagged, true).unwrap(); assert_eq!(order("kept", "desc")[0], "a"); + // Pinned first: the pinned banana tops every sort, the rest in the order asked for. + let pinned = |col: &str, dir: &str| -> Vec { + db.entries_in(1, None, Filter::All, None, 0, 50, &order_sql(col, dir, true)) + .unwrap() + .into_iter() + .map(|e| e.guid) + .collect() + }; + assert_eq!(pinned("published", "desc"), ["a", "c", "b"]); + assert_eq!(pinned("title", "desc"), ["a", "c", "b"]); + assert_eq!(pinned("kept", "asc")[2], "a", "sorting by the pin itself keeps its direction"); // An unknown column or direction is newest first; the name itself never reaches the SQL. assert_eq!(order("title; DROP TABLE entries", "sideways"), ["c", "b", "a"]); - assert!(!order_sql("x'; --", "asc").contains("x'")); + assert!(!order_sql("x'; --", "asc", false).contains("x'")); } #[test] @@ -1728,7 +1743,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, Some(600)).unwrap(); - let order = order_sql("published", "desc"); + let order = order_sql("published", "desc", false); let page = |user| db.entries_in(user, Some("f"), Filter::All, None, 0, 50, &order).unwrap(); let (ray, sam) = (page(1), page(2)); let ray_b = ray.iter().find(|e| e.guid == "b").unwrap(); @@ -1898,7 +1913,7 @@ mod tests { for f in [Filter::All, Filter::Unread, Filter::Downloaded, Filter::Flagged, Filter::InProgress] { // Both paths must run without erroring, and agree with each other. - let order = order_sql("published", "desc"); + let order = order_sql("published", "desc", false); let rows = db.entries_in(7, Some("f"), f, None, 0, 50, &order).unwrap(); let n = db.count_in(7, Some("f"), f, None).unwrap(); assert_eq!(rows.len() as i64, n, "{f:?} count disagrees with the page"); @@ -1917,7 +1932,7 @@ mod tests { "search is case-insensitive and covers the description"); // Currently Listening: started, not finished, and not just an accidental tap. - let order = order_sql("published", "desc"); + let order = order_sql("published", "desc", false); 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::>() diff --git a/src/web.rs b/src/web.rs index 9b3fcfc..b40f03b 100644 --- a/src/web.rs +++ b/src/web.rs @@ -1146,9 +1146,11 @@ fn entry_page( let filter = crate::db::Filter::parse(page.filter.as_deref().unwrap_or("all")); let search = page.q.as_deref().map(str::trim).filter(|q| !q.is_empty()); let db = &state.ctx.db; + // Currently Listening keeps its own order, pinned or not: it is what you are part-way through. let order = crate::db::order_sql( page.sort.as_deref().unwrap_or("published"), page.dir.as_deref().unwrap_or("desc"), + filter != crate::db::Filter::InProgress, ); let mut rows = db.entries_in(user_id, feed, filter, search, page.offset, page.limit.clamp(1, 200), &order)?; diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index a0d3ceb..9985cb4 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -1299,3 +1299,20 @@ test('a pinned feed, even one from inside a folder, goes to the top of the list' await expect(page.locator('.feed.pinned')).toHaveCount(0); await expect(page.locator('.feed.child', { hasText: name })).toHaveCount(1); }); + +test('a pinned item goes to the top of its list, and back when unpinned', async ({ page }) => { + await page.locator('.feed', { hasText: 'Test Show' }).click(); + await page.locator('.tabs button', { hasText: 'All' }).first().click(); + await expect(page.locator('.ep').nth(1)).toBeVisible({ timeout: 20_000 }); + const guids = () => page.locator('.ep').evaluateAll(rows => rows.map(r => r.dataset.guid)); + const before = await guids(); + const last = before[before.length - 1]; + const row = page.locator(`.ep[data-guid="${last}"]`); + await row.locator('[data-a="flag"]').click(); + await expect.poll(async () => (await guids())[0]).toBe(last); + // It is the server's order, so it holds on a reload. + await page.reload(); + await expect.poll(async () => (await guids())[0]).toBe(last); + await page.locator(`.ep[data-guid="${last}"] [data-a="flag"]`).click(); + await expect.poll(guids).toEqual(before); +}); diff --git a/web/src/items.ts b/web/src/items.ts index e748f62..6789ef5 100644 --- a/web/src/items.ts +++ b/web/src/items.ts @@ -314,7 +314,9 @@ async function epAction(a: string, e, el, encId?: number){ const path=`/api/entries/${encodeURIComponent(e.feed_id)}/${encodeURIComponent(e.guid)}`; try{ if(a==='play') play(e, encId!=null ? enc : undefined); - if(a==='flag'){ e.flagged=!e.flagged; await api(path+'/flags',{method:'POST',body:JSON.stringify({flagged:e.flagged})}); redraw(); } + // A pinned item sits at the top of its list (the server sorts it there), so the list is + // asked for again rather than the row redrawn where it stands. + if(a==='flag'){ e.flagged=!e.flagged; await api(path+'/flags',{method:'POST',body:JSON.stringify({flagged:e.flagged})}); redraw(); loadEntries(); } if(a==='read'){ await setRead(e,!e.read); redraw(); } if(a==='get'){ if(!enc) return;