Skip to content

Commit

Permalink
Remove redirect type from router api
Browse files Browse the repository at this point in the history
Router API has dropped support for temporary redirects, as they aren't a
used feature.
  • Loading branch information
theseanything committed Sep 4, 2024
1 parent 7da9940 commit ea820d0
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 47 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

* Remove ability to specify "temporary" redirects for Router API.

# 96.0.3

* Add support for getting content with embeds from the Publishing API [PR](https://github.com/alphagov/gds-api-adapters/pull/1283)
Expand Down
6 changes: 1 addition & 5 deletions lib/gds_api/content_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def call
"#{Plek.new.website_root}#{destination_uri}"
end

[url, status_code(redirect)]
[url, 301]
end

private_class_method :new
Expand Down Expand Up @@ -105,10 +105,6 @@ def prefix_destination(redirect, path, query)

uri.to_s
end

def status_code(redirect)
redirect["redirect_type"] == "temporary" ? 302 : 301
end
end

class UnresolvedRedirect < GdsApi::BaseError; end
Expand Down
3 changes: 1 addition & 2 deletions lib/gds_api/router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,14 @@ def add_route(path, type, backend_id)
)
end

def add_redirect_route(path, type, destination, redirect_type = "permanent", options = {})
def add_redirect_route(path, type, destination, options = {})
put_json(
"#{endpoint}/routes",
route: {
incoming_path: path,
route_type: type,
handler: "redirect",
redirect_to: destination,
redirect_type:,
segments_mode: options[:segments_mode],
},
)
Expand Down
7 changes: 3 additions & 4 deletions lib/gds_api/test_helpers/router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def stub_router_has_backend_route(path, backend_id:, route_type: "exact", disabl
stub_router_has_route(path, handler: "backend", backend_id:, disabled:, route_type:)
end

def stub_router_has_redirect_route(path, redirect_to:, redirect_type: "permanent", route_type: "exact", disabled: false)
stub_router_has_route(path, handler: "redirect", redirect_to:, redirect_type:, disabled:, route_type:)
def stub_router_has_redirect_route(path, redirect_to:, route_type: "exact", disabled: false)
stub_router_has_route(path, handler: "redirect", redirect_to:, disabled:, route_type:)
end

def stub_router_has_gone_route(path, route_type: "exact", disabled: false)
Expand Down Expand Up @@ -52,14 +52,13 @@ def stub_route_registration(path, type, backend_id)
})
end

def stub_redirect_registration(path, type, destination, redirect_type, segments_mode = nil)
def stub_redirect_registration(path, type, destination, segments_mode = nil)
stub_route_put({
route: {
incoming_path: path,
route_type: type,
handler: "redirect",
redirect_to: destination,
redirect_type:,
segments_mode:,
},
})
Expand Down
17 changes: 3 additions & 14 deletions test/content_store_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,13 @@ def create_redirect(
path:,
destination: "/destination",
type: "exact",
segments_mode: "ignore",
redirect_type: "permanent"
segments_mode: "ignore"
)
{
"path" => path,
"destination" => destination,
"type" => type,
"segments_mode" => segments_mode,
"redirect_type" => redirect_type,
}
end

Expand Down Expand Up @@ -94,24 +92,15 @@ def create_redirect(
assert_equal "https://example.com/b", destination
end

it "includes a 301 status code for a permanent redirect" do
it "includes a 301 status code for a redirect" do
@content_item["redirects"] = [
create_redirect(path: "/a", redirect_type: "permanent"),
create_redirect(path: "/a"),
]

_, status_code = GdsApi::ContentStore.redirect_for_path(@content_item, "/a")
assert_equal 301, status_code
end

it "includes a 301 status code for a temporary redirect" do
@content_item["redirects"] = [
create_redirect(path: "/a", redirect_type: "temporary"),
]

_, status_code = GdsApi::ContentStore.redirect_for_path(@content_item, "/a")
assert_equal 302, status_code
end

it "returns an absolute URL redirect unmodified" do
@content_item["redirects"] = [
create_redirect(path: "/a", destination: "https://example.com/b"),
Expand Down
23 changes: 1 addition & 22 deletions test/router_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,6 @@
"route_type" => "exact",
"handler" => "redirect",
"redirect_to" => "/bar",
"redirect_type" => "permanent",
"segments_mode" => nil }
req = WebMock.stub_request(:put, "#{@base_api_url}/routes")
.with(body: { "route" => route_data }.to_json)
Expand All @@ -258,36 +257,17 @@
assert_requested(req)
end

it "should allow creating/updating a temporary redirect route" do
route_data = { "incoming_path" => "/foo",
"route_type" => "exact",
"handler" => "redirect",
"redirect_to" => "/bar",
"redirect_type" => "temporary",
"segments_mode" => nil }
req = WebMock.stub_request(:put, "#{@base_api_url}/routes")
.with(body: { "route" => route_data }.to_json)
.to_return(status: 201, body: route_data.to_json, headers: { "Content-type" => "application/json" })

response = @api.add_redirect_route("/foo", "exact", "/bar", "temporary")
assert_equal 201, response.code
assert_equal "/bar", response["redirect_to"]

assert_requested(req)
end

it "should allow creating/updating a redirect route which preserves segments" do
route_data = { "incoming_path" => "/foo",
"route_type" => "exact",
"handler" => "redirect",
"redirect_to" => "/bar",
"redirect_type" => "temporary",
"segments_mode" => "preserve" }
req = WebMock.stub_request(:put, "#{@base_api_url}/routes")
.with(body: { "route" => route_data }.to_json)
.to_return(status: 201, body: route_data.to_json, headers: { "Content-type" => "application/json" })

response = @api.add_redirect_route("/foo", "exact", "/bar", "temporary", segments_mode: "preserve")
response = @api.add_redirect_route("/foo", "exact", "/bar", segments_mode: "preserve")
assert_equal 201, response.code
assert_equal "/bar", response["redirect_to"]

Expand All @@ -299,7 +279,6 @@
"route_type" => "exact",
"handler" => "redirect",
"redirect_to" => "bar",
"redirect_type" => "permanent",
"segments_mode" => nil }
response_data = route_data.merge("errors" => { "redirect_to" => "is not a valid URL path" })

Expand Down

0 comments on commit ea820d0

Please sign in to comment.