From a3f437cfe0cc6a696449d7df8096ca0b36f78111 Mon Sep 17 00:00:00 2001 From: Simon Hughesdon Date: Tue, 10 Sep 2019 17:07:40 +0100 Subject: [PATCH 1/2] Deduplicate FAQ page schema presentation At present we're showing the same FAQ schema on each page of a guide. This is a bit suboptimal. We have the option of splitting it up and showing one question and answer per part/chapter in the guide, but it seems much better to present it to Google as a joined up publication, because that's how it's been designed. To resolve this problem, we fall back to the Article schema for chapter pages, so we're still presenting some schema.org content, but not presenting duplicate content. --- app/views/content_items/guide.html.erb | 4 +++- test/integration/guide_test.rb | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/views/content_items/guide.html.erb b/app/views/content_items/guide.html.erb index d1c30a3b9..1cf7c25c8 100644 --- a/app/views/content_items/guide.html.erb +++ b/app/views/content_items/guide.html.erb @@ -1,6 +1,8 @@ <% content_for :extra_head_content do %> + <% schema = @content_item.requesting_a_part? ? :article : :faq %> + <%= machine_readable_metadata( - schema: :faq, + schema: schema, canonical_url: @content_item.canonical_url ) %> <% end %> diff --git a/test/integration/guide_test.rb b/test/integration/guide_test.rb index 227e18cc5..799e9e7f7 100644 --- a/test/integration/guide_test.rb +++ b/test/integration/guide_test.rb @@ -83,7 +83,31 @@ 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'] end + + test "guide chapters show the article 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'] + end + + def setup_and_visit_part_in_guide + @content_item = get_content_example("guide").tap do |item| + chapter_path = "#{item['base_path']}/key-stage-1-and-2" + content_store_has_item(chapter_path, item.to_json) + visit_with_cachebust(chapter_path) + end + end end From bea1e5e7ca85752dd53ff30e46e0d2b3a73f57a6 Mon Sep 17 00:00:00 2001 From: Simon Hughesdon Date: Tue, 10 Sep 2019 17:26:34 +0100 Subject: [PATCH 2/2] Resolve some test deprecation warnings Resolved due to messages like: > DEPRECATED: Use assert_nil if expecting nil from test/services/request_helper_test.rb:7. > This will fail in Minitest 6. --- test/helpers/application_helper_test.rb | 2 +- test/models/http_feature_flags_test.rb | 2 +- test/services/request_helper_test.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb index e4c5560ef..0a09ec3dc 100644 --- a/test/helpers/application_helper_test.rb +++ b/test/helpers/application_helper_test.rb @@ -15,7 +15,7 @@ class ApplicationHelperTest < ActionView::TestCase test "#t_locale_fallback returns nil for a string with a locale translation" do fallback = t_locale_fallback("content_item.schema_name.imported", count: 1, locale: :de) - assert_equal nil, fallback + assert_nil fallback end test "#t_locale_fallback returns default locale for a string with no locale translation" do diff --git a/test/models/http_feature_flags_test.rb b/test/models/http_feature_flags_test.rb index df2b90abc..7e505d63c 100644 --- a/test/models/http_feature_flags_test.rb +++ b/test/models/http_feature_flags_test.rb @@ -58,7 +58,7 @@ class HttpFeatureFlagsTest < ActiveSupport::TestCase feature_flag_value = instance.get_feature_flag('USE_MAGIC') - assert_equal nil, feature_flag_value + assert_nil feature_flag_value end test 'get_feature_flag returns feature flag value when feature flag exists' do diff --git a/test/services/request_helper_test.rb b/test/services/request_helper_test.rb index e7325ee27..1e7c5c948 100644 --- a/test/services/request_helper_test.rb +++ b/test/services/request_helper_test.rb @@ -4,7 +4,7 @@ class RequestHelperTest < ActiveSupport::TestCase test 'get_header returns nil when header does not exist' do header_val = RequestHelper.get_header('Govuk-Example-Header', {}) - assert_equal nil, header_val + assert_nil header_val end test 'get_header returns header when header exists' do