Merge pull request #325 from hmrc/fix-header-and-footer-links-when-generating-relative-links

TRG-668: Support sites deployed on a path other than "/" when generating header and footer links.

Author: lfdebrux

govuk_tech_docs.gemspec

@@ -37,6 +37,7 @@ Gem::Specification.new do |spec|
 
   spec.add_dependency "autoprefixer-rails", "~> 10.2"
   spec.add_dependency "chronic", "~> 0.10.2"
+  spec.add_dependency "haml", "< 6.0.0"
   spec.add_dependency "middleman", "~> 4.0"
   spec.add_dependency "middleman-autoprefixer", "~> 2.10.0"
   spec.add_dependency "middleman-compass", ">= 4.0.0"
@@ -47,7 +48,6 @@ Gem::Specification.new do |spec|
   spec.add_dependency "nokogiri"
   spec.add_dependency "openapi3_parser", "~> 0.9.0"
   spec.add_dependency "redcarpet", "~> 3.5.1"
-  spec.add_dependency "haml", "< 6.0.0"
 
   spec.add_development_dependency "byebug"
   spec.add_development_dependency "capybara", "~> 3.32"

lib/govuk_tech_docs/path_helpers.rb

@@ -1,18 +1,29 @@
+require "uri"
 module GovukTechDocs
   module PathHelpers
-    def get_path_to_resource(config, resource, current_page)
-      if config[:relative_links]
-        resource_path_segments = resource.path.split("/").reject(&:empty?)[0..-2]
-        resource_file_name = resource.path.split("/")[-1]
+    # Some useful notes from https://www.rubydoc.info/github/middleman/middleman/Middleman/Sitemap/Resource#url-instance_method :
+    # 'resource.path' is "The source path of this resource (relative to the source directory, without template extensions)."
+    # 'resource.destination_path', which is: "The output path in the build directory for this resource."
+    # 'resource.url' is based on 'resource.destination_path', but is further tweaked to optionally strip the index file and prefixed with any :http_prefix.
 
-        path_to_site_root = path_to_site_root config, current_page.path
-        resource_path = path_to_site_root + resource_path_segments
-                                           .push(resource_file_name)
-                                           .join("/")
+    # Calculates the path to the sought resource, taking in to account whether the site has been configured
+    # to generate relative or absolute links.
+    # Identifies whether the sought resource is an internal or external target: External targets are returned untouched. Path calculation is performed for internal targets.
+    # Works for both "Middleman::Sitemap::Resource" resources and plain strings (which may be passed from the site configuration when generating header links).
+    #
+    # @param [Object] config
+    # @param [Object] resource
+    # @param [Object] current_page
+    def get_path_to_resource(config, resource, current_page)
+      if resource.is_a?(Middleman::Sitemap::Resource)
+        config[:relative_links] ? get_resource_path_relative_to_current_page(config, current_page.path, resource.path) : resource.url
+      elsif external_url?(resource)
+        resource
+      elsif config[:relative_links]
+        get_resource_path_relative_to_current_page(config, current_page.path, resource)
       else
-        resource_path = resource.url
+        resource
       end
-      resource_path
     end
 
     def path_to_site_root(config, page_path)
@@ -26,5 +37,24 @@ def path_to_site_root(config, page_path)
       end
       path_to_site_root
     end
+
+  private
+
+    # Calculates the path to the sought resource, relative to the current page.
+    # @param [Object] config Middleman config.
+    # @param [String] current_page path of the current page, from the site root.
+    # @param [String] resource_path_from_site_root path of the sought resource, from the site root.
+    def get_resource_path_relative_to_current_page(config, current_page, resource_path_from_site_root)
+      path_segments = resource_path_from_site_root.split("/").reject(&:empty?)[0..-2]
+      path_file_name = resource_path_from_site_root.split("/")[-1]
+
+      path_to_site_root = path_to_site_root config, current_page
+      path_to_site_root + path_segments.push(path_file_name).join("/")
+    end
+
+    def external_url?(url)
+      uri = URI.parse(url)
+      uri.scheme || uri.to_s.split("/")[0]&.include?(".")
+    end
   end
 end

lib/source/layouts/_footer.erb

@@ -6,7 +6,7 @@
         <ul class="govuk-footer__inline-list">
           <% config[:tech_docs][:footer_links].each do |title, path| %>
             <li class="govuk-footer__inline-list-item">
