From 0887aee46dd760ab95e744782b03baafa0c5157d Mon Sep 17 00:00:00 2001 From: Sonia Hussain Date: Fri, 11 Oct 2024 10:48:44 +0100 Subject: [PATCH] Refactor contents list logic and integration tests - Updated the show_contents_list? method in contents_list.rb to ensure that the contents are displayed whenever items are present. This simplifies the logic by removing the condition that checked for more than two items. - Revised the integration tests related to the content list, removing unnecessary tests that required three or more items to render, as these are no longer relevant. Tests now ensure check whether a content list appears if any items are present, rather than based on the number of items. - Refactored the document collection tests to better reflect their behaviour, focusing on the number of document collection groups that contain documents as the behaviour is adjusted for document collections. --- app/presenters/content_item/contents_list.rb | 3 +- .../corporate_information_page_test.rb | 13 ---- test/integration/detailed_guide_test.rb | 5 -- test/integration/document_collection_test.rb | 59 ++++++++++----- test/integration/specialist_document_test.rb | 5 +- test/integration/statistical_data_set_test.rb | 13 ---- .../topical_event_about_page_test.rb | 11 +-- test/integration/working_group_test.rb | 14 +--- .../content_item/contents_list_test.rb | 74 +++++++------------ 9 files changed, 76 insertions(+), 121 deletions(-) diff --git a/app/presenters/content_item/contents_list.rb b/app/presenters/content_item/contents_list.rb index 2fb462803..9bcdeec0f 100644 --- a/app/presenters/content_item/contents_list.rb +++ b/app/presenters/content_item/contents_list.rb @@ -19,8 +19,7 @@ def contents_items end def show_contents_list? - return false if contents_items.count < 2 - return true if contents_items.count > 2 + return true if contents_items.present? return false if no_first_item? first_item_size_requirements_met?(character_count, table_row_count) diff --git a/test/integration/corporate_information_page_test.rb b/test/integration/corporate_information_page_test.rb index e2e622159..25af2b528 100644 --- a/test/integration/corporate_information_page_test.rb +++ b/test/integration/corporate_information_page_test.rb @@ -17,19 +17,6 @@ class CorporateInformationPageTest < ActionDispatch::IntegrationTest ]) end - test "renders without contents list if it has fewer than 3 items" do - item = get_content_example("corporate_information_page") - item["details"]["body"] = "
-

Item one

Content about item one

-

Item two

Content about item two

-
" - - stub_content_store_has_item(item["base_path"], item.to_json) - visit_with_cachebust(item["base_path"]) - - assert_not page.has_css?(".gem-c-contents-list") - end - test "renders corporate information with body when present" do setup_and_visit_content_item("corporate_information_page") diff --git a/test/integration/detailed_guide_test.rb b/test/integration/detailed_guide_test.rb index 94ba6c670..e46d662bc 100644 --- a/test/integration/detailed_guide_test.rb +++ b/test/integration/detailed_guide_test.rb @@ -68,11 +68,6 @@ class DetailedGuideTest < ActionDispatch::IntegrationTest assert page.has_css?(".gem-c-contents-list") end - test "renders without contents list if it has fewer than 3 items" do - setup_and_visit_content_item("national_applicability_alternative_url_detailed_guide") - assert_not page.has_css?(".gem-c-contents-list") - end - test "conditionally renders a logo" do setup_and_visit_content_item("england-2014-to-2020-european-structural-and-investment-funds") diff --git a/test/integration/document_collection_test.rb b/test/integration/document_collection_test.rb index d6caaaf00..f14d1449a 100644 --- a/test/integration/document_collection_test.rb +++ b/test/integration/document_collection_test.rb @@ -5,16 +5,6 @@ class DocumentCollectionTest < ActionDispatch::IntegrationTest setup_and_visit_content_item("document_collection") assert_has_component_title(@content_item["title"]) assert page.has_text?(@content_item["description"]) - assert page.has_css?(".gem-c-contents-list") - end - - test "document collection with no body and 2 collection groups where 1st group has long body" do - content_item = get_content_example("document_collection") - content_item["details"]["collection_groups"][0]["body"] = Faker::Lorem.characters(number: 416) - stub_content_store_has_item(content_item["base_path"], content_item.to_json) - visit(content_item["base_path"]) - - assert page.has_css?(".gem-c-contents-list") end test "renders metadata and document footer" do @@ -34,25 +24,56 @@ class DocumentCollectionTest < ActionDispatch::IntegrationTest assert page.has_text?("Each regime page provides a current list of asset freeze targets designated by the United Nations (UN), European Union and United Kingdom, under legislation relating to current financial sanctions regimes.") end - test "renders contents with link to each collection group" do + test "adds a contents list, with one list item per document collection group, if the group contains documents" do setup_and_visit_content_item("document_collection") + assert_equal 6, @content_item["details"]["collection_groups"].size + @content_item["details"]["collection_groups"].each do |group| assert page.has_css?("nav a", text: group["title"]) end end - test "renders without contents list if it has fewer than 3 items" do - item = get_content_example("document_collection") - item["details"]["collection_groups"] = [ + test "ignores document collection groups that have no documents when presenting the contents list" do + setup_and_visit_content_item("document_collection") + @content_item["details"]["collection_groups"] << { "title" => "Empty Group", "documents" => [] } + assert_equal 7, @content_item["details"]["collection_groups"].size + + content_list_items = all("nav.gem-c-contents-list .gem-c-contents-list__list-item") + assert_equal 6, content_list_items.size + + @content_item["details"]["collection_groups"].each do |group| + next if group["documents"].empty? + + assert page.has_css?("nav a", text: group["title"]) + end + + assert page.has_css?(".gem-c-contents-list", text: "Contents") + end + + test "renders no contents list if body has multiple h2s and is long, but collection groups are empty" do + content_item = get_content_example("document_collection") + + content_item["details"]["body"] = <<~HTML +
+

