Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 66 additions & 38 deletions frontend/src/pages/articleView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,27 +99,22 @@ 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"
authorQuery?: string
sectionFilterSlug?: string
subsectionFilterSlug?: string
publishedFilter?: PublishedFilter
breakingFilter?: FlagFilter
featuredFilter?: FlagFilter
// Each narrows to the flagged articles when on and does not filter at all
// when off, so neither ever asks for the unflagged half. The endpoint still
// accepts breaking=false/featured=false; nothing here sends it.
breakingOnly?: boolean
featuredOnly?: boolean
dateSortDirection?: "asc" | "desc"
pageSize?: number
}

const flagFilterParam = (filter: FlagFilter) => (filter === "on" ? "true" : "false")

type ArticleResultsCacheEntry = {
items: ArticleItem[]
totalArticleCount: number
Expand All @@ -141,6 +136,48 @@ const writeSessionJSON = (key: string, value: unknown) => {
window.sessionStorage.setItem(key, JSON.stringify(value))
}

// A switch rather than a checkbox because these read as "on/off", not as items
// ticked off a list. Hand-rolled: there is no switch in components/ui and no
// @radix-ui/react-switch in the tree, and pulling one in for two toggles would
// be more code than this.
//
// role="switch" with aria-checked is what makes it announce as a switch; the
// track and knob are presentational, so they are spans inside the button rather
// than focusable elements of their own.
function FilterSwitch({
checked,
label,
onChange,
}: {
checked: boolean
label: string
onChange: (next: boolean) => void
}) {
return (
<button
aria-checked={checked}
className="group flex items-center gap-2 text-sm text-foreground cursor-pointer rounded-md focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary/40"
onClick={() => onChange(!checked)}
role="switch"
type="button"
>
<span
aria-hidden="true"
className={`relative inline-flex h-5 w-9 shrink-0 items-center rounded-full border transition-colors ${
checked ? "bg-primary border-primary" : "bg-muted border-border group-hover:border-primary"
}`}
>
<span
className={`inline-block h-3.5 w-3.5 rounded-full bg-background shadow-sm transition-transform ${
checked ? "translate-x-[17px]" : "translate-x-[3px]"
}`}
/>
</span>
{label}
</button>
)
}

function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: ArticleViewProps) {
const navigate = useNavigate()
const apiFetch = useApiFetch()
Expand All @@ -166,8 +203,8 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article
const [subsections, setSubsections] = useState<ApiTaxonomyItem[]>([])
const [subsectionFilterSlug, setSubsectionFilterSlug] = useState(() => loadUIState().subsectionFilterSlug ?? "")
const [publishedFilter, setPublishedFilter] = useState<PublishedFilter>(() => loadUIState().publishedFilter ?? "all")
const [breakingFilter, setBreakingFilter] = useState<FlagFilter>(() => loadUIState().breakingFilter ?? "all")
const [featuredFilter, setFeaturedFilter] = useState<FlagFilter>(() => loadUIState().featuredFilter ?? "all")
const [breakingOnly, setBreakingOnly] = useState(() => loadUIState().breakingOnly ?? false)
const [featuredOnly, setFeaturedOnly] = useState(() => loadUIState().featuredOnly ?? false)
const [dateSortDirection, setDateSortDirection] = useState<"asc" | "desc">(() => loadUIState().dateSortDirection ?? "desc")
const [isLoading, setIsLoading] = useState(true)
const [error, setError] = useState<string | null>(null)
Expand All @@ -183,17 +220,17 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article
sectionFilterSlug,
subsectionFilterSlug,
publishedFilter,
breakingFilter,
featuredFilter,
breakingOnly,
featuredOnly,
dateSortDirection,
pageSize,
} satisfies ArticleViewUIState)
}, [
activeTab,
authorQuery,
breakingFilter,
breakingOnly,
dateSortDirection,
featuredFilter,
featuredOnly,
pageSize,
publishedFilter,
searchQuery,
Expand Down Expand Up @@ -408,11 +445,11 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article
if (subsectionFilterSlug) {
params.set("subsection_slug", subsectionFilterSlug)
}
if (breakingFilter !== "all") {
params.set("breaking", flagFilterParam(breakingFilter))
if (breakingOnly) {
params.set("breaking", "true")
}
if (featuredFilter !== "all") {
params.set("featured", flagFilterParam(featuredFilter))
if (featuredOnly) {
params.set("featured", "true")
}
if (fixedType) {
params.set("type", fixedType)
Expand Down Expand Up @@ -494,7 +531,7 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article
return () => {
cancelled = true
}
}, [activeTab, apiFetch, authorQuery, breakingFilter, dateSortDirection, excludeType, featuredFilter, fixedType, page, pageSize, publishedFilter, resultsCacheKey, searchQuery, sectionFilterSlug, selectedAuthorSlug, subsectionFilterSlug])
}, [activeTab, apiFetch, authorQuery, breakingOnly, dateSortDirection, excludeType, featuredOnly, fixedType, page, pageSize, publishedFilter, resultsCacheKey, searchQuery, sectionFilterSlug, selectedAuthorSlug, subsectionFilterSlug])

