Skip to content

Commit

Permalink
Have CompoundBlock extract subblocks
Browse files Browse the repository at this point in the history
... instead of doing it with pattern matching in BlockFactory.

We want these to be optional in most cases, but putting them in the
patterns means we only use the correct block type if they're present.
Instead, we just use type: in the pattern, and have the blocks take care
of finding their config inside source.
  • Loading branch information
richardTowers committed Dec 19, 2024
1 parent caee21f commit 7c1ddbc
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 28 deletions.
16 changes: 7 additions & 9 deletions app/models/landing_page/block_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@ def self.build_all(blocks, images)

def self.build(block, images)
case block.with_indifferent_access
in { type: "box", box_content: { blocks: } }
LandingPage::CompoundBlock.new(block, images, "box_content", blocks)
in { type: "card", card_content: { blocks: } }
LandingPage::CompoundBlock.new(block, images, "card_content", blocks)
in { type: "featured", featured_content: { blocks: } }
LandingPage::FeaturedBlock.new(block, images, blocks)
in { type: "hero", hero_content: { blocks: } }
LandingPage::HeroBlock.new(block, images, blocks)
in { type: "box" }
LandingPage::CompoundBlock.new(block, images, "box_content")
in { type: "card" }
LandingPage::CompoundBlock.new(block, images, "card_content")
in { type: "featured" }
LandingPage::FeaturedBlock.new(block, images)
in { type: "hero" }
LandingPage::HeroBlock.new(block, images, nil)
LandingPage::HeroBlock.new(block, images)
in { type: "image" }
LandingPage::ImageBlock.new(block, images)
in { type: String, blocks: Array }
Expand Down
3 changes: 2 additions & 1 deletion app/models/landing_page/compound_block.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ class LandingPage::CompoundBlock < LandingPage::BaseBlock
content_blocks.each { |b| errors.merge!(b.errors) if b.invalid? }
end

def initialize(source, images, content_block_key, content_blocks)
def initialize(source, images, content_block_key)
super(source, images)
@content_block_key = content_block_key
content_blocks = source.dig(content_block_key, "blocks")
@content_blocks = LandingPage::BlockFactory.build_all(content_blocks, images)
end

Expand Down
4 changes: 2 additions & 2 deletions app/models/landing_page/featured_block.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ class LandingPage::FeaturedBlock < LandingPage::CompoundBlock
include ActiveModel::API
include LandingPageImageBlock

def initialize(source, images, content_blocks)
super(source, images, "featured_content", content_blocks)
def initialize(source, images)
super(source, images, "featured_content")

image_sources = @source.dig("image", "sources") || {}
@desktop_image = find_image(image_sources["desktop"])
Expand Down
4 changes: 2 additions & 2 deletions app/models/landing_page/hero_block.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ class LandingPage::HeroBlock < LandingPage::CompoundBlock
record.errors.add(attr, "is of the wrong image kind: #{actual_kind}") if actual_kind != expected_kind
end

def initialize(source, images, content_blocks)
super(source, images, "hero_content", content_blocks)
def initialize(source, images)
super(source, images, "hero_content")

image_sources = @source.dig("image", "sources") || {}
@desktop_image = find_image(image_sources["desktop"])
Expand Down
21 changes: 13 additions & 8 deletions test/unit/app/models/landing_page/featured_block_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ class FeaturedBlockTest < ActiveSupport::TestCase
@valid_featured_images = [
build(:image, image_data: build(:landing_page_image_data, file: upload_fixture("landing_page_image.png", "image/png"))),
]
@valid_featured_content_blocks = [{ "type" => "some-block-type" }]
@valid_featured_block_config = {
"type" => "featured",
"image" => {
Expand All @@ -16,17 +15,17 @@ class FeaturedBlockTest < ActiveSupport::TestCase
"mobile" => "[Image: landing_page_image.png]",
},
},
"featured_content" => { "blocks" => @valid_featured_content_blocks },
"featured_content" => { "blocks" => [{ "type" => "some-block-type" }] },
}
end

test "valid when given correct params" do
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, @valid_featured_images, @valid_featured_content_blocks)
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, @valid_featured_images)
assert subject.valid?
end

