From 49168e531aedc710a4bdcc97e86945cf5354cb4e Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Thu, 28 Sep 2023 16:07:12 +0100 Subject: [PATCH 1/2] Add test for gone content item with nil redirects The `GonePresenter` in Publishing API does not include a value for `redirects` [1], therefore it will be `nil`. Our current test for 'gone' content items assumes the value of `redirects` will be `[]`, which is not true. This appears to be causing an error when we try to unpublish former redirects with a 'gone' content item in their place. Therefore adding a test that reproduces the problem we have seen in production. 1: https://github.com/alphagov/publishing-api/blob/712b548ee6e75c85a15d808d667678c68e2d3145/app/presenters/gone_presenter.rb#L42-L56 --- spec/models/route_set_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/models/route_set_spec.rb b/spec/models/route_set_spec.rb index 2704b97e..588cd779 100644 --- a/spec/models/route_set_spec.rb +++ b/spec/models/route_set_spec.rb @@ -49,6 +49,28 @@ ] route_set = RouteSet.from_content_item(item) + + expect(route_set.is_gone).to eq(true) + expected_routes = [ + { path: "/path", type: "exact" }, + { path: "/path.json", type: "exact" }, + { path: "/path/subpath", type: "prefix" }, + ] + expect(route_set.routes).to eq([]) + expect(route_set.gone_routes).to match_array(expected_routes) + expect(route_set.redirects).to eq([]) + end + + it "constructs a route set from a gone content item with nil redirects" do + item = build(:gone_content_item, base_path: "/path", redirects: nil) + item.routes = [ + { "path" => "/path", "type" => "exact" }, + { "path" => "/path.json", "type" => "exact" }, + { "path" => "/path/subpath", "type" => "prefix" }, + ] + + route_set = RouteSet.from_content_item(item) + expect(route_set.is_gone).to eq(true) expected_routes = [ { path: "/path", type: "exact" }, From aa00d00ffcd79fc2d24cafbe0f049a808fb0ecea Mon Sep 17 00:00:00 2001 From: Bruce Bolt Date: Thu, 28 Sep 2023 16:09:27 +0100 Subject: [PATCH 2/2] Don't assume `items.redirects` will always be an array `items.redirects` can sometimes be `nil`. Therefore we need to convert it to an array (even if empty) before doing `map`, else we get an error as `nil` does not have a `map` method. This will resolve an issue in production where we are unable to unpublish former redirects. --- app/models/route_set.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/route_set.rb b/app/models/route_set.rb index 0e7220a2..f93e43c5 100644 --- a/app/models/route_set.rb +++ b/app/models/route_set.rb @@ -27,7 +27,7 @@ def self.from_content_item(item) routes = item.routes.map(&:to_h).map(&:deep_symbolize_keys) end - redirects = item.redirects.map(&:to_h).map(&:deep_symbolize_keys) + redirects = Array(item.redirects).map(&:to_h).map(&:deep_symbolize_keys) new( routes:,