const onChangeTab = (tab: "all" | "trash") => {
setActiveTab(tab)
Expand All @@ -508,7 +545,7 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article

useEffect(() => {
setPage(0)
}, [authorQuery, publishedFilter, breakingFilter, featuredFilter, dateSortDirection, searchQuery, sectionFilterSlug, subsectionFilterSlug, pageSize])
}, [authorQuery, publishedFilter, breakingOnly, featuredOnly, dateSortDirection, searchQuery, sectionFilterSlug, subsectionFilterSlug, pageSize])

const effectiveTotalCount = Math.max(totalArticleCount, (page * pageSize) + articles.length)
const totalPages = Math.max(1, Math.ceil(effectiveTotalCount / pageSize))
Expand Down Expand Up @@ -779,23 +816,14 @@ 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. */}
still flagged as breaking, and what is currently pinned. Each one
narrows to its flag or does nothing, so it is a switch rather than a
row of chips -- there is no third thing to pick. */}
<div className="flex flex-col gap-1.5">
<span className="text-xs font-semibold text-muted-foreground uppercase tracking-wide">Breaking</span>
<div className="flex gap-1.5">
<button aria-pressed={breakingFilter === "all"} className={filterTagClass(breakingFilter === "all")} onClick={() => setBreakingFilter("all")} type="button">All</button>
<button aria-pressed={breakingFilter === "on"} className={filterTagClass(breakingFilter === "on")} onClick={() => setBreakingFilter("on")} type="button">Breaking</button>
<button aria-pressed={breakingFilter === "off"} className={filterTagClass(breakingFilter === "off")} onClick={() => setBreakingFilter("off")} type="button">Not breaking</button>
</div>
</div>

