From 5808732badd15c8042fd13fc3a4ffe51b7ae502f Mon Sep 17 00:00:00 2001 From: ssavutu Date: Thu, 3 Sep 2026 22:43:32 -0400 Subject: [PATCH 1/2] Let the taxonomy table be the only list of sections Three places kept their own copy of the section tree, each frozen at whatever the desk had configured on the day it was written. article_params.go held a section -> subsection map used to validate ?section_slug= and ?subsection_slug= whenever there was no database handle. It had drifted: it knew 41 subsections where the live tree has 93, and it had no special-editions at all. It only ran in tests, which is worse than it sounds -- the tests were asserting against a fiction, so a subsection an editor added resolved in production and failed here. Without a handle there is now simply no taxonomy, and every slug is unknown. The two behaviours that map really pinned, a subsection resolving to its root section and a subsection that contradicts the named section staying a 400, move to taxonomy_integration_test.go where a real table can answer. The sections screen ordered sections by a list of seven slugs; anything else was alphabetised onto the end. It now orders by id, the order the sections were created in, which is what the articles screen's own filter already did -- so the two screens agree and a new section appears in both. New rows take MAX(id)+1, so they land at the end exactly as the unlisted ones used to. Live, this is a no-op: sections are ids 1-7 in precisely the order the list gave, and Graduation (id 37) sorted last as an unlisted slug and sorts last by id too. The dashboard's section count fell back to counting homepage blocks against a list of six keys, so it answered "6" however many sections existed; there are eight. It keeps its placeholder instead when taxonomy is unavailable, rather than showing a wrong number that looks like a real one. Also drops the typeLabel special case that printed "Column" for anything parented by the literal slug "columns". The homepage handler's slug/key/limit table is deliberately left alone: those JSON keys are the contract Scalene reads, not a filter. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01L4qBhBdQto1yNp1zP7VLYc --- frontend/src/pages/DashboardPage.tsx | 30 ++-- frontend/src/pages/sectionsView.tsx | 30 +--- server/internal/handlers/article_params.go | 130 +++++------------- .../internal/handlers/article_params_test.go | 51 +------ .../handlers/taxonomy_integration_test.go | 60 ++++++++ 5 files changed, 117 insertions(+), 184 deletions(-) diff --git a/frontend/src/pages/DashboardPage.tsx b/frontend/src/pages/DashboardPage.tsx index 2a72e67..076dca5 100644 --- a/frontend/src/pages/DashboardPage.tsx +++ b/frontend/src/pages/DashboardPage.tsx @@ -161,34 +161,22 @@ export default function DashboardPage() { if (!fallback.ok) throw new Error("taxonomy unavailable") return fallback.json() }) - .then(async (d) => { - let totalSections = Array.isArray(d) + // site_taxonomy is the only place that knows how many sections there are. + // This used to fall back to counting the homepage blocks against a list of + // six keys written here, which answered "6" however many sections the desk + // had actually configured -- a wrong number that looked like a real one. + // The tile keeps its placeholder instead when taxonomy is unavailable. + .then((d) => { + const totalSections = Array.isArray(d) ? d.filter((item) => item?.type === "section").length || d.length : null - if (totalSections == null || totalSections === 0) { - const homepageRes = await apiFetch("/v1/homepage") - if (homepageRes.ok) { - const homepage = await homepageRes.json() - const sectionKeys = ["news", "opinion", "sports", "entertainment", "candp", "columns"] - totalSections = sectionKeys.filter((key) => Array.isArray(homepage?.[key])).length - } - } setStats((s) => ({ ...s, totalSections, })) }) - .catch(async () => { - try { - const homepageRes = await apiFetch("/v1/homepage") - if (!homepageRes.ok) return - const homepage = await homepageRes.json() - const sectionKeys = ["news", "opinion", "sports", "entertainment", "candp", "columns"] - const totalSections = sectionKeys.filter((key) => Array.isArray(homepage?.[key])).length - setStats((s) => ({ ...s, totalSections })) - } catch { - // Keep placeholder when both sources are unavailable. - } + .catch(() => { + // Keep the placeholder rather than guessing. }) apiFetch("/v1/articles?limit=1") diff --git a/frontend/src/pages/sectionsView.tsx b/frontend/src/pages/sectionsView.tsx index a12b8fd..13d27a1 100644 --- a/frontend/src/pages/sectionsView.tsx +++ b/frontend/src/pages/sectionsView.tsx @@ -49,16 +49,6 @@ type EditorState = | { mode: "create"; label: string } | { mode: "edit"; item: TaxonomyItem; label: string } -const SECTION_ORDER = [ - "news", - "sports", - "opinion", - "columns", - "entertainment", - "comics-puzzles", - "special-editions", -] - const emptyForm: FormState = { type: "section", canonicalTitle: "", @@ -96,9 +86,7 @@ async function readErrorMessage(response: Response) { } function typeLabel(item: TaxonomyItem) { - if (item.type === "section") return "Section" - if (item.parent_slug === "columns") return "Column" - return "Subsection" + return item.type === "section" ? "Section" : "Subsection" } export default function SectionsView() { @@ -144,16 +132,12 @@ export default function SectionsView() { const parentSections = useMemo(() => { return items .filter((item) => item.type === "section") - .sort((left, right) => { - const leftIndex = SECTION_ORDER.indexOf(left.slug) - const rightIndex = SECTION_ORDER.indexOf(right.slug) - if (leftIndex !== -1 || rightIndex !== -1) { - if (leftIndex === -1) return 1 - if (rightIndex === -1) return -1 - return leftIndex - rightIndex - } - return left.canonical_title.localeCompare(right.canonical_title) - }) + // By id, which is the order the sections were created in and so the + // running order the desk gave them. The articles screen orders its + // section filter the same way, so the two screens agree, and a section + // added today lands at the end of both instead of being invisible to a + // list of slugs frozen in this file. + .sort((left, right) => left.id - right.id) }, [items]) const childrenByParent = useMemo(() => { diff --git a/server/internal/handlers/article_params.go b/server/internal/handlers/article_params.go index 7929908..c5f3be0 100644 --- a/server/internal/handlers/article_params.go +++ b/server/internal/handlers/article_params.go @@ -32,63 +32,6 @@ func articleParamsStatus(err, pathParamErr error) int { return http.StatusBadRequest } -var allowedSubsectionsBySection = map[string]map[string]struct{}{ - "news": { - "academic-transformation": {}, - "transit": {}, - "public-safety": {}, - "campus": {}, - "city": {}, - "national": {}, - "world": {}, - }, - "sports": { - "mens-basketball": {}, - "womens-basketball": {}, - "big-5": {}, - "philly-sports": {}, - "field-hockey": {}, - "mens-soccer": {}, - "womens-soccer": {}, - "nil": {}, - "squash": {}, - }, - "opinion": { - "science-tech": {}, - "from-the-editor": {}, - "politics": {}, - "lifestyle": {}, - }, - "columns": { - "the-love-triangle": {}, - "tri-this-sweet-treat": {}, - "from-the-playbook": {}, - "jack-of-all-takes": {}, - "the-green-angle": {}, - "the-overall-score": {}, - }, - "entertainment": { - "movies": {}, - "music": {}, - "happening-in-philly": {}, - "cooking": {}, - "books": {}, - "gaming": {}, - "listicles": {}, - "performing-arts": {}, - "the-drawing-board": {}, - }, - "comics-puzzles": { - "political-cartoons": {}, - "crossword": {}, - "sudoku": {}, - "comics": {}, - "puzzles": {}, - "satire": {}, - }, - "graduation": {}, -} - func normalizeAndValidateArticleParams(ctx context.Context, conn *sql.DB, params ArticleParams) (ArticleParams, error) { params.AuthorSlug = strings.TrimSpace(params.AuthorSlug) params.AuthorSearch = strings.TrimSpace(params.AuthorSearch) @@ -150,10 +93,13 @@ func normalizeSectionSlug(value string) string { return strings.ToLower(strings.TrimSpace(value)) } +// site_taxonomy is the only list of what a section is. Without a handle there +// is nothing to check against, so every slug is unknown: a hard-coded mirror of +// the tree here would answer for the seven sections it was written with and +// deny the ones an editor has added since. func taxonomySectionExists(ctx context.Context, conn *sql.DB, section string) (bool, error) { if conn == nil { - _, ok := allowedSubsectionsBySection[section] - return ok, nil + return false, nil } var exists int @@ -168,9 +114,8 @@ func taxonomySectionExists(ctx context.Context, conn *sql.DB, section string) (b } // sectionMatchSlugs returns the section plus every subsection below it, at any -// depth, which together define what "articles in this section" means. Falls back -// to the section alone when there is no database handle, matching the behaviour -// of the other taxonomy lookups here. +// depth, which together define what "articles in this section" means. Without a +// database handle there are no subsections to find, so the section stands alone. // // Transitive because the tree is three levels: A&E holds Food, and Food holds // Beer Reviews. One hop would have listed Food's own articles under A&E while @@ -181,11 +126,7 @@ func sectionMatchSlugs(ctx context.Context, conn *sql.DB, section string) ([]str return nil, nil } if conn == nil { - slugs := []string{trimmed} - for subsection := range allowedSubsectionsBySection[trimmed] { - slugs = append(slugs, subsection) - } - return slugs, nil + return []string{trimmed}, nil } return db.TaxonomyDescendants(ctx, conn, trimmed) } @@ -197,37 +138,34 @@ func sectionMatchSlugs(ctx context.Context, conn *sql.DB, section string) ([]str // ?subsection_slug= agrees with ?section_slug=, and the caller names a section. // Returning "food" for beer-reviews would fail that check against the only // section a reader could have arrived from. +// +// Without a handle nothing resolves, for the reason taxonomySectionExists gives. func rootSectionForSubsection(ctx context.Context, conn *sql.DB, subsection string) (string, bool, error) { - if conn != nil { - var parent sql.NullString - err := conn.QueryRowContext(ctx, - "SELECT parent_slug FROM site_taxonomy WHERE kind = ? AND slug = ? LIMIT 1", - string(models.TaxonomyTypeSubsection), subsection, - ).Scan(&parent) - if err == sql.ErrNoRows { - return "", false, nil - } - if err != nil { - return "", false, err - } - if !parent.Valid || strings.TrimSpace(parent.String) == "" { - return "", false, nil - } - ancestors, err := db.TaxonomyAncestors(ctx, conn, subsection) - if err != nil { - return "", false, err - } - if len(ancestors) == 0 { - return "", false, nil - } - // Nearest first, so the last entry is the top of the chain. - return ancestors[len(ancestors)-1], true, nil + if conn == nil { + return "", false, nil } - for section, subsections := range allowedSubsectionsBySection { - if _, ok := subsections[subsection]; ok { - return section, true, nil - } + var parent sql.NullString + err := conn.QueryRowContext(ctx, + "SELECT parent_slug FROM site_taxonomy WHERE kind = ? AND slug = ? LIMIT 1", + string(models.TaxonomyTypeSubsection), subsection, + ).Scan(&parent) + if err == sql.ErrNoRows { + return "", false, nil + } + if err != nil { + return "", false, err + } + if !parent.Valid || strings.TrimSpace(parent.String) == "" { + return "", false, nil + } + ancestors, err := db.TaxonomyAncestors(ctx, conn, subsection) + if err != nil { + return "", false, err + } + if len(ancestors) == 0 { + return "", false, nil } - return "", false, nil + // Nearest first, so the last entry is the top of the chain. + return ancestors[len(ancestors)-1], true, nil } diff --git a/server/internal/handlers/article_params_test.go b/server/internal/handlers/article_params_test.go index a181f95..c272778 100644 --- a/server/internal/handlers/article_params_test.go +++ b/server/internal/handlers/article_params_test.go @@ -26,9 +26,13 @@ func TestNormalizeSectionSlug(t *testing.T) { } } +// site_taxonomy is the only definition of a section, so with no handle to it +// nothing resolves. The section and subsection tree itself is exercised against +// a real database in taxonomy_integration_test.go; these cases only pin the +// no-taxonomy contract and the status-code mapping, which need no rows. func TestNormalizeAndValidateArticleParams_RejectsUnknownSectionSlug(t *testing.T) { _, err := normalizeAndValidateArticleParams(context.Background(), nil, ArticleParams{ - Section: "candp", + Section: "sports", }) if err == nil { t.Fatal("expected error for unknown section_slug") @@ -47,21 +51,6 @@ func TestNormalizeAndValidateArticleParams_RejectsUnknownSubsectionSlug(t *testi } } -// A subsection that exists but sits under a different parent is a genuinely -// contradictory request, so it must stay a 400 rather than becoming a 404. -func TestArticleParamsStatus_MismatchedParentStaysBadRequest(t *testing.T) { - _, err := normalizeAndValidateArticleParams(context.Background(), nil, ArticleParams{ - Section: "sports", - Subsection: "the-love-triangle", - }) - if err == nil { - t.Fatal("expected error for subsection outside the named section") - } - if got := articleParamsStatus(err, errSubsectionNotFound); got != http.StatusBadRequest { - t.Fatalf("status = %d, want %d", got, http.StatusBadRequest) - } -} - func TestArticleParamsStatus_OnlyPathParamDegradesToNotFound(t *testing.T) { cases := []struct { name string @@ -85,8 +74,8 @@ func TestArticleParamsStatus_OnlyPathParamDegradesToNotFound(t *testing.T) { } } -// The handlers validate against the static taxonomy fallback when there is no -// database handle, so an unknown slug is answered before anything is queried. +// With no database handle no slug resolves, so the handlers answer before +// anything is queried. func TestSectionArticlesHandlers_UnknownPathSlugReturnsNotFound(t *testing.T) { cases := []struct { name string @@ -125,29 +114,3 @@ func TestSectionArticlesHandlers_UnknownPathSlugReturnsNotFound(t *testing.T) { }) } } - -// The section_slug/subsection_slug query filters are caller-chosen filters -// rather than the addressed resource, so they keep answering 400. -func TestGetSectionArticles_UnknownSubsectionFilterStaysBadRequest(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/v1/sections/sports/articles?subsection_slug=not-a-subsection", nil) - req.SetPathValue("section_slug", "sports") - rec := httptest.NewRecorder() - - GetSectionArticles(nil)(rec, req) - - if rec.Code != http.StatusBadRequest { - t.Fatalf("status = %d, want %d (body %s)", rec.Code, http.StatusBadRequest, rec.Body.String()) - } -} - -func TestNormalizeAndValidateArticleParams_FallbackAllowsKnownColumn(t *testing.T) { - got, err := normalizeAndValidateArticleParams(context.Background(), nil, ArticleParams{ - Subsection: "the-love-triangle", - }) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if got.Section != "columns" { - t.Fatalf("section = %q, want columns", got.Section) - } -} diff --git a/server/internal/handlers/taxonomy_integration_test.go b/server/internal/handlers/taxonomy_integration_test.go index d36f563..29c05b1 100644 --- a/server/internal/handlers/taxonomy_integration_test.go +++ b/server/internal/handlers/taxonomy_integration_test.go @@ -819,6 +819,66 @@ func TestTaxonomyHTTPThreeLevelNesting(t *testing.T) { } } +// TestTaxonomyHTTPArticleFiltersResolveFromTheTable is what replaced the +// hard-coded section/subsection table that used to back these checks when there +// was no database handle. That mirror was frozen at the seven sections it was +// written with, so a subsection an editor added resolved in production and was +// rejected in the tests, and the tests still passed. site_taxonomy is now the +// only source, which is why this has to be an integration test. +func TestTaxonomyHTTPArticleFiltersResolveFromTheTable(t *testing.T) { + conn := taxonomyHTTPTestDB(t) + ctx := context.Background() + + create := func(kind, slug, title string, parent any) { + t.Helper() + recorder := taxonomyRequest(t, PostTaxonomy(conn), http.MethodPost, "/v1/taxonomy", map[string]any{ + "type": kind, + "slug": slug, + "canonical_title": title, + "parent_slug": parent, + }) + if recorder.Code != http.StatusCreated { + t.Fatalf("POST %s = %d: %s", slug, recorder.Code, recorder.Body.String()) + } + } + + create("section", "sports", "Sports", nil) + create("section", "columns", "Columns", nil) + create("subsection", "the-love-triangle", "The Love Triangle", "columns") + + // A subsection resolves to the section that actually holds it. + got, err := normalizeAndValidateArticleParams(ctx, conn, ArticleParams{Subsection: "the-love-triangle"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.Section != "columns" { + t.Errorf("section = %q, want columns", got.Section) + } + + // A subsection that exists but sits under a different parent is a + // contradictory request, not a missing one, so it stays a 400. + _, err = normalizeAndValidateArticleParams(ctx, conn, ArticleParams{ + Section: "sports", + Subsection: "the-love-triangle", + }) + if err == nil { + t.Fatal("expected an error for a subsection outside the named section") + } + if status := articleParamsStatus(err, errSubsectionNotFound); status != http.StatusBadRequest { + t.Errorf("status = %d, want %d", status, http.StatusBadRequest) + } + + // The same distinction over HTTP: the section in the path exists, so the + // unknown subsection is a bad filter rather than a missing resource. + req := httptest.NewRequest(http.MethodGet, "/v1/sections/sports/articles?subsection_slug=not-a-subsection", nil) + req.SetPathValue("section_slug", "sports") + rec := httptest.NewRecorder() + GetSectionArticles(conn)(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("GET with an unknown subsection filter = %d, want %d: %s", rec.Code, http.StatusBadRequest, rec.Body.String()) + } +} + // TestTaxonomyHTTPRejectsCircularParent covers the other way a walk could fail // to terminate: making an ancestor into a descendant. func TestTaxonomyHTTPRejectsCircularParent(t *testing.T) { From 6dee5a05201d29486a0a15589748a7502d914c61 Mon Sep 17 00:00:00 2001 From: ssavutu Date: Thu, 3 Sep 2026 22:44:01 -0400 Subject: [PATCH 2/2] Filter the articles list by breaking and by featured Both flags are set from the article form and shown as badges in the listing, but there was no way to ask the listing for them: finding what is still flagged as breaking, or what is currently pinned, meant paging through everything and reading badges. GET /v1/articles takes ?breaking= and ?featured=, and the articles screen gets a three-state control for each. Three-state because "not breaking" has to be reachable: an editor clearing a false alarm wants the stories that are NOT flagged, which is not the same request as not filtering. The off-side condition is COALESCE(col, 0) = 0 rather than col = 0. Both columns are nullable, so any row holding NULL rather than 0 would be dropped from the "off" half by the plain comparison -- and the WordPress archive is nine thousand of the ten thousand rows. Both columns scan into sql.NullBool, so live values can only be NULL, 0 or 1, which is what makes col = 1 right for the on-side. An unparseable value is treated as absent rather than guessed at, so a typo widens the listing instead of narrowing it to the wrong half. priority is the featured column; is_featured is the name it goes by over the wire, and GetFeaturedArticles already reads it the same way. There is no index on it, but every listing already scans -- the artifact filter is not sargable -- and the homepage already runs an unindexed priority = 1 on every load. The listing also gains a Featured badge next to the Breaking one, so a filtered list says why each row is in it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01L4qBhBdQto1yNp1zP7VLYc --- frontend/src/pages/articleView.tsx | 56 ++++++- server/docs/docs.go | 12 ++ server/docs/swagger.json | 12 ++ server/docs/swagger.yaml | 10 ++ .../article_flag_filters_integration_test.go | 149 ++++++++++++++++++ server/internal/handlers/handlers.go | 45 ++++++ 6 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 server/internal/handlers/article_flag_filters_integration_test.go diff --git a/frontend/src/pages/articleView.tsx b/frontend/src/pages/articleView.tsx index 2e986ff..cd541ea 100644 --- a/frontend/src/pages/articleView.tsx +++ b/frontend/src/pages/articleView.tsx @@ -17,6 +17,7 @@ type ArticleItem = { slug?: string featuredImage?: string breakingNews: boolean + isFeatured: boolean } type ApiArticle = { @@ -28,6 +29,7 @@ type ApiArticle = { creation_date?: string featured_image?: string breaking_news?: boolean + is_featured?: boolean authors?: Array<{ name?: string }> @@ -97,6 +99,12 @@ type ArticleViewProps = { // means send no status filter at all. type PublishedFilter = "all" | "published" | "draft" | "scheduled" +// A flag filter is three-state: "all" sends no parameter at all, the other two +// send breaking/featured explicitly. "Not breaking" has to be a real choice +// rather than the absence of one -- an editor clearing a false alarm wants the +// stories that are NOT flagged. +type FlagFilter = "all" | "on" | "off" + type ArticleViewUIState = { searchQuery?: string activeTab?: "all" | "trash" @@ -104,10 +112,14 @@ type ArticleViewUIState = { sectionFilterSlug?: string subsectionFilterSlug?: string publishedFilter?: PublishedFilter + breakingFilter?: FlagFilter + featuredFilter?: FlagFilter dateSortDirection?: "asc" | "desc" pageSize?: number } +const flagFilterParam = (filter: FlagFilter) => (filter === "on" ? "true" : "false") + type ArticleResultsCacheEntry = { items: ArticleItem[] totalArticleCount: number @@ -154,6 +166,8 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article const [subsections, setSubsections] = useState([]) const [subsectionFilterSlug, setSubsectionFilterSlug] = useState(() => loadUIState().subsectionFilterSlug ?? "") const [publishedFilter, setPublishedFilter] = useState(() => loadUIState().publishedFilter ?? "all") + const [breakingFilter, setBreakingFilter] = useState(() => loadUIState().breakingFilter ?? "all") + const [featuredFilter, setFeaturedFilter] = useState(() => loadUIState().featuredFilter ?? "all") const [dateSortDirection, setDateSortDirection] = useState<"asc" | "desc">(() => loadUIState().dateSortDirection ?? "desc") const [isLoading, setIsLoading] = useState(true) const [error, setError] = useState(null) @@ -169,13 +183,17 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article sectionFilterSlug, subsectionFilterSlug, publishedFilter, + breakingFilter, + featuredFilter, dateSortDirection, pageSize, } satisfies ArticleViewUIState) }, [ activeTab, authorQuery, + breakingFilter, dateSortDirection, + featuredFilter, pageSize, publishedFilter, searchQuery, @@ -390,6 +408,12 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article if (subsectionFilterSlug) { params.set("subsection_slug", subsectionFilterSlug) } + if (breakingFilter !== "all") { + params.set("breaking", flagFilterParam(breakingFilter)) + } + if (featuredFilter !== "all") { + params.set("featured", flagFilterParam(featuredFilter)) + } if (fixedType) { params.set("type", fixedType) } @@ -432,6 +456,7 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article slug: item.slug, featuredImage: item.featured_image, breakingNews: Boolean(item.breaking_news), + isFeatured: Boolean(item.is_featured), })) if (!cancelled) { @@ -469,7 +494,7 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article return () => { cancelled = true } - }, [activeTab, apiFetch, authorQuery, dateSortDirection, excludeType, fixedType, page, pageSize, publishedFilter, resultsCacheKey, searchQuery, sectionFilterSlug, selectedAuthorSlug, subsectionFilterSlug]) + }, [activeTab, apiFetch, authorQuery, breakingFilter, dateSortDirection, excludeType, featuredFilter, fixedType, page, pageSize, publishedFilter, resultsCacheKey, searchQuery, sectionFilterSlug, selectedAuthorSlug, subsectionFilterSlug]) const onChangeTab = (tab: "all" | "trash") => { setActiveTab(tab) @@ -483,7 +508,7 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article useEffect(() => { setPage(0) - }, [authorQuery, publishedFilter, dateSortDirection, searchQuery, sectionFilterSlug, subsectionFilterSlug, pageSize]) + }, [authorQuery, publishedFilter, breakingFilter, featuredFilter, dateSortDirection, searchQuery, sectionFilterSlug, subsectionFilterSlug, pageSize]) const effectiveTotalCount = Math.max(totalArticleCount, (page * pageSize) + articles.length) const totalPages = Math.max(1, Math.ceil(effectiveTotalCount / pageSize)) @@ -751,6 +776,28 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article )} + + {/* Breaking and Featured are the two flags an editor sets from the + article form, and the two they need to audit: which stories are + still flagged as breaking, and what is currently pinned. Both are + three-state, so "not flagged" is reachable rather than implied. */} +
+ Breaking +
+ + + +
+
+ +
+ Featured +
+ + + +
+
{/* Tabs */} @@ -842,6 +889,11 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article Breaking ) : null} + {item.isFeatured ? ( + + Featured + + ) : null} {item.authors || "-"} diff --git a/server/docs/docs.go b/server/docs/docs.go index 5925717..0d85cf0 100644 --- a/server/docs/docs.go +++ b/server/docs/docs.go @@ -142,6 +142,18 @@ const docTemplate = `{ "name": "slug", "in": "query" }, + { + "type": "boolean", + "description": "When true, return only articles flagged as breaking news; when false, only those not flagged.", + "name": "breaking", + "in": "query" + }, + { + "type": "boolean", + "description": "When true, return only featured (pinned) articles; when false, only those not featured.", + "name": "featured", + "in": "query" + }, { "enum": [ "title", diff --git a/server/docs/swagger.json b/server/docs/swagger.json index cb88e26..f835a1c 100644 --- a/server/docs/swagger.json +++ b/server/docs/swagger.json @@ -139,6 +139,18 @@ "name": "slug", "in": "query" }, + { + "type": "boolean", + "description": "When true, return only articles flagged as breaking news; when false, only those not flagged.", + "name": "breaking", + "in": "query" + }, + { + "type": "boolean", + "description": "When true, return only featured (pinned) articles; when false, only those not featured.", + "name": "featured", + "in": "query" + }, { "enum": [ "title", diff --git a/server/docs/swagger.yaml b/server/docs/swagger.yaml index 3dedc7e..42fb8a2 100644 --- a/server/docs/swagger.yaml +++ b/server/docs/swagger.yaml @@ -1362,6 +1362,16 @@ paths: in: query name: slug type: string + - description: When true, return only articles flagged as breaking news; when + false, only those not flagged. + in: query + name: breaking + type: boolean + - description: When true, return only featured (pinned) articles; when false, + only those not featured. + in: query + name: featured + type: boolean - description: Sort field enum: - title diff --git a/server/internal/handlers/article_flag_filters_integration_test.go b/server/internal/handlers/article_flag_filters_integration_test.go new file mode 100644 index 0000000..ec85a2e --- /dev/null +++ b/server/internal/handlers/article_flag_filters_integration_test.go @@ -0,0 +1,149 @@ +package handlers + +import ( + "context" + "database/sql" + "sort" + "testing" + "time" + + _ "github.com/go-sql-driver/mysql" + + db "server/internal/database" +) + +// The Articles screen offers Breaking and Featured as filters, so both have to +// narrow the listing on their own and in combination with the section filter +// the editor may already have set. +// +// The awkward part is the "off" half. Both columns are nullable and the +// WordPress archive leaves them NULL rather than 0, so `breaking_news` = 0 +// matches none of it; a filter written that way would answer "not breaking" +// with only the handful of rows the CMS itself has written. +func seedFlagFilterArticles(t *testing.T, conn *sql.DB) { + t.Helper() + ctx := context.Background() + past := time.Now().UTC().Add(-48 * time.Hour).Format("2006-01-02 15:04:05") + + rows := []struct { + title string + slug string + categories string + breaking any + priority any + }{ + {"Breaking campus story", "breaking-campus-story", `["News"]`, 1, 0}, + {"Breaking sports story", "breaking-sports-story", `["Sports"]`, 1, 1}, + {"Featured campus story", "featured-campus-story", `["News"]`, 0, 1}, + {"Ordinary campus story", "ordinary-campus-story", `["News"]`, 0, 0}, + // The archive shape: neither flag ever written. + {"Imported campus story", "imported-campus-story", `["News"]`, nil, nil}, + } + for _, row := range rows { + if _, err := conn.ExecContext(ctx, + "INSERT INTO articles (title, slug, `text`, authors, categories, pub_date, breaking_news, priority, creation_date) VALUES (?, ?, ?, ?, ?, ?, ?, ?, UTC_TIMESTAMP())", + row.title, row.slug, "Body", `["Rui Zhao"]`, row.categories, past, row.breaking, row.priority, + ); err != nil { + t.Fatalf("seed %s: %v", row.slug, err) + } + } + + // One case stacks the flag filter on ?section_slug=, which needs a real + // section row to validate against and the category index the listing + // actually matches on. Raw INSERTs bypass both. + // + // The harness creates site_taxonomy but does not drop it, so rows survive + // from whichever test ran before this one in the package. Clearing first is + // what makes the tree here exactly the tree this test describes -- and a + // literal id would otherwise collide with a leftover row, which passes when + // this test runs alone and fails in the suite. + if _, err := conn.ExecContext(ctx, "DELETE FROM site_taxonomy"); err != nil { + t.Fatalf("clear site_taxonomy: %v", err) + } + if _, err := conn.ExecContext(ctx, + "INSERT INTO site_taxonomy (id, kind, slug, canonical_title, parent_slug) VALUES (1, 'section', 'news', 'News', NULL)", + ); err != nil { + t.Fatalf("seed the news section: %v", err) + } + // The alias cache is package-level state that outlives a single test, so it + // has to be reloaded from the table this test just rewrote. + if err := db.RefreshCategoryAliases(ctx, conn); err != nil { + t.Fatalf("refresh category aliases: %v", err) + } + if err := db.RebuildArticleCategories(ctx, conn); err != nil { + t.Fatalf("rebuild article categories: %v", err) + } +} + +func TestArticleFlagFiltersHTTP(t *testing.T) { + conn := articlePatchTestDB(t) + seedFlagFilterArticles(t, conn) + + for _, tc := range []struct { + name string + query string + want []string + }{ + { + name: "breaking only", + query: "?breaking=true", + want: []string{"breaking-campus-story", "breaking-sports-story"}, + }, + { + // The NULL row has to be here, or "not breaking" hides the archive. + name: "not breaking", + query: "?breaking=false", + want: []string{"featured-campus-story", "imported-campus-story", "ordinary-campus-story"}, + }, + { + name: "featured only", + query: "?featured=true", + want: []string{"breaking-sports-story", "featured-campus-story"}, + }, + { + name: "not featured", + query: "?featured=false", + want: []string{"breaking-campus-story", "imported-campus-story", "ordinary-campus-story"}, + }, + { + // Both at once: the article that is breaking AND pinned. + name: "breaking and featured", + query: "?breaking=true&featured=true", + want: []string{"breaking-sports-story"}, + }, + { + // And stacked on the section filter the editor already had set. + name: "breaking within a section", + query: "?breaking=true§ion_slug=news", + want: []string{"breaking-campus-story"}, + }, + { + // An unparseable value does not filter, rather than guessing a half. + name: "unparseable value does not filter", + query: "?breaking=maybe", + want: []string{ + "breaking-campus-story", "breaking-sports-story", "featured-campus-story", + "imported-campus-story", "ordinary-campus-story", + }, + }, + { + // Bare ?breaking is how a form spells true. + name: "bare param means true", + query: "?breaking", + want: []string{"breaking-campus-story", "breaking-sports-story"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + got := slugsOf(listArticlesAsEditor(t, conn, tc.query)) + sort.Strings(got) + if len(got) != len(tc.want) { + t.Fatalf("%s returned %v, want %v", tc.query, got, tc.want) + } + for i, slug := range tc.want { + if got[i] != slug { + t.Fatalf("%s returned %v, want %v", tc.query, got, tc.want) + } + } + }) + } +} diff --git a/server/internal/handlers/handlers.go b/server/internal/handlers/handlers.go index 097eb81..9de4367 100644 --- a/server/internal/handlers/handlers.go +++ b/server/internal/handlers/handlers.go @@ -1260,6 +1260,8 @@ func GetAuthorArticles(conn *sql.DB) http.HandlerFunc { // @Param archived query bool false "When true, return only soft-deleted articles. Ignored for unauthenticated callers." // @Param title query string false "Filter by title (partial match)" // @Param slug query string false "Filter by slug (partial match)" +// @Param breaking query bool false "When true, return only articles flagged as breaking news; when false, only those not flagged." +// @Param featured query bool false "When true, return only featured (pinned) articles; when false, only those not featured." // @Param sort_by query string false "Sort field" Enums(title,slug,creation_date,published_date,status,comment_status) // @Param sort_direction query string false "Sort direction" Enums(asc,desc) // @Success 200 {object} models.ArticlesResponse @@ -1706,9 +1708,52 @@ func articleQueryFilters(r *http.Request, params ArticleParams) ([]string, []any args = append(args, "%"+slug+"%") } + // Both columns are nullable and default to 0, and rows imported from + // WordPress leave them NULL rather than 0. COALESCE on the "off" side is + // what keeps ?breaking=false from hiding the whole archive, since + // `breaking_news` = 0 is never true for NULL. + if breaking, ok := parseBoolFilter(q, "breaking"); ok { + if breaking { + conditions = append(conditions, "`breaking_news` = 1") + } else { + conditions = append(conditions, "COALESCE(`breaking_news`, 0) = 0") + } + } + + // `priority` is the featured/pinned flag; is_featured is the name it goes by + // over the wire. GetFeaturedArticles reads the same column. + if featured, ok := parseBoolFilter(q, "featured"); ok { + if featured { + conditions = append(conditions, "`priority` = 1") + } else { + conditions = append(conditions, "COALESCE(`priority`, 0) = 0") + } + } + return conditions, args } +// parseBoolFilter reads an optional boolean query param. The second result says +// whether the caller asked at all, so an absent param and an explicit false stay +// distinguishable: absent means "do not filter", false means "only the ones that +// are off". A value that parses as neither is treated as absent rather than +// guessed at, so a typo widens the listing instead of silently narrowing it to +// the wrong half. +func parseBoolFilter(q url.Values, key string) (bool, bool) { + if _, provided := q[key]; !provided { + return false, false + } + switch strings.ToLower(strings.TrimSpace(q.Get(key))) { + case "", "1", "true", "yes": + // Bare ?breaking is the HTML-form spelling of true. + return true, true + case "0", "false", "no": + return false, true + default: + return false, false + } +} + // appendCategorySlugCondition narrows to articles filed under any of the given // slugs. Patterns come from db.CategoryMatchPatterns so the listing and the // taxonomy counts can never drift apart.