From bbef5342a6a70c365c5a148ec45cec4715edd59b Mon Sep 17 00:00:00 2001 From: Simon Hughesdon Date: Mon, 30 Jul 2018 17:49:54 +0100 Subject: [PATCH 1/3] Add test for services presenter Adding this flushed out that we were making an unnecessary request when taxon ids weren't provided. --- app/presenters/supergroups/services.rb | 17 +++++--- test/presenters/supergroups/services_test.rb | 42 ++++++++++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 test/presenters/supergroups/services_test.rb diff --git a/app/presenters/supergroups/services.rb b/app/presenters/supergroups/services.rb index f477de18c..d34422e53 100644 --- a/app/presenters/supergroups/services.rb +++ b/app/presenters/supergroups/services.rb @@ -4,11 +4,8 @@ class Services def initialize(current_path, taxon_ids) @taxon_ids = taxon_ids - @content = MostPopularContent.fetch( - content_ids: @taxon_ids, - current_path: current_path, - filter_content_purpose_supergroup: "services" - ) + @current_path = current_path + @content = fetch end def all_services @@ -34,6 +31,16 @@ def promoted_content private + def fetch + return [] if @taxon_ids.empty? + + MostPopularContent.fetch( + content_ids: @taxon_ids, + current_path: @current_path, + filter_content_purpose_supergroup: "services" + ) + end + def promoted_content_count 3 end diff --git a/test/presenters/supergroups/services_test.rb b/test/presenters/supergroups/services_test.rb new file mode 100644 index 000000000..035040bc0 --- /dev/null +++ b/test/presenters/supergroups/services_test.rb @@ -0,0 +1,42 @@ +require 'test_helper' + +class ServicesTest < ActiveSupport::TestCase + include RummagerHelpers + + test "services returns no results if taxon ids is a blank array" do + services = Supergroups::Services.new("/a-random-path", []) + refute services.any_services? + end + + test "services returns no results if there are taxon ids but no results" do + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + + stub_most_popular_content("/a-random-path", taxon_content_ids, 0, "services") + services = Supergroups::Services.new("/a-random-path", taxon_content_ids) + refute services.any_services? + end + + test "tagged_content returns hash with with 2 featured items and 0 normal items with 2 results" do + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + + stub_most_popular_content("/a-random-path", taxon_content_ids, 2, "services") + + services = Supergroups::Services.new("/a-random-path", taxon_content_ids) + + assert services.any_services? + assert_equal 0, services.tagged_content.count + assert_equal 2, services.promoted_content.count + end + + test "tagged_content returns hash with with 3 featured items and 2 normal items if there are enough results" do + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + + stub_most_popular_content("/a-random-path", taxon_content_ids, 5, "services") + + services = Supergroups::Services.new("/a-random-path", taxon_content_ids) + + assert services.any_services? + assert_equal 2, services.tagged_content.count + assert_equal 3, services.promoted_content.count + end +end From c1cb611b79e04609a06a92e0d9c8fda5d9089464 Mon Sep 17 00:00:00 2001 From: Simon Hughesdon Date: Tue, 31 Jul 2018 08:35:08 +0100 Subject: [PATCH 2/3] Indicate that test taxon ids are not magic ids Using guids can be confusing for future us --- .../supergroups/guidance_and_regulation_test.rb | 10 +++++----- .../supergroups/policy_and_engagement_test.rb | 10 +++++----- test/presenters/supergroups/transparency_test.rb | 10 +++++----- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/test/presenters/supergroups/guidance_and_regulation_test.rb b/test/presenters/supergroups/guidance_and_regulation_test.rb index f26adcc77..0d8833cfa 100644 --- a/test/presenters/supergroups/guidance_and_regulation_test.rb +++ b/test/presenters/supergroups/guidance_and_regulation_test.rb @@ -3,22 +3,22 @@ class GuidanceAndRegulationTest < ActiveSupport::TestCase include RummagerHelpers - def taxon_content_ids - ['c3c860fc-a271-4114-b512-1c48c0f82564', 'ff0e8e1f-4dea-42ff-b1d5-f1ae37807af2'] - end - test "tagged_content returns empty array if taxon ids is a blank array" do guidance_and_regulation = Supergroups::GuidanceAndRegulation.new("/no-particular-page", []) assert_equal [], guidance_and_regulation.tagged_content end test "tagged_content returns empty array if there are taxon ids but no results" do - stub_most_popular_content("/no-particular-page", [], 0, "guidance_and_regulation") + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + + stub_most_popular_content("/no-particular-page", taxon_content_ids, 0, "guidance_and_regulation") guidance_and_regulation = Supergroups::GuidanceAndRegulation.new("/no-particular-page", []) assert_equal [], guidance_and_regulation.tagged_content end test "tagged_content returns array with 2 items if there are 2 results" do + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + stub_most_popular_content("/no-particular-page", taxon_content_ids, 2, "guidance_and_regulation") guidance_and_regulation = Supergroups::GuidanceAndRegulation.new("/no-particular-page", taxon_content_ids) assert_equal 2, guidance_and_regulation.tagged_content.count diff --git a/test/presenters/supergroups/policy_and_engagement_test.rb b/test/presenters/supergroups/policy_and_engagement_test.rb index 5d0a6d6d8..fe21edbe0 100644 --- a/test/presenters/supergroups/policy_and_engagement_test.rb +++ b/test/presenters/supergroups/policy_and_engagement_test.rb @@ -3,22 +3,22 @@ class PolicyAndEngagementTest < ActiveSupport::TestCase include RummagerHelpers - def taxon_content_ids - ['c3c860fc-a271-4114-b512-1c48c0f82564', 'ff0e8e1f-4dea-42ff-b1d5-f1ae37807af2'] - end - test "tagged_content returns empty array if taxon ids is a blank array" do policy_and_engagement = Supergroups::PolicyAndEngagement.new("/a-random-path", []) assert_equal [], policy_and_engagement.tagged_content end test "tagged_content returns empty array if there are taxon ids but no results" do - stub_most_recent_content("/a-random-path", [], 0, "policy_and_engagement") + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + + stub_most_recent_content("/a-random-path", taxon_content_ids, 0, "policy_and_engagement") policy_and_engagement = Supergroups::PolicyAndEngagement.new("/a-random-path", []) assert_equal [], policy_and_engagement.tagged_content end test "tagged_content returns array with 2 items if there are 2 results" do + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + stub_most_recent_content("/a-random-path", taxon_content_ids, 2, "policy_and_engagement") policy_and_engagement = Supergroups::PolicyAndEngagement.new("/a-random-path", taxon_content_ids) assert_equal 2, policy_and_engagement.tagged_content.count diff --git a/test/presenters/supergroups/transparency_test.rb b/test/presenters/supergroups/transparency_test.rb index 7db6ae250..683123e48 100644 --- a/test/presenters/supergroups/transparency_test.rb +++ b/test/presenters/supergroups/transparency_test.rb @@ -3,22 +3,22 @@ class TransparencyTest < ActiveSupport::TestCase include RummagerHelpers - def taxon_content_ids - ['c3c860fc-a271-4114-b512-1c48c0f82564', 'ff0e8e1f-4dea-42ff-b1d5-f1ae37807af2'] - end - test "tagged_content returns empty array if taxon ids is a blank array" do policy_and_engagement = Supergroups::Transparency.new("/a-random-path", []) assert_equal [], policy_and_engagement.tagged_content end test "tagged_content returns empty array if there are taxon ids but no results" do - stub_most_recent_content("/a-random-path", [], 0, "transparency") + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + + stub_most_recent_content("/a-random-path", taxon_content_ids, 0, "transparency") policy_and_engagement = Supergroups::Transparency.new("/a-random-path", []) assert_equal [], policy_and_engagement.tagged_content end test "tagged_content returns array with 2 items if there are 2 results" do + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + stub_most_recent_content("/a-random-path", taxon_content_ids, 2, "transparency") policy_and_engagement = Supergroups::Transparency.new("/a-random-path", taxon_content_ids) assert_equal 2, policy_and_engagement.tagged_content.count From 4e661170ef513cd92ea463e5debfd3c5b19cac53 Mon Sep 17 00:00:00 2001 From: Simon Hughesdon Date: Tue, 31 Jul 2018 08:36:27 +0100 Subject: [PATCH 3/3] Add news and communications section News and comms are different from the other sections in that we show the image from the article if it's a featured link (and the image is present). This will be updated in future to get content images from rummager which removes the need to call content store for each item. We'll also update this to display results as one list rather than separate items. --- app/controllers/content_items_controller.rb | 10 ++- .../supergroups/news_and_communications.rb | 75 +++++++++++++++++ app/presenters/supergroups/supergroup.rb | 6 +- .../shared/_taxonomy_navigation.html.erb | 33 ++++++++ config/locales/en.yml | 1 + .../content_pages_navigation_test.rb | 41 ++++++++++ .../news_and_communications_test.rb | 82 +++++++++++++++++++ test/support/content_pages_nav_test_helper.rb | 2 + 8 files changed, 243 insertions(+), 7 deletions(-) create mode 100644 app/presenters/supergroups/news_and_communications.rb create mode 100644 test/presenters/supergroups/news_and_communications_test.rb diff --git a/app/controllers/content_items_controller.rb b/app/controllers/content_items_controller.rb index c753f2074..4d78f72ab 100644 --- a/app/controllers/content_items_controller.rb +++ b/app/controllers/content_items_controller.rb @@ -70,14 +70,16 @@ def load_taxonomy_navigation taxon_ids = taxons.map { |taxon| taxon["content_id"] } services = Supergroups::Services.new(content_item_path, taxon_ids) guidance_and_regulation = Supergroups::GuidanceAndRegulation.new(content_item_path, taxon_ids) + news = Supergroups::NewsAndCommunications.new(content_item_path, taxon_ids) policy_and_engagement = Supergroups::PolicyAndEngagement.new(content_item_path, taxon_ids) transparency = Supergroups::Transparency.new(content_item_path, taxon_ids) @taxonomy_navigation = { - services: (services.all_services if services.any_services?), - guidance_and_regulation: guidance_and_regulation.tagged_content, - policy_and_engagement: policy_and_engagement.tagged_content, - transparency: transparency.tagged_content, + services: (services.all_services if services.any_services?), + guidance_and_regulation: guidance_and_regulation.tagged_content, + news_and_communication: (news.all_news if news.any_news?), + policy_and_engagement: policy_and_engagement.tagged_content, + transparency: transparency.tagged_content, } @tagged_taxons = taxons.map do |taxon| diff --git a/app/presenters/supergroups/news_and_communications.rb b/app/presenters/supergroups/news_and_communications.rb new file mode 100644 index 000000000..2d7d1da6f --- /dev/null +++ b/app/presenters/supergroups/news_and_communications.rb @@ -0,0 +1,75 @@ +module Supergroups + class NewsAndCommunications < Supergroup + attr_reader :content + + def initialize(current_path, taxon_ids) + @taxon_ids = taxon_ids + @current_path = current_path + @content = fetch + end + + def all_news + { + documents: tagged_content, + promoted_content: promoted_content, + } + end + + def any_news? + @content.any? + end + + def tagged_content + items = @content.drop(promoted_content_count) + format_document_data(items) + end + + def promoted_content + items = @content.shift(promoted_content_count) + content = format_document_data(items, data_category: "ImageCardClicked") + + content.each do |document| + document_image = news_item_photo(document[:link][:path]) + document[:image] = { + url: document_image["url"], + alt: document_image["alt_text"], + context: document_image["context"] + } + end + + content + end + + private + + def fetch + return [] if @taxon_ids.empty? + + MostRecentContent.fetch( + content_ids: @taxon_ids, + current_path: @current_path, + filter_content_purpose_supergroup: "news_and_communications" + ) + end + + def promoted_content_count + 3 + end + + def news_item_photo(base_path) + default_news_image = { + "url" => "https://assets.publishing.service.gov.uk/government/assets/placeholder.jpg", + "alt_text" => "" + } + + news_item = ::Services.content_store.content_item(base_path).to_h + + image = news_item["details"]["image"] || default_news_image + date = Date.parse(news_item["public_updated_at"]).strftime("%d %B %Y") + document_type = news_item["document_type"].humanize + image["context"] = "#{document_type} - #{date}" + + image + end + end +end diff --git a/app/presenters/supergroups/supergroup.rb b/app/presenters/supergroups/supergroup.rb index 5f4b0b0d1..5fbba0876 100644 --- a/app/presenters/supergroups/supergroup.rb +++ b/app/presenters/supergroups/supergroup.rb @@ -8,7 +8,7 @@ def format_document_data(documents, data_category: nil, include_timestamp: true) link: { text: document["title"], path: document["link"], - data_attributes: data_attributes(document["link"], document["title"], index) + data_attributes: data_attributes(document["link"], document["title"], index, data_category) }, metadata: { document_type: document["content_store_document_type"].humanize @@ -27,9 +27,9 @@ def format_document_data(documents, data_category: nil, include_timestamp: true) end end - def data_attributes(base_path, link_text, index) + def data_attributes(base_path, link_text, index, data_category) { - track_category: data_module_label + "DocumentListClicked", + track_category: data_module_label + (data_category || "DocumentListClicked"), track_action: index, track_label: base_path, track_options: { diff --git a/app/views/shared/_taxonomy_navigation.html.erb b/app/views/shared/_taxonomy_navigation.html.erb index ce7e73721..210c86693 100644 --- a/app/views/shared/_taxonomy_navigation.html.erb +++ b/app/views/shared/_taxonomy_navigation.html.erb @@ -41,6 +41,39 @@ <% end %> + <% if taxonomy_navigation[:news_and_communication].present? %> +
+ <%= render "govuk_publishing_components/components/heading", { + text: t('supergroups.news_and_communications'), + heading_level: 3, + font_size: 24, + margin_bottom: 2 + } %> + + <% taxonomy_navigation[:news_and_communication][:promoted_content].in_groups_of(3, false) do |promo_group| %> +
+ <% promo_group.each do |promo| %> +
+ <%= render "govuk_publishing_components/components/image_card", { + context: promo[:image][:context], + href: promo[:link][:path], + heading_text: promo[:link][:text], + image_src: promo[:image][:url], + image_alt: promo[:image][:alt], + heading_level: 0, + href_data_attributes: promo[:link][:data_attributes] + } %> +
+ <% end %> +
+ <% end %> + + <%= render "govuk_publishing_components/components/document_list", { + items: taxonomy_navigation[:news_and_communication][:documents] + } %> +
+ <% end %> + <% if taxonomy_navigation[:policy_and_engagement].present? %>
<%= render "govuk_publishing_components/components/heading", { diff --git a/config/locales/en.yml b/config/locales/en.yml index 6506b5991..36739e5d6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -46,6 +46,7 @@ en: supergroups: services: "Services" guidance_and_regulation: "Guidance and regulation" + news_and_communications: "News and communications" policy_and_engagement: "Policy and engagement" transparency: "Transparency" see_more_supergroups: "See more %{supergroup}" diff --git a/test/integration/content_pages_navigation_test.rb b/test/integration/content_pages_navigation_test.rb index f5d70cdea..e3c04d642 100644 --- a/test/integration/content_pages_navigation_test.rb +++ b/test/integration/content_pages_navigation_test.rb @@ -58,6 +58,7 @@ class ContentPagesNavigationTest < ActionDispatch::IntegrationTest test "shows the Services section title and documents with tracking" do stub_rummager stub_empty_guidance + stub_empty_news stub_empty_policies stub_empty_transparency setup_variant_b @@ -91,6 +92,7 @@ class ContentPagesNavigationTest < ActionDispatch::IntegrationTest test "shows the Policy section title and documents with tracking" do stub_rummager stub_empty_guidance + stub_empty_news stub_empty_services stub_empty_transparency setup_variant_b @@ -120,6 +122,7 @@ class ContentPagesNavigationTest < ActionDispatch::IntegrationTest test "shows the Guidance section title and documents with tracking" do stub_rummager stub_empty_policies + stub_empty_news stub_empty_services stub_empty_transparency setup_variant_b @@ -149,6 +152,7 @@ class ContentPagesNavigationTest < ActionDispatch::IntegrationTest test "shows the Transparency section title and documents with tracking" do stub_rummager stub_empty_guidance + stub_empty_news stub_empty_policies stub_empty_services setup_variant_b @@ -175,6 +179,39 @@ class ContentPagesNavigationTest < ActionDispatch::IntegrationTest refute page.has_css?('h3', text: "Guidance and regulation") end + test "shows the News and comms section title and documents with tracking" do + stub_rummager + stub_empty_services + stub_empty_guidance + stub_empty_policies + stub_empty_transparency + setup_variant_b + + taxons = SINGLE_TAXON + + setup_and_visit_content_item_with_taxons('guide', taxons) + + assert page.has_css?('h3', text: "News and communications") + assert page.has_css?('.gem-c-image-card__title', text: 'Free school meals form') + assert page.has_css?('.gem-c-image-card__title-link[data-track-category="newsAndCommunicationsImageCardClicked"]', text: 'Free school meals form') + assert page.has_css?('.gem-c-image-card__title-link[data-track-action="1"]', text: 'Free school meals form') + assert page.has_css?('.gem-c-image-card__title-link[data-track-label="/government/publications/meals"]', text: 'Free school meals form') + + assert page.has_css?('.gem-c-document-list__item a[data-track-category="newsAndCommunicationsDocumentListClicked"]', text: 'Free school meals form') + assert page.has_css?('.gem-c-document-list__item a[data-track-action="1"]', text: 'Free school meals form') + assert page.has_css?('.gem-c-document-list__item a[data-track-label="/government/publications/meals"]', text: 'Free school meals form') + end + + test "does not show the News and comms section if there is no tagged content" do + stub_empty_rummager + setup_variant_b + + taxons = SINGLE_TAXON + + setup_and_visit_content_item_with_taxons('guide', taxons) + + refute page.has_css?('h3', text: "News and communications") + end def stub_empty_services Supergroups::Services.any_instance.stubs(:all_services).returns({}) @@ -184,6 +221,10 @@ def stub_empty_guidance Supergroups::GuidanceAndRegulation.any_instance.stubs(:tagged_content).returns([]) end + def stub_empty_news + Supergroups::NewsAndCommunications.any_instance.stubs(:tagged_content).returns([]) + end + def stub_empty_policies Supergroups::PolicyAndEngagement.any_instance.stubs(:tagged_content).returns([]) end diff --git a/test/presenters/supergroups/news_and_communications_test.rb b/test/presenters/supergroups/news_and_communications_test.rb new file mode 100644 index 000000000..a5aa607e0 --- /dev/null +++ b/test/presenters/supergroups/news_and_communications_test.rb @@ -0,0 +1,82 @@ +require 'test_helper' + +class NewsAndCommunicationsTest < ActiveSupport::TestCase + include RummagerHelpers + + test "finds no results if taxon ids is a blank array" do + news_and_comms = Supergroups::NewsAndCommunications.new("/a-random-path", []) + refute news_and_comms.any_news? + end + + test "finds no results if there are taxon ids but no results" do + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + + stub_most_recent_content("/a-random-path", taxon_content_ids, 0, "news_and_communications") + news_and_comms = Supergroups::NewsAndCommunications.new("/a-random-path", taxon_content_ids) + refute news_and_comms.any_news? + end + + test "finds 2 featured items and 0 normal items with 2 results" do + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + stub_most_recent_content("/a-random-path", taxon_content_ids, 2, "news_and_communications") + stub_content_store_items(2) + + news_and_comms = Supergroups::NewsAndCommunications.new("/a-random-path", taxon_content_ids) + + assert news_and_comms.any_news? + assert_equal 0, news_and_comms.tagged_content.count + assert_equal 2, news_and_comms.promoted_content.count + end + + test "finds 3 promoted items and 2 normal items if there are enough results" do + taxon_content_ids = ['any-old-taxon', 'some-other-taxon-id'] + stub_most_recent_content("/a-random-path", taxon_content_ids, 5, "news_and_communications") + stub_content_store_items(3) + + news_and_comms = Supergroups::NewsAndCommunications.new("/a-random-path", taxon_content_ids) + + news_and_comms.any_news? + assert_equal 2, news_and_comms.tagged_content.count + assert_equal 3, news_and_comms.promoted_content.count + end + + test "promoted content includes placeholder images if the content doesn't have one" do + placeholder_image = "https://assets.publishing.service.gov.uk/government/assets/placeholder.jpg" + taxon_content_ids = ['any-old-taxon'] + stub_most_recent_content("/a-random-path", taxon_content_ids, 1, "news_and_communications") + stub_content_store_items(1) + + news_and_comms = Supergroups::NewsAndCommunications.new("/a-random-path", taxon_content_ids) + + news_item = news_and_comms.promoted_content.first + + assert_equal news_item[:image][:url], placeholder_image + end + + test "promoted content includes the content image URL if the content has one" do + taxon_content_ids = ['any-old-taxon'] + stub_most_recent_content("/a-random-path", taxon_content_ids, 1, "news_and_communications") + + content = content_item_for_base_path("/content-item-0").merge( + "details": { + "image": { + "url": "an/image/path", + "alt_text": "some alt text" + } + } + ) + content_store_has_item("/content-item-0", content) + + news_and_comms = Supergroups::NewsAndCommunications.new("/a-random-path", taxon_content_ids) + + news_item = news_and_comms.promoted_content.first + + assert_equal news_item[:image][:url], "an/image/path" + end + + def stub_content_store_items(count) + count.times.map do |i| + content_store_has_item("/content-item-#{i}") + end + end +end diff --git a/test/support/content_pages_nav_test_helper.rb b/test/support/content_pages_nav_test_helper.rb index 49bea9472..8e9919aef 100644 --- a/test/support/content_pages_nav_test_helper.rb +++ b/test/support/content_pages_nav_test_helper.rb @@ -19,6 +19,8 @@ def stub_rummager "suggested_queries": [] }.to_json ) + + content_store_has_item('/government/publications/meals') end def stub_empty_rummager