-              <a class="govuk-footer__link" href="<%= url_for path %>"><%= title %></a>
+              <a class="govuk-footer__link" href="<%= get_path_to_resource config, path, current_page %>"><%= title %></a>
             </li>
           <% end %>
         </ul>

lib/source/layouts/_header.erb

@@ -2,7 +2,7 @@
   <div class="govuk-header__container govuk-header__container--full-width">
     <div class="govuk-header__logo">
       <% if config[:tech_docs][:service_link] %>
-      <a href="<%= url_for config[:tech_docs][:service_link] %>" class="govuk-header__link govuk-header__link--homepage">
+      <a href="<%= get_path_to_resource config, config[:tech_docs][:service_link], current_page %>" class="govuk-header__link govuk-header__link--homepage">
       <% else %>
       <span class="govuk-header__link govuk-header__link--homepage">
       <% end %>
@@ -46,7 +46,7 @@
         <ul id="navigation" class="govuk-header__navigation-list">
           <% config[:tech_docs][:header_links].each do |title, path| %>
             <li class="govuk-header__navigation-item<% if active_page(path) %> govuk-header__navigation-item--active<% end %>">
-              <a class="govuk-header__link" href="<%= url_for path %>"><%= title %></a>
+              <a class="govuk-header__link" href="<%= get_path_to_resource config, path, current_page %>"><%= title %></a>
             </li>
           <% end %>
         </ul>

spec/govuk_tech_docs/doubles.rb

@@ -0,0 +1,5 @@
+def create_resource_double(**kwargs)
+  resource = instance_double(Middleman::Sitemap::Resource, **kwargs)
+  allow(resource).to receive(:is_a?).with(Middleman::Sitemap::Resource).and_return(true)
+  resource
+end

spec/govuk_tech_docs/pages_spec.rb

@@ -1,10 +1,12 @@
+require "govuk_tech_docs/doubles"
+
 RSpec.describe GovukTechDocs::Pages do
   describe "#to_json" do
     it "returns the pages as JSON when using absolute links" do
       current_page = double(path: "/api/pages.json")
       sitemap = double(resources: [
-        double(url: "/a.html", data: double(title: "A thing", owner_slack: "#2ndline", last_reviewed_on: Date.yesterday, review_in: "0 days")),
-        double(url: "/b.html", data: double(title: "B thing", owner_slack: "#2ndline", last_reviewed_on: Date.yesterday, review_in: "2 days")),
+        create_resource_double(url: "/a.html", data: double(title: "A thing", owner_slack: "#2ndline", last_reviewed_on: Date.yesterday, review_in: "0 days")),
+        create_resource_double(url: "/b.html", data: double(title: "B thing", owner_slack: "#2ndline", last_reviewed_on: Date.yesterday, review_in: "2 days")),
       ])
 
       json = described_class.new(sitemap, {}, current_page).to_json
@@ -18,8 +20,8 @@
     it "returns the pages as JSON when using relative links" do
       current_page = double(path: "/api/pages.json")
       sitemap = double(resources: [
-        double(url: "/a.html", path: "/a.html", data: double(title: "A thing", owner_slack: "#2ndline", last_reviewed_on: Date.yesterday, review_in: "0 days")),
-        double(url: "/b/c.html", path: "/b/c.html", data: double(title: "B thing", owner_slack: "#2ndline", last_reviewed_on: Date.yesterday, review_in: "2 days")),
+        create_resource_double(url: "/a.html", path: "/a.html", data: double(title: "A thing", owner_slack: "#2ndline", last_reviewed_on: Date.yesterday, review_in: "0 days")),
+        create_resource_double(url: "/b/c.html", path: "/b/c.html", data: double(title: "B thing", owner_slack: "#2ndline", last_reviewed_on: Date.yesterday, review_in: "2 days")),
       ])
 
       json = described_class.new(sitemap, { relative_links: true }, current_page).to_json

spec/govuk_tech_docs/path_helpers_spec.rb

@@ -1,63 +1,123 @@
+require "govuk_tech_docs/doubles"
+
 RSpec.describe GovukTechDocs::PathHelpers do
   include GovukTechDocs::PathHelpers
 
   describe "#get_path_to_resource" do
-    it "calculates the path to a resource when using absolute links" do
+    context "calculate paths for internal resources" do
       resource_url = "/documentation/introduction/index.html"
 
