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

@@ -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<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()
.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<String> {
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::<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 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();