test "presents featured blocks to publishing api" do
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, @valid_featured_images, @valid_featured_content_blocks)
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, @valid_featured_images)
expected_result = {
"type" => "featured",
"image" => {
Expand All @@ -48,7 +47,7 @@ class FeaturedBlockTest < ActiveSupport::TestCase
end

test "invalid when missing images" do
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config.except("image"), @valid_featured_images, @valid_featured_content_blocks)
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config.except("image"), @valid_featured_images)
assert subject.invalid?
assert_equal [
"Desktop image can't be blank",
Expand All @@ -59,7 +58,7 @@ class FeaturedBlockTest < ActiveSupport::TestCase

test "invalid when image expressions are not found" do
no_images = []
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, no_images, @valid_featured_content_blocks)
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, no_images)
assert subject.invalid?
assert_equal [
"Desktop image can't be blank",
Expand All @@ -69,13 +68,19 @@ class FeaturedBlockTest < ActiveSupport::TestCase
end

test "valid when missing featured content blocks" do
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, @valid_featured_images, nil)
subject = LandingPage::FeaturedBlock.new(
@valid_featured_block_config.except("featured_content"),
@valid_featured_images,
)
assert subject.valid?
end

test "invalid when featured content blocks are invalid" do
invalid_blocks_config = [{ "invalid" => "because I do not have a type" }]
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, @valid_featured_images, invalid_blocks_config)
subject = LandingPage::FeaturedBlock.new(
@valid_featured_block_config.tap { _1["featured_content"]["blocks"] = invalid_blocks_config },
@valid_featured_images,
)
assert subject.invalid?
assert_equal ["Type can't be blank"], subject.errors.to_a
end
Expand Down
15 changes: 9 additions & 6 deletions test/unit/app/models/landing_page/hero_block_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ class HeroBlockTest < ActiveSupport::TestCase
end

test "valid when given correct params" do
subject = LandingPage::HeroBlock.new(@valid_hero_block_config, @valid_hero_block_images, @valid_hero_content_blocks)
subject = LandingPage::HeroBlock.new(@valid_hero_block_config, @valid_hero_block_images)
assert subject.valid?
end

test "presents hero blocks to publishing api" do
subject = LandingPage::HeroBlock.new(@valid_hero_block_config, @valid_hero_block_images, @valid_hero_content_blocks)
subject = LandingPage::HeroBlock.new(@valid_hero_block_config, @valid_hero_block_images)
expected_result = {
"type" => "hero",
"image" => {
Expand All @@ -49,7 +49,7 @@ class HeroBlockTest < ActiveSupport::TestCase
end

test "invalid when missing images" do
subject = LandingPage::HeroBlock.new(@valid_hero_block_config.except("image"), @valid_hero_block_images, @valid_hero_content_blocks)
subject = LandingPage::HeroBlock.new(@valid_hero_block_config.except("image"), @valid_hero_block_images)
assert subject.invalid?
assert_equal [
"Desktop image can't be blank",
Expand All @@ -60,7 +60,7 @@ class HeroBlockTest < ActiveSupport::TestCase

test "invalid when image expressions are not found" do
no_images = []
subject = LandingPage::HeroBlock.new(@valid_hero_block_config, no_images, @valid_hero_content_blocks)
subject = LandingPage::HeroBlock.new(@valid_hero_block_config, no_images)
assert subject.invalid?
assert_equal [
"Desktop image can't be blank",
Expand All @@ -70,13 +70,16 @@ class HeroBlockTest < ActiveSupport::TestCase
end

test "valid when missing hero content blocks" do
subject = LandingPage::HeroBlock.new(@valid_hero_block_config, @valid_hero_block_images, nil)
subject = LandingPage::HeroBlock.new(@valid_hero_block_config.except("hero_content"), @valid_hero_block_images)
subject.valid?
end

test "invalid when hero content blocks are invalid" do
invalid_blocks_config = [{ "invalid" => "because I do not have a type" }]
subject = LandingPage::HeroBlock.new(@valid_hero_block_config, @valid_hero_block_images, invalid_blocks_config)
subject = LandingPage::HeroBlock.new(
@valid_hero_block_config.tap { _1["hero_content"]["blocks"] = invalid_blocks_config },
@valid_hero_block_images,
)
assert subject.invalid?
assert_equal ["Type can't be blank"], subject.errors.to_a
end
Expand Down

0 comments on commit 7c1ddbc

Please sign in to comment.