From f0de06b84c4dba8c7344e798c62453b84a5c818d Mon Sep 17 00:00:00 2001
From: Jamie Tanna <jamie@jamietanna.co.uk>
Date: Wed, 10 Nov 2021 13:05:37 +0000
Subject: [PATCH] Fix: Don't break TOC when OpenAPI description includes
 headers

As noted in [0], the ability to render a CommonMark document through
OpenAPI specifications' `info.description` is breaking due to how the
Table of Contents code works.

We'd assumed that the `id` would always be set on headers - by Middleman
- but as it's possible for an OpenAPI description to include headers, we
won't always have one.

To solve this, we can make it possible for a link to not be present for
a heading, instead returning just a `<span> for the table of contents.

This allows the headings to still be present, but won't be actionable.

Until the `openapi3_parser` gem supports making this configurable, i.e.
be allowing us to inject in headers, this is a good middle ground.

[0]: https://github.com/alphagov/tdt-documentation/issues/156
---
 .../table_of_contents/heading.rb              |  2 +-
 .../heading_tree_renderer.rb                  |  6 +++-
 spec/table_of_contents/helpers_spec.rb        | 28 +++++++++++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/lib/govuk_tech_docs/table_of_contents/heading.rb b/lib/govuk_tech_docs/table_of_contents/heading.rb
index 120772d8..0068d3a1 100644
--- a/lib/govuk_tech_docs/table_of_contents/heading.rb
+++ b/lib/govuk_tech_docs/table_of_contents/heading.rb
@@ -15,7 +15,7 @@ def size
       def href
         if @page_url != "" && size == 1
           @page_url
-        else
+        elsif @attributes["id"]
           @page_url + "#" + @attributes["id"]
         end
       end
diff --git a/lib/govuk_tech_docs/table_of_contents/heading_tree_renderer.rb b/lib/govuk_tech_docs/table_of_contents/heading_tree_renderer.rb
index 843775a0..9cc8c1a7 100644
--- a/lib/govuk_tech_docs/table_of_contents/heading_tree_renderer.rb
+++ b/lib/govuk_tech_docs/table_of_contents/heading_tree_renderer.rb
@@ -20,7 +20,11 @@ def render_tree(tree, indentation: DEFAULT_INDENTATION, level: nil)
         output = ""
 
         if tree.heading
-          output += indentation + %{<a href="#{tree.heading.href}"><span>#{tree.heading.title}</span></a>\n}
+          output += if tree.heading.href
+                      indentation + %{<a href="#{tree.heading.href}"><span>#{tree.heading.title}</span></a>\n}
+                    else
+                      indentation + %{<span>#{tree.heading.title}</span>\n}
+                    end
         end
 
         if tree.children.any? && level < @max_level
diff --git a/spec/table_of_contents/helpers_spec.rb b/spec/table_of_contents/helpers_spec.rb
index dd2a63bc..1b2bd5f1 100644
--- a/spec/table_of_contents/helpers_spec.rb
+++ b/spec/table_of_contents/helpers_spec.rb
@@ -36,6 +36,34 @@ class Subject
       expect(subject.single_page_table_of_contents(html).strip).to eq(expected_single_page_table_of_contents.strip)
     end
 
+    it "builds a table of contents from html, when items do not include an `id`, for instance if included as Markdown in an OpenAPI document" do
+      html = %{
+        <h1>Fruit</h1>
+        <h1 id="apples">Apples</h1>
+        <p>A fruit</p>
+        <h2 id="apple-recipes">Apple recipes</h2>
+        <p>Get some apples..</p>
+      }
+
+      expected_single_page_table_of_contents = %{
+<ul>
+  <li>
+    <span>Fruit</span>
+  </li>
+  <li>
+    <a href="#apples"><span>Apples</span></a>
+    <ul>
+      <li>
+        <a href="#apple-recipes"><span>Apple recipes</span></a>
+      </li>
+    </ul>
+  </li>
+</ul>
+      }
+
+      expect(subject.single_page_table_of_contents(html).strip).to eq(expected_single_page_table_of_contents.strip)
+    end
+
     it "builds a table of contents from html when headings suddenly change by more than one size" do
       html = %{
         <h1 id="fruit">Fruit</h1>