TRG-1096: Calculate redirects as relative paths when site is configured to use relative links.

(cherry picked from commit 396ccb1)

Author: eddgrant

lib/govuk_tech_docs/path_helpers.rb

@@ -1,26 +1,28 @@
 require "uri"
 module GovukTechDocs
   module PathHelpers
-    # 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.
-
-    # Calculates the path to the sought resource, taking in to account whether the site has been configured
+    # 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
+    # Some useful notes from label[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.
+    #
+    # @param [Object] config from Middleman::ConfigContext
+    # @param [Middleman::Sitemap::Resource, String] resource
+    # @param [Middleman::Sitemap::Resource, String] current_page
     def get_path_to_resource(config, resource, current_page)
+      current_page_path = current_page.is_a?(Middleman::Sitemap::Resource) ? current_page.path : 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
+        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)
+        get_resource_path_relative_to_current_page(config, current_page_path, resource)
       else
         resource
       end

lib/govuk_tech_docs/redirects.rb

@@ -1,6 +1,11 @@
+require "govuk_tech_docs/path_helpers"
+require "ostruct"
+
 module GovukTechDocs
   class Redirects
-    LEADING_SLASH = %r{\A/}.freeze
+    include GovukTechDocs::PathHelpers
+
+    LEADING_SLASH = %r{\A/}
 
     def initialize(context)
       @context = context
@@ -11,7 +16,7 @@ def redirects
 
       all_redirects.map do |from, to|
         # Middleman needs paths without leading slashes
-        [from.sub(LEADING_SLASH, ""), { to: to.sub(LEADING_SLASH, "") }]
+        [from.sub(LEADING_SLASH, ""), { to: get_path_to_resource(@context.config, to.sub(LEADING_SLASH, ""), from.sub(LEADING_SLASH, "")) }]
       end
     end
 

spec/govuk_tech_docs/pages_spec.rb

@@ -3,7 +3,7 @@
 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")
