From db2f5968df5e0b181b79dae4ef01019227a6de6b Mon Sep 17 00:00:00 2001 From: Simon Hughesdon Date: Thu, 19 Sep 2019 10:16:36 +0100 Subject: [PATCH 1/2] Use the updated FAQ schema The FAQ schema now supports single page bodies rather than multipart guide sections. Some tests in this repo exercise a path where a guide has no parts which is why the conditional in the guide.html.erb view is required. The change to the schema allows us to add it to mainstream answers too. --- Gemfile | 2 +- Gemfile.lock | 10 +++++----- app/views/content_items/answer.html.erb | 2 +- app/views/content_items/guide.html.erb | 7 +++---- test/integration/answer_test.rb | 10 ++++++++++ test/integration/guide_test.rb | 14 ++++---------- 6 files changed, 24 insertions(+), 21 deletions(-) diff --git a/Gemfile b/Gemfile index 52bde6bb5..d09f647a2 100644 --- a/Gemfile +++ b/Gemfile @@ -16,7 +16,7 @@ gem 'gds-api-adapters', '~> 60.0' gem 'govuk_ab_testing', '~> 2.4' gem 'govuk_app_config', '~> 2.0' gem 'govuk_frontend_toolkit', '~> 8.2.0' -gem 'govuk_publishing_components', '~> 20.5.2' +gem 'govuk_publishing_components', '~> 21.0.0' gem 'plek', '~> 3.0' gem 'slimmer', '~> 13.1' diff --git a/Gemfile.lock b/Gemfile.lock index 35d014d2c..26c4bbf36 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -91,7 +91,7 @@ GEM faraday (0.15.4) multipart-post (>= 1.2, < 3) ffi (1.11.1) - gds-api-adapters (60.0.0) + gds-api-adapters (60.1.0) addressable link_header null_logger @@ -114,7 +114,7 @@ GEM govuk_frontend_toolkit (8.2.0) railties (>= 3.1.0) sass (>= 3.2.0) - govuk_publishing_components (20.5.2) + govuk_publishing_components (21.0.0) gds-api-adapters govuk_app_config kramdown @@ -254,7 +254,7 @@ GEM mime-types (>= 1.16, < 4.0) netrc (~> 0.8) robotex (1.0.0) - rouge (3.10.0) + rouge (3.11.0) rubocop (0.74.0) jaro_winkler (~> 1.5.1) parallel (~> 1.10) @@ -281,7 +281,7 @@ GEM sprockets (>= 2.8, < 4.0) sprockets-rails (>= 2.0, < 4.0) tilt (>= 1.1, < 3) - sassc (2.2.0) + sassc (2.2.1) ffi (~> 1.9) sassc-rails (2.1.2) railties (>= 4.0.0) @@ -367,7 +367,7 @@ DEPENDENCIES govuk_ab_testing (~> 2.4) govuk_app_config (~> 2.0) govuk_frontend_toolkit (~> 8.2.0) - govuk_publishing_components (~> 20.5.2) + govuk_publishing_components (~> 21.0.0) govuk_schemas (~> 4.0) govuk_test htmlentities (~> 4.3) diff --git a/app/views/content_items/answer.html.erb b/app/views/content_items/answer.html.erb index 1cdf8c43d..c38923472 100644 --- a/app/views/content_items/answer.html.erb +++ b/app/views/content_items/answer.html.erb @@ -1,6 +1,6 @@ <% content_for :extra_head_content do %> <%= machine_readable_metadata( - schema: :article + schema: :faq ) %> <% end %> diff --git a/app/views/content_items/guide.html.erb b/app/views/content_items/guide.html.erb index 1cf7c25c8..1be4779e2 100644 --- a/app/views/content_items/guide.html.erb +++ b/app/views/content_items/guide.html.erb @@ -1,9 +1,8 @@ <% content_for :extra_head_content do %> - <% schema = @content_item.requesting_a_part? ? :article : :faq %> - <%= machine_readable_metadata( - schema: schema, - canonical_url: @content_item.canonical_url + schema: :faq, + canonical_url: @content_item.canonical_url, + body: @content_item.has_valid_part? ? @content_item.current_part_body : nil ) %> <% end %> diff --git a/test/integration/answer_test.rb b/test/integration/answer_test.rb index ad7cab73b..e2e971df6 100644 --- a/test/integration/answer_test.rb +++ b/test/integration/answer_test.rb @@ -20,4 +20,14 @@ class AnswerTest < ActionDispatch::IntegrationTest assert page.has_css?('.gem-c-related-navigation__section-link--other[href="' + first_related_link["url"] + '"]', text: first_related_link["title"]) end end + + test "renders FAQ structured data" do + setup_and_visit_content_item('answer') + + schema_sections = page.find_all("script[type='application/ld+json']", visible: false) + schemas = schema_sections.map { |section| JSON.parse(section.text(:all)) } + + faq_schema = schemas.detect { |schema| schema["@type"] == "FAQPage" } + assert_equal faq_schema["headline"], @content_item['title'] + end end diff --git a/test/integration/guide_test.rb b/test/integration/guide_test.rb index 799e9e7f7..a0a099af4 100644 --- a/test/integration/guide_test.rb +++ b/test/integration/guide_test.rb @@ -83,24 +83,18 @@ class GuideTest < ActionDispatch::IntegrationTest schema_sections = page.find_all("script[type='application/ld+json']", visible: false) schemas = schema_sections.map { |section| JSON.parse(section.text(:all)) } - article_schema = schemas.detect { |schema| schema["@type"] == "Article" } - assert_nil article_schema - - qa_page_schema = schemas.detect { |schema| schema["@type"] == "FAQPage" } - assert_equal qa_page_schema["headline"], @content_item['title'] + faq_schema = schemas.detect { |schema| schema["@type"] == "FAQPage" } + assert_equal faq_schema["headline"], @content_item['title'] end - test "guide chapters show the article schema" do + test "guide chapters show the faq schema" do setup_and_visit_part_in_guide schema_sections = page.find_all("script[type='application/ld+json']", visible: false) schemas = schema_sections.map { |section| JSON.parse(section.text(:all)) } faq_schema = schemas.detect { |schema| schema["@type"] == "FAQPage" } - assert_nil faq_schema - - article_schema = schemas.detect { |schema| schema["@type"] == "Article" } - assert_equal article_schema["headline"], @content_item['title'] + assert_equal faq_schema["headline"], @content_item['title'] end def setup_and_visit_part_in_guide From 94568918386c7ecf3703be759c6a40b346ebfb94 Mon Sep 17 00:00:00 2001 From: Simon Hughesdon Date: Thu, 19 Sep 2019 15:56:32 +0100 Subject: [PATCH 2/2] Extract schema finding logic to helper This is repeated a few times now and doesn't add much by being there. --- test/integration/answer_test.rb | 5 +---- test/integration/guide_test.rb | 10 ++-------- test/test_helper.rb | 7 +++++++ 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/test/integration/answer_test.rb b/test/integration/answer_test.rb index e2e971df6..8fca111c5 100644 --- a/test/integration/answer_test.rb +++ b/test/integration/answer_test.rb @@ -23,11 +23,8 @@ class AnswerTest < ActionDispatch::IntegrationTest test "renders FAQ structured data" do setup_and_visit_content_item('answer') + faq_schema = find_structured_data(page, "FAQPage") - schema_sections = page.find_all("script[type='application/ld+json']", visible: false) - schemas = schema_sections.map { |section| JSON.parse(section.text(:all)) } - - faq_schema = schemas.detect { |schema| schema["@type"] == "FAQPage" } assert_equal faq_schema["headline"], @content_item['title'] end end diff --git a/test/integration/guide_test.rb b/test/integration/guide_test.rb index a0a099af4..d4ab510cc 100644 --- a/test/integration/guide_test.rb +++ b/test/integration/guide_test.rb @@ -79,21 +79,15 @@ class GuideTest < ActionDispatch::IntegrationTest test "guides show the faq page schema" do setup_and_visit_content_item('guide') + faq_schema = find_structured_data(page, "FAQPage") - schema_sections = page.find_all("script[type='application/ld+json']", visible: false) - schemas = schema_sections.map { |section| JSON.parse(section.text(:all)) } - - faq_schema = schemas.detect { |schema| schema["@type"] == "FAQPage" } assert_equal faq_schema["headline"], @content_item['title'] end test "guide chapters show the faq schema" do setup_and_visit_part_in_guide + faq_schema = find_structured_data(page, "FAQPage") - schema_sections = page.find_all("script[type='application/ld+json']", visible: false) - schemas = schema_sections.map { |section| JSON.parse(section.text(:all)) } - - faq_schema = schemas.detect { |schema| schema["@type"] == "FAQPage" } assert_equal faq_schema["headline"], @content_item['title'] end diff --git a/test/test_helper.rb b/test/test_helper.rb index 0428ad276..b8060339e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -182,4 +182,11 @@ def visit_with_cachebust(visit_uri) visit(uri) end + + def find_structured_data(page, schema_name) + schema_sections = page.find_all("script[type='application/ld+json']", visible: false) + schemas = schema_sections.map { |section| JSON.parse(section.text(:all)) } + + schemas.detect { |schema| schema["@type"] == schema_name } + end end