From f09bb4a11c55259ecd562cc70d5b4e150cc8c1fa Mon Sep 17 00:00:00 2001 From: rays Date: Fri, 18 Sep 2026 22:08:37 +0000 Subject: [PATCH] Revert pinned items rising to the top of their list Sorting by the pin column, or the Pinned tab, was enough. order_sql loses its pinned_first option, pinning no longer reloads the list, and the tests and changelog line for #35 go. The NULLS FIRST/LAST ordering from the Postgres work stays. Closes #36. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 8 +++----- src/db.rs | 34 ++++++++++++---------------------- src/web.rs | 7 +------ tests/ui/app.spec.js | 17 ----------------- web/src/items.ts | 4 +--- 5 files changed, 17 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0adf11c..35a4e27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,11 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The database is reached through SeaORM, on the way to Postgres (issue #18); it is still the same SQLite file, and nothing you see changes. On Postgres, sorting by title or feed follows - the language's order (an accented letter beside the plain one) rather than raw bytes. A database from before 0.7 has to be opened by a - 0.7 release first, which brings its tables up to date. -- 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. + the language's order (an accented letter beside the plain one) rather than raw bytes. A + database from before 0.7 has to be opened by a 0.7 release first, which brings its tables up + to date. ## [0.7.0] - 2026-09-18 diff --git a/src/db.rs b/src/db.rs index 91fa768..ac692d4 100644 --- a/src/db.rs +++ b/src/db.rs @@ -765,10 +765,10 @@ fn entries_from(a: &mut Args, user_id: i64, feed_id: Option<&str>, filter: Filte /// /// 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. -/// `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 { +/// +/// Pinned items are not lifted above the rest: sorting by the pin column, or the Pinned tab, does +/// that when it is wanted, and lifting them always was undone (issues #35 and #36). +pub fn order_sql(col: &str, dir: &str) -> String { let expr = match col { "kept" => "coalesce(s.flagged, false)", "title" => "lower(coalesce(e.title, ''))", @@ -781,9 +781,8 @@ pub fn order_sql(col: &str, dir: &str, pinned_first: bool) -> String { // and Postgres as the largest, so "largest first" on Postgres opened with every item that has // no file. NULLS FIRST going up and LAST going down keeps what SQLite did. let dir = if dir == "asc" { "ASC NULLS FIRST" } else { "DESC NULLS LAST" }; - let pins = if pinned_first && col != "kept" { "coalesce(s.flagged, false) DESC, " } else { "" }; // The guid breaks what ties remain: SQLite's rowid did, and Postgres has none. - format!("{pins}{expr} {dir}, coalesce(e.published, e.first_seen) DESC, e.guid DESC") + format!("{expr} {dir}, coalesce(e.published, e.first_seen) DESC, e.guid DESC") } /// Which slice of a feed the UI is asking for. @@ -1729,7 +1728,7 @@ mod tests { ).await .unwrap(); let order = async |col: &str, dir: &str| -> Vec { - db.entries_in(1, None, Filter::All, None, 0, 50, &order_sql(col, dir, false)).await + db.entries_in(1, None, Filter::All, None, 0, 50, &order_sql(col, dir)).await .unwrap() .into_iter() .map(|e| e.guid) @@ -1751,20 +1750,11 @@ mod tests { assert_eq!(order("published", "desc").await, ["c", "b", "a"]); db.set_entry_flag(1, "f", "a", EntryFlag::Flagged, true).await.unwrap(); assert_eq!(order("kept", "desc").await[0], "a"); - // Pinned first: the pinned banana tops every sort, the rest in the order asked for. - let pinned = async |col: &str, dir: &str| -> Vec { - db.entries_in(1, None, Filter::All, None, 0, 50, &order_sql(col, dir, true)).await - .unwrap() - .into_iter() - .map(|e| e.guid) - .collect() - }; - assert_eq!(pinned("published", "desc").await, ["a", "c", "b"]); - assert_eq!(pinned("title", "desc").await, ["a", "c", "b"]); - assert_eq!(pinned("kept", "asc").await[2], "a", "sorting by the pin itself keeps its direction"); + // A pin does not lift an item above the others in any other sort (#36). + assert_eq!(order("published", "desc").await, ["c", "b", "a"]); // An unknown column or direction is newest first; the name itself never reaches the SQL. assert_eq!(order("title; DROP TABLE entries", "sideways").await, ["c", "b", "a"]); - assert!(!order_sql("x'; --", "asc", false).contains("x'")); + assert!(!order_sql("x'; --", "asc").contains("x'")); } #[tokio::test] @@ -1813,7 +1803,7 @@ mod tests { // Starring and position are just as private. db.set_entry_flag(1, "f", "b", EntryFlag::Flagged, true).await.unwrap(); db.set_position(2, "f", "b", 42, Some(600)).await.unwrap(); - let order = order_sql("published", "desc", false); + let order = order_sql("published", "desc"); let page = async |user| db.entries_in(user, Some("f"), Filter::All, None, 0, 50, &order).await.unwrap(); let (ray, sam) = (page(1).await, page(2).await); let ray_b = ray.iter().find(|e| e.guid == "b").unwrap(); @@ -1927,7 +1917,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", false); + let order = order_sql("published", "desc"); let rows = db.entries_in(7, Some("f"), f, None, 0, 50, &order).await.unwrap(); let n = db.count_in(7, Some("f"), f, None).await.unwrap(); assert_eq!(rows.len() as i64, n, "{f:?} count disagrees with the page"); @@ -1946,7 +1936,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", false); + let order = order_sql("published", "desc"); let listening = async || { let rows = db.entries_in(7, Some("f"), Filter::InProgress, None, 0, 50, &order).await.unwrap(); rows.into_iter().map(|e| e.guid).collect::>() diff --git a/src/web.rs b/src/web.rs index f40ca22..a8928b2 100644 --- a/src/web.rs +++ b/src/web.rs @@ -1146,12 +1146,7 @@ async 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 order = crate::db::order_sql(page.sort.as_deref().unwrap_or("published"), page.dir.as_deref().unwrap_or("desc")); let mut rows = db.entries_in(user_id, feed, filter, search, page.offset, page.limit.clamp(1, 200), &order).await?; let mut sanitizer = feed_sanitizer(); diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index 9985cb4..a0d3ceb 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -1299,20 +1299,3 @@ 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 6789ef5..e748f62 100644 --- a/web/src/items.ts +++ b/web/src/items.ts @@ -314,9 +314,7 @@ 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); - // 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==='flag'){ e.flagged=!e.flagged; await api(path+'/flags',{method:'POST',body:JSON.stringify({flagged:e.flagged})}); redraw(); } if(a==='read'){ await setRead(e,!e.read); redraw(); } if(a==='get'){ if(!enc) return;