<div className="flex flex-col gap-1.5">
<span className="text-xs font-semibold text-muted-foreground uppercase tracking-wide">Featured</span>
<div className="flex gap-1.5">
<button aria-pressed={featuredFilter === "all"} className={filterTagClass(featuredFilter === "all")} onClick={() => setFeaturedFilter("all")} type="button">All</button>
<button aria-pressed={featuredFilter === "on"} className={filterTagClass(featuredFilter === "on")} onClick={() => setFeaturedFilter("on")} type="button">Featured</button>
<button aria-pressed={featuredFilter === "off"} className={filterTagClass(featuredFilter === "off")} onClick={() => setFeaturedFilter("off")} type="button">Not featured</button>
<span className="text-xs font-semibold text-muted-foreground uppercase tracking-wide">Flags</span>
<div className="flex flex-wrap items-center gap-4 py-1">
<FilterSwitch checked={breakingOnly} label="Breaking only" onChange={setBreakingOnly} />
<FilterSwitch checked={featuredOnly} label="Featured only" onChange={setFeaturedOnly} />
</div>
</div>
</div>
Expand Down
4 changes: 2 additions & 2 deletions server/docs/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,13 @@ const docTemplate = `{
},
{
"type": "boolean",
"description": "When true, return only articles flagged as breaking news; when false, only those not flagged.",
"description": "When true, return only articles flagged as breaking news. Any other value does not filter.",
"name": "breaking",
"in": "query"
},
{
"type": "boolean",
"description": "When true, return only featured (pinned) articles; when false, only those not featured.",
"description": "When true, return only featured (pinned) articles. Any other value does not filter.",
"name": "featured",
"in": "query"
},
Expand Down
4 changes: 2 additions & 2 deletions server/docs/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,13 @@
},
{
"type": "boolean",
"description": "When true, return only articles flagged as breaking news; when false, only those not flagged.",
"description": "When true, return only articles flagged as breaking news. Any other value does not filter.",
"name": "breaking",
"in": "query"
},
{
"type": "boolean",
"description": "When true, return only featured (pinned) articles; when false, only those not featured.",
"description": "When true, return only featured (pinned) articles. Any other value does not filter.",
"name": "featured",
"in": "query"
},
Expand Down
8 changes: 4 additions & 4 deletions server/docs/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1362,13 +1362,13 @@ paths:
in: query
name: slug
type: string
- description: When true, return only articles flagged as breaking news; when
false, only those not flagged.
- description: When true, return only articles flagged as breaking news. Any
other value does not filter.
in: query
name: breaking
type: boolean
- description: When true, return only featured (pinned) articles; when false,
only those not featured.
- description: When true, return only featured (pinned) articles. Any other
value does not filter.
in: query
name: featured
type: boolean
Expand Down
27 changes: 13 additions & 14 deletions server/internal/handlers/article_flag_filters_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import (
db "server/internal/database"
)

// The Articles screen offers Breaking and Featured as filters, so both have to
// The Articles screen offers Breaking and Featured as switches, 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.
// The seed keeps a row with both flags NULL, which is the shape the WordPress
// archive is in. It must stay out of every "on" result: `= 1` is false for
// NULL, and a truthiness test written some other way could let nine thousand
// imported rows into a list of breaking stories.
func seedFlagFilterArticles(t *testing.T, conn *sql.DB) {
t.Helper()
ctx := context.Background()
Expand Down Expand Up @@ -89,21 +89,20 @@ func TestArticleFlagFiltersHTTP(t *testing.T) {
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"},
// A switch cannot ask for the unflagged half, so false is inert. The
// failure this pins is it being read as "not breaking" instead.
name: "false does not filter",
query: "?breaking=false",
want: []string{
"breaking-campus-story", "breaking-sports-story", "featured-campus-story",
"imported-campus-story", "ordinary-campus-story",
},
},
{
// Both at once: the article that is breaking AND pinned.
Expand Down
55 changes: 21 additions & 34 deletions server/internal/handlers/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1260,8 +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 breaking query bool false "When true, return only articles flagged as breaking news. Any other value does not filter."
// @Param featured query bool false "When true, return only featured (pinned) articles. Any other value does not filter."
// @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
Expand Down Expand Up @@ -1708,49 +1708,36 @@ 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")
}
// Both flags narrow to the articles that carry them. `priority` is the
// featured/pinned column; is_featured is the name it goes by over the wire,
// and GetFeaturedArticles reads the same one.
if flagFilterOn(q, "breaking") {
conditions = append(conditions, "`breaking_news` = 1")
}

// `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")
}
if flagFilterOn(q, "featured") {
conditions = append(conditions, "`priority` = 1")
}

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) {
// flagFilterOn reports whether a flag filter is switched on. Absent, false and
// unparseable all mean "do not filter", which is the whole of the contract: the
// parameter is a switch, and a switch cannot ask for the unflagged half.
//
// `= 1` rather than a truthiness test because both columns are nullable, and
// NULL is what the WordPress archive holds -- it must read as unflagged, not as
// unknown.
func flagFilterOn(q url.Values, key string) bool {
if _, provided := q[key]; !provided {
return false, false
return false
}
switch strings.ToLower(strings.TrimSpace(q.Get(key))) {
// Bare ?breaking is the HTML-form spelling of true.
case "", "1", "true", "yes":
// Bare ?breaking is the HTML-form spelling of true.
return true, true
case "0", "false", "no":
return false, true
return true
default:
return false, false
return false
}
}

Expand Down
Loading