From cf7ffc9cfbb1b777ae5a1718cb0cb2411db1d3a0 Mon Sep 17 00:00:00 2001 From: Sarah Schneider Date: Fri, 2 Oct 2020 19:36:23 -0400 Subject: [PATCH] Fix bug in path handler (#15858) * minor clarification in comment * handle scenario where a new version has been injected into an old path * update oldEnterprisePath regex and comment * lint --- lib/old-versions-utils.js | 26 +++++++++++++++++++++----- lib/path-utils.js | 5 +++-- lib/patterns.js | 6 ++++-- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/lib/old-versions-utils.js b/lib/old-versions-utils.js index e49cf493bdc6..f17ad8c4c34c 100644 --- a/lib/old-versions-utils.js +++ b/lib/old-versions-utils.js @@ -15,7 +15,8 @@ const newVersions = Object.keys(require('./all-versions')) // Given a new version like enterprise-server@2.21, // return an old version like 2.21. -// Fall back to latest version if one can't be found. +// Fall back to latest GHES version if one can't be found, +// for example, if the new version is private-instances@latest. function getOldVersionFromNewVersion (newVersion) { return newVersion === nonEnterpriseDefaultVersion ? 'dotcom' @@ -24,7 +25,7 @@ function getOldVersionFromNewVersion (newVersion) { // Given an old version like 2.21, // return a new version like enterprise-server@2.21. -// Fall back to latest version if one can't be found. +// Fall back to latest GHES version if one can't be found. function getNewVersionFromOldVersion (oldVersion) { return oldVersion === 'dotcom' ? nonEnterpriseDefaultVersion @@ -33,8 +34,11 @@ function getNewVersionFromOldVersion (oldVersion) { // Given an old path like /enterprise/2.21/user/github/category/article, // return an old version like 2.21. -function getOldVersionFromOldPath (oldPath) { +function getOldVersionFromOldPath (oldPath, languageCode) { + // We should never be calling this function on a path that starts with a new version, + // so we can assume the path either uses the old /enterprise format or it's dotcom. if (!patterns.enterprise.test(oldPath)) return 'dotcom' + const ghesNumber = oldPath.match(patterns.getEnterpriseVersionNumber) return ghesNumber ? ghesNumber[1] : latest } @@ -42,8 +46,20 @@ function getOldVersionFromOldPath (oldPath) { // Given an old path like /en/enterprise/2.21/user/github/category/article, // return a new path like /en/enterprise-server@2.21/github/category/article. function getNewVersionedPath (oldPath, languageCode = '') { - const oldVersion = getOldVersionFromOldPath(oldPath) - const newVersion = getNewVersionFromOldVersion(oldVersion) + // It's possible a new version has been injected into an old path + // via syntax like: /en/enterprise/{{ currentVersion }}/admin/category/article + // which could resolve to /en/enterprise/private-instances@latest/admin/category/article, + // in which case the new version is the `private-instances@latest` segment. + // Get the second or third segment depending on whether there is a lang code. + const pathParts = oldPath.split('/') + const possibleVersion = languageCode ? pathParts[3] : pathParts[2] + let newVersion = newVersions.includes(possibleVersion) ? possibleVersion : '' + + // If no new version was found, assume path contains an old version, like 2.21 + if (!newVersion) { + const oldVersion = getOldVersionFromOldPath(oldPath, languageCode) + newVersion = getNewVersionFromOldVersion(oldVersion) + } // Remove /?/enterprise?/?/user? if present. // This leaves only the part of the string that starts with the product. diff --git a/lib/path-utils.js b/lib/path-utils.js index c062ab8d59e7..f180d28f07e2 100644 --- a/lib/path-utils.js +++ b/lib/path-utils.js @@ -22,8 +22,9 @@ function getVersionedPathWithoutLanguage (href, version) { // example: enterprise-server@2.22 or free-pro-team@latest let versionFromPath = getVersionStringFromPath(href) - // if this is an old versioned path, convert to new versioned path - // OLD: /enterprise/2.22/admin/installation or /enterprise/admin/installation + // if versionFromPath doesn't match any current versions, this may be an old + // versioned path that should be converted to new versioned path. Examples: + // OLD: /enterprise/2.22/admin/installation OR /enterprise/admin/installation // NEW: /enterprise-server@2.22/admin/installation // OLD: /desktop/installing-and-configuring-github-desktop // NEW: /free-pro-team@latest/desktop/installing-and-configuring-github-desktop diff --git a/lib/patterns.js b/lib/patterns.js index 0696818c65c7..44a621ea0c71 100644 --- a/lib/patterns.js +++ b/lib/patterns.js @@ -33,8 +33,10 @@ module.exports = { oldApiPath: /\/v[34]\/(?!guides|overview).+?\/.+/, staticRedirect: //, enterpriseNoVersion: /\/enterprise\/([^\d].*$)/, - // currentVersion in Liquid statements may inject 'enterprise-server@release' into old paths - oldEnterprisePath: /\/([a-z]{2}\/)?(enterprise\/)?(enterprise-server@)?(\d.\d+\/)?(user[/$])?/, + // a {{ currentVersion }} in internal links may inject '' into old paths, + // so the oldEnterprisePath regex must match: /enterprise/private-instances@latest/user, + // /enterprise/enterprise-server@2.22/user, /enterprise/2.22/user, and /enterprise/user + oldEnterprisePath: /\/([a-z]{2}\/)?(enterprise\/)?(\S+?@(\S+?\/))?(\d.\d+\/)?(user[/$])?/, // new versioning format patterns adminProduct: /\/admin(\/|$|\?|#)/, enterpriseServer: /\/enterprise-server@/,