From d8ae1c08f9fb800206d8e32cc21d76433d96febc Mon Sep 17 00:00:00 2001
From: Steve Laing <steve.laing@gmail.com>
Date: Wed, 17 Jan 2018 17:56:08 +0000
Subject: [PATCH] Move detailed guides to universal layout

---
 app/presenters/detailed_guide_presenter.rb    | 22 ++++--
 .../content_items/detailed_guide.html.erb     | 76 +++++++++---------
 test/integration/detailed_guide_test.rb       | 78 +++++++++++++++----
 3 files changed, 115 insertions(+), 61 deletions(-)

diff --git a/app/presenters/detailed_guide_presenter.rb b/app/presenters/detailed_guide_presenter.rb
index 107a3d7600..52c2a814ec 100644
--- a/app/presenters/detailed_guide_presenter.rb
+++ b/app/presenters/detailed_guide_presenter.rb
@@ -12,14 +12,6 @@ def title_and_context
     end
   end
 
-  def related_guides
-    links("related_guides")
-  end
-
-  def related_mainstream_content
-    links("related_mainstream_content")
-  end
-
   def image
     content_item["details"]["image"]["url"] if content_item["details"]["image"]
   end
@@ -30,8 +22,22 @@ def document_footer
     end
   end
 
+  def related_navigation
+    nav = super
+    nav[:related_items] += related_links("related_mainstream_content")
+    nav[:related_guides] = related_links("related_guides")
+    nav
+  end
+
 private
 
+  def related_links(key)
+    raw_links = content_item["links"]
+    guides = raw_links.fetch(key, [])
+    guides.map { |g| { text: g["title"], path: g["base_path"] } }
+  end
+
+
   def related_guides_title
     I18n.t('detailed_guide.related_guides')
   end
diff --git a/app/views/content_items/detailed_guide.html.erb b/app/views/content_items/detailed_guide.html.erb
index 56fda7f464..1c30d94ab5 100644
--- a/app/views/content_items/detailed_guide.html.erb
+++ b/app/views/content_items/detailed_guide.html.erb
@@ -1,45 +1,49 @@
 <div class="grid-row">
   <div class="column-two-thirds">
-    <%= render 'govuk_component/title', @content_item.title_and_context %>
+    <%= render 'govuk_component/title',
+       title: @content_item.title,
+       average_title_length: "long" %>
   </div>
-  <div class="column-third">
-    <%= render 'components/translation-nav', translations: @content_item.available_translations %>
-    <% if @content_item.image.present? %>
-      <%= image_tag @content_item.image, class: "logo-image" %>
-    <% end %>
-  </div>
-</div>
-<%= render 'components/notice', @content_item.withdrawal_notice_component  %>
-<%= render 'shared/metadata', content_item: @content_item %>
-<%= render 'shared/history_notice', content_item: @content_item %>
-<%= render 'govuk_component/lead_paragraph', text: @content_item.description %>
-
-<div
-  class="grid-row sidebar-with-body"
-  data-module="sticky-element-container"
-  id="contents"
->
-  <% if @content_item.contents.any? %>
+  <% if @content_item.available_translations.length > 1 %>
     <div class="column-third">
-      <%= render 'components/contents-list', contents: @content_item.contents %>
+      <%= render 'components/translation-nav',
+          translations: @content_item.available_translations %>
     </div>
   <% end %>
-  <div class="column-two-thirds <% unless @content_item.contents.any? %>offset-one-third<% end %>">
-    <% if @content_item.related_mainstream_content.any? %>
-      <aside class="related-mainstream-content" role="complementary">
-        <h4>
-          <%= raw( t('detailed_guide.related_mainstream_content') ) %>
-        </h4>
-        <% @content_item.related_mainstream_content.each do |link| %>
-          <%= link %><br />
-        <% end %>
-      </aside>
-    <% end %>
-    <%= render 'govuk_component/govspeak', @content_item.govspeak_body %>
-  </div>
-  <div data-sticky-element class="govuk-sticky-element">
-    <%= render 'components/back-to-top', href: "#contents" %>
+  <div class="column-two-thirds">
+    <%= render 'govuk_component/lead_paragraph', text: @content_item.description %>
+    <%= render 'components/notice', @content_item.withdrawal_notice_component  %>
+    <%= render 'shared/history_notice', content_item: @content_item %>
+    <%= render "components/publisher-metadata", @content_item.publisher_metadata %>
   </div>
 </div>
 
