diff --git a/app/components/Search.tsx b/app/components/Search.tsx index 19716e8..3b00c26 100644 --- a/app/components/Search.tsx +++ b/app/components/Search.tsx @@ -29,6 +29,7 @@ import { Link, useNavigate } from 'react-router' import Icon from '~/components/Icon' import StatusBadge from '~/components/StatusBadge' +import { formatRfdNum } from '~/utils/canonicalUrl' import { useSteppedScroll } from '~/hooks/use-stepped-scroll' import type { RfdItem } from '~/services/rfd.server' @@ -170,7 +171,7 @@ const SearchWrapper = ({ dismissSearch }: { dismissSearch: () => void }) => { if (e.key === 'Enter') { const selectedItem = flattenedHits[selectedIdx] if (!selectedItem) return - navigate(`/rfd/${selectedItem.rfd_number}#${selectedItem.anchor}`) + navigate(`/rfd/${formatRfdNum(selectedItem.rfd_number)}#${selectedItem.anchor}`) // needed despite key={pathname + hash} logic in case we navigate // to the page we're already on dismissSearch() @@ -377,7 +378,7 @@ const HitItem = ({ hit, isSelected }: { hit: RFDHit; isSelected: boolean }) => { )} /> )} - +
  • { item.level === 1 && (
    diff --git a/app/components/rfd/MoreDropdown.tsx b/app/components/rfd/MoreDropdown.tsx index 962ce86..53bdacf 100644 --- a/app/components/rfd/MoreDropdown.tsx +++ b/app/components/rfd/MoreDropdown.tsx @@ -47,7 +47,7 @@ const MoreDropdown = () => { )} - View PDF + View PDF diff --git a/app/routes/$slug.tsx b/app/routes/$slug.tsx index 4398312..1784b94 100644 --- a/app/routes/$slug.tsx +++ b/app/routes/$slug.tsx @@ -8,11 +8,14 @@ import { redirect, type LoaderFunctionArgs } from 'react-router' +import { formatRfdNum } from '~/utils/canonicalUrl' import { parseRfdNum } from '~/utils/parseRfdNum' export async function loader({ params: { slug } }: LoaderFunctionArgs) { - if (parseRfdNum(slug)) { - return redirect(`/rfd/${slug}`) + const num = parseRfdNum(slug) + if (num) { + // Always redirect to canonical URL format (zero-padded) + return redirect(`/rfd/${formatRfdNum(num)}`) } else { throw new Response('Not Found', { status: 404 }) } diff --git a/app/routes/_index.tsx b/app/routes/_index.tsx index b5b2bd0..949b669 100644 --- a/app/routes/_index.tsx +++ b/app/routes/_index.tsx @@ -22,6 +22,7 @@ import { type LoaderFunctionArgs, } from 'react-router' +import { canonicalUrl } from '~/utils/canonicalUrl' import { ClientOnly } from '~/components/ClientOnly' import Container from '~/components/Container' import { SortArrowBottom, SortArrowTop } from '~/components/CustomIcons' @@ -37,6 +38,10 @@ import { sortBy } from '~/utils/array' import { fuzz } from '~/utils/fuzz' import { parseSortOrder, type SortAttr } from '~/utils/rfdSortOrder.server' +export const links = () => [ + { rel: 'canonical', href: canonicalUrl('/') }, +] + export const loader = async ({ request }: LoaderFunctionArgs) => { const cookieHeader = request.headers.get('Cookie') return parseSortOrder(await rfdSortCookie.parse(cookieHeader)) diff --git a/app/routes/login.tsx b/app/routes/login.tsx index 30875db..6816120 100644 --- a/app/routes/login.tsx +++ b/app/routes/login.tsx @@ -19,8 +19,13 @@ import { } from 'react-router' import { auth, getUserFromSession } from '~/services/auth.server' +import { canonicalUrl } from '~/utils/canonicalUrl' import { returnToCookie } from '~/services/cookies.server' +export const links = () => [ + { rel: 'canonical', href: canonicalUrl('/login') }, +] + export const loader = async ({ request }: LoaderFunctionArgs) => { const url = new URL(request.url) const returnTo = url.searchParams.get('returnTo') diff --git a/app/routes/rfd.$slug.discussion.tsx b/app/routes/rfd.$slug.discussion.tsx index e66c556..9c02566 100644 --- a/app/routes/rfd.$slug.discussion.tsx +++ b/app/routes/rfd.$slug.discussion.tsx @@ -10,6 +10,7 @@ import { redirect, type LoaderFunctionArgs } from 'react-router' import { authenticate } from '~/services/auth.server' import { fetchRfd } from '~/services/rfd.server' +import { formatRfdNum } from '~/utils/canonicalUrl' import { parseRfdNum } from '~/utils/parseRfdNum' import { resp404 } from './rfd.$slug' @@ -24,7 +25,7 @@ export async function loader({ request, params: { slug } }: LoaderFunctionArgs) // !rfd covers both non-existent and private RFDs for the logged-out user. In // both cases, once they log in, if they have permission to read it, they'll // get the redirect, otherwise they will get 404. - if (!rfd && !user) throw redirect(`/login?returnTo=/rfd/${num}/discussion`) + if (!rfd && !user) throw redirect(`/login?returnTo=/rfd/${formatRfdNum(num)}/discussion`) // If you don't see an RFD but you are logged in, you can't tell whether you // don't have access or it doesn't exist. That's fine. diff --git a/app/routes/rfd.$slug.tsx b/app/routes/rfd.$slug.tsx index b944e59..8e7388c 100644 --- a/app/routes/rfd.$slug.tsx +++ b/app/routes/rfd.$slug.tsx @@ -48,6 +48,7 @@ import StatusBadge from '~/components/StatusBadge' import { useRootLoaderData } from '~/root' import { authenticate } from '~/services/auth.server' import { fetchGroups, fetchRfd } from '~/services/rfd.server' +import { canonicalRfdUrl, formatRfdNum } from '~/utils/canonicalUrl' import { parseRfdNum } from '~/utils/parseRfdNum' import { can } from '~/utils/permission' @@ -77,13 +78,19 @@ export async function loader({ request, params: { slug } }: LoaderFunctionArgs) const num = parseRfdNum(slug) if (!num) throw resp404() + // Redirect to canonical URL if the slug is not in the canonical format (zero-padded) + const canonicalSlug = formatRfdNum(num) + if (slug !== canonicalSlug) { + throw redirect(`/rfd/${canonicalSlug}`) + } + const user = await authenticate(request) const rfd = await fetchRfd(num, user) // If someone goes to a private RFD but they're not logged in, they will // want to log in and see it. - if (!rfd && !user) throw redirect(`/login?returnTo=/rfd/${num}`) + if (!rfd && !user) throw redirect(`/login?returnTo=/rfd/${canonicalSlug}`) // If you don't see an RFD but you are logged in, you can't tell whether you // don't have access or it doesn't exist. That's fine. @@ -114,7 +121,10 @@ export async function loader({ request, params: { slug } }: LoaderFunctionArgs) export const meta: MetaFunction = ({ data }) => { if (data && data.rfd) { - return [{ title: `${data.rfd.number} - ${data.rfd.title} / RFD / Oxide` }] + return [ + { title: `${data.rfd.number} - ${data.rfd.title} / RFD / Oxide` }, + { tagName: 'link', rel: 'canonical', href: canonicalRfdUrl(data.rfd.number) }, + ] } else { return [{ title: 'Page not found / Oxide' }] } diff --git a/app/utils/canonicalUrl.test.ts b/app/utils/canonicalUrl.test.ts new file mode 100644 index 0000000..049efac --- /dev/null +++ b/app/utils/canonicalUrl.test.ts @@ -0,0 +1,36 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ + +import { expect, test } from 'vitest' + +import { canonicalRfdUrl, canonicalUrl, formatRfdNum, SITE_URL } from './canonicalUrl' + +test.each([ + [1, '0001'], + [53, '0053'], + [321, '0321'], + [9999, '9999'], +])('formatRfdNum(%i) -> %s', (input: number, result: string) => { + expect(formatRfdNum(input)).toEqual(result) +}) + +test.each([ + [1, `${SITE_URL}/rfd/0001`], + [53, `${SITE_URL}/rfd/0053`], + [321, `${SITE_URL}/rfd/0321`], +])('canonicalRfdUrl(%i) -> %s', (input: number, result: string) => { + expect(canonicalRfdUrl(input)).toEqual(result) +}) + +test.each([ + ['/', `${SITE_URL}/`], + ['/login', `${SITE_URL}/login`], + ['/rfd/0053', `${SITE_URL}/rfd/0053`], +])('canonicalUrl(%s) -> %s', (input: string, result: string) => { + expect(canonicalUrl(input)).toEqual(result) +}) diff --git a/app/utils/canonicalUrl.ts b/app/utils/canonicalUrl.ts new file mode 100644 index 0000000..52397cd --- /dev/null +++ b/app/utils/canonicalUrl.ts @@ -0,0 +1,25 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ + +export const SITE_URL = 'https://rfd.shared.oxide.computer' + +/** + * Format an RFD number as a zero-padded 4-digit string (e.g., 53 -> "0053") + */ +export const formatRfdNum = (num: number): string => num.toString().padStart(4, '0') + +/** + * Generate the canonical URL for an RFD page + */ +export const canonicalRfdUrl = (num: number): string => + `${SITE_URL}/rfd/${formatRfdNum(num)}` + +/** + * Generate a canonical URL for a given path (strips query params) + */ +export const canonicalUrl = (path: string): string => `${SITE_URL}${path}`