Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calculate redirects as relative paths when site is configured to use relative links. #335

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions lib/govuk_tech_docs/path_helpers.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
7 changes: 6 additions & 1 deletion lib/govuk_tech_docs/redirects.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
require "govuk_tech_docs/path_helpers"
require "ostruct"

module GovukTechDocs
class Redirects
include GovukTechDocs::PathHelpers

LEADING_SLASH = %r{\A/}.freeze

def initialize(context)
Expand All @@ -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

Expand Down
4 changes: 2 additions & 2 deletions spec/govuk_tech_docs/pages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand All @@ -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")),
Expand Down
4 changes: 2 additions & 2 deletions spec/govuk_tech_docs/path_helpers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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")
Expand Down
63 changes: 63 additions & 0 deletions spec/govuk_tech_docs/redirects_spec.rb
Original file line number Diff line number Diff line change
@@ -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
19 changes: 9 additions & 10 deletions spec/table_of_contents/helpers_spec.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
require "govuk_tech_docs/doubles"
require "spec_helper"

describe GovukTechDocs::TableOfContents::Helpers do
Expand Down Expand Up @@ -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>'

Expand Down Expand Up @@ -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>'

Expand Down