Skip to content

Commit

Permalink
Ensure show_contents_list? returns a boolean
Browse files Browse the repository at this point in the history
The original implementation could result in the method returning nil
rather than true or false. Which is unexpected, and has been a barrier
to changing when and where we display contents lists because of confusing
test failures.

The test amended in this commit should have been failing all along, because
the body object passed in contains only a single H2 with no proceeding content of
416 characters, and therefore fails all of the requirements for showing a contents list.
Now that the show_contents_list returns true or false, this test has been updated accordingly.
  • Loading branch information
hannako committed Oct 14, 2024
1 parent 21a1a53 commit a8932bb
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 21 deletions.
33 changes: 13 additions & 20 deletions app/presenters/content_item/contents_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,23 @@ def show_contents_list?
return true if contents_items.count > 2
return false if no_first_item?

first_item_has_long_content? ||
first_item_has_long_table? ||
first_item_has_image_and_long_content? ||
first_item_has_image_and_long_table?
first_item_size_requirements_met?(character_count, table_row_count)
end

private

def first_item_size_requirements_met?(char_count, table_row_count)
first_item_character_count > char_count || first_item_table_rows > table_row_count
end

def character_count
first_item_has_image? ? MINIMUM_CHARACTER_COUNT_IF_IMAGE_PRESENT : MINIMUM_CHARACTER_COUNT
end

def table_row_count
first_item_has_image? ? MINIMUM_TABLE_ROW_COUNT_IF_IMAGE_PRESENT : MINIMUM_TABLE_ROW_COUNT
end

def extract_headings_with_ids
headings = parsed_body.css("h2").map do |heading|
id = heading.attribute("id")
Expand All @@ -39,10 +48,6 @@ def extract_headings_with_ids
headings.compact
end

def first_item_has_long_content?
first_item_character_count > MINIMUM_CHARACTER_COUNT
end

def first_item_content
element = first_item
first_item_text = ""
Expand All @@ -60,10 +65,6 @@ def first_item_character_count
@first_item_character_count ||= first_item_content.length
end

def first_item_has_long_table?
first_item_table_rows > MINIMUM_TABLE_ROW_COUNT
end

def find_first_table
element = first_item

Expand Down Expand Up @@ -91,14 +92,6 @@ def first_item_has_image?
end
end

def first_item_has_image_and_long_content?
first_item_has_image? && first_item_character_count > MINIMUM_CHARACTER_COUNT_IF_IMAGE_PRESENT
end

def first_item_has_image_and_long_table?
first_item_has_image? && first_item_table_rows > MINIMUM_TABLE_ROW_COUNT_IF_IMAGE_PRESENT
end

def parsed_body
@parsed_body ||= Nokogiri::HTML(body)
end
Expand Down
3 changes: 2 additions & 1 deletion test/presenters/content_item/contents_list_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ def setup
test "memoises the contents to avoid repeated processing and extraction" do
class << @contents_list
def body
'<h2 id="custom">A heading</h2>'
"<h2 id='one'>One</h2>
<p>#{Faker::Lorem.characters(number: 450)}</p>"
end
end

Expand Down

0 comments on commit a8932bb

Please sign in to comment.