-<%= render 'shared/footer', @content_item.document_footer %>
+<div class="grid-row">
+  <div class="column-two-thirds">
+    <%= render 'components/important-metadata',
+        items: @content_item.metadata[:other] %>
+
+    <%= render "components/contents-list-with-body", contents: @content_item.contents do %>
+      <%# if @content_item.related_mainstream_content.any? %>
+        <!-- aside class="related-mainstream-content" role="complementary">
+          <h4>
+            <%= raw( t('detailed_guide.related_mainstream_content') ) %>
+          </h4>
+          <%# @content_item.related_mainstream_content.each do |link| %>
+            <%#= link %><br />
+          <%# end %>
+        </aside -->
+      <%# end %>
+      <%= render 'govuk_component/govspeak', @content_item.govspeak_body %>
+
+      <div class="responsive-bottom-margin">
+        <%= render 'components/published-dates', {
+            published: @content_item.published,
+            last_updated: @content_item.updated,
+            history: @content_item.history
+          } %>
+      </div>
+    <% end %>
+  </div>
+  <%= render "shared/sidebar_navigation" %>
+</div>
diff --git a/test/integration/detailed_guide_test.rb b/test/integration/detailed_guide_test.rb
index 0cfd90b526..90c7f1676e 100644
--- a/test/integration/detailed_guide_test.rb
+++ b/test/integration/detailed_guide_test.rb
@@ -11,21 +11,28 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest
     assert_has_component_title(@content_item["title"])
     assert page.has_text?(@content_item["description"])
 
-    assert_has_component_metadata_pair("first_published", "12 June 2014")
-    assert_has_component_metadata_pair("last_updated", "18 February 2016")
-    link1 = "<a href=\"/topic/business-tax/paye\">PAYE</a>"
-    link2 = "<a href=\"/topic/business-tax\">Business tax</a>"
-    assert_has_component_metadata_pair("part_of", [link1, link2])
-    assert_has_component_document_footer_pair("part_of", [link1, link2])
+    assert_has_publisher_metadata(
+      published: "Published 12 June 2014",
+      last_updated: "Last updated 18 February 2016",
+      history_link: true,
+      metadata: {
+        "From:": {
+          link: "/government/organisations/hm-revenue-customs",
+          text: "HM Revenue & Customs"
+        }
+      }
+    )
   end
 
   test "renders back to contents elements" do
     setup_and_visit_content_item('detailed_guide')
+
     assert page.has_css?(".app-c-back-to-top[href='#contents']")
   end
 
   test "withdrawn detailed guide" do
     setup_and_visit_content_item('withdrawn_detailed_guide')
+
     assert page.has_css?('title', text: "[Withdrawn]", visible: false)
 
     within ".app-c-notice" do
@@ -35,19 +42,50 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test "renders related topics" do
+    setup_and_visit_content_item('detailed_guide')
+
+    assert_has_related_navigation([
+      {
+        section_name: "related-nav-topics",
+        section_text: "Explore the topic",
+        links: [
+          { text: "PAYE", href: "/topic/business-tax/paye" },
+          { text: "Business tax", href: "/topic/business-tax" },
+        ]
+      }
+    ])
+  end
+
   test "shows related detailed guides" do
     setup_and_visit_content_item('political_detailed_guide')
-    assert_has_component_document_footer_pair("Related guides", ['<a href="/guidance/offshore-wind-part-of-the-uks-energy-mix">Offshore wind: part of the UK&#39;s energy mix</a>'])
+
+    assert_has_related_navigation([
+      {
+        section_name: "related-nav-related_guides",
+        section_text: "Detailed guidance",
+        links: [
+          {
+            text: "Offshore wind: part of the UK's energy mix",
+            href: "/guidance/offshore-wind-part-of-the-uks-energy-mix"
+          }
+        ]
+      }
+    ])
   end
 
-  test "shows related mainstream content" do
+  test "shows related mainstream links" do
     setup_and_visit_content_item('related_mainstream_detailed_guide')
 
-    within ".related-mainstream-content" do
-      assert page.has_text?('Too much detail?')
-      assert page.has_css?('a[href="/overseas-passports"]', text: 'Overseas British passport applications')
-      assert page.has_css?('a[href="/report-a-lost-or-stolen-passport"]', text: 'Cancel a lost or stolen passport')
-    end
+    assert_has_related_navigation([
+      {
+        section_name: "related-nav-related_items",
+        links: [
+          { text: "Overseas British passport applications", href: "/overseas-passports" },
+          { text: "Cancel a lost or stolen passport", href: "/report-a-lost-or-stolen-passport" },
+        ]
+      }
+    ])
   end
 
   test "historically political detailed guide" do
@@ -61,15 +99,21 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest
   test 'detailed guide that only applies to a set of nations' do
     setup_and_visit_content_item('national_applicability_detailed_guide')
 
-    assert_has_component_metadata_pair('Applies to', 'England')
+    assert_has_publisher_metadata_other(
+      "Applies to:": {
+        link: "/government/organisations/hm-revenue-customs",
+        text: "England"
+      }
+    )
   end
 
   test 'detailed guide that only applies to a set of nations, with alternative urls' do
     setup_and_visit_content_item('national_applicability_alternative_url_detailed_guide')
 
-    assert_has_component_metadata_pair(
-      'Applies to',
-      'England, Scotland, and Wales (see guidance for <a rel="external" href="http://www.dardni.gov.uk/news-dard-pa022-a-13-new-procedure-for">Northern Ireland</a>)'
+    assert_has_publisher_metadata_other(
+      'Applies to:': {
+        text: 'England, Scotland, and Wales (see guidance for Northern Ireland)'
+      }
     )
   end