Merge pull request #16965 from github/revert-16952-revert-16947-optimize-sitetree

Site tree optimizations

Author: sarahs

content/developers/github-marketplace/creating-apps-for-github-marketplace.md

@@ -4,6 +4,5 @@ intro: 'You can list free and paid tools for developers to use in {% data variab
 mapTopic: true
 versions:
   free-pro-team: '*'
-  enterprise-server: '*'
 ---
 

content/github/site-policy/github-insights-and-data-protection-for-your-organization.md

@@ -4,8 +4,11 @@ intro: '{% data variables.product.prodname_insights %} analyzes your {% data var
 product: '{% data reusables.gated-features.github-insights %}'
 redirect_from:
   - /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
 versions:
-  enterprise-server: '*'
+  free-pro-team: '*'
 ---
 
 For more information about the terms that govern {% data variables.product.prodname_insights %}, see your {% data variables.product.prodname_ghe_one %} subscription agreement.

content/insights/installing-and-configuring-github-insights/about-data-in-github-insights.md

@@ -19,4 +19,4 @@ You can choose which contributors are included in metrics and create teams of co
 
 ### Further reading
 - "[Metrics available with {% data variables.product.prodname_insights %}](/insights/exploring-your-usage-of-github-enterprise/metrics-available-with-github-insights)"
-- "[{% data variables.product.prodname_insights %} and data protection for your organization](/github/site-policy/github-insights-and-data-protection-for-your-organization)"
+- "<a href="/github/site-policy/github-insights-and-data-protection-for-your-organization" class="dotcom-only">{% data variables.product.prodname_insights %} and data protection for your organization</a>"

content/insights/installing-and-configuring-github-insights/installing-github-insights.md

