Skip to content

Commit 07c8fc3

Browse files
authored
Decouple redirects from language (github#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
1 parent b381520 commit 07c8fc3

File tree

29 files changed

+5169
-14292
lines changed

29 files changed

+5169
-14292
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,4 @@ mkdir translations
2020
# front-matter will be at play.
2121
# These static redirects json files are notoriously large
2222
echo '[]' > lib/redirects/static/archived-frontmatter-fallbacks.json
23-
echo '{}' > lib/redirects/static/developer.json
2423
echo '{}' > lib/redirects/static/archived-redirects-from-213-to-217.json

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ coverage/
1616
blc_output.log
1717
blc_output_internal.log
1818
broken_links.md
19-
lib/redirects/.redirects-cache_*.json
19+
lib/redirects/.redirects-cache.json
2020

2121
# During the preview deploy untrusted user code may be cloned into this directory
2222
# We ignore it from git to keep things deterministic

content/admin/index.md

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,7 @@ redirect_from:
1010
- /github/installing-and-configuring-github-insights/key-metrics-for-collaboration-in-pull-requests
1111
- /github/installing-and-configuring-github-insights/viewing-and-filtering-key-metrics-and-reports
1212
- /github/installing-and-configuring-github-insights/github-insights-and-data-protection-for-your-organization
13-
- /enterprise-server@2.22/github/site-policy/github-insights-and-data-protection-for-your-organization
14-
- /enterprise-server@2.21/github/site-policy/github-insights-and-data-protection-for-your-organization
15-
- /enterprise-server@2.20/github/site-policy/github-insights-and-data-protection-for-your-organization
13+
- /github/site-policy/github-insights-and-data-protection-for-your-organization
1614
- /insights/installing-and-configuring-github-insights/configuring-the-connection-between-github-insights-and-github-enterprise
1715
- /github/installing-and-configuring-github-insights/navigating-between-github-insights-and-github-enterprise
1816
- /github/installing-and-configuring-github-insights/enabling-a-link-between-github-insights-and-github-enterprise
@@ -134,4 +132,3 @@ children:
134132
- /release-notes
135133
- /all-releases
136134
---
137-

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ redirect_from:
1010
- /github/articles/managing-allowed-ip-addresses-for-organizations-in-your-enterprise-account
1111
- /github/setting-up-and-managing-your-enterprise-account/enforcing-security-settings-in-your-enterprise-account
1212
- /github/setting-up-and-managing-your-enterprise/enforcing-security-settings-in-your-enterprise-account
13-
- github/setting-up-and-managing-your-enterprise/setting-policies-for-organizations-in-your-enterprise-account/enforcing-security-settings-in-your-enterprise-account
13+
- /github/setting-up-and-managing-your-enterprise/setting-policies-for-organizations-in-your-enterprise-account/enforcing-security-settings-in-your-enterprise-account
1414
versions:
1515
ghec: '*'
1616
ghes: '*'
@@ -73,7 +73,7 @@ Enterprise owners can restrict access to assets owned by organizations in an ent
7373

7474
{% data reusables.identity-and-permissions.ip-allow-lists-cidr-notation %}
7575

76-
{% data reusables.identity-and-permissions.ip-allow-lists-enable %} {% data reusables.identity-and-permissions.ip-allow-lists-enterprise %}
76+
{% data reusables.identity-and-permissions.ip-allow-lists-enable %} {% data reusables.identity-and-permissions.ip-allow-lists-enterprise %}
7777

7878
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)."
7979

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
title: About Git rebase
33
redirect_from:
44
- /rebase
5-
- articles/interactive-rebase/
5+
- /articles/interactive-rebase
66
- /articles/about-git-rebase
77
- /github/using-git/about-git-rebase
88
- /github/getting-started-with-github/about-git-rebase

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ title: Managing scheduled reminders for your team
33
intro: You can get reminders in Slack when your team has pull requests waiting for review.
44
redirect_from:
55
- /github/setting-up-and-managing-organizations-and-teams/managing-scheduled-reminders-for-pull-requests
6-
- /github/setting-up-and-managing-organizations-and-teams/managing-scheduled-reminders-for-your team
6+
- /github/setting-up-and-managing-organizations-and-teams/managing-scheduled-reminders-for-your-team
77
versions:
88
fpt: '*'
99
ghec: '*'

