Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

repo sync #25219

Merged
merged 6 commits into from
Apr 25, 2023
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
3 changes: 1 addition & 2 deletions .github/actions-scripts/rendered-content-link-checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const EXTERNAL_LINK_CHECKER_DB =
process.env.EXTERNAL_LINK_CHECKER_DB || 'external-link-checker-db.json'

const adapter = new JSONFile(EXTERNAL_LINK_CHECKER_DB)
const externalLinkCheckerDB = new Low(adapter)
const externalLinkCheckerDB = new Low(adapter, { urls: {} })

// Given a number and a percentage, return the same number with a *percentage*
// max change of making a bit larger or smaller.
Expand Down Expand Up @@ -233,7 +233,6 @@ async function main(core, octokit, uploadArtifact, opts = {}) {
}

await externalLinkCheckerDB.read()
externalLinkCheckerDB.data ||= { urls: {} }

debugTimeStart(core, 'processPages')
const t0 = new Date().getTime()
Expand Down
13 changes: 8 additions & 5 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"liquidjs": "^10.7.0",
"lodash": "^4.17.21",
"lodash-es": "^4.17.21",
"lowdb": "5.0.5",
"lowdb": "6.0.0",
"mdast-util-from-markdown": "^1.2.0",
"mdast-util-to-markdown": "1.5.0",
"mdast-util-to-string": "^3.1.0",
Expand Down
28 changes: 25 additions & 3 deletions src/pageinfo/middleware.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import express from 'express'

import statsd from '../../lib/statsd.js'
import findPage from '../../lib/find-page.js'
import { defaultCacheControl } from '../../middleware/cache-control.js'
import catchMiddlewareError from '../../middleware/catch-middleware-error.js'
import {
Expand All @@ -12,19 +11,32 @@ import {
import shortVersions from '../../middleware/contextualizers/short-versions.js'
import contextualize from '../../middleware/context.js'
import features from '../../middleware/contextualizers/features.js'
import getRedirect from '../../lib/get-redirect.js'

const router = express.Router()

const validationMiddleware = (req, res, next) => {
const { pathname } = req.query
let { pathname } = req.query
if (!pathname) {
return res.status(400).json({ error: `No 'pathname' query` })
}
if (!pathname.trim()) {
return res.status(400).json({ error: `'pathname' query empty` })
}

const page = findPage(pathname, req.context.pages, req.context.redirects)
// We can't use the `findPage` middleware utility function because we
// need to know when the pathname is a redirect.
// This is important so that the final `pathname` value
// matches the page's permalinks.
// This is important when rendering a page because of translations,
// if it needs to do a fallback, it needs to know the correct
// equivalent English page.
const redirectsContext = { pages: req.context.pages, redirects: req.context.redirects }
const redirect = getRedirect(pathname, redirectsContext)
if (redirect) {
pathname = redirect
}
const page = req.context.pages[pathname]

if (!page) {
return res.status(400).json({ error: `No page found for '${pathname}'` })
Expand All @@ -42,8 +54,18 @@ router.get(
'/v1',
validationMiddleware,
catchMiddlewareError(async function pageInfo(req, res) {
// Remember, the `validationMiddleware` will use redirects if the
// `pathname` used is a redirect (e.g. /en/articles/foo or
// /articles or '/en/enterprise-server@latest/foo/bar)
// So by the time we get here, the pathname should be one of the
// page's valid permalinks.
const { page, pathname } = req.pageinfo

const pagePermalinks = page.permalinks.map((p) => p.href)
if (!pagePermalinks.includes(pathname)) {
throw new Error(`pathname '${pathname}' not one of the page's permalinks`)
}

const renderingReq = {
path: pathname,
language: page.languageCode,
Expand Down
37 changes: 28 additions & 9 deletions src/pageinfo/tests/pageinfo.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { get } from '../../../tests/helpers/e2etest.js'
import { SURROGATE_ENUMS } from '../../../middleware/set-fastly-surrogate-key.js'

const makeURL = (pathname) => `/api/pageinfo/v1?${new URLSearchParams({ pathname })}`

describe('pageinfo api', () => {
test('redirects without version suffix', async () => {
const res = await get('/api/pageinfo')
Expand All @@ -9,7 +11,7 @@ describe('pageinfo api', () => {
})

test('happy path', async () => {
const res = await get('/api/pageinfo/v1?pathname=/en/get-started/quickstart')
const res = await get(makeURL('/en/get-started/quickstart'))
expect(res.statusCode).toBe(200)
const { info } = JSON.parse(res.body)
expect(info.product).toBe('Get started')
Expand All @@ -27,7 +29,7 @@ describe('pageinfo api', () => {
})

test('a pathname that does not exist', async () => {
const res = await get('/api/pageinfo/v1?pathname=/en/never/heard/of')
const res = await get(makeURL('/en/never/heard/of'))
expect(res.statusCode).toBe(400)
const { error } = JSON.parse(res.body)
expect(error).toBe("No page found for '/en/never/heard/of'")
Expand All @@ -47,13 +49,30 @@ describe('pageinfo api', () => {
expect(error).toBe("'pathname' query empty")
})

test('redirects are automatically respected', async () => {
// This is based on the fixture content content/index.md which
// has a `redirect_from`.
const res = await get('/api/pageinfo/v1?pathname=/en/olden-days')
expect(res.statusCode).toBe(200)
const { info } = JSON.parse(res.body)
expect(info.title).toBe('GitHub Fixture Documentation')
test('redirects correct the URL', async () => {
// Regular redirect from `redirect_from`
{
const res = await get(makeURL('/en/olden-days'))
expect(res.statusCode).toBe(200)
const { info } = JSON.parse(res.body)
expect(info.title).toBe('github.com Fixture Documentation')
}
// Short code for latest version
{
const res = await get(makeURL('/en/enterprise-server@latest/get-started/liquid/ifversion'))
expect(res.statusCode).toBe(200)
const { info } = JSON.parse(res.body)
expect(info.intro).toMatch(/\(not on fpt\)/)
}
// A URL that doesn't have fpt as an available version
{
const res = await get(
'/api/pageinfo/v1?pathname=/en/get-started/versioning/only-ghec-and-ghes'
)
expect(res.statusCode).toBe(200)
const { info } = JSON.parse(res.body)
expect(info.title).toBe('Only in Enterprise Cloud and Enterprise Server')
}
})

test('a page that uses non-trivial Liquid to render', async () => {
Expand Down