From d1b092c7d56178bf5f064621f5718e2eb6925c2f Mon Sep 17 00:00:00 2001 From: Keith Lawrence Date: Mon, 1 Jul 2024 14:39:58 +0100 Subject: [PATCH] WIP 2 --- .../requests/content_items_controller_spec.rb | 166 ++++++++---------- ...roller_spec.rb => service_sign_in_spec.rb} | 14 +- ...roller_spec.rb => step_navigation_spec.rb} | 19 +- spec/support/govuk_content_schema_examples.rb | 6 + 4 files changed, 93 insertions(+), 112 deletions(-) rename spec/requests/{service_sign_in_content_item_controller_spec.rb => service_sign_in_spec.rb} (91%) rename spec/requests/{step_navigation_controller_spec.rb => step_navigation_spec.rb} (59%) diff --git a/spec/requests/content_items_controller_spec.rb b/spec/requests/content_items_controller_spec.rb index a96b7d7789..8cb09b907e 100644 --- a/spec/requests/content_items_controller_spec.rb +++ b/spec/requests/content_items_controller_spec.rb @@ -1,29 +1,29 @@ RSpec.describe(ContentItemsController, type: :request) do include GovukAbTesting::MinitestHelpers - it("routing handles paths with no format or locale") do + it "routes paths with no format or locale" do assert_routing("/government/news/statement-the-status-of-eu-nationals-in-the-uk", controller: "content_items", action: "show", path: "government/news/statement-the-status-of-eu-nationals-in-the-uk") end - it("routing handles paths for all supported locales") do + it "route paths for all supported locales" do I18n.available_locales.each do |locale| assert_routing("/government/news/statement-the-status-of-eu-nationals-in-the-uk.#{locale}", controller: "content_items", action: "show", path: "government/news/statement-the-status-of-eu-nationals-in-the-uk", locale: locale.to_s) end end - it("routing handles paths with just format") do + it "routes paths with just format" do assert_routing("/government/news/statement-the-status-of-eu-nationals-in-the-uk.atom", controller: "content_items", action: "show", path: "government/news/statement-the-status-of-eu-nationals-in-the-uk", format: "atom") end - it("routing handles paths with format and locale") do + it "routes paths with format and locale" do assert_routing("/government/news/statement-the-status-of-eu-nationals-in-the-uk.es.atom", controller: "content_items", action: "show", path: "government/news/statement-the-status-of-eu-nationals-in-the-uk", format: "atom", locale: "es") end - it("routing handles paths with print variant") do + it "routes paths with print variant" do assert_routing("/government/news/statement-the-status-of-eu-nationals-in-the-uk/print", controller: "content_items", action: "show", path: "government/news/statement-the-status-of-eu-nationals-in-the-uk", variant: "print") end - it("redirects route with invalid parts to base path") do + it "redirects route with invalid parts to base path" do content_item = content_store_has_schema_example("travel_advice", "full-country") invalid_part_path = "#{path_for(content_item)}/not-a-valid-part" stub_request(:get, /#{invalid_part_path}/).to_return(status: 200, body: content_item.to_json, headers: {}) @@ -32,43 +32,40 @@ expect(response).to redirect_to(content_item["base_path"]) end - it("redirects route for first path to base path") do + it "redirects route for first path to base path" do content_item = content_store_has_schema_example("guide", "guide") invalid_part_path = "#{path_for(content_item)}/#{content_item['details']['parts'].first['slug']}" stub_request(:get, /#{invalid_part_path}/).to_return(status: 200, body: content_item.to_json, headers: {}) - @controller.stubs(:page_in_scope?).returns(false) + allow_any_instance_of(ContentItemsController).to receive(:page_in_scope?).and_return(false) get "/#{invalid_part_path}" expect(response).to redirect_to(content_item["base_path"]) end - it("returns HTML when an unspecific accepts header is requested (eg by IE8 and below)") do - request.headers["Accept"] = "*/*" + it "returns HTML when an unspecific accepts header is requested (eg by IE8 and below)" do content_item = content_store_has_schema_example("travel_advice", "full-country") - get "/#{path_for(content_item)}" + get "/#{path_for(content_item)}", headers: { Accept: "*/*" } expect(response.headers["Content-Type"]).to(match(/text\/html/)) expect(response.status).to eq(200) assert_select("#wrapper") end - it("returns a 406 for XMLHttpRequests without an Accept header set to a supported format") do - request.headers["X-Requested-With"] = "XMLHttpRequest" + it "returns a 406 for XMLHttpRequests without an Accept header set to a supported format" do content_item = content_store_has_schema_example("case_study", "case_study") - get "/#{path_for(content_item)}" + get "/#{path_for(content_item)}", headers: { "X-Requested-With" => "XMLHttpRequest", "Accept" => nil } expect(response.status).to eq(406) end - it("returns a 406 for unsupported format requests, eg text/javascript") do - request.headers["Accept"] = "text/javascript" + it "returns a 406 for unsupported format requests, eg text/javascript" do content_item = content_store_has_schema_example("case_study", "case_study") - get "/#{path_for(content_item)}" + get "/#{path_for(content_item)}", headers: { Accept: "text/javascript" } expect(response.status).to eq(406) end - it("gets item from content store") do + it "gets an item from content store" do content_item = content_store_has_schema_example("case_study", "case_study") get "/#{path_for(content_item)}" @@ -76,64 +73,64 @@ expect(assigns[:content_item].title).to(eq(content_item["title"])) end - it("gets item from content store and keeps existing ordered_related_items when links already exist") do + it "gets an item from content store and keeps existing ordered_related_items when links already exist" do content_item = content_store_has_schema_example("guide", "guide") get "/#{path_for(content_item)}" expect(response.status).to eq(200) - assert_not_empty(content_item["links"]["ordered_related_items"], "Content item should have existing related links") - assert_not_empty(content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links") - expect(assigns[:content_item].content_item["links"]["ordered_related_items"]).to(eq(content_item["links"]["ordered_related_items"])) + expect(content_item["links"]["ordered_related_items"]).not_to be_empty + expect(content_item["links"]["suggested_ordered_related_items"]).not_to be_empty + expect(assigns[:content_item].content_item["links"]["ordered_related_items"]).to eq(content_item["links"]["ordered_related_items"]) end - it("gets item from content store and does not change ordered_related_items when link overrides exist") do + it "gets an item from content store and does not change ordered_related_items when link overrides exist" do content_item = content_store_has_schema_example("guide", "guide-with-related-link-overrides") get "/#{path_for(content_item)}" expect(response.status).to eq(200) expect(content_item["links"]["ordered_related_items"]).to(be_nil) - assert_not_empty(content_item["links"]["ordered_related_items_overrides"], "Content item should have existing related link overrides") - assert_not_empty(content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links") + expect(content_item["links"]["ordered_related_items_overrides"]).not_to be_empty + expect(content_item["links"]["suggested_ordered_related_items"]).not_to be_empty expect(content_item["links"]["ordered_related_items"]).to(be_nil) end - it("gets item from content store and replaces ordered_related_items there are no existing links or overrides") do + it "gets an item from content store and replaces ordered_related_items there are no existing links or overrides" do content_item = content_store_has_schema_example("case_study", "case_study") get "/#{path_for(content_item)}" expect(response.status).to eq(200) expect(content_item["links"]["ordered_related_items"]).to(be_empty) - assert_not_empty(content_item["links"]["suggested_ordered_related_items"], "Content item should have existing suggested related links") - expect(content_item["links"]["suggested_ordered_related_items"]).to(eq(assigns[:content_item].content_item["links"]["ordered_related_items"])) + expect(content_item["links"]["suggested_ordered_related_items"]).not_to be_empty + expect(content_item["links"]["suggested_ordered_related_items"]).to eq(assigns[:content_item].content_item["links"]["ordered_related_items"]) end - it("sets the expiry as sent by content-store") do + it "sets the expiry as sent by content-store" do content_item = content_store_has_schema_example("case_study", "case_study") stub_content_store_has_item(content_item["base_path"], content_item, max_age: 20) get "/#{path_for(content_item)}" expect(response.status).to eq(200) - expect(@response.headers["Cache-Control"]).to(eq("max-age=20, public")) + expect(response.headers["Cache-Control"]).to eq("max-age=20, public") end - it("sets a longer cache-control header for travel advice atom feeds") do + it "sets a longer cache-control header for travel advice atom feeds" do content_item = content_store_has_schema_example("travel_advice", "full-country") get "/#{path_for(content_item)}", params: { format: :atom } expect(response.status).to eq(200) - expect(@response.headers["Cache-Control"]).to(eq("max-age=300, public")) + expect(response.headers["Cache-Control"]).to eq("max-age=300, public") end - it("honours cache-control private items") do + it "honours cache-control private items" do content_item = content_store_has_schema_example("case_study", "case_study") stub_content_store_has_item(content_item["base_path"], content_item, private: true) get "/#{path_for(content_item)}" expect(response.status).to eq(200) - expect(@response.headers["Cache-Control"]).to(eq("max-age=900, private")) + expect(response.headers["Cache-Control"]).to eq("max-age=900, private") end - it("renders translated content items in their locale") do + it "renders translated content items in their locale" do content_item = content_store_has_schema_example("case_study", "translated") locale = content_item["locale"] translated_schema_name = I18n.t("content_item.schema_name.case_study", count: 1, locale:) @@ -143,7 +140,7 @@ assert_select("title", /#{translated_schema_name}/) end - it("renders atom feeds") do + it "renders atom feeds" do content_item = content_store_has_schema_example("travel_advice", "full-country") get "/#{path_for(content_item)}", params: { format: :atom } @@ -151,34 +148,32 @@ assert_select("feed title", "Travel Advice Summary") end - it("renders print variants") do + it "renders print variants" do content_item = content_store_has_schema_example("travel_advice", "full-country") get "/#{path_for(content_item)}", params: { variant: :print } - expect(response.status).to eq(200) - expect([:print]).to(eq(request.variant)) assert_select("#travel-advice-print") end - it("gets item from content store even when url contains multi-byte UTF8 character") do + it "gets an item from content store even when url contains multi-byte UTF8 character" do content_item = content_store_has_schema_example("case_study", "case_study") utf8_path = "government/case-studies/caf\u00E9-culture" content_item["base_path"] = "/#{utf8_path}" stub_content_store_has_item(content_item["base_path"], content_item) - get "/#{utf8_path}" + get "/#{CGI.escape(utf8_path)}" expect(response.status).to eq(200) end - it("returns 404 for invalid url") do + it "returns 404 for invalid url" do path = "foreign-travel-advice/egypt]" stub_content_store_does_not_have_item("/#{path}") - get "/#{path}" + get "/#{CGI.escape(path)}" expect(response.status).to eq(404) end - it("returns 404 for item not in content store") do + it "returns 404 if the item is not in content store" do path = "government/case-studies/boost-chocolate-production" stub_content_store_does_not_have_item("/#{path}") get "/#{path}" @@ -186,7 +181,7 @@ expect(response.status).to eq(404) end - it("returns 404 if content store falls through to special route") do + it "returns 404 if content store falls through to special route" do path = "government/item-not-here" content_item = content_store_has_schema_example("special_route", "special_route") content_item["base_path"] = "/government" @@ -196,7 +191,7 @@ expect(response.status).to eq(404) end - it("returns 403 for access-limited item") do + it "returns 403 for access-limited item" do path = "government/case-studies/super-sekrit-document" url = "#{content_store_endpoint}/content/#{path}" stub_request(:get, url).to_return(status: 403, headers: {}) @@ -205,21 +200,21 @@ expect(response.status).to eq(403) end - it("returns 406 for schema types which don't support provided format") do + it "returns 406 for schema types which don't support provided format" do content_item_without_atom = content_store_has_schema_example("case_study", "case_study") - get(:show, params: { path: path_for(content_item_without_atom), format: "atom" }) + get "/#{path_for(content_item_without_atom)}", params: { format: "atom" } expect(response.status).to eq(406) end - it("returns 410 for content items that are gone") do + it "returns 410 for content items that are gone" do stub_content_store_has_gone_item("/gone-item") get "/gone-item" expect(response.status).to eq(410) end - it("returns a redirect when content item is a redirect") do + it "returns a redirect when content item is a redirect" do content_item = content_store_has_schema_example("redirect", "redirect") stub_content_store_has_item("/406beacon", content_item) get "/406beacon" @@ -227,7 +222,7 @@ expect(response).to redirect_to("https://www.test.gov.uk/maritime-safety-weather-and-navigation/register-406-mhz-beacons?query=answer#fragment") end - it("returns a prefixed redirect when content item is a prefix redirect") do + it "returns a prefixed redirect when content item is a prefix redirect" do content_item = content_store_has_schema_example("redirect", "redirect") stub_content_store_has_item("/406beacon/prefix/to-preserve", content_item) get "/406beacon/prefix/to-preserve" @@ -235,86 +230,73 @@ expect(response).to redirect_to("https://www.test.gov.uk/new-406-beacons-destination/to-preserve") end - it("sets the Access-Control-Allow-Origin header for atom pages") do + it "sets the Access-Control-Allow-Origin header for atom pages" do content_store_has_schema_example("travel_advice", "full-country") - get(:show, params: { path: "foreign-travel-advice/albania", format: "atom" }) + get "/foreign-travel-advice/albania", params: { format: "atom" } - expect("*").to(eq(response.headers["Access-Control-Allow-Origin"])) + expect(response.headers["Access-Control-Allow-Origin"]).to eq("*") end - it("sets GOVUK-Account-Session-Flash in the Vary header") do + it "sets GOVUK-Account-Session-Flash in the Vary header" do content_item = content_store_has_schema_example("case_study", "case_study") - get(:show, params: { path: path_for(content_item) }) + get "/#{path_for(content_item)}" - expect(response.headers["Vary"].include?("GOVUK-Account-Session-Flash")).to(eq(true)) + expect(response.headers["Vary"].include?("GOVUK-Account-Session-Flash")).to eq(true) end %w[publication consultation detailed_guide call_for_evidence].each do |schema_name| - it("#{schema_name} displays the subscription success banner when the 'email-subscription-success' flash is present") do + it "#{schema_name} displays the subscription success banner when the 'email-subscription-success' flash is present" do example_name = if %w[consultation call_for_evidence].include?(schema_name) "open_#{schema_name}" else schema_name end content_item = content_store_has_schema_example(schema_name, example_name) - request.headers["GOVUK-Account-Session"] = GovukPersonalisation::Flash.encode_session("session-id", %w[email-subscription-success]) - get(:show, params: { path: path_for(content_item) }) + get( + "/#{path_for(content_item)}", + headers: { "GOVUK-Account-Session" => GovukPersonalisation::Flash.encode_session("session-id", %w[email-subscription-success]) }, + ) - expect(response.body.include?("subscribed to emails about this page")).to(eq(true)) + expect(response.body).to include("subscribed to emails about this page") end - it("#{schema_name} displays the unsubscribe success banner when the 'email-unsubscribe-success' flash is present") do + it "#{schema_name} displays the unsubscribe success banner when the 'email-unsubscribe-success' flash is present" do example_name = if %w[consultation call_for_evidence].include?(schema_name) "open_#{schema_name}" else schema_name end content_item = content_store_has_schema_example(schema_name, example_name) - request.headers["GOVUK-Account-Session"] = GovukPersonalisation::Flash.encode_session("session-id", %w[email-unsubscribe-success]) - get(:show, params: { path: path_for(content_item) }) + get( + "/#{path_for(content_item)}", + headers: { "GOVUK-Account-Session" => GovukPersonalisation::Flash.encode_session("session-id", %w[email-unsubscribe-success]) }, + ) - expect(response.body.include?("unsubscribed from emails about this page")).to(eq(true)) + expect(response.body).to include("unsubscribed from emails about this page") end - it("#{schema_name} displays the already subscribed success banner when the 'email-subscribe-already-subscribed' flash is present") do + it "#{schema_name} displays the already subscribed success banner when the 'email-subscribe-already-subscribed' flash is present" do example_name = if %w[consultation call_for_evidence].include?(schema_name) "open_#{schema_name}" else schema_name end content_item = content_store_has_schema_example(schema_name, example_name) - request.headers["GOVUK-Account-Session"] = GovukPersonalisation::Flash.encode_session("session-id", %w[email-subscription-already-subscribed]) - get(:show, params: { path: path_for(content_item) }) + get( + "/#{path_for(content_item)}", + headers: { "GOVUK-Account-Session" => GovukPersonalisation::Flash.encode_session("session-id", %w[email-subscription-already-subscribed]) }, + ) - expect(response.body.include?("already getting emails about this page")).to(eq(true)) + expect(response.body).to include("already getting emails about this page") end end - it("renders service_manual_guides") do + it "renders service_manual_guides" do content_item = content_store_has_schema_example("service_manual_guide", "service_manual_guide") - get(:show, params: { path: path_for(content_item) }) - - assert_response(:success) - expect(assigns[:content_item].title).to(eq(content_item["title"])) - end - - it("guides should tell slimmer to scope search results to the manual") do - content_item = content_store_has_schema_example("service_manual_guide", "service_manual_guide") - get(:show, params: { path: path_for(content_item) }) - - expect(@response.headers[Slimmer::Headers::SEARCH_PARAMETERS_HEADER]).to(eq({ filter_manual: "/service-manual" }.to_json)) - end - - it("the homepage should tell slimmer not to include a search box in the header") do - content_item = content_store_has_schema_example("service_manual_homepage", "service_manual_homepage") - get(:show, params: { path: path_for(content_item) }) + get "/#{path_for(content_item)}" - expect(@response.headers[Slimmer::Headers::REMOVE_SEARCH_HEADER]).to(eq("true")) + expect(response.status).to eq(200) + expect(assigns[:content_item].title).to eq(content_item["title"]) end - def path_for(content_item, locale = nil) - base_path = content_item["base_path"].sub(/^\//, "") - base_path.gsub!(/\.#{locale}$/, "") if locale - base_path - end end diff --git a/spec/requests/service_sign_in_content_item_controller_spec.rb b/spec/requests/service_sign_in_spec.rb similarity index 91% rename from spec/requests/service_sign_in_content_item_controller_spec.rb rename to spec/requests/service_sign_in_spec.rb index 68ed6ab191..af27c9da4e 100644 --- a/spec/requests/service_sign_in_content_item_controller_spec.rb +++ b/spec/requests/service_sign_in_spec.rb @@ -1,4 +1,4 @@ -RSpec.describe(ContentItemsController, type: :request) do +RSpec.describe("Service Sign In", type: :request) do it "redirects route for root service_sign_in page" do content_item = content_store_has_schema_example("service_sign_in", "service_sign_in") invalid_part_path = path_for(content_item) @@ -49,7 +49,7 @@ value = option["text"].parameterize link = option["url"] stub_request(:get, /#{path}/).to_return(status: 200, body: content_item.to_json, headers: {}) - post "/#{path}" + post "/#{path}", params: { option: value } expect(response).to redirect_to(link) end @@ -60,7 +60,7 @@ stub_request(:get, /#{path}/).to_return(status: 200, body: content_item.to_json, headers: {}) post "/#{path}" - expect(@controller.instance_variable_get(:@error)).to_not(be_nil) + expect(@controller.instance_variable_get(:@error)).not_to be_nil expect(response).to render_template(:service_sign_in) end @@ -71,7 +71,7 @@ stub_request(:get, /#{path}/).to_return(status: 200, body: content_item.to_json, headers: {}) post "/#{path}" - expect(@controller.instance_variable_get(:@error)).to_not(be_nil) + expect(@controller.instance_variable_get(:@error)).not_to be_nil expect(response).to render_template(:service_sign_in) end @@ -88,10 +88,4 @@ expect(response).to redirect_to("https://www.horse.service.gov.uk/account?horse=brown&_ga=1.1111111.1111111.111111111") end - - def path_for(content_item, locale = nil) - base_path = content_item["base_path"].sub(/^\//, "") - base_path.gsub!(/\.#{locale}$/, "") if locale - base_path - end end diff --git a/spec/requests/step_navigation_controller_spec.rb b/spec/requests/step_navigation_spec.rb similarity index 59% rename from spec/requests/step_navigation_controller_spec.rb rename to spec/requests/step_navigation_spec.rb index 97552260b8..fe81aeb282 100644 --- a/spec/requests/step_navigation_controller_spec.rb +++ b/spec/requests/step_navigation_spec.rb @@ -1,27 +1,26 @@ -RSpec.describe(ContentItemsController, type: :request) do +RSpec.describe("Step Navigation", type: :request) do %w[guide answer publication].each do |schema_name| it "#{schema_name} shows step by step navigation where relevant" do content_item = content_store_has_schema_example(schema_name, "#{schema_name}-with-step-navs") content_item["base_path"] = "/pass-plus" - path = content_item["base_path"][(1..)] stub_content_store_has_item(content_item["base_path"], content_item) - @controller.stubs(:page_in_scope?).returns(false) - get(:show, params: { path: }) + allow_any_instance_of(ContentItemsController).to receive(:page_in_scope?).and_return(false) + get content_item["base_path"] - expect(response.status_code).to eq(:ok) + expect(response.status).to eq(200) expect(response.body).to include("Learn to drive a car: step by step") end it "#{schema_name} does not show step by step navigation where relevant" do content_item = content_store_has_schema_example(schema_name, schema_name) content_item["base_path"] = "/not-part-of-a-step-by-step" - path = content_item["base_path"][(1..)] stub_content_store_has_item(content_item["base_path"], content_item) - @controller.stubs(:page_in_scope?).returns(false) - get(:show, params: { path: }) + allow_any_instance_of(ContentItemsController).to receive(:page_in_scope?).and_return(false) - expect(response.status_code).to eq(:ok) - expect(response.body).not_to include?("Learn to drive a car: step by step") + get content_item["base_path"] + + expect(response.status).to eq(200) + expect(response.body).not_to include("Learn to drive a car: step by step") end end end diff --git a/spec/support/govuk_content_schema_examples.rb b/spec/support/govuk_content_schema_examples.rb index d143b76a4d..9187df4078 100644 --- a/spec/support/govuk_content_schema_examples.rb +++ b/spec/support/govuk_content_schema_examples.rb @@ -36,6 +36,12 @@ def stub_parent_breadcrumbs(document, schema) stub_content_store_has_item(parents.first["base_path"], document) if schema == "html_publication" end + def path_for(content_item, locale = nil) + base_path = content_item["base_path"].sub(/^\//, "") + base_path.gsub!(/\.#{locale}$/, "") if locale + base_path + end + module ClassMethods def all_examples_for_supported_schemas supported_schemas.flat_map { |format| GovukSchemas::Example.find_all(format) }