lib/find-page.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
11
import { getLanguageCode } from './patterns.js'
2+
import getRedirect from './get-redirect.js'
23

34
export default function findPage(href, pageMap, redirects) {
45
// remove any fragments
56
href = href.replace(/#.*$/, '')
67

8+
const redirectsContext = { redirects, pages: pageMap }
9+
710
// find the page
8-
const page = pageMap[href] || pageMap[redirects[href]]
11+
const page = pageMap[href] || pageMap[getRedirect(href, redirectsContext)]
912
if (page) return page
1013

1114
// get the current language
1215
const currentLang = getLanguageCode.test(href) ? href.match(getLanguageCode)[1] : 'en'
1316

1417
// try to fall back to English if the translated page can't be found
1518
const englishHref = href.replace(`/${currentLang}/`, '/en/')
16-
return pageMap[englishHref] || pageMap[redirects[englishHref]]
19+
return pageMap[englishHref] || pageMap[getRedirect(englishHref, redirectsContext)]
1720
}

lib/get-redirect.js

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
import { languageKeys } from './languages.js'
2+
import nonEnterpriseDefaultVersion from './non-enterprise-default-version.js'
3+
4+
import { allVersions } from './all-versions.js'
5+
import { latest, supported } from './enterprise-server-releases.js'
6+
7+
const languagePrefixRegex = new RegExp(`^/(${languageKeys.join('|')})/`)
8+
const nonEnterpriseDefaultVersionPrefix = `/${nonEnterpriseDefaultVersion}`
9+
10+
// Return the new URI if there is one, otherwise return undefined.
11+
export default function getRedirect(uri, context) {
12+
const { redirects, pages } = context
13+
14+
let language = 'en'
15+
let withoutLanguage = uri
16+
if (languagePrefixRegex.test(uri)) {
17+
language = uri.match(languagePrefixRegex)[1]
18+
withoutLanguage = uri.replace(languagePrefixRegex, '/')
19+
}
20+
21+
let destination
22+
23+
// `redirects` is sourced from more than one thing. The primary use
24+
// case is gathering up the `redirect_from` frontmatter key.
25+
// But we also has `developer.json` which contains legacy redirects.
26+
// For example, the `developer.json` will have entries such
27+
// `/enterprise/v4/enum/auditlogorderfield` which clearly is using
28+
// the old formatting of the version. So to leverage the redirects
29+
// from `developer.json` we'll look at it right away.
30+
if (withoutLanguage in redirects) {
31+
return `/${language}` + redirects[withoutLanguage]
32+
}
33+
34+
let basicCorrection
35+
36+
if (withoutLanguage.startsWith(nonEnterpriseDefaultVersionPrefix)) {
37+
// E.g. '/free-pro-team@latest/foo/bar' or '/free-pro-team@latest'
38+
basicCorrection =
39+
`/${language}` + withoutLanguage.replace(nonEnterpriseDefaultVersionPrefix, '')
40+
} else if (withoutLanguage.replace('/', '') in allVersions && !languagePrefixRegex.test(uri)) {
41+
// E.g. just '/github-ae@latest' or '/enterprise-cloud@latest'
42+
basicCorrection = `/${language}` + withoutLanguage
43+
return basicCorrection
44+
}
45+
46+
if (
47+
withoutLanguage === '/enterprise-server' ||
48+
withoutLanguage.startsWith('/enterprise-server/')
49+
) {
50+
// E.g. '/enterprise-server' or '/enterprise-server/3.0/foo'
51+
basicCorrection =
52+
`/${language}` + withoutLanguage.replace('/enterprise-server', `/enterprise-server@${latest}`)
53+
// If it's now just the version, without anything after, exit here
54+
if (withoutLanguage === '/enterprise-server') {
55+
return basicCorrection
56+
}
57+
// console.log({ basicCorrection })
58+
} else if (withoutLanguage.startsWith('/enterprise-server@latest')) {
59+
// E.g. '/enterprise-server@latest' or '/enterprise-server@latest/3.3/foo'
60+
basicCorrection =
61+
`/${language}` +
62+
withoutLanguage.replace('/enterprise-server@latest', `/enterprise-server@${latest}`)
63+
// If it was *just* '/enterprise-server@latest' all that's needed is
64+
// the language but with 'latest' replaced with the value of `latest`
65+
if (withoutLanguage === '/enterprise-server@latest') {
66+
return basicCorrection
67+
}
68+
} else if (
69+
withoutLanguage.startsWith('/enterprise/') &&
70+
supported.includes(withoutLanguage.split('/')[2])
71+
) {
72+
// E.g. '/enterprise/3.3' or '/enterprise/3.3/foo'
73+
74+
// If the URL is without a language, and no redirect is necessary,
75+
// but it has as version prefix, the language has to be there
76+
// otherwise it will never be found in `req.context.pages`
77+
const version = withoutLanguage.split('/')[2]
78+
if (withoutLanguage === `/enterprise/${version}`) {
79+
// E.g. `/enterprise/3.0`
80+
basicCorrection =
81+
`/${language}` +
82+
withoutLanguage.replace(`/enterprise/${version}`, `/enterprise-server@${version}`)
83+
return basicCorrection
84+
} else {
85+
basicCorrection =
86+
`/${language}` +
87+
withoutLanguage.replace(`/enterprise/${version}/`, `/enterprise-server@${version}/`)
88+
}
89+
} else if (withoutLanguage === '/enterprise') {
90+
// E.g. `/enterprise` exactly
91+
basicCorrection = `/${language}/enterprise-server@${latest}`
92+
return basicCorrection
93+
} else if (
94+
withoutLanguage.startsWith('/enterprise/') &&
95+
!supported.includes(withoutLanguage.split('/')[2])
96+
) {
97+
// E.g. '/en/enterprise/user/github/foo'
98+
// If the URL is without a language, and no redirect is necessary,
99+
// but it has as version prefix, the language has to be there
100+
// otherwise it will never be found in `req.context.pages`
101+
basicCorrection =
102+
`/${language}` +
103+
withoutLanguage
104+
.replace(`/enterprise/`, `/enterprise-server@${latest}/`)
105+
.replace('/user/', '/')
106+
} else if (withoutLanguage.startsWith('/insights')) {
107+
// E.g. '/insights/foo'
108+
basicCorrection = uri.replace('/insights', `${language}/enterprise-server@${latest}/insights`)
109+
}
110+
111+
if (basicCorrection) {
112+
return (
113+
getRedirect(basicCorrection, {
114+
redirects,
115+
pages,
116+
}) || basicCorrection
117+
)
118+
}
119+
120+
if (withoutLanguage.startsWith('/admin/')) {
121+
const prefix = `/enterprise-server@${latest}`
122+
let suffix = withoutLanguage
123+
if (suffix.startsWith('/admin/guides/')) {
124+
suffix = suffix.replace('/admin/guides/', '/admin/')
125+
}
126+
const newURL = prefix + suffix
127+
destination = redirects[newURL] || newURL
128+
} else if (
129+
withoutLanguage.split('/')[1].includes('@') &&
130+
withoutLanguage.split('/')[1] in allVersions
131+
) {
132+
// E.g. '/enterprise-server@latest' or '/github-ae@latest' or '/enterprise-server@3.3'
133+
const majorVersion = withoutLanguage.split('/')[1].split('@')[0]
134+
const split = withoutLanguage.split('/')
135+
const version = split[1].split('@')[1]
136+
let prefix
137+
let suffix
138+
139+
if (supported.includes(version) || version === 'latest') {
140+
prefix = `/${majorVersion}@${version}`
141+
suffix = '/' + split.slice(2).join('/')
142+
143+
if (
144+
suffix.includes('/user') ||
145+
suffix.startsWith('/admin/guide') ||
146+
suffix.startsWith('/articles/user')
147+
) {
148+
suffix = tryReplacements(prefix, suffix, context) || suffix
149+
}
150+
}
151+
152+
const newURL = prefix + suffix
153+
if (newURL !== withoutLanguage) {
154+
// At least the prefix changed!
155+
destination = redirects[newURL] || newURL
156+
} else {
157+
destination = redirects[newURL]
158+
}
159+
} else if (withoutLanguage.startsWith('/desktop/guides/')) {
160+
// E.g. /desktop/guides/contributing-and-collaborat
161+
const newURL = withoutLanguage.replace('/desktop/guides/', '/desktop/')
162+
destination = redirects[newURL] || newURL
163+
} else {
164+
destination = redirects[withoutLanguage]
165+
}
166+
167+
if (destination !== undefined) {
168+
// There's hope! Now we just need to attach the correct language
169+
// to the destination URL.
170+
return `/${language}${destination}`
171+
}
172+
}
173+
174+
// Over time, we've developed multiple ambiguous patterns of URLs
175+
// You can't simply assume that all `/admin/guides` should become
176+
// `/admin` for example.
177+
// This function tries different string replacement on the suffix
178+
// (the pathname after the language and version part) until it
179+
// finds one string replacement that yields either a page or a redirect.
180+
function tryReplacements(prefix, suffix, { pages, redirects }) {
181+
const test = (suffix) => {
182+
const candidateAsRedirect = prefix + suffix
183+
const candidateAsURL = '/en' + candidateAsRedirect
184+
return candidateAsRedirect in redirects || candidateAsURL in pages
185+
}
186+
187+
let attempt = suffix.replace('/user', '/github')
188+
if (test(attempt)) return attempt
189+
190+
attempt = suffix.replace('/user', '')
191+
if (test(attempt)) return attempt
192+
193+
attempt = suffix.replace('/admin/guides', '/admin')
194+
if (test(attempt)) return attempt
195+
196+
attempt = suffix.replace('/admin/guides/user', '/admin/github')
197+
if (test(attempt)) return attempt
198+
199+
attempt = suffix.replace('/admin/guides', '/admin').replace('/user', '/github')
200+
if (test(attempt)) return attempt
201+
}

lib/languages.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,12 @@ if (process.env.ENABLED_LANGUAGES) {
5353

5454
export const languageKeys = Object.keys(languages)
5555

56-
export const languagePrefixPathRegex = new RegExp(`^/(${languageKeys.join('|')})/`)
56+
export const languagePrefixPathRegex = new RegExp(`^/(${languageKeys.join('|')})(/|$)`)
5757

58+
/** Return true if the URL is something like /en/foo or /ja but return false
59+
* if it's something like /foo or /foo/bar or /fr (because French (fr)
60+
* is currently not an active language)
61+
*/
5862
export function pathLanguagePrefixed(path) {
5963
return languagePrefixPathRegex.test(path)
6064
}

lib/page.js

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -135,14 +135,16 @@ class Page {
135135
}
136136

137137
buildRedirects() {
138-
// create backwards-compatible old paths for page permalinks and frontmatter redirects
139-
this.redirects = generateRedirectsForPermalinks(
140-
this.permalinks,
141-
this.redirect_from,
142-
this.applicableVersions
143-
)
138+
if (!this.redirect_from) {
139+
return {}
140+
}
141+
// this.redirect_from comes from frontmatter Yaml
142+
// E.g `redirect_from: /old/path`
143+
const redirectFrontmatter = Array.isArray(this.redirect_from)
144+
? this.redirect_from
145+
: [this.redirect_from]
144146

145-
return this.redirects
147+
return generateRedirectsForPermalinks(this.permalinks, redirectFrontmatter)
146148
}
147149

148150
// Infer the parent product ID from the page's relative file path

0 commit comments

Comments
 (0)