feat(podcasts): reverse order — position DESC, date-agnostic; single order helper

The 'Oldest first' order sorts by PUB DATE, which breaks on Scribl's
migrated podiobooks feeds (messy/all-equal dates). New order value
'reverse' = feed position DESC with a date-DESC tiebreak — dates are
not involved, so nothing can break.

- services.OrderEpisodes: THE single order source of truth (§0 items
  3+5a). The four surfaces (sequence JSON, reader chain context, SSR
  page list) previously carried three near-identical handwritten
  comparators — all now route through one generic helper. The SSR page
  list gains a SortDate key (lexical 2006-01-02) so the pub-date
  tiebreak is actually chronological; the old comparator compared
  'Jan 2, 2006' display strings alphabetically (latent drift bug).
- Dropdown becomes four options: Feed order / Feed order (reversed) /
  Oldest first (pub date) / Newest first (pub date).
- Serial suggestion button renamed 'Reverse feed order' (sets reverse,
  not oldest) and its response out-of-band-swaps the order select
  (PodcastOrderSelectOOB — same mechanism as Download All's policy
  select), so the dropdown mirrors the server state immediately.
- Validation: oneof=native reverse oldest newest everywhere.
- Tests: reverse binds via the form wire; messy-dates sequence
  returns 3,2,1; existing reorder-safety stays green.
This commit is contained in:
John O'Keefe committed 2026-10-30 09:15:03 -04:00
1 parent 7c7c08748e
commit e3675536aa
9 files changed
+608 -363

No files matched your search