One

+

#{Faker::Lorem.characters(number: 200)}

+

Two

+

#{Faker::Lorem.characters(number: 200)}

+

Three

+

#{Faker::Lorem.characters(number: 200)}

+
+ HTML + + content_item["details"]["collection_groups"] = [ { - "title" => "Item one", - "body" => "

Content about item one

", - "documents" => %w[a-content-id], + "body" => "
\n
", + "documents" => [], + "title" => "Empty Group", }, ] - stub_content_store_has_item(item["base_path"], item.to_json) - visit_with_cachebust(item["base_path"]) + stub_content_store_has_item(content_item["base_path"], content_item.to_json) + visit(content_item["base_path"]) assert_not page.has_css?(".gem-c-contents-list") end diff --git a/test/integration/specialist_document_test.rb b/test/integration/specialist_document_test.rb index 0f0d6cf69..fd129e0db 100644 --- a/test/integration/specialist_document_test.rb +++ b/test/integration/specialist_document_test.rb @@ -136,10 +136,9 @@ def assert_nested_content_item(heading) assert page.has_content?("on the Big Issue Invest website") end - test "does not render a contents list if there are fewer than three items in the contents list" do + test "renders a content list" do setup_and_visit_content_item("aaib-reports") - - assert_not page.has_css?("#contents .gem-c-contents-list") + assert page.has_css?(".gem-c-contents-list", text: "Contents") end test "renders a link to statutory instruments finder" do diff --git a/test/integration/statistical_data_set_test.rb b/test/integration/statistical_data_set_test.rb index 34c870914..9c97372f8 100644 --- a/test/integration/statistical_data_set_test.rb +++ b/test/integration/statistical_data_set_test.rb @@ -54,19 +54,6 @@ class StatisticalDataSetTest < ActionDispatch::IntegrationTest ]) end - test "renders without contents list if it has fewer than 3 items" do - item = get_content_example("statistical_data_set") - item["details"]["body"] = "
-

Item one

Content about item one

-

Item two

Content about item two

-
" - - stub_content_store_has_item(item["base_path"], item.to_json) - visit_with_cachebust(item["base_path"]) - - assert_not page.has_css?(".gem-c-contents-list") - end - test "does not render with the single page notification button" do setup_and_visit_content_item("statistical_data_set") assert_not page.has_css?(".gem-c-single-page-notification-button") diff --git a/test/integration/topical_event_about_page_test.rb b/test/integration/topical_event_about_page_test.rb index 8db25bc66..4ae748449 100644 --- a/test/integration/topical_event_about_page_test.rb +++ b/test/integration/topical_event_about_page_test.rb @@ -22,14 +22,9 @@ class TopicalEventAboutPageTest < ActionDispatch::IntegrationTest assert_not page.has_css?(".contents-list.contents-list-dashed") end - test "contents list not displayed when fewer than three items" do - @content_item = get_content_example("topical_event_about_page") - @content_item["details"]["body"] = body_with_two_contents_list_items - - stub_content_store_has_item(@content_item["base_path"], @content_item.to_json) - - visit_with_cachebust @content_item["base_path"] - assert_not page.has_css?(".gem-c-contents-list") + test "renders a content list" do + setup_and_visit_content_item("topical_event_about_page") + assert page.has_css?(".gem-c-contents-list", text: "Contents") end test "contents list displayed when fewer than three items and first item word count is greater than 100" do diff --git a/test/integration/working_group_test.rb b/test/integration/working_group_test.rb index 5c9aa4c2c..2124b96d0 100644 --- a/test/integration/working_group_test.rb +++ b/test/integration/working_group_test.rb @@ -42,17 +42,9 @@ class WorkingGroupTest < ActionDispatch::IntegrationTest assert page.has_text?("Some content") end - test "renders without contents list if it has fewer than 3 items" do - item = get_content_example("short") - item["details"]["body"] = "
-

Item one

Content about item one

-

Item two

Content about item two

-
" - - stub_content_store_has_item(item["base_path"], item.to_json) - visit_with_cachebust(item["base_path"]) - - assert_not page.has_css?(".gem-c-contents-list") + test "renders a content list" do + setup_and_visit_content_item("with_policies") + assert page.has_css?(".gem-c-contents-list", text: "Contents") end test "does not render with the single page notification button" do diff --git a/test/presenters/content_item/contents_list_test.rb b/test/presenters/content_item/contents_list_test.rb index 6008b6dd1..45504fa2c 100644 --- a/test/presenters/content_item/contents_list_test.rb +++ b/test/presenters/content_item/contents_list_test.rb @@ -84,71 +84,62 @@ def body @contents_list.contents_items end - test "#show_contents_list? returns true if number of contents items is less than 3 and the first item's character count is above 415" do + test "displays content list if there is an H2" do class << @contents_list def body - "

One

-

#{Faker::Lorem.characters(number: 220)}

-

#{Faker::Lorem.characters(number: 196)}

-

Two

-

#{Faker::Lorem.sentence}

" + "

One

" end end assert @contents_list.show_contents_list? end - test "#show_contents_list? returns true if number of contents items is 2 and the first item's character count is above 415 including a list" do + test "displays no contents list if there is no H2 and first item is less than 415" do class << @contents_list def body - "

One

-

#{Faker::Lorem.characters(number: 40)}

- -

#{Faker::Lorem.characters(number: 40)}

-

Two

-

#{Faker::Lorem.sentence}

" + "
+

#{Faker::Lorem.characters(number: 400)}

+
+ " end end - assert @contents_list.show_contents_list? + assert_not @contents_list.show_contents_list? end - test "#show_contents_list? returns true if number of contents items is 3 or more" do + test "displays contents list if the first item's character count is above 415, including a list" do class << @contents_list def body "

One

-

#{Faker::Lorem.sentence}

+

#{Faker::Lorem.characters(number: 40)}

+ +

#{Faker::Lorem.characters(number: 40)}

Two

-

#{Faker::Lorem.sentence}

-

Three

-

Pi

-

#{Faker::Lorem.sentence}

" +

#{Faker::Lorem.sentence}

" end end assert @contents_list.show_contents_list? end - test "#show_contents_list? returns false if number of contents times is less than 3 and first item's character count is less than 415" do + test "does not display contents list if the first item's character count is less than 415" do class << @contents_list def body - "

One

-

#{Faker::Lorem.characters(number: 10)}

-

#{Faker::Lorem.characters(number: 10)}

-

Two

+ "

#{Faker::Lorem.characters(number: 20)}

+

#{Faker::Lorem.characters(number: 20)}

#{Faker::Lorem.sentence}

" end end assert_not @contents_list.show_contents_list? end - test "#show_contents_list? returns true if number of table rows in the first item is more than 13" do + test "displays contents list if number of table rows in the first item is more than 13" do class << @contents_list def body base = "

One

\n\n" 14.times do - base += "\n\n\n" + base += "\n\n\n\n" end base += "\n
#{Faker::Lorem.word}#{Faker::Lorem.word}/td>\n
#{Faker::Lorem.word}#{Faker::Lorem.word}

Two

" end @@ -156,19 +147,19 @@ def body assert @contents_list.show_contents_list? end - test "#show_contents_list? returns false if number of table rows in the first item is less than 13" do + test "does not display contents list if number of table rows in the first item is less than 13" do class << @contents_list def body - "

One

\n\n + "
\n\n \n\n\n \n\n\n - \n
#{Faker::Lorem.word}#{Faker::Lorem.word}/td>\n
#{Faker::Lorem.word}#{Faker::Lorem.word}/td>\n

Two

" + \n" end end assert_not @contents_list.show_contents_list? end - test "#show_contents_list? returns true if image and over 224 characters are present in the first item" do + test "displays contents list if image is present and the first_item's character count is over 224" do class << @contents_list def body "

One

@@ -181,7 +172,7 @@ def body assert @contents_list.show_contents_list? end - test "#show_contents_list? returns true if image and table over 6 rows are present in the first item" do + test "displays contents list if an image and a table with more than 6 rows are present in the first item" do class << @contents_list def body base = "

One

@@ -195,17 +186,6 @@ def body assert @contents_list.show_contents_list? end - test "#show_contents_list? returns true if body is over 415 characters and has no h2s present" do - class << @contents_list - def body - "

#{Faker::Lorem.characters(number: 416)}

" - end - end - @contents_list.stubs(:contents_items).returns(["item 1", "item2"]) - @contents_list.stubs(:first_item).returns(first_element) - assert @contents_list.show_contents_list? - end - def first_element Nokogiri::HTML(@contents_list.body).css("div").first.first_element_child end