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 <noreply@anthropic.com>
This commit is contained in:
2026-09-18 22:08:37 +00:00
parent 973ebdd33a
commit f09bb4a11c
5 changed files with 17 additions and 53 deletions

View File

@@ -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 - 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 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 the language's order (an accented letter beside the plain one) rather than raw bytes. A
0.7 release first, which brings its tables up to date. database from before 0.7 has to be opened by a 0.7 release first, which brings its tables up
- A pinned item sits at the top of its list, above everything else in whatever order you sort to date.
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 ## [0.7.0] - 2026-09-18

View File

@@ -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 /// 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. /// 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, /// Pinned items are not lifted above the rest: sorting by the pin column, or the Pinned tab, does
/// where the direction chosen is the point. /// that when it is wanted, and lifting them always was undone (issues #35 and #36).
pub fn order_sql(col: &str, dir: &str, pinned_first: bool) -> String { pub fn order_sql(col: &str, dir: &str) -> String {
let expr = match col { let expr = match col {
"kept" => "coalesce(s.flagged, false)", "kept" => "coalesce(s.flagged, false)",
"title" => "lower(coalesce(e.title, ''))", "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 // 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. // 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 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. // 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. /// Which slice of a feed the UI is asking for.
@@ -1729,7 +1728,7 @@ mod tests {
).await ).await
.unwrap(); .unwrap();
let order = async |col: &str, dir: &str| -> Vec<String> { let order = async |col: &str, dir: &str| -> Vec<String> {
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() .unwrap()
.into_iter() .into_iter()
.map(|e| e.guid) .map(|e| e.guid)
@@ -1751,20 +1750,11 @@ mod tests {
assert_eq!(order("published", "desc").await, ["c", "b", "a"]); assert_eq!(order("published", "desc").await, ["c", "b", "a"]);
db.set_entry_flag(1, "f", "a", EntryFlag::Flagged, true).await.unwrap(); db.set_entry_flag(1, "f", "a", EntryFlag::Flagged, true).await.unwrap();
assert_eq!(order("kept", "desc").await[0], "a"); assert_eq!(order("kept", "desc").await[0], "a");
// Pinned first: the pinned banana tops every sort, the rest in the order asked for. // A pin does not lift an item above the others in any other sort (#36).
let pinned = async |col: &str, dir: &str| -> Vec<String> { assert_eq!(order("published", "desc").await, ["c", "b", "a"]);
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");
// An unknown column or direction is newest first; the name itself never reaches the SQL. // 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_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] #[tokio::test]
@@ -1813,7 +1803,7 @@ mod tests {
// Starring and position are just as private. // Starring and position are just as private.
db.set_entry_flag(1, "f", "b", EntryFlag::Flagged, true).await.unwrap(); db.set_entry_flag(1, "f", "b", EntryFlag::Flagged, true).await.unwrap();
db.set_position(2, "f", "b", 42, Some(600)).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 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, sam) = (page(1).await, page(2).await);
let ray_b = ray.iter().find(|e| e.guid == "b").unwrap(); 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] { for f in [Filter::All, Filter::Unread, Filter::Downloaded, Filter::Flagged, Filter::InProgress] {
// Both paths must run without erroring, and agree with each other. // 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 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(); 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"); 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"); "search is case-insensitive and covers the description");
// Currently Listening: started, not finished, and not just an accidental tap. // 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 listening = async || {
let rows = db.entries_in(7, Some("f"), Filter::InProgress, None, 0, 50, &order).await.unwrap(); let rows = db.entries_in(7, Some("f"), Filter::InProgress, None, 0, 50, &order).await.unwrap();
rows.into_iter().map(|e| e.guid).collect::<Vec<_>>() rows.into_iter().map(|e| e.guid).collect::<Vec<_>>()

View File

@@ -1146,12 +1146,7 @@ async fn entry_page(
let filter = crate::db::Filter::parse(page.filter.as_deref().unwrap_or("all")); 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 search = page.q.as_deref().map(str::trim).filter(|q| !q.is_empty());
let db = &state.ctx.db; 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"));
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 = let mut rows =
db.entries_in(user_id, feed, filter, search, page.offset, page.limit.clamp(1, 200), &order).await?; db.entries_in(user_id, feed, filter, search, page.offset, page.limit.clamp(1, 200), &order).await?;
let mut sanitizer = feed_sanitizer(); let mut sanitizer = feed_sanitizer();

View File

@@ -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.pinned')).toHaveCount(0);
await expect(page.locator('.feed.child', { hasText: name })).toHaveCount(1); 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);
});

View File

@@ -314,9 +314,7 @@ async function epAction(a: string, e, el, encId?: number){
const path=`/api/entries/${encodeURIComponent(e.feed_id)}/${encodeURIComponent(e.guid)}`; const path=`/api/entries/${encodeURIComponent(e.feed_id)}/${encodeURIComponent(e.guid)}`;
try{ try{
if(a==='play') play(e, encId!=null ? enc : undefined); 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 if(a==='flag'){ e.flagged=!e.flagged; await api(path+'/flags',{method:'POST',body:JSON.stringify({flagged:e.flagged})}); redraw(); }
// 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==='read'){ await setRead(e,!e.read); redraw(); }
if(a==='get'){ if(a==='get'){
if(!enc) return; if(!enc) return;