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/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/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/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/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/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. 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) {