+65 -3
View File
@@ -11,12 +11,12 @@ package main
import (
"bytes"
"net/url"
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"os"
"os/exec"
"path/filepath"
@@ -664,8 +664,6 @@ func adminScopeExec(t *testing.T, setup *TestServerSetup, adminID pgtype.UUID, s
require.NoError(t, err)
}
// ---- task 1.9 residue: the htmx wire format is FORM-encoded (issue #2) ----
func TestPodcastOrderFormEncoding(t *testing.T) {
@@ -695,10 +693,74 @@ func TestPodcastOrderFormEncoding(t *testing.T) {
assert.Equal(t, http.StatusOK, formPut("order=oldest"), "form-encoded order must bind")
assert.Equal(t, http.StatusOK, formPut("order=native"), "native binds too")
assert.Equal(t, http.StatusOK, formPut("order=reverse"), "reverse binds too (§0 item 3)")
assert.Equal(t, http.StatusBadRequest, formPut(""), "missing order stays rejected (required)")
assert.Equal(t, http.StatusBadRequest, formPut("order=sideways"), "invalid order stays rejected")
}
// ---- §0 item 3: reverse = feed position DESC (no pub dates involved) ----
func TestPodcastReverseOrder(t *testing.T) {
setup := setupTestServer(t)
adminCtx := adminSeedCtx(t, setup)
adminID := adminCtxValue(t, setup)
lib, _ := createPodcastsLibrary(t, setup, "test ReverseOrder Podcasts")
feed, err := setup.DB.CreatePodcastFeed(adminCtx, database.CreatePodcastFeedParams{
LibraryID: lib.ID,
FeedUrl: "https://feed.invalid/reverse-order.xml",
Title: "Reverse Order Feed",
})
require.NoError(t, err)
feedIDStr := uuid.UUID(feed.ID.Bytes).String()
// Episodes with MESSY dates (the Scribl condition): all equal.
// reverse must be driven by feed position alone — position 3 first.
epA := upsertTestEpisode(t, setup, feed, "rr-1", "Reverse Alpha", 1)
epB := upsertTestEpisode(t, setup, feed, "rr-2", "Reverse Beta", 2)
epC := upsertTestEpisode(t, setup, feed, "rr-3", "Reverse Gamma", 3)
adminScopeExec(t, setup, adminID, `
UPDATE media_items SET
download_state = 'downloaded',
file_size = 1000,
date_published = '2020-01-01T00:00:00Z'::timestamptz
WHERE id IN ($1, $2, $3) AND source = 'podcast_episode'`, epA.ID, epB.ID, epC.ID)
subscribeRaw(t, setup, uuid.UUID(adminID.Bytes), feed.ID)
// Set the override to reverse (the wire the "Reverse feed order"
// button uses).
formPut := func(body string) int {
req, _ := http.NewRequest(http.MethodPut,
setup.Server.URL+"/api/podcasts/feeds/"+feedIDStr+"/order",
strings.NewReader(body))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("Authorization", "Bearer "+setup.Token)
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
defer func() { _ = resp.Body.Close() }()
return resp.StatusCode
}
require.Equal(t, http.StatusOK, formPut("order=reverse"))
// The SEQUENCE endpoint must return position 3, 2, 1 — driven by
// feed position DESC, dates being all equal (the Scribl guard).
_, body := podcastAuthed(t, setup, setup.Token, "GET",
"/api/podcasts/feeds/"+feedIDStr+"/sequence", "")
var seq struct {
Order string `json:"order"`
Episodes []struct {
Title string `json:"title"`
FeedPosition int `json:"feed_position"`
} `json:"episodes"`
}
require.NoError(t, json.Unmarshal(body, &seq))
assert.Equal(t, "reverse", seq.Order, "the endpoint echoes the effective order")
require.Len(t, seq.Episodes, 3)
assert.Equal(t, 3, seq.Episodes[0].FeedPosition, "PIN: reverse = position DESC, date-agnostic")
assert.Equal(t, 1, seq.Episodes[2].FeedPosition)
assert.Equal(t, "Reverse Gamma", seq.Episodes[0].Title,
"position 3 first — a messy-date feed reorders by position, not dates")
}
// ---- issue #8: page search hits EPISODE titles/descriptions too ----
func TestPodcastPageSearchEpisodes(t *testing.T) {
+21 -32
View File
@@ -9,7 +9,6 @@ package handlers
import (
"context"
"encoding/json"
"fmt"
"log"
"sort"
@@ -39,8 +38,11 @@ type PodcastHandler struct {
// EpisodeListRenderer renders the post-action episode list for an htmx
// request. includePolicySelect additionally out-of-band-swaps the
// auto-download select (Download All flips the caller's policy — the
// dropdown must reflect it immediately, issue #3).
type EpisodeListRenderer func(c *echo.Context, feedID string, includePolicySelect bool) error
// dropdown must reflect it immediately, issue #3);
// includeOrderSelect likewise rides along for the order endpoint (the
// "Reverse feed order" button flips to reverse — the dropdown must
// mirror the server state, §0 item 3).
type EpisodeListRenderer func(c *echo.Context, feedID string, includePolicySelect, includeOrderSelect bool) error
// SetEpisodeListRenderer wires the renderer (router-only; product wiring).
func (h *PodcastHandler) SetEpisodeListRenderer(r EpisodeListRenderer) {
@@ -49,9 +51,9 @@ func (h *PodcastHandler) SetEpisodeListRenderer(r EpisodeListRenderer) {
// hxEpisodeList answers an htmx action with the refreshed partial when
// a renderer is wired; otherwise falls back to 204 + HX-Trigger.
func (h *PodcastHandler) hxEpisodeList(c *echo.Context, feedID string, includePolicySelect bool) error {
func (h *PodcastHandler) hxEpisodeList(c *echo.Context, feedID string, includePolicySelect, includeOrderSelect bool) error {
if h.episodeListRenderer != nil {
return h.episodeListRenderer(c, feedID, includePolicySelect)
return h.episodeListRenderer(c, feedID, includePolicySelect, includeOrderSelect)
}
return hxAccepted(c, "")
}
@@ -328,7 +330,7 @@ func (h *PodcastHandler) UpdateSubscription(c *echo.Context) error {
if isHXRequest(c) {
// Policy changes feed the auto-download union on the next poll.
_, _ = h.worker.ApplyAutoPolicies(ctx, feedID)
return h.hxEpisodeList(c, feedID.String(), true)
return h.hxEpisodeList(c, feedID.String(), true, false)
}
return c.JSON(http.StatusOK, updated)
}
@@ -367,7 +369,7 @@ func (h *PodcastHandler) RefreshFeed(c *echo.Context) error {
return podcastErrorResponse(c, err)
}
if isHXRequest(c) {
return h.hxEpisodeList(c, feedID.String(), false)
return h.hxEpisodeList(c, feedID.String(), false, false)
}
return c.JSON(http.StatusOK, map[string]string{"message": "refreshed"})
}
@@ -418,7 +420,7 @@ func (h *PodcastHandler) DownloadEpisodes(c *echo.Context) error {
// Download All flips the caller's policy to all (issue #3)
// — the oob select keeps the dropdown honest.
h.flipSubscriptionToAll(ctx, user.ID, feedID)
return h.hxEpisodeList(c, feedID.String(), true)
return h.hxEpisodeList(c, feedID.String(), true, false)
}
return c.JSON(http.StatusAccepted, map[string]interface{}{"queued": queued})
}
@@ -430,7 +432,7 @@ func (h *PodcastHandler) DownloadEpisodes(c *echo.Context) error {
return podcastErrorResponse(c, err)
}
if isHXRequest(c) {
return h.hxEpisodeList(c, feedID.String(), false)
return h.hxEpisodeList(c, feedID.String(), false, false)
}
return c.JSON(http.StatusAccepted, map[string]string{"message": "queued"})
}
@@ -503,9 +505,10 @@ func podcastOrderSettingKey(feedID string) string {
return "podcast_order:" + feedID
}
// podcastOrderValid: native (feed position) | oldest | newest.
// podcastOrderValid native (feed position) | reverse (position DESC,
// no dates) | oldest | newest.
func podcastOrderValid(o string) bool {
return o == "native" || o == "oldest" || o == "newest"
return services.ValidPodcastEpisodeOrder(o)
}
// GetFeedOrder reads the caller's effective order for a feed
@@ -538,7 +541,7 @@ func (h *PodcastHandler) SetFeedOrder(c *echo.Context) error {
return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid feed id"})
}
var req struct {
Order string `json:"order" form:"order" validate:"required,oneof=native oldest newest"`
Order string `json:"order" form:"order" validate:"required,oneof=native reverse oldest newest"`
}
if err := c.Bind(&req); err != nil {
return c.JSON(http.StatusBadRequest, map[string]string{"error": "invalid request"})
@@ -564,7 +567,7 @@ func (h *PodcastHandler) SetFeedOrder(c *echo.Context) error {
}
}
if isHXRequest(c) {
return h.hxEpisodeList(c, feedID.String(), false)
return h.hxEpisodeList(c, feedID.String(), false, true)
}
return c.JSON(http.StatusOK, map[string]string{"order": req.Order})
}
@@ -590,19 +593,11 @@ func (h *PodcastHandler) GetFeedSequence(c *echo.Context) error {
for i := range eps {
views = append(views, episodeView(&eps[i]))
}
sortSlice(views, func(a, b PodcastEpisodeView) bool {
switch order {
case "oldest":
return episodeSortKey(a) < episodeSortKey(b)
case "newest":
return episodeSortKey(a) > episodeSortKey(b)
default: // native: feed position (fetch-maintained)
if a.FeedPosition != b.FeedPosition {
return a.FeedPosition < b.FeedPosition
}
return a.Date < b.Date
}
})
// Effective listening order (order consolidation §0 item 5a).
services.OrderEpisodes(views, order,
func(v PodcastEpisodeView) int32 { return int32(v.FeedPosition) },
func(v PodcastEpisodeView) string { return v.Date },
)
sequence := make([]PodcastEpisodeView, 0, len(views))
gaps := make([]PodcastEpisodeView, 0)
@@ -621,12 +616,6 @@ func (h *PodcastHandler) GetFeedSequence(c *echo.Context) error {
})
}
// episodeSortKey: chronological key for oldest/newest (date, then
// native position as the same-day tiebreaker).
func episodeSortKey(v PodcastEpisodeView) string {
return v.Date + fmt.Sprintf("%06d", v.FeedPosition)
}
// sortSlice: sort.Slice with a controller-friendly wrapper.
func sortSlice[T any](s []T, less func(a, b T) bool) {
sort.Slice(s, func(i, j int) bool { return less(s[i], s[j]) })
+9 -1
View File
@@ -20,7 +20,7 @@ func registerPodcastRoutes(cfg *Config) {
// refreshed partial directly — no event indirection to miss. The
// view assembly lives here (templates) while the action logic lives
// in the handler; this closure is the sanctioned seam.
cfg.PodcastHandler.SetEpisodeListRenderer(func(c *echo.Context, feedID string, includePolicySelect bool) error {
cfg.PodcastHandler.SetEpisodeListRenderer(func(c *echo.Context, feedID string, includePolicySelect, includeOrderSelect bool) error {
user, err := getTemplateUserWithTheme(c, cfg)
if err != nil {
return err
@@ -48,6 +48,14 @@ func registerPodcastRoutes(cfg *Config) {
return err
}
}
if includeOrderSelect {
// §0 item 3: out-of-band swap of the order select — the
// "Reverse feed order" button flips to reverse; the
// dropdown mirrors the server state immediately.
if err := templates.PodcastOrderSelectOOB(data.Feed.ID, data.Order).Render(c.Request().Context(), &buf); err != nil {
return err
}
}
return c.HTML(http.StatusOK, buf.String())
})
+14 -16
View File
@@ -9,6 +9,7 @@ package router
import (
"bookhoard/internal/database"
"bookhoard/internal/handlers"
"bookhoard/internal/services"
"bookhoard/internal/utils"
"bookhoard/templates"
"bytes"
@@ -316,7 +317,9 @@ func buildPodcastDetailData(c *echo.Context, cfg *Config, user templates.User, f
rowID := uuid.UUID(ep.ID.Bytes).String()
p := progressByItem[rowID]
dateDisplay := ""
dateSort := ""
if ep.DatePublished.Valid {
dateSort = ep.DatePublished.Time.Format("2006-01-02")
dateDisplay = ep.DatePublished.Time.In(utils.UserLocation(user.Timezone)).Format("Jan 2, 2006")
}
rows = append(rows, templates.PodcastEpisodeRow{
@@ -325,6 +328,7 @@ func buildPodcastDetailData(c *echo.Context, cfg *Config, user templates.User, f
Title: ep.Title,
Description: firstLine(ep.Description.String, 200),
Date: dateDisplay,
SortDate: dateSort,
Position: int(ep.FeedPosition.Int32),
Duration: templates.FormatEpisodeDuration(int(ep.DeclaredDurationSeconds.Int32)),
Size: templates.FormatEpisodeSize(ep.FileSize.Int64),
@@ -337,17 +341,13 @@ func buildPodcastDetailData(c *echo.Context, cfg *Config, user templates.User, f
}
// Order override reshapes the LIST too (the sequence endpoint is the
// strict downloaded-only view; the list shows everything in the
// chosen order so pending episodes stay visible).
if order == "oldest" || order == "newest" {
sort.SliceStable(rows, func(a, b int) bool {
ka := rows[a].Date + itoaPad(rows[a].Position)
kb := rows[b].Date + itoaPad(rows[b].Position)
if order == "newest" {
return ka > kb
}
return ka < kb
})
}
// chosen order so pending episodes stay visible). Consolidated sort
// (§0 item 5a) — SortDate is the lexical pub-date key so the
// tiebreak is chronological even in display-tz date strings.
services.OrderEpisodes(rows, order,
func(r templates.PodcastEpisodeRow) int32 { return int32(r.Position) },
func(r templates.PodcastEpisodeRow) string { return r.SortDate },
)
detail := &templates.PodcastDetailData{
Feed: card,
@@ -396,12 +396,11 @@ func podcastOrderForFeed(cfg *Config, c *echo.Context, feedID string) string {
if err := json.Unmarshal(row, &v); err != nil {
return "native"
}
switch v.Order {
case "oldest", "newest":
return v.Order
}
if !services.ValidPodcastEpisodeOrder(v.Order) {
return "native"
}
return v.Order
}
// facetValuesFromCards derives the filter bar values.
func facetValuesFromCards(feeds []templates.PodcastFeedCard) map[string][]string {
@@ -455,7 +454,6 @@ func itoaPad(n int) string {
return fmt.Sprintf("%06d", n)
}
// parsePodcastUUID parses an id path param (handlers.parseUUID is
// package-private).
func parsePodcastUUID(s string) (pgtype.UUID, error) {
+6 -14
View File
@@ -9,9 +9,7 @@ import (
"bookhoard/templates"
"bytes"
"encoding/json"
"fmt"
"net/http"
"sort"
"github.com/google/uuid"
"github.com/jackc/pgx/v5/pgtype"
@@ -186,7 +184,7 @@ func buildPodcastSequenceContext(c *echo.Context, cfg *Config, item *database.Me
var v struct {
Order string `json:"order"`
}
if json.Unmarshal(row, &v) == nil && (v.Order == "oldest" || v.Order == "newest") {
if json.Unmarshal(row, &v) == nil && services.ValidPodcastEpisodeOrder(v.Order) {
order = v.Order
}
}
@@ -215,17 +213,11 @@ func buildPodcastSequenceContext(c *echo.Context, cfg *Config, item *database.Me
pos: ep.FeedPosition.Int32,
})
}
sort.SliceStable(ordered, func(a, b int) bool {
ka := ordered[a].date + fmt.Sprintf("%06d", ordered[a].pos)
kb := ordered[b].date + fmt.Sprintf("%06d", ordered[b].pos)
if order == "newest" {
return ka > kb
}
if order == "oldest" {
return ka < kb
}
return ordered[a].pos < ordered[b].pos
})
// Effective listening order (order consolidation §0 item 5a).
services.OrderEpisodes(ordered, order,
func(e seqEp) int32 { return e.pos },
func(e seqEp) string { return e.date },
)
idx := -1
for i, ep := range ordered {
+60
View File
@@ -0,0 +1,60 @@
package services
import (
"fmt"
"sort"
)
// Podcast episode ordering — THE single source of order truth (§0 item
// 3 + item 5a, 2026-10-30). Every surface (sequence JSON, reader chain
// context, SSR page list, download queueing) routes through
// OrderEpisodes; per-surface sort blocks were the drift risk (three
// near-identical comparators existed by hand).
// PodcastEpisodeOrder values:
// - native: feed position ASC, date ASC tiebreak (fetch-maintained order)
// - reverse: feed position DESC, date DESC tiebreak ("Reverse feed
// order" — no pub dates involved, so Scribl's messy/all-equal
// migrated podiobooks dates can't break it)
// - oldest / newest: chronological by pub date string (2006-01-02 or
// anything lexical-comparable), position as same-date tiebreak
func ValidPodcastEpisodeOrder(o string) bool {
return o == "native" || o == "reverse" || o == "oldest" || o == "newest"
}
// OrderEpisodes sorts eps in place into the given order. Accessor
// functions supply each episode's feed position and pub date key
// (LEN lexical-comparable, e.g. RFC3339 or 2006-01-02).
func OrderEpisodes[T any](eps []T, order string, pos func(T) int32, date func(T) string) {
switch order {
case "reverse":
sort.SliceStable(eps, func(i, j int) bool {
if pos(eps[i]) != pos(eps[j]) {
return pos(eps[i]) > pos(eps[j])
}
return date(eps[i]) > date(eps[j])
})
case "oldest", "newest":
sort.SliceStable(eps, func(i, j int) bool {
ka := date(eps[i]) + episodePosKey(pos(eps[i]))
kb := date(eps[j]) + episodePosKey(pos(eps[j]))
if order == "newest" {
return ka > kb
}
return ka < kb
})
default: // native
sort.SliceStable(eps, func(i, j int) bool {
if pos(eps[i]) != pos(eps[j]) {
return pos(eps[i]) < pos(eps[j])
}
return date(eps[i]) < date(eps[j])
})
}
}
// episodePosKey: zero-padded position so lexical date keys tiebreak
// numerically.
func episodePosKey(pos int32) string {
return fmt.Sprintf("%06d", pos)
}
+30 -6
View File
@@ -116,21 +116,24 @@ templ PodcastDetailPage(user User, data PodcastDetailData) {
hx-swap="outerHTML"
>
<option value="native" selected={ data.Order == "native" }>Feed order</option>
<option value="oldest" selected={ data.Order == "oldest" }>Oldest first</option>
<option value="newest" selected={ data.Order == "newest" }>Newest first</option>
<option value="reverse" selected={ data.Order == "reverse" }>Feed order (reversed)</option>
<option value="oldest" selected={ data.Order == "oldest" }>Oldest first (pub date)</option>
<option value="newest" selected={ data.Order == "newest" }>Newest first (pub date)</option>
</select>
<!-- Serial suggestion (D5): one-click, never silent -->
<!-- Serial suggestion (D5, §0 item 3 rename): one-click,
never silent. "reverse" sorts by FEED POSITION DESC —
no pub dates involved, so messy dates can't break it. -->
if data.IsSerial && data.Order == "native" {
<button
class="btn btn-secondary text-sm"
hx-put={ "/api/podcasts/feeds/" + data.Feed.ID + "/order" }
hx-vals='{"order": "oldest"}'
hx-vals='{"order": "reverse"}'
hx-target="#episode-list"
hx-swap="outerHTML"
title="Serial feed: publishers list newest first — listen oldest-first"
title="Serial feed: publishers list newest first — reverse the feed order to start at chapter 1"
>
<span>Serial? Listen oldest-first</span>
<span>Reverse feed order</span>
</button>
}
@@ -382,6 +385,27 @@ templ PodcastAutoDownloadSelect(feed PodcastFeedCard) {
</select>
}
// PodcastOrderSelectOOB is the out-of-band variant of the order
// dropdown: same element id + hx-swap-oob so htmx replaces the
// action-row select in place (§0 item 3 — the "Reverse feed order"
// button flips the order server-side; the dropdown must show it).
templ PodcastOrderSelectOOB(feedID string, order string) {
<select
id="podcast-order-select"
name="order"
hx-swap-oob="true"
class="input w-auto py-1.5 cursor-pointer"
hx-put={ "/api/podcasts/feeds/" + feedID + "/order" }
hx-target="#episode-list"
hx-swap="outerHTML"
>
<option value="native" selected={ order == "native" }>Feed order</option>
<option value="reverse" selected={ order == "reverse" }>Feed order (reversed)</option>
<option value="oldest" selected={ order == "oldest" }>Oldest first (pub date)</option>
<option value="newest" selected={ order == "newest" }>Newest first (pub date)</option>
</select>
}
// PodcastAutoDownloadSelectOOB is the out-of-band variant: same element
// id + hx-swap-oob so htmx replaces the action-row select in place.
templ PodcastAutoDownloadSelectOOB(feed PodcastFeedCard) {
File diff suppressed because it is too large. Load diff
+1
View File
@@ -86,6 +86,7 @@ type PodcastEpisodeRow struct {
Title string
Description string
Date string // display-formatted (user tz)
SortDate string // lexical-comparable pub-date key (2006-01-02) for ordering
Position int
Duration string // human ("58:02" / "1:02:10")
Size string // human MB/GB when known