-      config = {}
-      resource = double("resource", url: resource_url)
+      it "when the site is configured to generate absolute links" do
+        config = {}
+
+        resource = create_resource_double(url: resource_url)
+        resource_path = get_path_to_resource(config, resource, nil)
+
+        expect(resource_path).to eql(resource_url)
+      end
+
+      it "when the site is configured to generate relative links" do
+        config = {
+          relative_links: true,
+        }
 
-      resource_path = get_path_to_resource(config, resource, nil)
-      expect(resource_path).to eql(resource_url)
+        current_page_path = "/documentation/introduction/section-one/index.html"
+        current_page = double("current_page", path: current_page_path)
+        resource = create_resource_double(url: resource_url, path: resource_url)
+        resource_path = get_path_to_resource(config, resource, current_page)
+
+        expect(resource_path).to eql("../../../documentation/introduction/index.html")
+      end
     end
 
-    it "calculates the path to a resource when using relative links" do
+    context "calculate paths for internal page paths (which are represented as strings)" do
       current_page_path = "/documentation/introduction/section-one/index.html"
-      url = "/documentation/introduction/index.html"
 
-      config = {
-        relative_links: true,
-      }
-      resource = double("resource", url: url, path: url)
-      current_page = double("current_page", path: current_page_path)
+      it "when the site is configured to generate absolute links" do
+        config = {}
+
+        url = "/documentation/introduction/index.html"
+        current_page = double("current_page", path: current_page_path)
+        resource_path = get_path_to_resource(config, url, current_page)
+
+        expect(resource_path).to eql(url)
+      end
+
+      it "when the site is configured to generate relative links" do
+        config = {
+          relative_links: true,
+        }
+
+        url = "/documentation/introduction/index.html"
+        current_page = double("current_page", path: current_page_path)
+        resource_path = get_path_to_resource(config, url, current_page)
+
+        expect(resource_path).to eql("../../../documentation/introduction/index.html")
+      end
+    end
+
+    context "calculate URLs for external URLs" do
+      external_urls = [
+        "https://www.example.com/some-page.html", # With scheme included.
+        "www.example.com/some-page.html", # With scheme omitted.
+      ]
+      external_urls.each do |external_url|
+        current_page_path = "/documentation/introduction/section-one/index.html"
+
+        it "when the site is configured to generate absolute links" do
+          config = {}
 
-      resource_path = get_path_to_resource(config, resource, current_page)
-      expect(resource_path).to eql("../../../documentation/introduction/index.html")
+          current_page = double("current_page", path: current_page_path)
+          resource_path = get_path_to_resource(config, external_url, current_page)
+
+          expect(resource_path).to eql(external_url)
+        end
+
+        it "when the site is configured to generate relative links" do
+          config = {
+            relative_links: true,
+          }
+
+          current_page = double("current_page", path: current_page_path)
+          resource_path = get_path_to_resource(config, external_url, current_page)
+
+          expect(resource_path).to eql(external_url)
+        end
+      end
     end
   end
 
   describe "#path_to_site_root" do
-    it "calculates the path from a page to the site root when using absolute links" do
-      page_path = "/documentation/introduction/index.html"
-
+    it "calculates the path from a page to the site root when the site is configured to generate absolute links" do
       config = {
         http_prefix: "/", # This is Middleman's default setting.
       }
 
+      page_path = "/documentation/introduction/index.html"
       path_to_site_root = path_to_site_root(config, page_path)
+
       expect(path_to_site_root).to eql("/")
     end
 
     it "calculates the path from a page to the site root when a middleman http prefix" do
-      page_path = "/bananas/documentation/introduction/index.html"
-
       config = {
         http_prefix: "/bananas",
       }
 
+      page_path = "/bananas/documentation/introduction/index.html"
       path_to_site_root = path_to_site_root(config, page_path)
+
       expect(path_to_site_root).to eql("/bananas/")
     end
 
-    it "calculates the path from a page to the site root when using relative links" do
-      page_path = "/documentation/introduction/index.html"
-
+    it "calculates the path from a page to the site root when the site is configured to generate relative links" do
       config = {
         relative_links: true,
       }
 
+      page_path = "/documentation/introduction/index.html"
       path_to_site_root = path_to_site_root(config, page_path)
+
       expect(path_to_site_root).to eql("../../")
     end
   end

spec/table_of_contents/helpers_spec.rb

@@ -129,6 +129,10 @@ def render(_layout)
           def add_children(children)
             @children.concat children
           end
+
+          def is_a?(klass)
+            klass.to_s == "Middleman::Sitemap::Resource"
+          end
         end,
       )
     end