Merge pull request #19046 from github/repo-sync

repo sync

Author: Octomerger

middleware/redirects/handle-redirects.js

@@ -1,6 +1,7 @@
 import patterns from '../../lib/patterns.js'
 import { URL } from 'url'
 import { pathLanguagePrefixed } from '../../lib/languages.js'
+import { deprecatedWithFunctionalRedirects } from '../../lib/enterprise-server-releases.js'
 import getRedirect from '../../lib/get-redirect.js'
 import { cacheControlFactory } from '../cache-control.js'
 
@@ -65,7 +66,7 @@ export default function handleRedirects(req, res, next) {
     // 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) {
+    if (possibleRedirectTo in req.context.pages || isDeprecatedAdminReleaseNotes(req.path)) {
       const language = getLanguage(req)
 
       // Note, it's important to use `req.url` here and not `req.path`
@@ -82,7 +83,10 @@ export default function handleRedirects(req, res, next) {
   }
 
   // do not redirect if the redirected page can't be found
-  if (!req.context.pages[removeQueryParams(redirect)] && !redirect.includes('://')) {
+  if (
+    !(req.context.pages[removeQueryParams(redirect)] || isDeprecatedAdminReleaseNotes(req.path)) &&
+    !redirect.includes('://')
+  ) {
     // display error on the page in development, but not in production
     // include final full redirect path in the message
     if (process.env.NODE_ENV !== 'production' && req.context) {
@@ -134,3 +138,20 @@ function usePermanentRedirect(req) {
 function removeQueryParams(redirect) {
   return new URL(redirect, 'https://docs.github.com').pathname
 }
+
+function isDeprecatedAdminReleaseNotes(path) {
+  // When we rewrote how redirects work, from a lookup model to a
+  // functional model, the enterprise-server releases that got
+  // deprecated since then fall between the cracks. Especially
+  // for custom NextJS page-like pages like /admin/release-notes
+  // These URLs don't come from any remaining .json lookup file
+  // and they're not active pages either (e.g. req.context.pages)
+  if (path.includes('admin/release-notes')) {
+    for (const version of deprecatedWithFunctionalRedirects) {
+      if (path.includes(`enterprise-server@${version}`)) {
+        return true
+      }
+    }
+  }
+  return false
+}

tests/routing/redirects.js

@@ -3,7 +3,9 @@ import path from 'path'
 import { isPlainObject } from 'lodash-es'
 import { describe, expect, jest, test } from '@jest/globals'
 
-import enterpriseServerReleases from '../../lib/enterprise-server-releases.js'
+import enterpriseServerReleases, {
+  deprecatedWithFunctionalRedirects,
+} from '../../lib/enterprise-server-releases.js'
 import Page from '../../lib/page.js'
 import { get, head } from '../helpers/e2etest.js'
 import versionSatisfiesRange from '../../lib/version-satisfies-range.js'
@@ -564,6 +566,27 @@ describe('redirects', () => {
     })
   })
 
+  describe('admin/release-notes redirects that lack language', () => {
+    test('test redirect to an active enterprise-server version', async () => {
+      const url = `/enterprise-server@${enterpriseServerReleases.latest}/admin/release-notes`
+      const res = await get(url)
+      expect(res.statusCode).toBe(302)
+      expect(res.headers.location).toBe(`/en${url}`)
+    })
+    test('test redirect to a deprecated old enterprise-server version', async () => {
+      const url = `/enterprise-server@2.22/admin/release-notes`
+      const res = await get(url)
+      expect(res.statusCode).toBe(302)
+      expect(res.headers.location).toBe(`/en${url}`)
+    })
+    test('test redirect to a recently deprecated enterprise-server version', async () => {
+      const url = `/enterprise-server@${deprecatedWithFunctionalRedirects[0]}/admin/release-notes`
+      const res = await get(url)
+      expect(res.statusCode).toBe(302)
+      expect(res.headers.location).toBe(`/en${url}`)
+    })
+  })
+
   // These tests exists because of issue #1960
   describe('rest reference redirects with default product', () => {
     test('rest subcategory with fpt in URL', async () => {