Skip to content

Commit

Permalink
Merge pull request #1286 from alphagov/remove-redirect-type-for-route…
Browse files Browse the repository at this point in the history
…r-api

Remove redirect type from router api
  • Loading branch information
theseanything authored Sep 4, 2024
2 parents 7da9940 + ea820d0 commit 17f29b9
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 17f29b9

Please sign in to comment.