Decouple redirects from language (#24597)

* experimenting with redirects

* cleanup developer.json

* wip

* clean console.log

* progress

* some progress

* progress

* much progress

* debugging tests

* hacky progress

* ditch latest -> number redirects

* minor

* hacky progress

* lots of progress

* some small fixes

* fix rendering tests

* small fixes

* progress

* undo debugging

* better

* routing tests OK

* more cleaning

* unit tests

* undoing lineending edit

* undoing temporary debugging

* don't ever set this.redirects on Page

* cope with archived version redirects

* adding code comments on the major if statements

* address all feedback

* update README about redirects

* delete invalid test

* fix feedback

.github/actions-scripts/prune-for-preview-env.sh

@@ -20,5 +20,4 @@ mkdir translations
 # front-matter will be at play.
 # These static redirects json files are notoriously large
 echo '[]' > lib/redirects/static/archived-frontmatter-fallbacks.json
-echo '{}' > lib/redirects/static/developer.json
 echo '{}' > lib/redirects/static/archived-redirects-from-213-to-217.json

.gitignore

@@ -16,7 +16,7 @@ coverage/
 blc_output.log
 blc_output_internal.log
 broken_links.md
-lib/redirects/.redirects-cache_*.json
+lib/redirects/.redirects-cache.json
 
 # During the preview deploy untrusted user code may be cloned into this directory
 # We ignore it from git to keep things deterministic

content/admin/index.md

@@ -10,9 +10,7 @@ redirect_from:
   - /github/installing-and-configuring-github-insights/key-metrics-for-collaboration-in-pull-requests
   - /github/installing-and-configuring-github-insights/viewing-and-filtering-key-metrics-and-reports
   - /github/installing-and-configuring-github-insights/github-insights-and-data-protection-for-your-organization
-  - /enterprise-server@2.22/github/site-policy/github-insights-and-data-protection-for-your-organization
-  - /enterprise-server@2.21/github/site-policy/github-insights-and-data-protection-for-your-organization
-  - /enterprise-server@2.20/github/site-policy/github-insights-and-data-protection-for-your-organization
+  - /github/site-policy/github-insights-and-data-protection-for-your-organization
   - /insights/installing-and-configuring-github-insights/configuring-the-connection-between-github-insights-and-github-enterprise
   - /github/installing-and-configuring-github-insights/navigating-between-github-insights-and-github-enterprise
   - /github/installing-and-configuring-github-insights/enabling-a-link-between-github-insights-and-github-enterprise
@@ -134,4 +132,3 @@ children:
   - /release-notes
   - /all-releases
 ---
-

content/admin/policies/enforcing-policies-for-your-enterprise/enforcing-policies-for-security-settings-in-your-enterprise.md

@@ -10,7 +10,7 @@ redirect_from:
   - /github/articles/managing-allowed-ip-addresses-for-organizations-in-your-enterprise-account
   - /github/setting-up-and-managing-your-enterprise-account/enforcing-security-settings-in-your-enterprise-account
   - /github/setting-up-and-managing-your-enterprise/enforcing-security-settings-in-your-enterprise-account
-  - github/setting-up-and-managing-your-enterprise/setting-policies-for-organizations-in-your-enterprise-account/enforcing-security-settings-in-your-enterprise-account
+  - /github/setting-up-and-managing-your-enterprise/setting-policies-for-organizations-in-your-enterprise-account/enforcing-security-settings-in-your-enterprise-account
 versions:
   ghec: '*'
   ghes: '*'
@@ -73,7 +73,7 @@ Enterprise owners can restrict access to assets owned by organizations in an ent
 
 {% data reusables.identity-and-permissions.ip-allow-lists-cidr-notation %}
 
-{% data reusables.identity-and-permissions.ip-allow-lists-enable %} {% data reusables.identity-and-permissions.ip-allow-lists-enterprise %} 
+{% data reusables.identity-and-permissions.ip-allow-lists-enable %} {% data reusables.identity-and-permissions.ip-allow-lists-enterprise %}
 
 You can also configure allowed IP addresses for an individual organization. For more information, see "[Managing allowed IP addresses for your organization](/organizations/keeping-your-organization-secure/managing-allowed-ip-addresses-for-your-organization)."
 

content/get-started/using-git/about-git-rebase.md

@@ -2,7 +2,7 @@
 title: About Git rebase
 redirect_from:
   - /rebase
-  - articles/interactive-rebase/
+  - /articles/interactive-rebase
   - /articles/about-git-rebase
   - /github/using-git/about-git-rebase
   - /github/getting-started-with-github/about-git-rebase

content/organizations/organizing-members-into-teams/managing-scheduled-reminders-for-your-team.md

@@ -3,7 +3,7 @@ title: Managing scheduled reminders for your team
 intro: You can get reminders in Slack when your team has pull requests waiting for review.
 redirect_from:
   - /github/setting-up-and-managing-organizations-and-teams/managing-scheduled-reminders-for-pull-requests
-  - /github/setting-up-and-managing-organizations-and-teams/managing-scheduled-reminders-for-your team
+  - /github/setting-up-and-managing-organizations-and-teams/managing-scheduled-reminders-for-your-team
 versions:
   fpt: '*'
   ghec: '*'

lib/find-page.js

@@ -1,11 +1,14 @@
 import { getLanguageCode } from './patterns.js'
+import getRedirect from './get-redirect.js'
 
 export default function findPage(href, pageMap, redirects) {
   // remove any fragments
   href = href.replace(/#.*$/, '')
 
+  const redirectsContext = { redirects, pages: pageMap }
+
   // find the page
-  const page = pageMap[href] || pageMap[redirects[href]]
+  const page = pageMap[href] || pageMap[getRedirect(href, redirectsContext)]
   if (page) return page
 
   // get the current language
@@ -13,5 +16,5 @@ export default function findPage(href, pageMap, redirects) {
 
   // try to fall back to English if the translated page can't be found
   const englishHref = href.replace(`/${currentLang}/`, '/en/')
-  return pageMap[englishHref] || pageMap[redirects[englishHref]]
+  return pageMap[englishHref] || pageMap[getRedirect(englishHref, redirectsContext)]
 }

lib/get-redirect.js

@@ -0,0 +1,201 @@
+import { languageKeys } from './languages.js'
+import nonEnterpriseDefaultVersion from './non-enterprise-default-version.js'
+
+import { allVersions } from './all-versions.js'
+import { latest, supported } from './enterprise-server-releases.js'
+
+const languagePrefixRegex = new RegExp(`^/(${languageKeys.join('|')})/`)
+const nonEnterpriseDefaultVersionPrefix = `/${nonEnterpriseDefaultVersion}`
+
+// Return the new URI if there is one, otherwise return undefined.
+export default function getRedirect(uri, context) {
+  const { redirects, pages } = context
+
+  let language = 'en'
+  let withoutLanguage = uri
+  if (languagePrefixRegex.test(uri)) {
+    language = uri.match(languagePrefixRegex)[1]
+    withoutLanguage = uri.replace(languagePrefixRegex, '/')
+  }
+
+  let destination
+
+  // `redirects` is sourced from more than one thing. The primary use
+  // case is gathering up the `redirect_from` frontmatter key.
+  // But we also has `developer.json` which contains legacy redirects.
+  // For example, the `developer.json` will have entries such
+  // `/enterprise/v4/enum/auditlogorderfield` which clearly is using
+  // the old formatting of the version. So to leverage the redirects
+  // from `developer.json` we'll look at it right away.
+  if (withoutLanguage in redirects) {
+    return `/${language}` + redirects[withoutLanguage]
+  }
+
+  let basicCorrection
+
+  if (withoutLanguage.startsWith(nonEnterpriseDefaultVersionPrefix)) {
+    // E.g. '/free-pro-team@latest/foo/bar' or '/free-pro-team@latest'
+    basicCorrection =
+      `/${language}` + withoutLanguage.replace(nonEnterpriseDefaultVersionPrefix, '')
+  } else if (withoutLanguage.replace('/', '') in allVersions && !languagePrefixRegex.test(uri)) {
+    // E.g. just '/github-ae@latest' or '/enterprise-cloud@latest'
+    basicCorrection = `/${language}` + withoutLanguage
+    return basicCorrection
+  }
+
+  if (
+    withoutLanguage === '/enterprise-server' ||
+    withoutLanguage.startsWith('/enterprise-server/')
+  ) {
+    // E.g. '/enterprise-server' or '/enterprise-server/3.0/foo'
+    basicCorrection =
+      `/${language}` + withoutLanguage.replace('/enterprise-server', `/enterprise-server@${latest}`)
+    // If it's now just the version, without anything after, exit here
+    if (withoutLanguage === '/enterprise-server') {
+      return basicCorrection
+    }
+    // console.log({ basicCorrection })
+  } else if (withoutLanguage.startsWith('/enterprise-server@latest')) {
+    // E.g. '/enterprise-server@latest' or '/enterprise-server@latest/3.3/foo'
+    basicCorrection =
+      `/${language}` +
+      withoutLanguage.replace('/enterprise-server@latest', `/enterprise-server@${latest}`)
+    // If it was *just* '/enterprise-server@latest' all that's needed is
+    // the language but with 'latest' replaced with the value of `latest`
+    if (withoutLanguage === '/enterprise-server@latest') {
+      return basicCorrection
+    }
+  } else if (
+    withoutLanguage.startsWith('/enterprise/') &&
+    supported.includes(withoutLanguage.split('/')[2])
+  ) {
+    // E.g. '/enterprise/3.3' or '/enterprise/3.3/foo'
+
+    // If the URL is without a language, and no redirect is necessary,
+    // but it has as version prefix, the language has to be there
+    // otherwise it will never be found in `req.context.pages`
+    const version = withoutLanguage.split('/')[2]
+    if (withoutLanguage === `/enterprise/${version}`) {
+      // E.g. `/enterprise/3.0`
+      basicCorrection =
+        `/${language}` +
+        withoutLanguage.replace(`/enterprise/${version}`, `/enterprise-server@${version}`)
+      return basicCorrection
+    } else {
+      basicCorrection =
+        `/${language}` +
+        withoutLanguage.replace(`/enterprise/${version}/`, `/enterprise-server@${version}/`)
+    }
+  } else if (withoutLanguage === '/enterprise') {
+    // E.g. `/enterprise` exactly
+    basicCorrection = `/${language}/enterprise-server@${latest}`
+    return basicCorrection
+  } else if (
+    withoutLanguage.startsWith('/enterprise/') &&
+    !supported.includes(withoutLanguage.split('/')[2])
+  ) {
+    // E.g. '/en/enterprise/user/github/foo'
+    // If the URL is without a language, and no redirect is necessary,
+    // but it has as version prefix, the language has to be there
+    // otherwise it will never be found in `req.context.pages`
+    basicCorrection =
+      `/${language}` +
+      withoutLanguage
+        .replace(`/enterprise/`, `/enterprise-server@${latest}/`)
+        .replace('/user/', '/')
+  } else if (withoutLanguage.startsWith('/insights')) {
+    // E.g. '/insights/foo'
+    basicCorrection = uri.replace('/insights', `${language}/enterprise-server@${latest}/insights`)
+  }
+
+  if (basicCorrection) {
+    return (
+      getRedirect(basicCorrection, {
+        redirects,
+        pages,
+      }) || basicCorrection
+    )
+  }
+
+  if (withoutLanguage.startsWith('/admin/')) {
+    const prefix = `/enterprise-server@${latest}`
+    let suffix = withoutLanguage
+    if (suffix.startsWith('/admin/guides/')) {
+      suffix = suffix.replace('/admin/guides/', '/admin/')
+    }
+    const newURL = prefix + suffix
+    destination = redirects[newURL] || newURL
+  } else if (
+    withoutLanguage.split('/')[1].includes('@') &&
+    withoutLanguage.split('/')[1] in allVersions
+  ) {
+    // E.g. '/enterprise-server@latest' or '/github-ae@latest'  or '/enterprise-server@3.3'
+    const majorVersion = withoutLanguage.split('/')[1].split('@')[0]
+    const split = withoutLanguage.split('/')
+    const version = split[1].split('@')[1]
+    let prefix
+    let suffix
+
+    if (supported.includes(version) || version === 'latest') {
+      prefix = `/${majorVersion}@${version}`
+      suffix = '/' + split.slice(2).join('/')
+
+      if (
+        suffix.includes('/user') ||
+        suffix.startsWith('/admin/guide') ||
+        suffix.startsWith('/articles/user')
+      ) {
+        suffix = tryReplacements(prefix, suffix, context) || suffix
+      }
+    }
+
+    const newURL = prefix + suffix
+    if (newURL !== withoutLanguage) {
+      // At least the prefix changed!
+      destination = redirects[newURL] || newURL
+    } else {
+      destination = redirects[newURL]
+    }
+  } else if (withoutLanguage.startsWith('/desktop/guides/')) {
+    // E.g. /desktop/guides/contributing-and-collaborat
+    const newURL = withoutLanguage.replace('/desktop/guides/', '/desktop/')
+    destination = redirects[newURL] || newURL
+  } else {
+    destination = redirects[withoutLanguage]
+  }
+
+  if (destination !== undefined) {
+    // There's hope! Now we just need to attach the correct language
+    // to the destination URL.
+    return `/${language}${destination}`
+  }
+}
+
+// Over time, we've developed multiple ambiguous patterns of URLs
+// You can't simply assume that all `/admin/guides` should become
+// `/admin` for example.
+// This function tries different string replacement on the suffix
+// (the pathname after the language and version part) until it
+// finds one string replacement that yields either a page or a redirect.
+function tryReplacements(prefix, suffix, { pages, redirects }) {
+  const test = (suffix) => {
+    const candidateAsRedirect = prefix + suffix
+    const candidateAsURL = '/en' + candidateAsRedirect
+    return candidateAsRedirect in redirects || candidateAsURL in pages
+  }
+
+  let attempt = suffix.replace('/user', '/github')
+  if (test(attempt)) return attempt
+
+  attempt = suffix.replace('/user', '')
+  if (test(attempt)) return attempt
+
+  attempt = suffix.replace('/admin/guides', '/admin')
+  if (test(attempt)) return attempt
+
+  attempt = suffix.replace('/admin/guides/user', '/admin/github')
+  if (test(attempt)) return attempt
+
+  attempt = suffix.replace('/admin/guides', '/admin').replace('/user', '/github')
+  if (test(attempt)) return attempt
+}

lib/languages.js

@@ -53,8 +53,12 @@ if (process.env.ENABLED_LANGUAGES) {
 
 export const languageKeys = Object.keys(languages)
 
-export const languagePrefixPathRegex = new RegExp(`^/(${languageKeys.join('|')})/`)
+export const languagePrefixPathRegex = new RegExp(`^/(${languageKeys.join('|')})(/|$)`)
 
+/** Return true if the URL is something like /en/foo or /ja but return false
+ * if it's something like /foo or /foo/bar or /fr (because French (fr)
+ * is currently not an active language)
+ */
 export function pathLanguagePrefixed(path) {
   return languagePrefixPathRegex.test(path)
 }

lib/page.js

@@ -135,14 +135,16 @@ class Page {
   }
 
   buildRedirects() {
-    // create backwards-compatible old paths for page permalinks and frontmatter redirects
-    this.redirects = generateRedirectsForPermalinks(
-      this.permalinks,
-      this.redirect_from,
-      this.applicableVersions
-    )
+    if (!this.redirect_from) {
+      return {}
+    }
+    // this.redirect_from comes from frontmatter Yaml
+    // E.g `redirect_from: /old/path`
+    const redirectFrontmatter = Array.isArray(this.redirect_from)
+      ? this.redirect_from
+      : [this.redirect_from]
 
-    return this.redirects
+    return generateRedirectsForPermalinks(this.permalinks, redirectFrontmatter)
   }
 
   // Infer the parent product ID from the page's relative file path

lib/path-utils.js

@@ -33,7 +33,7 @@ export function getPathWithoutVersion(href) {
 }
 
 // Return the version segment in a path
-export function getVersionStringFromPath(href) {
+export function getVersionStringFromPath(href, supportedOnly = false) {
   href = getPathWithoutLanguage(href)
 
   // Return immediately if this is a link to the homepage
@@ -65,6 +65,16 @@ export function getVersionStringFromPath(href) {
     return allVersions[planObject.latestVersion].version
   }
 
+  // If the caller of this function explicitly wants to know if the
+  // version part is *not* supported, they get back `undefined`.
+  // But this function is used in many other places where it potentially
+  // doesn't care if the version is supported.
+  // For example, in lib/redirects/permalinks.js it needs to know if the
+  // URL didn't contain a valid version.
+  if (supportedOnly) {
+    return
+  }
+
   // Otherwise, return the first segment as-is, which may not be a real supported version,
   // but additional checks are done on this segment in getVersionedPathWithoutLanguage
   return versionFromPath

lib/permalink.js

@@ -13,9 +13,12 @@ class Permalink {
 
     const permalinkSuffix = this.constructor.relativePathToSuffix(relativePath)
 
-    this.href = removeFPTFromPath(
-      path.posix.join('/', languageCode, pageVersion, permalinkSuffix)
+    this.hrefWithoutLanguage = removeFPTFromPath(
+      path.posix.join('/', pageVersion, permalinkSuffix)
     ).replace(patterns.trailingSlash, '$1')
+    this.href = `/${languageCode}${
+      this.hrefWithoutLanguage === '/' ? '' : this.hrefWithoutLanguage
+    }`
 
     this.pageVersionTitle = allVersions[pageVersion].versionTitle
 

lib/redirects/README.md

@@ -14,16 +14,27 @@ Precompiled redirects account for the majority of the docs site's redirect handl
 
 When [`lib/warm-server.js`](lib/warm-server.js) runs on server start, it creates all pages in the site by instantiating the [`Page` class](lib/page.js) for each content file, then passes the pages to `lib/redirects/precompile.js` to create redirects. The precompile script runs `lib/redirects/permalinks.js`, which:
 
-1. Loops over each page's [permalinks](contributing/permalinks.md) and creates an array of legacy paths for each one (via `lib/redirects/get-old-paths-from-permalink.js`). For example, a permalink that starts with `/en/enterprise-server@2.22` results in an array that includes `/en/enterprise/2.22`, `/enterprise/2.22`, etc.
+1. Includes all legacy redirects from `static/developerjson`
 2. Loops over each page's [frontmatter `redirect_from` entries](content/README.md#redirect_from) and creates an array of legacy paths for each one (using the same handling as for permalinks).
+3. Any other exceptions from the `static/redirect-exceptions.txt` file
 
-The results comprise the `page.redirects` object, where the **keys are legacy paths** and the **values are current permalinks**.
-
-Additionally, a [static JSON file](lib/redirects/static/developer.json) gets `require`d that contains keys with legacy developer.github.com paths (e.g., `/v4/object/app`) and values with new docs.github.com paths (e.g., `/graphql/reference/objects#app`).
+The results comprise the `page.redirects` object, whose keys are always only the path without language.
+Sometimes it contains the specific plan/version (e.g. `/enterprise-server@3.0/v3/integrations` to `enterprise-server@3.0/developers/apps`) and sometimes it's just the plain path
+(e.g. `/articles/viewing-your-repositorys-workflows` to `/actions/monitoring-and-troubleshooting-workflows`)
 
 All of the above are merged into a global redirects object. This object gets added to `req.context` via `middleware/context.js` and is made accessible on every request.
 
-Because the redirects are precompiled via `warm-server`, that means `middleware/redirects/handle-redirects.js` just needs to do a simple lookup of the requested path in the redirects object.
+In the `handle-redirects.js` middleware, the language part of the URL is
+removed, looked up, and if matched to something, redirects with language
+put back in. Demonstrated with pseudo code:
+
+```js
+var fullPath = '/ja/foo'
+var newPath = redirects['/foo']
+if (newPath) {
+  redirect('/ja' + newPath)
+}
+```
 
 ### Archived Enterprise redirects
 
@@ -51,4 +62,4 @@ This is admittedly an inefficient brute-force approach. But requests for archive
 
 Redirect tests are mainly found in `tests/routing/*`, with some additional tests in `tests/rendering/server.js`.
 
-The `tests/fixtures/*` directory includes `developer-redirects.json`, `graphql-redirects.json`, and `rest-redirects.json`.
+The `tests/fixtures/*` directory includes `developer-redirects.json`, `graphql-redirects.json`, and `rest-redirects.json`.

lib/redirects/get-old-paths-from-permalink.js

@@ -1,151 +0,0 @@
-import {
-  latest,
-  deprecated,
-  lastReleaseWithLegacyFormat,
-  firstRestoredAdminGuides,
-} from '../enterprise-server-releases.js'
-import {
-  getPathWithoutLanguage,
-  getPathWithLanguage,
-  getVersionStringFromPath,
-} from '../path-utils.js'
-import patterns from '../patterns.js'
-import versionSatisfiesRange from '../version-satisfies-range.js'
-import { allVersions } from '../all-versions.js'
-import nonEnterpriseDefaultVersion from '../non-enterprise-default-version.js'
-const currentlySupportedVersions = Object.keys(allVersions)
-
-// This function takes a current path, applies what we know about historically
-// supported paths, and returns an array of ALL possible associated old
-// paths that users might try to hit.
-export default function getOldPathsFromPath(currentPath, languageCode, currentVersion) {
-  const oldPaths = new Set()
-
-  const versionFromPath = getVersionStringFromPath(currentPath)
-
-  // This only applies to Dotcom paths, so no need to determine whether the version is deprecated
-  // create old path /free-pro-team@latest/github from new path /github (or from a frontmatter `redirect_from` path like /articles)
-  if (
-    versionFromPath === 'homepage' ||
-    !(
-      currentlySupportedVersions.includes(versionFromPath) || deprecated.includes(versionFromPath)
-    ) ||
-    (versionFromPath === nonEnterpriseDefaultVersion &&
-      !currentPath.includes(nonEnterpriseDefaultVersion))
-  ) {
-    oldPaths.add(
-      currentPath.replace(`/${languageCode}`, `/${languageCode}/${nonEnterpriseDefaultVersion}`)
-    )
-  }
-
-  // ------ BEGIN LEGACY VERSION FORMAT REPLACEMENTS ------//
-  // These remain relevant to handle legacy-formatted frontmatter redirects
-  // and archived versions paths.
-
-  // create old path /insights from current path /enterprise/version/insights
-  oldPaths.add(
-    currentPath.replace(`/${languageCode}/enterprise/${latest}/user/insights`, '/insights')
-  )
-
-  // create old path /desktop/guides from current path /desktop
-  if (currentPath.includes('/desktop') && !currentPath.includes('/guides')) {
-    oldPaths.add(currentPath.replace('/desktop', '/desktop/guides'))
-  }
-
-  // create old path /admin/guides from current path /admin
-  if (currentPath.includes('admin') && !currentPath.includes('/guides')) {
-    // ... but ONLY on versions <2.21 and in deep links on all versions
-    if (
-      versionSatisfiesRange(currentVersion, `<${firstRestoredAdminGuides}`) ||
-      !currentPath.endsWith('/admin')
-    ) {
-      oldPaths.add(currentPath.replace('/admin', '/admin/guides'))
-    }
-  }
-
-  // create old path /user from current path /user/github on 2.16+ only
-  if (
-    currentlySupportedVersions.includes(currentVersion) ||
-    versionSatisfiesRange(currentVersion, '>2.15')
-  ) {
-    oldPaths.add(currentPath.replace('/user/github', '/user'))
-  }
-
-  // create old path /enterprise from current path /enterprise/latest
-  oldPaths.add(currentPath.replace(`/enterprise/${latest}`, '/enterprise'))
-
-  // create old path /enterprise/foo from current path /enterprise/user/foo
-  // this supports old developer paths like /enterprise/webhooks with no /user in them
-  if (currentPath.includes('/enterprise/')) {
-    oldPaths.add(currentPath.replace('/user/', '/'))
-  }
-
-  // ------ END LEGACY VERSION FORMAT REPLACEMENTS ------//
-
-  // ------ BEGIN MODERN VERSION FORMAT REPLACEMENTS ------//
-  if (
-    currentlySupportedVersions.includes(currentVersion) ||
-    versionSatisfiesRange(currentVersion, `>${lastReleaseWithLegacyFormat}`)
-  ) {
-    new Set(oldPaths).forEach((oldPath) => {
-      // create old path /enterprise/<version> from new path /enterprise-server@<version>
-      oldPaths.add(oldPath.replace(/\/enterprise-server@(\d)/, '/enterprise/$1'))
-
-      // create old path /enterprise/<version>/user from new path /enterprise-server@<version>/github
-      oldPaths.add(oldPath.replace(/\/enterprise-server@(\d.+?)\/github/, '/enterprise/$1/user'))
-
-      // create old path /insights from new path /enterprise-server@<latest>/insights
-      oldPaths.add(oldPath.replace(`/enterprise-server@${latest}/insights`, '/insights'))
-
-      // create old path /admin from new path /enterprise-server@<latest>/admin
-      oldPaths.add(oldPath.replace(`/enterprise-server@${latest}/admin`, '/admin'))
-
-      // create old path /enterprise from new path /enterprise-server@<latest>
-      oldPaths.add(oldPath.replace(`/enterprise-server@${latest}`, '/enterprise'))
-
-      // create old path /enterprise-server from new path /enterprise-server@<latest>
-      oldPaths.add(oldPath.replace(`/enterprise-server@${latest}`, '/enterprise-server'))
-
-      // create old path /enterprise-server@latest from new path /enterprise-server@<latest>
-      oldPaths.add(oldPath.replace(`/enterprise-server@${latest}`, '/enterprise-server@latest'))
-
-      if (!patterns.adminProduct.test(oldPath)) {
-        // create old path /enterprise/<version>/user/foo from new path /enterprise-server@<version>/foo
-        oldPaths.add(currentPath.replace(/\/enterprise-server@(\d.+?)\//, '/enterprise/$1/user/'))
-
-        // create old path /enterprise/user/foo from new path /enterprise-server@<latest>/foo
-        oldPaths.add(currentPath.replace(`/enterprise-server@${latest}/`, '/enterprise/user/'))
-      }
-    })
-  }
-  // ------ END MODERN VERSION FORMAT REPLACEMENTS ------//
-
-  // ------ BEGIN ONEOFF REPLACEMENTS ------//
-
-  // create special old path /enterprise-server-releases from current path /enterprise-server@<release>/admin/all-releases
-  if (
-    versionSatisfiesRange(currentVersion, `=${latest}`) &&
-    currentPath.endsWith('/admin/all-releases')
-  ) {
-    oldPaths.add('/enterprise-server-releases')
-  }
-
-  // ------ END ONEOFF REPLACEMENTS ------//
-
-  // For each old path added to the set above, do the following...
-  new Set(oldPaths).forEach((oldPath) => {
-    // for English only, remove language code
-    if (languageCode === 'en') {
-      oldPaths.add(getPathWithoutLanguage(oldPath))
-    }
-
-    // add language code
-    oldPaths.add(getPathWithLanguage(oldPath, languageCode))
-  })
-
-  // exclude any empty old paths that may have been derived
-  oldPaths.delete('')
-  oldPaths.delete('/')
-
-  return oldPaths
-}

lib/redirects/permalinks.js

@@ -1,77 +1,48 @@
-import path from 'path'
-import patterns from '../patterns.js'
-import { allVersions } from '../all-versions.js'
-import getOldPathsFromPermalink from './get-old-paths-from-permalink.js'
 import { getVersionStringFromPath } from '../path-utils.js'
-import { getNewVersionedPath } from '../old-versions-utils.js'
-import removeFPTFromPath from '../remove-fpt-from-path.js'
-const supportedVersions = new Set(Object.keys(allVersions))
+import nonEnterpriseDefaultVersion from '../non-enterprise-default-version.js'
 
-export default function generateRedirectsForPermalinks(permalinks, redirectFrontmatter, versions) {
-  // account for Array or String frontmatter entries
-  const redirectFrontmatterOldPaths = redirectFrontmatter
-    ? Array.from([redirectFrontmatter]).flat()
-    : []
+export default function generateRedirectsForPermalinks(permalinks, redirectFrontmatter) {
+  if (!Array.isArray(redirectFrontmatter)) {
+    // TypeScript could have prevented this from ever happening.
+    throw new Error(`redirectFrontmatter is supposed to be an array`)
+  }
 
   const redirects = {}
 
-  // for every permalink...
-  permalinks.forEach((permalink) => {
-    // get an array of possible old paths, e.g., /desktop/guides/ from current permalink /desktop/
-    const possibleOldPaths = getOldPathsFromPermalink(
-      permalink.href,
-      permalink.languageCode,
-      permalink.pageVersion
-    )
-
-    // for each old path, add a redirect to the current permalink
-    possibleOldPaths.forEach((oldPath) => {
-      redirects[oldPath] = permalink.href
-    })
-
-    // for every redirect frontmatter old path...
-    redirectFrontmatterOldPaths.forEach((frontmatterOldPath) => {
-      // remove trailing slashes (sometimes present in frontmatter)
-      frontmatterOldPath = frontmatterOldPath.replace(patterns.trailingSlash, '$1')
-
-      // support hardcoded versions in redirect frontmatter
-      if (supportedVersions.has(frontmatterOldPath.split('/')[1])) {
-        redirects[frontmatterOldPath] = permalink.href
-        redirects[`/en${frontmatterOldPath}`] = permalink.href
+  redirectFrontmatter.forEach((frontmatterOldPath) => {
+    if (!frontmatterOldPath.startsWith('/')) {
+      throw new Error(
+        `'${frontmatterOldPath}' is not a valid redirect_from frontmatter value because it doesn't start with a /`
+      )
+    }
+    permalinks.forEach((permalink) => {
+      // Exceptions where the `redirect_from` entries are too old
+      if (frontmatterOldPath.startsWith('/enterprise/admin/guides/')) {
+        // Let's pretend we didn't see that.
+        frontmatterOldPath = ('/' + frontmatterOldPath.split('/').slice(2).join('/')).replace(
+          '/admin/guides/',
+          '/admin/'
+        )
+      } else if (frontmatterOldPath.startsWith('/enterprise/')) {
+        // Let's pretend we didn't see that.
+        frontmatterOldPath = '/' + frontmatterOldPath.split('/').slice(2).join('/')
       }
 
-      // get the old path for the current permalink version
-      let versionedFrontmatterOldPath = path.posix.join(
-        '/',
-        permalink.languageCode,
-        getNewVersionedPath(frontmatterOldPath)
-      )
-      const versionFromPath = getVersionStringFromPath(versionedFrontmatterOldPath)
-      versionedFrontmatterOldPath = removeFPTFromPath(
-        versionedFrontmatterOldPath.replace(versionFromPath, permalink.pageVersion)
-      )
-
-      // add it to the redirects object
-      redirects[versionedFrontmatterOldPath] = permalink.href
-
-      // then get an array of possible alternative old paths from the current versioned old path
-      const possibleOldPathsForVersionedOldPaths = getOldPathsFromPermalink(
-        versionedFrontmatterOldPath,
-        permalink.languageCode,
-        permalink.pageVersion
-      )
-
-      // and add each one to the redirects object
-      possibleOldPathsForVersionedOldPaths.forEach((oldPath) => {
-        redirects[oldPath] = permalink.href
-      })
+      // We're only interested in the version string if it's a supported version.
+      const ver = getVersionStringFromPath(permalink.hrefWithoutLanguage, true)
+      // This tests if the permalink's version was free-pro-team.
+      // If that's the case, put an entry into the `redirects` without
+      // any version prefix.
+      // Some pages don't have a version which means it's supported by all
+      // versions (you'll find `versions: '*'` in frontmatter).
+      // E.g. /en/get-started/learning-about-github
+      if (!ver || ver === nonEnterpriseDefaultVersion) {
+        redirects[frontmatterOldPath] = permalink.hrefWithoutLanguage
+      } else if (ver) {
+        redirects[`/${ver}${frontmatterOldPath}`] = permalink.hrefWithoutLanguage
+      }
     })
   })
 
-  // filter for unique entries only
-  Object.entries(redirects).forEach(([oldPath, newPath]) => {
-    if (oldPath === newPath) delete redirects[oldPath]
-  })
-
   return redirects
 }

lib/redirects/precompile.js

@@ -5,9 +5,9 @@ import { isPromise } from 'util/types'
 import { fileURLToPath } from 'url'
 
 import { readCompressedJsonFileFallback } from '../read-json-file.js'
-import { latest } from '../../lib/enterprise-server-releases.js'
 import getExceptionRedirects from './exception-redirects.js'
-import { languageKeys } from '../languages.js'
+
+import { latest } from '../enterprise-server-releases.js'
 
 function diskMemoize(filePath, asyncFn, maxAgeSeconds = 60 * 60) {
   // The logging that the disk memoizer does is pretty useful to humans,
@@ -35,28 +35,25 @@ function diskMemoize(filePath, asyncFn, maxAgeSeconds = 60 * 60) {
     const promise = asyncFn(...args)
     assert(isPromise(promise), "memoized function didn't return a promise")
     return promise.then(async (value) => {
-      await fs.writeFile(filePath, JSON.stringify(value), 'utf-8')
+      await fs.writeFile(
+        filePath,
+        JSON.stringify(value, undefined, process.env.NODE_ENV === 'development' ? 2 : 0),
+        'utf-8'
+      )
+
       return value
     })
   }
 }
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url))
-const DISK_CACHE_FILEPATH = path.join(__dirname, `.redirects-cache_${languageKeys.join('_')}.json`)
+const DISK_CACHE_FILEPATH = path.join(__dirname, '.redirects-cache.json')
 
 // This function runs at server warmup and precompiles possible redirect routes.
 // It outputs them in key-value pairs within a neat Javascript object: { oldPath: newPath }
 const precompileRedirects = diskMemoize(DISK_CACHE_FILEPATH, async (pageList) => {
   const allRedirects = readCompressedJsonFileFallback('./lib/redirects/static/developer.json')
 
-  // Replace hardcoded 'latest' with real value in the redirected path
-  Object.entries(allRedirects).forEach(([oldPath, newPath]) => {
-    allRedirects[oldPath] = newPath.replace(
-      'enterprise-server@latest',
-      `enterprise-server@${latest}`
-    )
-  })
-
   // Exception redirects are those that are essentially unicorn one-offs.
   // For example, we have redirects for documents that used to be on
   // `free-pro-team@latest` but have now been moved to
@@ -65,16 +62,29 @@ const precompileRedirects = diskMemoize(DISK_CACHE_FILEPATH, async (pageList) =>
   // comments and it's also possible to write 1 destination URL once
   // for each N redirect origins
   const exceptions = getExceptionRedirects()
-  Object.entries(exceptions).forEach(([fromURL, toURL]) => {
-    allRedirects[fromURL] = `/en${toURL}`
-    for (const languageCode of languageKeys) {
-      allRedirects[`/${languageCode}${fromURL}`] = `/${languageCode}${toURL}`
-    }
-  })
+  Object.assign(allRedirects, exceptions)
 
   // CURRENT PAGES PERMALINKS AND FRONTMATTER
   // create backwards-compatible old paths for page permalinks and frontmatter redirects
-  pageList.forEach((page) => Object.assign(allRedirects, page.buildRedirects()))
+  pageList
+    .filter((page) => page.languageCode === 'en')
+    .forEach((page) => Object.assign(allRedirects, page.buildRedirects()))
+
+  Object.entries(allRedirects).forEach(([fromURI, toURI]) => {
+    // If the destination URL has a hardcoded `enterprise-server@latest` in
+    // it we need to rewrite that now.
+    // We never want to redirect to that as the final URL (in the 301 response)
+    // but it might make sense for it to be in the `developer.json`
+    // file since that it static.
+    //
+    //
+    if (toURI.includes('/enterprise-server@latest')) {
+      allRedirects[fromURI] = toURI.replace(
+        '/enterprise-server@latest',
+        `/enterprise-server@${latest}`
+      )
+    }
+  })
 
   return allRedirects
 })

middleware/find-page.js

@@ -1,3 +1,4 @@
+import getRedirect from '../lib/get-redirect.js'
 // This middleware uses the request path to find a page in the preloaded context.pages object
 
 export default async function findPage(req, res, next) {
@@ -16,7 +17,7 @@ export default async function findPage(req, res, next) {
     const englishPath = req.pagePath.replace(new RegExp(`^/${req.language}`), '/en')
     // NOTE the fallback page will have page.languageCode = 'en'
     page = req.context.pages[englishPath]
-    const redirectToPath = req.context.redirects[englishPath]
+    const redirectToPath = getRedirect(englishPath, req.context)
 
     // If the requested translated page has a 1-1 mapping in English,
     // redirect to that English page

middleware/index.js

@@ -214,6 +214,10 @@ export default function (app) {
   app.use(asyncMiddleware(instrument(context, './context'))) // Must come before early-access-*, handle-redirects
   app.use(asyncMiddleware(instrument(shortVersions, './contextualizers/short-versions'))) // Support version shorthands
 
+  // Must come before handleRedirects.
+  // This middleware might either redirect to serve something.
+  app.use(asyncMiddleware(instrument(archivedEnterpriseVersions, './archived-enterprise-versions')))
+
   // *** Redirects, 3xx responses ***
   // I ordered these by use frequency
   app.use(connectSlashes(false))
@@ -237,7 +241,6 @@ export default function (app) {
   // Check for a dropped connection before proceeding (again)
   app.use(haltOnDroppedConnection)
 
-  app.use(asyncMiddleware(instrument(archivedEnterpriseVersions, './archived-enterprise-versions')))
   app.use(instrument(robots, './robots'))
   app.use(
     /(\/.*)?\/early-access$/,

middleware/redirects/handle-redirects.js

@@ -1,6 +1,7 @@
 import patterns from '../../lib/patterns.js'
 import { URL } from 'url'
 import languages, { pathLanguagePrefixed } from '../../lib/languages.js'
+import getRedirect from '../../lib/get-redirect.js'
 import { cacheControlFactory } from '../cache-control.js'
 
 const cacheControl = cacheControlFactory(60 * 60 * 24) // one day
@@ -52,18 +53,42 @@ export default function handleRedirects(req, res, next) {
   // remove query params temporarily so we can find the path in the redirects object
   let redirectWithoutQueryParams = removeQueryParams(redirect)
 
-  // look for a redirect in the global object
-  // for example, given an incoming path /v3/activity/event_types
-  // find /en/developers/webhooks-and-events/github-event-types
-  redirectWithoutQueryParams =
-    req.context.redirects[redirectWithoutQueryParams] || redirectWithoutQueryParams
+  const redirectTo = getRedirect(redirectWithoutQueryParams, req.context)
+
+  redirectWithoutQueryParams = redirectTo || redirectWithoutQueryParams
 
-  // add query params back in
   redirect = queryParams ? redirectWithoutQueryParams + queryParams : redirectWithoutQueryParams
 
+  if (!redirectTo && !pathLanguagePrefixed(req.path)) {
+    // No redirect necessary, but perhaps it's to a known page, and the URL
+    // currently doesn't have a language prefix, then we need to add
+    // the language prefix.
+    // We can't always force on the language prefix because some URLs
+    // aren't pages. They're other middleware endpoints such as
+    // `/healthz` which should never redirect.
+    // But for example, a `/authentication/connecting-to-github-with-ssh`
+    // needs to become `/en/authentication/connecting-to-github-with-ssh`
+    const possibleRedirectTo = `/en${req.path}`
+    if (possibleRedirectTo in req.context.pages) {
+      // As of Jan 2022 we always redirect to `/en` if the URL doesn't
+      // specify a language.  ...except for the root home page (`/`).
+      // It's unfortunate but that's how it currently works.
+      // It's tracked in #1145
+      // Perhaps a more ideal solution would be to do something similar to
+      // the code above for `req.path === '/'` where we look at the user
+      // agent for a header and/or cookie.
+      // Note, it's important to use `req.url` here and not `req.path`
+      // because the full URL can contain query strings.
+      // E.g. `/foo?json=breadcrumbs`
+      redirect = `/en${req.url}`
+    }
+  }
+
   // do not redirect a path to itself
   // req._parsedUrl.path includes query params whereas req.path does not
-  if (redirect === req._parsedUrl.path) return next()
+  if (redirect === req._parsedUrl.path) {
+    return next()
+  }
 
   // do not redirect if the redirected page can't be found
   if (!req.context.pages[removeQueryParams(redirect)]) {

script/rendered-content-link-checker.mjs

@@ -16,6 +16,7 @@ import chalk from 'chalk'
 import shortVersions from '../middleware/contextualizers/short-versions.js'
 import contextualize from '../middleware/context.js'
 import { languageKeys } from '../lib/languages.js'
+import getRedirect from '../lib/get-redirect.js'
 import warmServer from '../lib/warm-server.js'
 import renderContent from '../lib/render-content/index.js'
 import { deprecated } from '../lib/enterprise-server-releases.js'
@@ -389,8 +390,8 @@ function checkHrefLink(href, $, redirects, pageMap, checkAnchors = false) {
         if (!fs.existsSync(staticFilePath)) {
           return { CRITICAL: `Static file not found ${staticFilePath} (${pathname})` }
         }
-      } else if (redirects[pathname]) {
-        return { WARNING: `Redirect to ${redirects[pathname]}` }
+      } else if (getRedirect(pathname, { redirects, pages: pageMap })) {
+        return { WARNING: `Redirect to ${getRedirect(pathname, { redirects, pages: pageMap })}` }
       } else if (!pageMap[pathname]) {
         if (deprecatedVersionPrefixesRegex.test(pathname)) {
           return

script/update-internal-links.js

@@ -28,6 +28,7 @@ import { allVersionKeys } from '../lib/all-versions.js'
 import frontmatter from '../lib/read-frontmatter.js'
 import renderContent from '../lib/render-content/index.js'
 import patterns from '../lib/patterns.js'
+import getRedirect from '../lib/get-redirect.js'
 const __dirname = path.dirname(fileURLToPath(import.meta.url))
 
 const walkFiles = (pathToWalk) => {
@@ -200,10 +201,11 @@ function findPage(tryPath, pageMap, redirects) {
     }
   }
 
-  if (pageMap[redirects[tryPath]]) {
+  const redirect = getRedirect(tryPath, { redirects, pages: pageMap })
+  if (pageMap[redirect]) {
     return {
-      title: pageMap[redirects[tryPath]].title,
-      path: redirects[tryPath],
+      title: pageMap[redirect].title,
+      path: redirect,
     }
   }
 }

tests/helpers/supertest.js

@@ -34,11 +34,17 @@ export const head = (helpers.head = async function (route, opts = { followRedire
 
 export const post = (helpers.post = (route) => request('post', route))
 
-export const getDOM = (helpers.getDOM = async function (route, headers, allow500s = false) {
+export const getDOM = (helpers.getDOM = async function (
+  route,
+  { headers, allow500s, allow404 } = { headers: undefined, allow500s: false, allow404: false }
+) {
   const res = await helpers.get(route, { followRedirects: true, headers })
   if (!allow500s && res.status >= 500) {
     throw new Error(`Server error (${res.status}) on ${route}`)
   }
+  if (!allow404 && res.status === 404) {
+    throw new Error(`Page not found on ${route}`)
+  }
   const $ = cheerio.load(res.text || '', { xmlMode: true })
   $.res = Object.assign({}, res)
   return $
@@ -51,5 +57,8 @@ export const getJSON = (helpers.getJSON = async function (route) {
   if (res.status >= 500) {
     throw new Error(`Server error (${res.status}) on ${route}`)
   }
+  if (res.status >= 400) {
+    console.warn(`${res.status} on ${route} and the response might not be JSON`)
+  }
   return JSON.parse(res.text)
 })

tests/rendering/footer.js

@@ -23,7 +23,7 @@ describe('footer', () => {
     })
 
     test('leads to dotcom support on 404 pages', async () => {
-      const $ = await getDOM('/en/delicious-snacks/donuts.php')
+      const $ = await getDOM('/delicious-snacks/donuts.php', { allow404: true })
       expect($('a#contact-us').attr('href')).toBe('https://support.github.com/contact')
     })
   })

tests/rendering/header.js

@@ -85,54 +85,54 @@ describe('header', () => {
 
     test("renders a link to the same page in user's preferred language, if available", async () => {
       const headers = { 'accept-language': 'ja' }
-      const $ = await getDOM('/en', headers)
+      const $ = await getDOM('/en', { headers })
       expect($('[data-testid=header-notification][data-type=TRANSLATION]').length).toBe(1)
       expect($('[data-testid=header-notification] a[href*="/ja"]').length).toBe(1)
     })
 
     test("renders a link to the same page if user's preferred language is Chinese - PRC", async () => {
       const headers = { 'accept-language': 'zh-CN' }
-      const $ = await getDOM('/en', headers)
+      const $ = await getDOM('/en', { headers })
       expect($('[data-testid=header-notification][data-type=TRANSLATION]').length).toBe(1)
       expect($('[data-testid=header-notification] a[href*="/cn"]').length).toBe(1)
     })
 
     test("does not render a link when user's preferred language is Chinese - Taiwan", async () => {
       const headers = { 'accept-language': 'zh-TW' }
-      const $ = await getDOM('/en', headers)
+      const $ = await getDOM('/en', { headers })
       expect($('[data-testid=header-notification]').length).toBe(0)
     })
 
     test("does not render a link when user's preferred language is English", async () => {
       const headers = { 'accept-language': 'en' }
-      const $ = await getDOM('/en', headers)
+      const $ = await getDOM('/en', { headers })
       expect($('[data-testid=header-notification]').length).toBe(0)
     })
 
     test("renders a link to the same page in user's preferred language from multiple, if available", async () => {
       const headers = { 'accept-language': 'ja, *;q=0.9' }
-      const $ = await getDOM('/en', headers)
+      const $ = await getDOM('/en', { headers })
       expect($('[data-testid=header-notification][data-type=TRANSLATION]').length).toBe(1)
       expect($('[data-testid=header-notification] a[href*="/ja"]').length).toBe(1)
     })
 
     test("renders a link to the same page in user's preferred language with weights, if available", async () => {
       const headers = { 'accept-language': 'ja;q=1.0, *;q=0.9' }
-      const $ = await getDOM('/en', headers)
+      const $ = await getDOM('/en', { headers })
       expect($('[data-testid=header-notification][data-type=TRANSLATION]').length).toBe(1)
       expect($('[data-testid=header-notification] a[href*="/ja"]').length).toBe(1)
     })
 
     test("renders a link to the user's 2nd preferred language if 1st is not available", async () => {
       const headers = { 'accept-language': 'zh-TW,zh;q=0.9,ja *;q=0.8' }
-      const $ = await getDOM('/en', headers)
+      const $ = await getDOM('/en', { headers })
       expect($('[data-testid=header-notification][data-type=TRANSLATION]').length).toBe(1)
       expect($('[data-testid=header-notification] a[href*="/ja"]').length).toBe(1)
     })
 
     test('renders no notices if no language preference is available', async () => {
       const headers = { 'accept-language': 'zh-TW,zh;q=0.9,zh-SG *;q=0.8' }
-      const $ = await getDOM('/en', headers)
+      const $ = await getDOM('/en', { headers })
       expect($('[data-testid=header-notification]').length).toBe(0)
     })
   })

tests/rendering/learning-tracks.js

@@ -52,7 +52,7 @@ describe('navigation banner', () => {
 
   test('render navigation banner when url is a redirect to a learning track URL', async () => {
     const $ = await getDOM(
-      '/enterprise/admin/enterprise-management/enabling-automatic-update-checks?learn=upgrade_your_instance'
+      '/en/enterprise/admin/enterprise-management/enabling-automatic-update-checks?learn=upgrade_your_instance'
     )
     expect($('[data-testid=learning-track-nav]')).toHaveLength(1)
     const $navLinks = $('[data-testid=learning-track-nav] a')

tests/rendering/server.js

@@ -144,7 +144,7 @@ describe('server', () => {
   })
 
   test('renders a 404 page', async () => {
-    const $ = await getDOM('/not-a-real-page')
+    const $ = await getDOM('/not-a-real-page', { allow404: true })
     expect($('h1').text()).toBe('Ooops!')
     expect($.text().includes("It looks like this page doesn't exist.")).toBe(true)
     expect(
@@ -162,17 +162,17 @@ describe('server', () => {
 
   // see issue 12427
   test('renders a 404 for leading slashes', async () => {
-    let $ = await getDOM('//foo.com/enterprise')
+    let $ = await getDOM('//foo.com/enterprise', { allow404: true })
     expect($('h1').text()).toBe('Ooops!')
     expect($.res.statusCode).toBe(404)
 
-    $ = await getDOM('///foo.com/enterprise')
+    $ = await getDOM('///foo.com/enterprise', { allow404: true })
     expect($('h1').text()).toBe('Ooops!')
     expect($.res.statusCode).toBe(404)
   })
 
   test('renders a 500 page when errors are thrown', async () => {
-    const $ = await getDOM('/_500', undefined, true)
+    const $ = await getDOM('/_500', { allow500s: true })
     expect($('h1').text()).toBe('Ooops!')
     expect($.text().includes('It looks like something went wrong.')).toBe(true)
     expect(
@@ -578,8 +578,10 @@ describe('server', () => {
 
     test('is not displayed if ghec article has only one version', async () => {
       const $ = await getDOM(
-        '/en/enterprise-cloud@latest/admin/managing-your-enterprise-users-with-your-identity-provider/about-enterprise-managed-users'
+        '/en/enterprise-cloud@latest/admin/managing-your-enterprise-users-with-your-identity-provider/about-enterprise-managed-users',
+        { allow404: true }
       )
+      expect($.res.statusCode).toBe(404)
       expect($('.article-versions').length).toBe(0)
     })
   })

tests/routing/redirects.js

@@ -4,7 +4,6 @@ import { isPlainObject } from 'lodash-es'
 import supertest from 'supertest'
 import createApp from '../../lib/app.js'
 import enterpriseServerReleases from '../../lib/enterprise-server-releases.js'
-import nonEnterpriseDefaultVersion from '../../lib/non-enterprise-default-version.js'
 import Page from '../../lib/page.js'
 import { get } from '../helpers/supertest.js'
 import versionSatisfiesRange from '../../lib/version-satisfies-range.js'
@@ -21,38 +20,34 @@ describe('redirects', () => {
     redirects = JSON.parse(res.text)
   })
 
-  test('page.redirects is an array', async () => {
+  test('page.buildRedirects() returns an array', async () => {
     const page = await Page.init({
       relativePath:
         'pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-branches.md',
       basePath: path.join(__dirname, '../../content'),
       languageCode: 'en',
     })
-    page.buildRedirects()
-    expect(isPlainObject(page.redirects)).toBe(true)
+    const pageRedirects = page.buildRedirects()
+    expect(isPlainObject(pageRedirects)).toBe(true)
   })
 
-  test('dotcom homepage page.redirects', async () => {
+  test('dotcom homepage page.buildRedirects()', async () => {
     const page = await Page.init({
       relativePath: 'github/index.md',
       basePath: path.join(__dirname, '../../content'),
       languageCode: 'en',
     })
-    page.buildRedirects()
-    expect(page.redirects[`/en/${nonEnterpriseDefaultVersion}/github`]).toBe('/en/github')
-    expect(page.redirects['/articles']).toBe('/en/github')
-    expect(page.redirects['/en/articles']).toBe('/en/github')
-    expect(page.redirects[`/en/${nonEnterpriseDefaultVersion}/articles`]).toBe('/en/github')
-    expect(page.redirects['/common-issues-and-questions']).toBe('/en/github')
-    expect(page.redirects['/en/common-issues-and-questions']).toBe('/en/github')
-    expect(page.redirects[`/en/enterprise/${enterpriseServerReleases.latest}/user/articles`]).toBe(
-      `/en/enterprise-server@${enterpriseServerReleases.latest}/github`
+    const pageRedirects = page.buildRedirects()
+    expect(pageRedirects['/articles']).toBe('/github')
+    expect(pageRedirects['/common-issues-and-questions']).toBe('/github')
+    expect(pageRedirects[`/enterprise-server@${enterpriseServerReleases.latest}/articles`]).toBe(
+      `/enterprise-server@${enterpriseServerReleases.latest}/github`
     )
     expect(
-      page.redirects[
-        `/en/enterprise/${enterpriseServerReleases.latest}/user/common-issues-and-questions`
+      pageRedirects[
+        `/enterprise-server@${enterpriseServerReleases.latest}/common-issues-and-questions`
       ]
-    ).toBe(`/en/enterprise-server@${enterpriseServerReleases.latest}/github`)
+    ).toBe(`/enterprise-server@${enterpriseServerReleases.latest}/github`)
   })
 
   test('converts single `redirect_from` strings values into arrays', async () => {
@@ -61,8 +56,8 @@ describe('redirects', () => {
       basePath: path.join(__dirname, '../fixtures'),
       languageCode: 'en',
     })
-    page.buildRedirects()
-    expect(page.redirects['/redirect-string']).toBe('/en/article-with-redirect-from-string')
+    const pageRedirects = page.buildRedirects()
+    expect(pageRedirects['/redirect-string']).toBe('/article-with-redirect-from-string')
   })
 
   describe('query params', () => {
@@ -101,12 +96,6 @@ describe('redirects', () => {
       const res = await get(reqPath)
       expect(res.statusCode).toBe(200)
     })
-
-    test('work on deprecated versions', async () => {
-      const res = await get('/enterprise/2.12/admin/search?utf8=%E2%9C%93&q=pulls')
-      expect(res.statusCode).toBe(301)
-      expect(res.headers.location).toBe('/enterprise/2.12/admin/search?utf8=%E2%9C%93&query=pulls')
-    })
   })
 
   describe('trailing slashes', () => {

tests/unit/get-redirect.js

@@ -0,0 +1,151 @@
+import getRedirect from '../../lib/get-redirect.js'
+import { latest } from '../../lib/enterprise-server-releases.js'
+
+describe('getRedirect basics', () => {
+  it('should sometimes not correct the version prefix', () => {
+    // This essentially tests legacy entries that come from the
+    // `developer.json` file. Normally, we would have first
+    // rewritten `/enterprise/3.0` to `/enterprise-server@3.0`
+    // and then, from there, worried about the remaining `/foo/bar`
+    // part.
+    // But some redirects from `developer.json` as old and static.
+    const uri = '/enterprise/3.0/foo/bar'
+    const ctx = {
+      pages: {},
+      redirects: {
+        '/enterprise/3.0/foo/bar': '/something/else',
+      },
+    }
+    expect(getRedirect(uri, ctx)).toBe('/en/something/else')
+  })
+
+  it('should return undefined if nothing could be found', () => {
+    const ctx = {
+      pages: {},
+      redirects: {},
+    }
+    expect(getRedirect('/foo/pizza', ctx)).toBeUndefined()
+  })
+
+  it('should just inject language on version "home pages"', () => {
+    const ctx = {
+      pages: {},
+      redirects: {},
+    }
+    expect(getRedirect('/github-ae@latest', ctx)).toBe('/en/github-ae@latest')
+
+    expect(getRedirect('/enterprise-cloud@latest', ctx)).toBe('/en/enterprise-cloud@latest')
+
+    expect(getRedirect('/enterprise-server@3.3', ctx)).toBe('/en/enterprise-server@3.3')
+
+    expect(getRedirect('/enterprise-server@latest', ctx)).toBe(`/en/enterprise-server@${latest}`)
+    expect(getRedirect('/enterprise-server', ctx)).toBe(`/en/enterprise-server@${latest}`)
+  })
+
+  it('should always "remove" the free-pro-team prefix', () => {
+    const ctx = {
+      pages: {},
+      redirects: {
+        '/foo': '/bar',
+      },
+    }
+    expect(getRedirect('/free-pro-team@latest', ctx)).toBe('/en')
+    // Language is fine, but the version needs to be "removed"
+    expect(getRedirect('/en/free-pro-team@latest', ctx)).toBe('/en')
+    expect(getRedirect('/free-pro-team@latest/pizza', ctx)).toBe('/en/pizza')
+    expect(getRedirect('/free-pro-team@latest/foo', ctx)).toBe('/en/bar')
+    expect(getRedirect('/free-pro-team@latest/github', ctx)).toBe('/en/github')
+  })
+
+  it('should handle some odd exceptions', () => {
+    const ctx = {
+      pages: {},
+      redirects: {},
+    }
+    expect(getRedirect('/desktop/guides/foo/bar', ctx)).toBe('/en/desktop/foo/bar')
+    expect(getRedirect('/admin/guides/foo/bar', ctx)).toBe(
+      `/en/enterprise-server@${latest}/admin/foo/bar`
+    )
+    expect(getRedirect('/admin/something/else', ctx)).toBe(
+      `/en/enterprise-server@${latest}/admin/something/else`
+    )
+    expect(getRedirect('/insights/stuff', ctx)).toBe(
+      `/en/enterprise-server@${latest}/insights/stuff`
+    )
+  })
+
+  it('should figure out redirect based on presence of pages in certain cases', () => {
+    const ctx = {
+      pages: {
+        '/en/enterprise-server@3.2/foo/bar': null,
+        '/en/enterprise-server@3.2/admin/github-management': null,
+      },
+      redirects: {},
+    }
+    // Replacing `/user` with `` worked because there exits a page of such name.
+    expect(getRedirect('/enterprise-server@3.2/user/foo/bar', ctx)).toBe(
+      '/en/enterprise-server@3.2/foo/bar'
+    )
+    expect(getRedirect('/enterprise-server@3.2/admin/guides/user-management', ctx)).toBe(
+      '/en/enterprise-server@3.2/admin/github-management'
+    )
+  })
+
+  it('should always correct the old enterprise prefix', () => {
+    const ctx = {
+      pages: {},
+      redirects: {
+        '/enterprise-server@3.3/foo': '/enterprise-server@3.3/bar',
+      },
+    }
+    expect(getRedirect('/enterprise', ctx)).toBe(`/en/enterprise-server@${latest}`)
+    expect(getRedirect('/enterprise/3.3', ctx)).toBe('/en/enterprise-server@3.3')
+    expect(getRedirect('/enterprise/3.3/something', ctx)).toBe(
+      '/en/enterprise-server@3.3/something'
+    )
+    // but also respect redirects if there are some
+    expect(getRedirect('/enterprise/3.3/foo', ctx)).toBe('/en/enterprise-server@3.3/bar')
+
+    // Unique snowflake pattern
+    expect(getRedirect('/enterprise/github/admin/foo', ctx)).toBe(
+      `/en/enterprise-server@${latest}/github/admin/foo`
+    )
+  })
+
+  it('should not do anything on some prefixes', () => {
+    const ctx = {
+      pages: {},
+      redirects: {},
+    }
+    // Nothing's needed here because it's not /admin/guides and
+    // it already has the enterprise-server prefix.
+    expect(getRedirect(`/en/enterprise-server@${latest}/admin/something/else`, ctx)).toBeUndefined()
+    expect(getRedirect(`/en/enterprise-cloud@latest/user/foo`, ctx)).toBeUndefined()
+  })
+
+  it('should only inject language sometimes', () => {
+    const ctx = {
+      pages: {},
+      redirects: {
+        '/foo': '/bar',
+      },
+    }
+    // Nothing's needed here because it's not /admin/guides and
+    // it already has the enterprise-server prefix.
+    expect(getRedirect('/foo', ctx)).toBe('/en/bar')
+    expect(getRedirect('/en/foo', ctx)).toBe('/en/bar')
+    expect(getRedirect('/ja/foo', ctx)).toBe('/ja/bar')
+  })
+
+  it('should redirect both the prefix and the path needs to change', () => {
+    const ctx = {
+      pages: {},
+      redirects: {
+        [`/enterprise-server@${latest}/foo`]: `/enterprise-server@${latest}/bar`,
+      },
+    }
+    // Nothing's needed here because it's not /admin/guides and
+    // it already has the enterprise-server prefix.
+    expect(getRedirect('/enterprise-server/foo', ctx)).toBe(`/en/enterprise-server@${latest}/bar`)
+  })
+})