+      current_page = create_resource_double(path: "/api/pages.json")
       sitemap = double(resources: [
         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")),
@@ -18,7 +18,7 @@
     end
 
     it "returns the pages as JSON when using relative links" do
-      current_page = double(path: "/api/pages.json")
+      current_page = create_resource_double(path: "/api/pages.json")
       sitemap = double(resources: [
         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")),

spec/govuk_tech_docs/path_helpers_spec.rb

@@ -22,7 +22,7 @@
         }
 
         current_page_path = "/documentation/introduction/section-one/index.html"
-        current_page = double("current_page", path: current_page_path)
+        current_page = create_resource_double(url: current_page_path, path: current_page_path)
         resource = create_resource_double(url: resource_url, path: resource_url)
         resource_path = get_path_to_resource(config, resource, current_page)
 
@@ -49,7 +49,7 @@
         }
 
         url = "/documentation/introduction/index.html"
-        current_page = double("current_page", path: current_page_path)
+        current_page = create_resource_double(url: current_page_path, path: current_page_path)
         resource_path = get_path_to_resource(config, url, current_page)
 
         expect(resource_path).to eql("../../../documentation/introduction/index.html")

spec/govuk_tech_docs/redirects_spec.rb

@@ -0,0 +1,63 @@
+require "govuk_tech_docs/doubles"
+
+RSpec.describe GovukTechDocs::Redirects do
+  describe "#redirects" do
+    it "calculates redirects as a relative path from the site root" do
+      config = {
+        relative_links: true,
+        tech_docs: {
+          redirects: {
+            # A page has been renamed in an existing folder
+            "documentation/some-location/old-page.html" => "documentation/some-location/new-page.html",
+            # A page has been moved between sibling folders
+            "documentation/old-location/index.html" => "documentation/new-location/index.html",
+            # A page has been moved and has changed its position in the folder hierarchy
+            "documentation/some-path/old-location/index.html" => "documentation/new-location/index.html",
+            # A page has been moved but within some shared common folder hierarchy
+            "documentation/some-common-path/old-location/index.html" => "documentation/some-common-path/new-location/index.html",
+          },
+        },
+      }
+      context = double("Middleman::ConfigContext")
+      allow(context).to receive(:config).and_return config
+      allow(context).to receive_message_chain(:sitemap, :resources).and_return []
+
+      under_test = described_class.new(context)
+      redirects = under_test.redirects
+
+      expect(redirects[0][0]).to eql("documentation/some-location/old-page.html")
+      expect(redirects[0][1][:to]).to eql("../../documentation/some-location/new-page.html")
+
+      expect(redirects[1][0]).to eql("documentation/old-location/index.html")
+      expect(redirects[1][1][:to]).to eql("../../documentation/new-location/index.html")
+
+      expect(redirects[2][0]).to eql("documentation/some-path/old-location/index.html")
+      expect(redirects[2][1][:to]).to eql("../../../documentation/new-location/index.html")
+
+      expect(redirects[3][0]).to eql("documentation/some-common-path/old-location/index.html")
+      expect(redirects[3][1][:to]).to eql("../../../documentation/some-common-path/new-location/index.html")
+    end
+
+    it "performs no manipulation of redirects when the site is not using relative links" do
+      old_location = "documentation/old-location/index.html"
+      new_location = "documentation/new-location/index.html"
+      config = {
+        relative_links: false,
+        tech_docs: {
+          redirects: {
+            old_location => new_location,
+          },
+        },
+      }
+      context = double("Middleman::ConfigContext")
+      allow(context).to receive(:config).and_return config
+      allow(context).to receive_message_chain(:sitemap, :resources).and_return []
+
+      under_test = described_class.new(context)
+      redirects = under_test.redirects
+
+      expect(redirects[0][0]).to eql(old_location)
+      expect(redirects[0][1][:to]).to eql(new_location)
+    end
+  end
+end

spec/table_of_contents/helpers_spec.rb

@@ -1,3 +1,4 @@
+require "govuk_tech_docs/doubles"
 require "spec_helper"
 
 describe GovukTechDocs::TableOfContents::Helpers do
@@ -224,11 +225,10 @@ def is_a?(klass)
       resources[5] = FakeResource.new("/1/5/6/e.html", '<h1 id="heading-one">Heading one</h1><h2 id="heading-two">Heading two</h2>', 60, "Sub page A", resources[0])
       resources[0].add_children [resources[1], resources[2], resources[3], resources[4], resources[5]]
 
-      current_page = double("current_page",
-                            data: double("page_frontmatter", description: "The description.", title: "The Title"),
-                            url: "/1/2/3/index.html",
-                            path: "/1/2/3/index.html",
-                            metadata: { locals: {} })
+      current_page = create_resource_double(data: double("page_frontmatter", description: "The description.", title: "The Title"),
+                                            url: "/1/2/3/index.html",
+                                            path: "/1/2/3/index.html",
+                                            metadata: { locals: {} })
 
       current_page_html = '<h1 id="heading-one">Heading one</h1><h2 id="heading-two">Heading two</h2>'
 
@@ -299,11 +299,10 @@ def is_a?(klass)
       resources[2] = FakeResource.new("/prefix/b.html", '<h1 id="heading-one">Heading one</h1><h2 id="heading-two">Heading two</h2>', 20, "Sub page B", resources[0])
       resources[0].add_children [resources[1], resources[2]]
 
-      current_page = double("current_page",
-                            data: double("page_frontmatter", description: "The description.", title: "The Title"),
-                            url: "/prefix/index.html",
-                            path: "/prefix/index.html",
-                            metadata: { locals: {} })
+      current_page = create_resource_double(data: double("page_frontmatter", description: "The description.", title: "The Title"),
+                                            url: "/prefix/index.html",
+                                            path: "/prefix/index.html",
+                                            metadata: { locals: {} })
 
       current_page_html = '<h1 id="heading-one">Heading one</h1><h2 id="heading-two">Heading two</h2>'