From 8d7c00e710fa6015fd7c9c64b95575c9a69346ee Mon Sep 17 00:00:00 2001 From: ssavutu Date: Fri, 7 Aug 2026 01:08:13 -0400 Subject: [PATCH] Let editors pick the story that leads the homepage The article schema has carried is_featured (the legacy `priority` column) since the WordPress import, but nothing ever set it and nothing ever read it: the ETL hardcodes it false on every row, no query orders by it, and Scalene has never referenced it on any branch. Editors asking how to set a featured article were right that there was no way to. Featured now means what the newsroom means by it: the big centre card at the top of the homepage, which Scalene renders from news[0]. The article is spliced into the news block rather than sorted into the news query, because the featured story may be filed under any section -- a featured sports story still takes the lead card, which a news-scoped ORDER BY could never do. It keeps its place in its own section block, the way a lead story does in print; only the news block dedupes, so featuring a news story promotes it instead of printing it twice. Featuring is exclusive on all three write paths. The homepage has one lead card, and leaving the previous pick flagged would let the tiebreak, rather than the editor, decide which story runs. The clear happens after the target write succeeds, so a failed save leaves the old lead standing instead of leaving the homepage with none. Drafts, scheduled and archived rows are skipped when resolving the lead: the flag alone must not be able to put an unpublished headline on the front page. The homepage response also states its freshness for the first time (60s, plus stale-while-revalidate). Without a Cache-Control header every intermediary applies its own heuristic, so an editor swapping the lead had no bound at all on when readers would see it. Also unpins TestTaxonomyAliasSeedAndCacheRoundTrip from a literal alias list. It asserted entertainment held exactly ["Arts & Entertainment"] and that news had no aliases, both of which c0f2ae9 invalidated when it filed the remaining orphaned categories. It now derives expectations from defaultCategoryAliases, so filing more categories cannot fail it, and checks the HTML-escaping regression explicitly rather than through a comparison that shares a marshaller with the code under test. Co-Authored-By: Claude Opus 5 --- frontend/src/pages/editArticleView.tsx | 24 +++ server/internal/database/featured_article.go | 61 ++++++ .../database/taxonomy_integration_test.go | 46 ++++- .../featured_article_integration_test.go | 195 ++++++++++++++++++ .../handlers/featured_article_test.go | 94 +++++++++ server/internal/handlers/handlers.go | 115 +++++++++++ 6 files changed, 526 insertions(+), 9 deletions(-) create mode 100644 server/internal/database/featured_article.go create mode 100644 server/internal/handlers/featured_article_integration_test.go create mode 100644 server/internal/handlers/featured_article_test.go diff --git a/frontend/src/pages/editArticleView.tsx b/frontend/src/pages/editArticleView.tsx index 32cfc86..a88b946 100644 --- a/frontend/src/pages/editArticleView.tsx +++ b/frontend/src/pages/editArticleView.tsx @@ -27,6 +27,7 @@ type ApiArticleDetail = { featured_image?: string featured_image_alt?: string breaking_news?: boolean + is_featured?: boolean categories?: Array<{ name?: string slug?: string @@ -63,6 +64,7 @@ type PatchPayload = { photo_url: string photo_alt: string breaking_news: boolean + is_featured: boolean categories: string[] tags: string[] authors: number[] @@ -263,6 +265,7 @@ function EditArticleView() { const [photoURL, setPhotoURL] = useState("") const [photoAlt, setPhotoAlt] = useState("") const [breakingNews, setBreakingNews] = useState(false) + const [isFeatured, setIsFeatured] = useState(false) const [selectedCategorySlugs, setSelectedCategorySlugs] = useState([]) const [sectionSearch, setSectionSearch] = useState("") const [legacyCategoryTitlesBySlug, setLegacyCategoryTitlesBySlug] = useState>({}) @@ -300,6 +303,7 @@ function EditArticleView() { photoURL, photoAlt, breakingNews, + isFeatured, selectedCategorySlugs, seoTags, seoTagDraft, @@ -319,6 +323,7 @@ function EditArticleView() { photoURL, photoAlt, breakingNews, + isFeatured, selectedCategorySlugs, seoTags, seoTagDraft, @@ -394,6 +399,7 @@ function EditArticleView() { setPhotoURL(payload.featured_image ?? "") setPhotoAlt(payload.featured_image_alt ?? "") setBreakingNews(Boolean(payload.breaking_news)) + setIsFeatured(Boolean(payload.is_featured)) const legacyCategories: Record = {} const categorySlugs = (payload.categories ?? []) .map((category) => { @@ -661,6 +667,7 @@ function EditArticleView() { photo_url: photoURL.trim(), photo_alt: photoAlt.trim(), breaking_news: breakingNews, + is_featured: isFeatured, categories, tags: seoTagsToSave, authors: selectedAuthorIds, @@ -706,6 +713,7 @@ function EditArticleView() { photo_url: photoURL.trim(), photo_alt: photoAlt.trim(), breaking_news: breakingNews, + is_featured: isFeatured, categories, tags: seoTagsToSave, authors: selectedAuthorIds, @@ -1370,6 +1378,22 @@ function EditArticleView() { Breaking news + +
Sections
diff --git a/server/internal/database/featured_article.go b/server/internal/database/featured_article.go new file mode 100644 index 0000000..bbfbf22 --- /dev/null +++ b/server/internal/database/featured_article.go @@ -0,0 +1,61 @@ +package database + +import ( + "context" + "database/sql" + + "server/internal/models" +) + +// The featured article is the one an editor has pinned to the big lead card in +// the middle of the homepage. It is stored in the legacy `priority` column and +// is exclusive: exactly zero or one article carries it at a time, enforced on +// write by ClearFeaturedExcept. + +const featuredArticleConditions = "WHERE `priority` = 1 AND `pub_date` IS NOT NULL AND `pub_date` <= UTC_TIMESTAMP() AND `archived_at` IS NULL" + +// GetFeaturedArticle returns the featured article, or nil when nothing is +// featured. Unpublished, scheduled and archived rows are skipped so an article +// cannot reach the homepage through the flag alone -- an editor who features a +// draft and forgets to publish it gets the normal newest-first lead, not a +// headline the public should not see yet. +// +// The ORDER BY is a defensive tiebreak: exclusivity is enforced on write, but a +// direct DB edit or an ETL reseed could leave two rows flagged, and the homepage +// must still resolve to one article rather than picking arbitrarily. +func GetFeaturedArticle(ctx context.Context, conn *sql.DB) (*models.Article, error) { + query := searchSelectColumns + featuredArticleConditions + " ORDER BY `pub_date` DESC, `id` DESC LIMIT 1" + rows, err := conn.QueryContext(ctx, query) + if err != nil { + return nil, err + } + defer rows.Close() + + articles, err := CollectArticles(rows) + if err != nil { + return nil, err + } + if len(articles) == 0 { + return nil, nil + } + return &articles[0], nil +} + +// ClearFeaturedExcept unfeatures every article other than the given slug. Call +// it after the write that features an article, never before: if the ordering +// were reversed and the main update then failed, the site would be left with no +// featured article at all instead of the one it had. +func ClearFeaturedExcept(ctx context.Context, conn *sql.DB, slug string) error { + _, err := conn.ExecContext(ctx, + "UPDATE `articles` SET `priority` = 0 WHERE `priority` = 1 AND `slug` <> ?", slug) + return err +} + +// ClearFeaturedExceptID is ClearFeaturedExcept for the create path, where the +// slug may have been generated from the title rather than supplied by the +// caller and the insert's id is the only handle on the new row. +func ClearFeaturedExceptID(ctx context.Context, conn *sql.DB, id int64) error { + _, err := conn.ExecContext(ctx, + "UPDATE `articles` SET `priority` = 0 WHERE `priority` = 1 AND `id` <> ?", id) + return err +} diff --git a/server/internal/database/taxonomy_integration_test.go b/server/internal/database/taxonomy_integration_test.go index f85fb96..9561961 100644 --- a/server/internal/database/taxonomy_integration_test.go +++ b/server/internal/database/taxonomy_integration_test.go @@ -4,11 +4,25 @@ import ( "context" "database/sql" "os" + "strings" "testing" _ "github.com/go-sql-driver/mysql" ) +// A section the seed has no defaults for, for the "keeps no aliases" case. +// Which slugs are unseeded changes as orphaned categories get filed -- news +// gained "Paid Post" -- so the choice is guarded by requireUnseeded rather than +// assumed, or the assertion would quietly start testing nothing. +const unseededSlug = "photo" + +func requireUnseeded(t *testing.T, slug string) { + t.Helper() + if _, seeded := defaultCategoryAliases[slug]; seeded { + t.Fatalf("%q now has seeded aliases; pick another slug for the no-defaults case", slug) + } +} + // These need a real MariaDB, because the whole point is the column, the seed // and the cache reload agreeing with each other. They skip unless CMS_TEST_DSN // is set, so CI without a database stays green. @@ -65,33 +79,47 @@ func TestTaxonomyAliasSeedAndCacheRoundTrip(t *testing.T) { if err := EnsureTaxonomyTable(ctx, conn); err != nil { t.Fatalf("ensure taxonomy table: %v", err) } + requireUnseeded(t, unseededSlug) insertTaxonomyRow(t, conn, 1, "section", "entertainment", "Entertainment") - insertTaxonomyRow(t, conn, 2, "section", "news", "News") + insertTaxonomyRow(t, conn, 2, "section", unseededSlug, "Photo") if err := EnsureTaxonomyTable(ctx, conn); err != nil { t.Fatalf("second ensure: %v", err) } - // entertainment must have picked up its seeded default... + // entertainment must have picked up its seeded defaults. Asserted against + // defaultCategoryAliases rather than a literal list: the map grows every + // time another orphaned category is filed, and a test that pins the + // contents fails on the filing rather than on anything being broken. patterns := CategoryMatchPatterns("entertainment") - if !containsPattern(patterns, `%"arts & entertainment"%`) { - t.Errorf("entertainment patterns %v missing the seeded alias", patterns) + for _, alias := range defaultCategoryAliases["entertainment"] { + want := `%"` + strings.ToLower(alias) + `"%` + if !containsPattern(patterns, want) { + t.Errorf("entertainment patterns %v missing the seeded alias %s", patterns, want) + } } - // ...and it must not be HTML-escaped in the column, or it would never - // match the articles it names. + // The aliases must not be HTML-escaped in the column, or one holding an + // ampersand would never match the articles it names. var stored string if err := conn.QueryRowContext(ctx, "SELECT category_aliases FROM site_taxonomy WHERE slug = 'entertainment'", ).Scan(&stored); err != nil { t.Fatalf("read stored aliases: %v", err) } - if want := `["Arts & Entertainment"]`; stored != want { + want, err := MarshalCategoryJSON(defaultCategoryAliases["entertainment"]) + if err != nil { + t.Fatalf("marshal expected aliases: %v", err) + } + if stored != want { t.Errorf("stored aliases = %s, want %s", stored, want) } + if strings.Contains(stored, "\\u0026") { + t.Errorf("stored aliases are HTML-escaped: %s", stored) + } // A slug with no default keeps no aliases. - if got := CategoryMatchPatterns("news"); len(got) != 1 { - t.Errorf("news patterns = %v, want just its own", got) + if got := CategoryMatchPatterns(unseededSlug); len(got) != 1 { + t.Errorf("%s patterns = %v, want just its own", unseededSlug, got) } } diff --git a/server/internal/handlers/featured_article_integration_test.go b/server/internal/handlers/featured_article_integration_test.go new file mode 100644 index 0000000..f9342a4 --- /dev/null +++ b/server/internal/handlers/featured_article_integration_test.go @@ -0,0 +1,195 @@ +package handlers + +import ( + "context" + "database/sql" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + db "server/internal/database" +) + +// CMS_TEST_DSN='user:pw@tcp(127.0.0.1:3306)/cms_test?parseTime=true&multiStatements=true' go test ./internal/handlers/ -run Featured -v + +func seedFeaturedTestArticle(t *testing.T, conn *sql.DB, slug string, priority bool, pubDate any, archivedAt any) { + t.Helper() + if _, err := conn.ExecContext(context.Background(), + "INSERT INTO articles (title, slug, `text`, categories, priority, pub_date, archived_at) VALUES (?, ?, ?, ?, ?, ?, ?)", + slug, slug, "Body", "News", priority, pubDate, archivedAt, + ); err != nil { + t.Fatalf("seed %s: %v", slug, err) + } +} + +func featuredSlugs(t *testing.T, conn *sql.DB) []string { + t.Helper() + rows, err := conn.QueryContext(context.Background(), "SELECT slug FROM articles WHERE priority = 1 ORDER BY slug") + if err != nil { + t.Fatalf("read featured slugs: %v", err) + } + defer rows.Close() + var slugs []string + for rows.Next() { + var slug string + if err := rows.Scan(&slug); err != nil { + t.Fatalf("scan slug: %v", err) + } + slugs = append(slugs, slug) + } + return slugs +} + +// The homepage has one lead card. If featuring a second article left the first +// one flagged, the tiebreak in GetFeaturedArticle -- not the editor -- would be +// deciding which story runs. +func TestFeaturedArticleHTTP_PatchUnfeaturesThePreviousPick(t *testing.T) { + conn := articlePatchTestDB(t) + published := time.Now().UTC().Add(-24 * time.Hour).Format("2006-01-02 15:04:05") + seedFeaturedTestArticle(t, conn, "old-lead", true, published, nil) + seedFeaturedTestArticle(t, conn, "new-lead", false, published, nil) + + rec := httptest.NewRecorder() + body := `{"title":"new-lead","excerpt":"","content":"Body","comment_status":"open",` + + `"photo_url":"","breaking_news":false,"is_featured":true,"categories":["News"],` + + `"authors":[],"focus_keyword":"","meta_description":"","seo_title":""}` + PatchArticle(conn).ServeHTTP(rec, patchArticleRequest("new-lead", body)) + + if rec.Code != http.StatusNoContent { + t.Fatalf("patch status = %d, want 204; body = %s", rec.Code, rec.Body.String()) + } + slugs := featuredSlugs(t, conn) + if len(slugs) != 1 || slugs[0] != "new-lead" { + t.Fatalf("featured slugs = %v, want [new-lead]", slugs) + } +} + +// Unfeaturing must not silently re-feature anything: the homepage falls back to +// its normal newest-first lead. +func TestFeaturedArticleHTTP_PatchCanClearTheFeaturedFlag(t *testing.T) { + conn := articlePatchTestDB(t) + published := time.Now().UTC().Add(-24 * time.Hour).Format("2006-01-02 15:04:05") + seedFeaturedTestArticle(t, conn, "only-lead", true, published, nil) + + rec := httptest.NewRecorder() + body := `{"title":"only-lead","excerpt":"","content":"Body","comment_status":"open",` + + `"photo_url":"","breaking_news":false,"is_featured":false,"categories":["News"],` + + `"authors":[],"focus_keyword":"","meta_description":"","seo_title":""}` + PatchArticle(conn).ServeHTTP(rec, patchArticleRequest("only-lead", body)) + + if rec.Code != http.StatusNoContent { + t.Fatalf("patch status = %d, want 204; body = %s", rec.Code, rec.Body.String()) + } + if slugs := featuredSlugs(t, conn); len(slugs) != 0 { + t.Fatalf("featured slugs = %v, want none", slugs) + } + featured, err := db.GetFeaturedArticle(context.Background(), conn) + if err != nil { + t.Fatalf("get featured article: %v", err) + } + if featured != nil { + t.Fatalf("featured article = %q, want none", featured.Slug) + } +} + +// The flag alone must not put a headline on the homepage: an editor who +// features a draft, a scheduled story or an archived one gets the normal lead +// rather than something the public should not see. +func TestFeaturedArticle_UnpublishedRowsNeverLead(t *testing.T) { + conn := articlePatchTestDB(t) + past := time.Now().UTC().Add(-24 * time.Hour).Format("2006-01-02 15:04:05") + future := time.Now().UTC().Add(48 * time.Hour).Format("2006-01-02 15:04:05") + + seedFeaturedTestArticle(t, conn, "featured-draft", true, nil, nil) + seedFeaturedTestArticle(t, conn, "featured-scheduled", true, future, nil) + seedFeaturedTestArticle(t, conn, "featured-archived", true, past, past) + + featured, err := db.GetFeaturedArticle(context.Background(), conn) + if err != nil { + t.Fatalf("get featured article: %v", err) + } + if featured != nil { + t.Fatalf("featured article = %q, want none", featured.Slug) + } + + seedFeaturedTestArticle(t, conn, "featured-live", true, past, nil) + featured, err = db.GetFeaturedArticle(context.Background(), conn) + if err != nil { + t.Fatalf("get featured article: %v", err) + } + if featured == nil || featured.Slug != "featured-live" { + t.Fatalf("featured article = %v, want featured-live", featured) + } +} + +// The end-to-end shape editors actually care about: feature a sports story, and +// the homepage news block -- which Scalene renders news[0] of as the big centre +// card -- leads with it. +func TestFeaturedArticleHTTP_HomepageLeadsWithTheFeaturedArticle(t *testing.T) { + conn := articlePatchTestDB(t) + ctx := context.Background() + if err := db.EnsureSettingsTable(ctx, conn); err != nil { + t.Fatalf("ensure settings table: %v", err) + } + if err := db.EnsureTaxonomyTable(ctx, conn); err != nil { + t.Fatalf("ensure taxonomy table: %v", err) + } + + older := time.Now().UTC().Add(-72 * time.Hour).Format("2006-01-02 15:04:05") + newer := time.Now().UTC().Add(-1 * time.Hour).Format("2006-01-02 15:04:05") + if _, err := conn.ExecContext(ctx, + "INSERT INTO articles (title, slug, `text`, categories, priority, pub_date) VALUES "+ + "(?, ?, ?, ?, ?, ?), (?, ?, ?, ?, ?, ?)", + "Newest news", "newest-news", "Body", `["News"]`, false, newer, + "Old sports story", "old-sports-story", "Body", `["Sports"]`, false, older, + ); err != nil { + t.Fatalf("seed articles: %v", err) + } + + homepageNewsSlugs := func() []string { + rec := httptest.NewRecorder() + GetHomepage(conn).ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/v1/homepage", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("homepage status = %d, want 200; body = %s", rec.Code, rec.Body.String()) + } + // Without a stated freshness bound, a featured-article change reaches + // readers whenever some intermediary's heuristic decides it should. + if got := rec.Header().Get("Cache-Control"); got != homepageCacheControl { + t.Errorf("homepage Cache-Control = %q, want %q", got, homepageCacheControl) + } + var body struct { + News []struct { + Slug string `json:"slug"` + } `json:"news"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("decode homepage: %v", err) + } + slugs := make([]string, 0, len(body.News)) + for _, item := range body.News { + slugs = append(slugs, item.Slug) + } + return slugs + } + + // Nothing featured: the lead is just the newest news story. + if slugs := homepageNewsSlugs(); len(slugs) == 0 || slugs[0] != "newest-news" { + t.Fatalf("default news block = %v, want it to lead with newest-news", slugs) + } + + rec := httptest.NewRecorder() + body := `{"title":"Old sports story","excerpt":"","content":"Body","comment_status":"open",` + + `"photo_url":"","breaking_news":false,"is_featured":true,"categories":["Sports"],` + + `"authors":[],"focus_keyword":"","meta_description":"","seo_title":""}` + PatchArticle(conn).ServeHTTP(rec, patchArticleRequest("old-sports-story", body)) + if rec.Code != http.StatusNoContent { + t.Fatalf("patch status = %d, want 204; body = %s", rec.Code, rec.Body.String()) + } + + slugs := homepageNewsSlugs() + if len(slugs) == 0 || slugs[0] != "old-sports-story" { + t.Fatalf("featured news block = %v, want it to lead with old-sports-story", slugs) + } +} diff --git a/server/internal/handlers/featured_article_test.go b/server/internal/handlers/featured_article_test.go new file mode 100644 index 0000000..9e6b081 --- /dev/null +++ b/server/internal/handlers/featured_article_test.go @@ -0,0 +1,94 @@ +package handlers + +import ( + "testing" + + "server/internal/models" +) + +func newsBlock(ids ...int64) []models.ArticleListItem { + items := make([]models.ArticleListItem, 0, len(ids)) + for _, id := range ids { + items = append(items, models.ArticleListItem{ID: id}) + } + return items +} + +func newsIDs(items []models.ArticleListItem) []int64 { + ids := make([]int64, 0, len(items)) + for _, item := range items { + ids = append(ids, item.ID) + } + return ids +} + +func equalIDs(a, b []int64) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func TestSpliceFeaturedLead(t *testing.T) { + tests := []struct { + name string + news []int64 + featured int64 + limit int + want []int64 + }{ + { + // A featured sports story is not in the news block at all, so the + // block grows by one and the oldest card falls off the end. + name: "article from another section takes the lead slot", + news: []int64{5, 4, 3}, + featured: 99, + limit: 3, + want: []int64{99, 5, 4}, + }, + { + // The layout's big centre card is news[0]; featuring a story + // already in the block must move it, not duplicate it. + name: "news article is promoted rather than duplicated", + news: []int64{5, 4, 3}, + featured: 3, + limit: 3, + want: []int64{3, 5, 4}, + }, + { + name: "already leading stays put", + news: []int64{5, 4, 3}, + featured: 5, + limit: 3, + want: []int64{5, 4, 3}, + }, + { + name: "short block is not padded or trimmed", + news: []int64{5}, + featured: 99, + limit: 13, + want: []int64{99, 5}, + }, + { + name: "empty news block still leads with the featured article", + news: nil, + featured: 99, + limit: 13, + want: []int64{99}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := spliceFeaturedLead(newsBlock(tc.news...), models.ArticleListItem{ID: tc.featured}, tc.limit) + if !equalIDs(newsIDs(got), tc.want) { + t.Errorf("news order = %v, want %v", newsIDs(got), tc.want) + } + }) + } +} diff --git a/server/internal/handlers/handlers.go b/server/internal/handlers/handlers.go index 6a84666..01ebcab 100644 --- a/server/internal/handlers/handlers.go +++ b/server/internal/handlers/handlers.go @@ -2101,6 +2101,13 @@ func PostArticles(conn *sql.DB) http.HandlerFunc { writeError(w, http.StatusInternalServerError, err.Error()) return } + // See PatchArticle: featuring is exclusive. + if body.IsFeatured { + if err := db.ClearFeaturedExceptID(r.Context(), conn, articleID); err != nil { + writeError(w, http.StatusInternalServerError, err.Error()) + return + } + } if err := db.ReplaceArticleAuthors(r.Context(), conn, articleID, body.Authors); err != nil { writeError(w, http.StatusInternalServerError, err.Error()) return @@ -2181,6 +2188,14 @@ func PutArticle(conn *sql.DB) http.HandlerFunc { writeError(w, http.StatusNotFound, "article not found") return } + // See PatchArticle: featuring is exclusive, so the previous pick is + // cleared once this one is safely written. + if body.IsFeatured { + if err := db.ClearFeaturedExcept(r.Context(), conn, body.Slug); err != nil { + writeError(w, http.StatusInternalServerError, err.Error()) + return + } + } if err := db.ReplaceArticleAuthorsBySlug(r.Context(), conn, body.Slug, authorIDsFromOverviews(body.Authors)); err != nil { if err == sql.ErrNoRows { writeError(w, http.StatusNotFound, "article not found") @@ -2272,6 +2287,7 @@ func PatchArticle(conn *sql.DB) http.HandlerFunc { } var publishedDateValue any var scheduledDateValue any + featuring := false publishedDateSet := false var statusValue models.ArticleStatus statusSet := false @@ -2379,6 +2395,15 @@ func PatchArticle(conn *sql.DB) http.HandlerFunc { } setCols = append(setCols, column) setArgs = append(setArgs, b) + case "is_featured": + b, ok := v.(bool) + if !ok { + writeError(w, http.StatusBadRequest, "is_featured must be a boolean") + return + } + setCols = append(setCols, column) + setArgs = append(setArgs, b) + featuring = b case "slug": s, ok := v.(string) if !ok { @@ -2427,6 +2452,15 @@ func PatchArticle(conn *sql.DB) http.HandlerFunc { targetSlug = strings.TrimSpace(newSlug) } } + // Exactly one article is featured at a time: the homepage has one lead + // card, and leaving the old pick flagged would make the tiebreak, not + // the editor, decide which story runs. + if featuring { + if err := db.ClearFeaturedExcept(r.Context(), conn, targetSlug); err != nil { + writeError(w, http.StatusInternalServerError, err.Error()) + return + } + } if authorIDs != nil { if err := db.ReplaceArticleAuthorsBySlug(r.Context(), conn, targetSlug, *authorIDs); err != nil { if err == sql.ErrNoRows { @@ -2546,6 +2580,57 @@ func RestoreArticle(conn *sql.DB) http.HandlerFunc { } } +// homepageCacheControl bounds how long a featured-article change can take to +// reach readers. 60s is short enough that an editor swapping the lead sees it +// on the next reload rather than wondering whether the save worked, and long +// enough that the homepage is not re-rendered per visitor. +const homepageCacheControl = "public, max-age=60, stale-while-revalidate=300" + +// leadWithFeaturedArticle moves the featured article to the front of the +// homepage news block. It is a no-op when nothing is featured, so the default +// homepage stays newest-first. +// +// The featured article keeps its place in its own section block: a featured +// sports story leads the homepage and still appears in the sports rundown, +// which is what a lead story does in print. Only the news block dedupes, so a +// featured news story is promoted rather than duplicated. +func leadWithFeaturedArticle(r *http.Request, conn *sql.DB, homepage *models.HomepageResponse, excerptWords, newsLimit int, newsMatchSlugs []string) error { + featured, err := db.GetFeaturedArticle(r.Context(), conn) + if err != nil || featured == nil { + return err + } + + articles := []models.Article{*featured} + if err := db.PopulateArticleAuthors(r.Context(), conn, articles); err != nil { + return err + } + items := articleListItems(articles, excerptWords, newsMatchSlugs...) + if len(items) == 0 { + return nil + } + homepage.News = spliceFeaturedLead(homepage.News, items[0], newsLimit) + return nil +} + +// spliceFeaturedLead puts the featured article at the head of the news block, +// dropping the copy already in the list so a featured news story is promoted +// rather than printed twice. The list is re-trimmed to limit because splicing in +// a story from another section would otherwise push the block one card past the +// layout it was sized for. +func spliceFeaturedLead(news []models.ArticleListItem, featured models.ArticleListItem, limit int) []models.ArticleListItem { + out := make([]models.ArticleListItem, 0, len(news)+1) + out = append(out, featured) + for _, item := range news { + if item.ID != featured.ID { + out = append(out, item) + } + } + if limit > 0 && len(out) > limit { + out = out[:limit] + } + return out +} + // @Summary Get homepage data // @Tags homepage // @Produce json @@ -2606,6 +2691,12 @@ func GetHomepage(conn *sql.DB) http.HandlerFunc { DevelopingStories: developingStories, } + // Captured from the news pass so the featured article, which may come + // from any section, gets the same category ordering as the block it is + // being spliced into. + var newsMatchSlugs []string + newsLimit := 0 + for _, section := range sections { if err := func(section struct { slug string @@ -2640,6 +2731,8 @@ func GetHomepage(conn *sql.DB) http.HandlerFunc { } switch section.key { case "news": + newsMatchSlugs = matchSlugs + newsLimit = limit sectionArticles.News = articleListItems(articles, excerptWords, matchSlugs...) case "opinion": sectionArticles.Opinion = articleListItems(articles, excerptWords, matchSlugs...) @@ -2658,6 +2751,28 @@ func GetHomepage(conn *sql.DB) http.HandlerFunc { return } } + + // The homepage lead is the first entry of the news block (Scalene's + // "3-6-3" layout renders news[0] as the big centre card), so featuring + // an article means moving it to the front of that list. It is spliced in + // rather than sorted into the news query because the featured article + // may be filed under any section -- a featured sports story still takes + // the lead card, which a news-scoped ORDER BY could never do. + if offset == 0 { + if err := leadWithFeaturedArticle(r, conn, §ionArticles, excerptWords, newsLimit, newsMatchSlugs); err != nil { + writeError(w, http.StatusInternalServerError, err.Error()) + return + } + } + + // The homepage carries the editor's live decisions -- the featured lead, + // the breaking-news banner, the carousel -- so it needs a short, stated + // freshness bound. Without a Cache-Control header every intermediary + // picks its own heuristic, and Scalene's fetch cache has no expiry to + // respect at all, so "featured" could take an unbounded time to appear. + // stale-while-revalidate keeps that bound cheap: a spike is still served + // from cache while one request refreshes it behind the scenes. + w.Header().Set("Cache-Control", homepageCacheControl) writeJSON(w, http.StatusOK, sectionArticles) } }