Revert "Revert "sitetree optimizations""

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/find-page.js

@@ -8,6 +8,10 @@ module.exports = function findPage (href, pageMap, redirects = {}, languageCode
   // remove trailing slash
   href = slash(href).replace(patterns.trailingSlash, '$1')
 
+  // do an initial lookup on the path as-is
+  let page = pageMap[removeFragment(href)]
+  if (page) return page
+
   // check all potential versions
   const versionedPathsToCheck = [...new Set(allVersions.map(version => {
     return getVersionedPathWithLanguage(href, version, languageCode)
@@ -22,8 +26,8 @@ module.exports = function findPage (href, pageMap, redirects = {}, languageCode
   // need to account for redirects again
   pathToPage = redirects[pathToPage] || pathToPage
 
-  // find the page
-  const page = pageMap[removeFragment(pathToPage)]
+  // try finding the page again
+  page = pageMap[removeFragment(pathToPage)]
 
   if (page) return page
 

lib/get-map-topic-content.js

@@ -1,22 +1,12 @@
-const findPage = require('./find-page')
+// get the childArticles set on map topics in lib/site-tree.js
+module.exports = function getMapTopicContent (parentProductId, breadcrumbs, siteTree) {
+  const childArticles = siteTree.products[parentProductId].categories[breadcrumbs.category.href].maptopics[breadcrumbs.maptopic.href].childArticles
 
-// 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
-
-  if (!englishPage) {
-    console.error(`cannot find english page: ${page.fullPath}`)
-    return
-  }
-
-  if (!englishPage.childArticles) {
-    console.error(`error getting child articles on map topic: ${page.fullPath}`)
-    return
+  if (!childArticles) {
+    console.error(`can't find child articles from siteTree for map topic '${breadcrumbs.maptopic.href}'`)
   }
 
-  return englishPage.childArticles
+  return childArticles
     .map(article => `{% link_with_intro /${article.href} %}`)
     .join('\n\n')
 }

lib/page.js

@@ -148,7 +148,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.breadcrumbs, context.siteTree[context.currentLanguage][context.currentVersion])
       : this.markdown
 
     // If the article is interactive parse the React!

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

@@ -23,7 +23,7 @@ module.exports = async (req, res, next) => {
   const product = req.context.siteTree[req.language][req.context.currentVersion].products[req.context.currentProduct]
 
   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
   }
 
@@ -32,7 +32,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 /[email protected]/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]
 
@@ -49,7 +49,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]
 
@@ -60,9 +60,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
@@ -77,17 +75,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 = {

middleware/early-access-breadcrumbs.js

@@ -40,8 +40,8 @@ module.exports = async (req, res, next) => {
 
   // get Early Access category path
   // e.g., `enforcing-best-practices-with-github-policies` in /free-pro-team@latest/early-access/github/enforcing-best-practices-with-github-policies
-  const categoryPath = path.posix.join('/', req.context.currentVersion, 'early-access', pathParts[0], pathParts[1])
-  const category = req.context.pages[path.posix.join('/en', categoryPath)]
+  const categoryPath = path.posix.join('/', 'en', req.context.currentVersion, 'early-access', pathParts[0], pathParts[1])
+  const category = req.context.pages[categoryPath]
 
   if (!category) return next()
 
@@ -54,7 +54,7 @@ module.exports = async (req, res, next) => {
 
   // for Early Access purposes, we don't need to differentiate between map topics and articles breadcrumbs
   const mapTopicOrArticlePath = path.posix.join(categoryPath, pathParts[2])
-  const mapTopicOrArticle = req.context.pages[path.posix.join('/en', mapTopicOrArticlePath)]
+  const mapTopicOrArticle = req.context.pages[mapTopicOrArticlePath]
 
   if (!mapTopicOrArticle) return next()
 

tests/content/site-tree.js

@@ -24,23 +24,23 @@ describe('siteTree', () => {
   test('object order', () => {
     expect(Object.keys(siteTree)[0]).toBe('en')
     expect(Object.keys(siteTree.en)[0]).toBe(nonEnterpriseDefaultVersion)
-    expect(Object.keys(siteTree.en[nonEnterpriseDefaultVersion].products.github.categories)[0]).toBe(`/${nonEnterpriseDefaultVersion}/github/getting-started-with-github`)
+    expect(Object.keys(siteTree.en[nonEnterpriseDefaultVersion].products.github.categories)[0]).toBe(`/en/${nonEnterpriseDefaultVersion}/github/getting-started-with-github`)
   })
 
   test('object structure', () => {
     expect(nonEnterpriseDefaultVersion in siteTree.en).toBe(true)
     expect(`enterprise-server@${latestEnterpriseRelease}` in siteTree.en).toBe(true)
-    expect(flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.href`]).toBe(`/${nonEnterpriseDefaultVersion}/github`)
-    expect(flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.categories./${nonEnterpriseDefaultVersion}/github/getting-started-with-github.href`]).toBe(`/${nonEnterpriseDefaultVersion}/github/getting-started-with-github`)
+    expect(flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.href`]).toBe(`/en/${nonEnterpriseDefaultVersion}/github`)
+    expect(flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.categories./en/${nonEnterpriseDefaultVersion}/github/getting-started-with-github.href`]).toBe(`/en/${nonEnterpriseDefaultVersion}/github/getting-started-with-github`)
   })
 
   describe('localized titles', () => {
     test('titles for categories', () => {
-      const japaneseTitle = flatTree[`ja.${nonEnterpriseDefaultVersion}.products.github.categories./${nonEnterpriseDefaultVersion}/github/getting-started-with-github.title`]
+      const japaneseTitle = flatTree[`ja.${nonEnterpriseDefaultVersion}.products.github.categories./ja/${nonEnterpriseDefaultVersion}/github/getting-started-with-github.title`]
       expect(typeof japaneseTitle).toBe('string')
       expect(japaneseCharacters.presentIn(japaneseTitle)).toBe(true)
 
-      const englishTitle = flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.categories./${nonEnterpriseDefaultVersion}/github/getting-started-with-github.title`]
+      const englishTitle = flatTree[`en.${nonEnterpriseDefaultVersion}.products.github.categories./en/${nonEnterpriseDefaultVersion}/github/getting-started-with-github.title`]
       expect(typeof englishTitle).toBe('string')
       expect(japaneseCharacters.presentIn(englishTitle)).toBe(false)
     })
@@ -52,7 +52,7 @@ describe('siteTree', () => {
     test('articles that include site data in liquid templating', () => {
       const pageWithDynamicTitle = siteTree.en[`enterprise-server@${latestEnterpriseRelease}`]
         .products.admin
-        .categories[`/enterprise-server@${latestEnterpriseRelease}/admin/enterprise-support`]
+        .categories[`/en/enterprise-server@${latestEnterpriseRelease}/admin/enterprise-support`]
       // Source frontmatter from article:
       // title: 'Working with {{ site.data.variables.contact.github_support }}'
       expect(pageWithDynamicTitle.title).toEqual('Working with GitHub Support')