@@ -108,4 +108,4 @@ To configure {% data variables.product.prodname_insights %} to connect to {% dat
 ### Further reading
 
 - "[Managing repositories](/insights/installing-and-configuring-github-insights/managing-repositories)"
-- "[{% data variables.product.prodname_insights %} and data protection for your organization](/github/site-policy/github-insights-and-data-protection-for-your-organization)"
+- "<a href="/github/site-policy/github-insights-and-data-protection-for-your-organization" class="dotcom-only">{% data variables.product.prodname_insights %} and data protection for your organization</a>"

includes/breadcrumbs.html

@@ -3,7 +3,7 @@
   {% if breadcrumb[1].href == '' %}
   <span>{{breadcrumb[1].title}}</span>
   {% else %}
-  <a title="{{ breadcrumb[0]}}: {{breadcrumb[1].title}}" href="/{{currentLanguage}}{{breadcrumb[1].href}}" class="d-inline-block {% if breadcrumb[1].href == currentPathWithoutLanguage %}text-gray-light{% endif %}">
+  <a title="{{ breadcrumb[0]}}: {{breadcrumb[1].title}}" href="{{{breadcrumb[1].href}}" class="d-inline-block {% if breadcrumb[1].href == currentPath %}text-gray-light{% endif %}">
     {{breadcrumb[1].title}}</a>
   {% endif %}
   {% endfor %}

includes/sidebar-guides.html

@@ -1,17 +1,17 @@
 <ul class="sidebar-categories">
   {% for category in product[1].categories %}
   <li class="sidebar-category {% if breadcrumbs.category.href == category[1].href %}active{% endif %}">
-    <a href="/{{currentLanguage}}{{category[1].href}}">{{ category[1].title }}</a>
+    <a href="{{category[1].href}}">{{ category[1].title }}</a>
     <!-- some categories have topics with child articles -->
     {% if category[1].maptopics %}
     <ul class="sidebar-maptopics">
       {% for maptopic in category[1].maptopics %}
       <li class="sidebar-maptopic {% if breadcrumbs.maptopic.href == maptopic[1].href %}active{% endif %}">
-        <a href="/{{currentLanguage}}{{maptopic[1].href}}">{{ maptopic[1].title }}</a>
+        <a href="{{maptopic[1].href}}">{{ maptopic[1].title }}</a>
         <ul class="sidebar-articles">
           {% for article in maptopic[1].articles %}
           <li class="sidebar-article {% if currentPath == article[1].href %}active{% endif %}">
-            <a href="/{{currentLanguage}}{{article[1].href}}">{{ article[1].title }}</a>
+            <a href="{{article[1].href}}">{{ article[1].title }}</a>
           </li>
           {% endfor %}
         </ul>
@@ -23,7 +23,7 @@
     <ul class="sidebar-articles">
       {% for article in category[1].articles %}
       <li class="sidebar-article{% if currentPath == article[1].href %} active{% endif %}">
-        <a href="/{{currentLanguage}}{{article[1].href}}">{{ article[1].title }}</a>
+        <a href="{{article[1].href}}">{{ article[1].title }}</a>
       </li>
       {% endfor %}
     </ul>

includes/sidebar-specific-product.html

@@ -10,12 +10,12 @@
 {% include all-products-link %}
 <li title="{{product.title}}" class="sidebar-product mb-2">
   {% unless page.hidden %}
-  <a href="/{{currentLanguage}}{{product.href}}" class="pl-4 pr-5 pb-1 f4">{{ product.title }}</a>
+  <a href="{{product.href}}" class="pl-4 pr-5 pb-1 f4">{{ product.title }}</a>
   {% endunless %}
 </li>
 <ul class="sidebar-categories list-style-none">
   {% for category in product.categories %}
-  {% capture fullPathToCategory %}/{{currentLanguage}}{{category[1].href}}{% endcapture %}
+  {% capture fullPathToCategory %}{{category[1].href}}{% endcapture %}
 
   <li class="sidebar-category py-1 {% if breadcrumbs.category.href == category[1].href %}active {% if currentPath == fullPathToCategory %}is-current-page {% endif %}{% endif %}{% if category[1].standalone %}standalone-category{% endif %}">
     {% if category[1].standalone %}
@@ -37,13 +37,13 @@
       <ul class="sidebar-topics list-style-none position-relative">
         {% for maptopic in category[1].maptopics %}
           {% unless maptopic[1].hidden %}
-            {% capture fullPathToMaptopic %}/{{currentLanguage}}{{maptopic[1].href}}{% endcapture %}
+            {% capture fullPathToMaptopic %}{{maptopic[1].href}}{% endcapture %}
 
             <li class="sidebar-maptopic {% if breadcrumbs.maptopic.href == maptopic[1].href %}active {% if currentPath == fullPathToMaptopic %}is-current-page{% endif %}{% endif %}">
               <a href="{{fullPathToMaptopic}}" class="pl-4 pr-5 py-2">{{ maptopic[1].title }}</a>
               <ul class="sidebar-articles my-2">
                 {% for article in maptopic[1].articles %}
-                {% capture fullPathToArticle %}/{{currentLanguage}}{{article[1].href}}{% endcapture %}
+                {% capture fullPathToArticle %}{{article[1].href}}{% endcapture %}
 
                 <li class="sidebar-article {% if breadcrumbs.article.href == article[1].href %}active {% if currentPath == fullPathToArticle %}is-current-page{% endif %}{% endif %}">
                   <a href="{{fullPathToArticle}}" class="pl-6 pr-5 py-1{% if forloop.last %} pb-2{% endif %}">{{ article[1].title }}</a>
@@ -58,7 +58,7 @@
       {% else %}
       <ul class="sidebar-articles list-style-none">
         {% for article in category[1].articles %}
-        {% capture fullPathToArticle %}/{{currentLanguage}}{{article[1].href}}{% endcapture %}
+        {% capture fullPathToArticle %}{{article[1].href}}{% endcapture %}
         <li class="sidebar-article {% if breadcrumbs.article.href == article[1].href %}active {% if currentPath == fullPathToArticle %}is-current-page{% endif %}{% endif %}">
           <a href="{{fullPathToArticle}}" class="pl-4 pr-5 py-1{% if forloop.last %} pb-2{% endif %}">{{ article[1].title }}</a>
         </li>

lib/get-map-topic-content.js

@@ -1,22 +1,31 @@
-const findPage = require('./find-page')
+const { get, last } = require('lodash')
+const { getLanguageCode } = require('../lib/patterns')
 
-// get the page.childArticles set on english map topics in lib/site-tree.js
-module.exports = function getMapTopicContent (page, pageMap, redirects) {
-  const englishPage = page.languageCode !== 'en'
-    ? findPage(`/${page.relativePath.replace(/.md$/, '')}`, pageMap, redirects, 'en')
-    : page
+// get the childArticles set on map topics in lib/site-tree.js
+module.exports = function getMapTopicContent (productId, siteTree, currentLanguage, currentVersion, currentPath) {
+  const maptopicPath = currentPath
+  const categoryPath = currentPath.replace(new RegExp(`/${last(currentPath.split('/'))}$`), '')
+  const siteTreePath = getSiteTreePath(currentVersion, productId, categoryPath, maptopicPath)
+  let childArticles = get(siteTree[currentLanguage], siteTreePath)
 
-  if (!englishPage) {
-    console.error(`cannot find english page: ${page.fullPath}`)
-    return
+  // try falling back to English if needed
+  if (!childArticles && currentLanguage !== 'en') {
+    const englishCategoryPath = categoryPath.replace(getLanguageCode, '/en')
+    const englishMaptopicPath = maptopicPath.replace(getLanguageCode, '/en')
+    const englishSiteTreePath = getSiteTreePath(currentVersion, productId, englishCategoryPath, englishMaptopicPath)
+    childArticles = get(siteTree.en, englishSiteTreePath)
   }
 
-  if (!englishPage.childArticles) {
-    console.error(`error getting child articles on map topic: ${page.fullPath}`)
-    return
+  if (!childArticles) {
+    console.error(`can't find child articles for map topic ${currentPath}`)
+    return ''
   }
 
-  return englishPage.childArticles
+  return childArticles
     .map(article => `{% link_with_intro /${article.href} %}`)
     .join('\n\n')
 }
+
+function getSiteTreePath (version, productId, categoryPath, maptopicPath) {
+  return [version, 'products', productId, 'categories', categoryPath, 'maptopics', maptopicPath, 'childArticles']
+}

lib/page.js

@@ -157,7 +157,8 @@ class Page {
     this.shortTitle = await renderContent(this.shortTitle, context, { textOnly: true, encodeEntities: true })
 
     let markdown = this.mapTopic
-      ? getMapTopicContent(this, context.pages, context.redirects)
+      // get the map topic child articles from the siteTree
+      ? getMapTopicContent(this.parentProduct.id, context.siteTree, context.currentLanguage, context.currentVersion, context.currentPath)
       : this.markdown
 
     // If the article is interactive parse the React!

lib/redirects/permalinks.js

@@ -1,5 +1,6 @@
 const patterns = require('../patterns')
 const { getVersionedPathWithLanguage } = require('../path-utils')
+const supportedVersions = new Set(Object.keys(require('../all-versions')))
 const getOldPathsFromPermalink = require('./get-old-paths-from-permalink')
 
 module.exports = function generateRedirectsForPermalinks (permalinks, redirectFrontmatter) {
@@ -25,6 +26,12 @@ module.exports = function generateRedirectsForPermalinks (permalinks, redirectFr
       // 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
+      }
+
       // get the old path for the current permalink version
       const versionedFrontmatterOldPath = getVersionedPathWithLanguage(frontmatterOldPath, permalink.pageVersion, permalink.languageCode)
 

lib/site-tree.js

@@ -1,7 +1,7 @@
 const path = require('path')
 const findPageInVersion = require('./find-page-in-version')
 const products = Object.values(require('../lib/all-products'))
-const { getVersionedPathWithoutLanguage } = require('./path-utils')
+const { getVersionedPathWithLanguage, getPathWithLanguage } = require('./path-utils')
 const languageCodes = Object.keys(require('./languages'))
 const addTitlesToTree = require('./site-tree-titles')
 const allVersions = Object.keys(require('./all-versions'))
@@ -35,15 +35,16 @@ module.exports = async function buildSiteTree (pageMap, site, redirects) {
           return
         }
 
-        product.href = item.href
+        // we don't want versioned product links because these links already have a default version in them
+        product.href = getPathWithLanguage(item.href, languageCode)
 
         // find the product TOC page and get TOC items
         const page = findPageInVersion(item.href, pageMap, redirects, languageCode, version)
 
         // skip if page can't be found in this version
         if (!page) return
 
-        product.categories = buildCategoriesTree(page.tocItems, item.href, pageMap, redirects, version, languageCode)
+        product.categories = buildCategoriesTree(page.tocItems, product.href, pageMap, redirects, version, languageCode)
 
         productTree[item.id] = product
         return null
@@ -67,11 +68,12 @@ function buildCategoriesTree (tocItems, productHref, pageMap, redirects, version
 
     const categoryHref = path.join(productHref, item.href)
 
-    const versionedCategoryHref = getVersionedPathWithoutLanguage(categoryHref, version)
+    // we DO want versioned category links
+    const versionedCategoryHref = getVersionedPathWithLanguage(categoryHref, version, languageCode)
     category.href = versionedCategoryHref
 
     // find the category TOC page and get its TOC items
-    const page = findPageInVersion(categoryHref, pageMap, redirects, languageCode, version)
+    const page = findPageInVersion(versionedCategoryHref, pageMap, redirects, languageCode, version)
 
     // skip if page can't be found in this version
     if (!page) return
@@ -90,9 +92,9 @@ function buildCategoriesTree (tocItems, productHref, pageMap, redirects, version
       // if TOC contains maptopics, build a maptopics tree
       // otherwise build an articles tree
       if (hasMaptopics) {
-        category.maptopics = buildMaptopicsTree(page.tocItems, categoryHref, pageMap, redirects, version, languageCode)
+        category.maptopics = buildMaptopicsTree(page.tocItems, versionedCategoryHref, pageMap, redirects, version, languageCode)
       } else {
-        category.articles = buildArticlesTree(page.tocItems, categoryHref, pageMap, redirects, version, languageCode)
+        category.articles = buildArticlesTree(page.tocItems, versionedCategoryHref, pageMap, redirects, version, languageCode)
       }
     }
 
@@ -102,7 +104,7 @@ function buildCategoriesTree (tocItems, productHref, pageMap, redirects, version
   return categoryTree
 }
 
-function buildMaptopicsTree (tocItems, categoryHref, pageMap, redirects, version, languageCode) {
+function buildMaptopicsTree (tocItems, versionedCategoryHref, pageMap, redirects, version, languageCode) {
   const maptopicTree = {}
 
   // for every maptopic in a category TOC...
@@ -111,38 +113,32 @@ function buildMaptopicsTree (tocItems, categoryHref, pageMap, redirects, version
     .forEach(item => {
       const maptopic = {}
 
-      const maptopicHref = path.join(categoryHref, item.href)
-
-      const versionedMaptopicHref = getVersionedPathWithoutLanguage(maptopicHref, version)
+      const versionedMaptopicHref = path.join(versionedCategoryHref, item.href)
       maptopic.href = versionedMaptopicHref
 
-      // we already have access to the child articles via the category TOC items
-      // but we still need the page to get the available versions
-      const page = findPageInVersion(maptopicHref, pageMap, redirects, languageCode, version)
+      // find the category TOC page and get its TOC items
+      const page = findPageInVersion(versionedMaptopicHref, pageMap, redirects, languageCode, version)
 
       // skip if page can't be found in this version
       if (!page) return
 
       // if this is not a maptopic, return early
       if (!page.mapTopic) return
 
-      const childArticles = getChildArticles(tocItems, item.href)
-
       maptopic.title = page.title
       maptopic.shortTitle = page.shortTitle
       maptopic.hidden = page.hidden
-
       // make the child articles accessible to the page object for maptopic rendering
-      if (!page.childArticles) page.childArticles = childArticles
+      maptopic.childArticles = getChildArticles(tocItems, item.href)
+      maptopic.articles = buildArticlesTree(maptopic.childArticles, versionedCategoryHref, pageMap, redirects, version, languageCode)
 
-      maptopic.articles = buildArticlesTree(childArticles, categoryHref, pageMap, redirects, version, languageCode)
       maptopicTree[versionedMaptopicHref] = maptopic
     })
 
   return maptopicTree
 }
 
-function buildArticlesTree (tocItems, categoryHref, pageMap, redirects, version, languageCode) {
+function buildArticlesTree (tocItems, versionedCategoryHref, pageMap, redirects, version, languageCode) {
   const articleTree = {}
 
   // REST categories may not have TOC items
@@ -152,12 +148,11 @@ function buildArticlesTree (tocItems, categoryHref, pageMap, redirects, version,
   tocItems.forEach(item => {
     const article = {}
 
-    const articleHref = path.join(categoryHref, item.href)
-
-    const versionedArticleHref = getVersionedPathWithoutLanguage(articleHref, version)
+    const versionedArticleHref = path.join(versionedCategoryHref, item.href)
     article.href = versionedArticleHref
 
-    const page = findPageInVersion(articleHref, pageMap, redirects, languageCode, version)
+    // find the category TOC page and get its TOC items
+    const page = findPageInVersion(versionedArticleHref, pageMap, redirects, languageCode, version)
 
     // skip if page can't be found in this version
     if (!page) return

middleware/breadcrumbs.js

@@ -27,7 +27,7 @@ module.exports = async (req, res, next) => {
   }
 
   req.context.breadcrumbs.product = {
-    href: path.posix.join('/', req.context.currentVersion, productPath),
+    href: path.posix.join('/', req.context.currentLanguage, req.context.currentVersion, productPath),
     title: product.title
   }
 
@@ -36,7 +36,7 @@ module.exports = async (req, res, next) => {
   // get category path
   // e.g., `getting-started-with-github` in /free-pro-team@latest/github/getting-started-with-github
   // or /enterprise-server@2.21/github/getting-started-with-github
-  const categoryPath = path.posix.join('/', req.context.currentVersion, productPath, pathParts[1])
+  const categoryPath = path.posix.join('/', req.context.currentLanguage, req.context.currentVersion, productPath, pathParts[1])
 
   const category = product.categories[categoryPath]
 
@@ -53,7 +53,7 @@ module.exports = async (req, res, next) => {
   // e.g., /github/getting-started-with-github/learning-about-github
   let maptopic
   if (req.context.page.mapTopic) {
-    const maptopicPath = path.posix.join(categoryPath, pathParts[2])
+    const maptopicPath = req.path
 
     maptopic = category.maptopics[maptopicPath]
 
@@ -64,9 +64,7 @@ module.exports = async (req, res, next) => {
       title: maptopic.shortTitle || maptopic.title
     }
   } else {
-    // get article path
-    // e.g., /github/getting-started-with-github/githubs-products
-    const articlePath = path.posix.join(categoryPath, pathParts[2])
+    const articlePath = req.path
 
     // find parent maptopic if one exists
     // some categories don't have maptopics, e.g. site-policy
@@ -81,17 +79,7 @@ module.exports = async (req, res, next) => {
       }
     }
 
-    let articleKey = '/' + req.language + articlePath
-    let articlePage = req.context.pages[articleKey]
-
-    // fall back to English if localized article does not exist
-    if (!articlePage && req.language !== 'en') {
-      articleKey = '/en' + articlePath
-      articlePage = req.context.pages[articleKey]
-    }
-
-    if (!articlePage) return next()
-
+    const articlePage = req.context.page
     const articleTitle = await articlePage.renderProp('shortTitle', req.context, { textOnly: true, encodeEntities: true })
 
     req.context.breadcrumbs.article = {