Skip to content

Commit

Permalink
Merge pull request #9662 from alphagov/landing-page-featured-images-2
Browse files Browse the repository at this point in the history
Add support for landing page images in image / featured blocks
  • Loading branch information
richardTowers authored Nov 28, 2024
2 parents 5a8efc2 + b7b11b5 commit 5e9bf71
Show file tree
Hide file tree
Showing 11 changed files with 311 additions and 5 deletions.
51 changes: 51 additions & 0 deletions app/models/concerns/landing_page_image_block.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
module LandingPageImageBlock
extend ActiveSupport::Concern

IMAGE_PATTERN = /^\[Image:\s*(.*?)\s*\]/
EXPECTED_IMAGE_KINDS = {
desktop_image: "landing_page_image",
tablet_image: "landing_page_image",
mobile_image: "landing_page_image",
}.freeze

included do
attr_reader :desktop_image, :tablet_image, :mobile_image, :alt

validates :desktop_image, :tablet_image, :mobile_image, presence: true
validates_each :desktop_image, :tablet_image, :mobile_image do |record, attr, value|
next if value.blank?

expected_kind = EXPECTED_IMAGE_KINDS.fetch(attr)
actual_kind = value.image_data.image_kind
record.errors.add(attr, "is of the wrong image kind: #{actual_kind}") if actual_kind != expected_kind
end

def present_image
{
"image" => {
"alt" => alt,
"sources" => present_image_sources,
},
}
end

def present_image_sources
{
"desktop" => desktop_image.url(:landing_page_desktop_1x),
"desktop_2x" => desktop_image.url(:landing_page_desktop_2x),
"tablet" => tablet_image.url(:landing_page_tablet_1x),
"tablet_2x" => tablet_image.url(:landing_page_tablet_2x),
"mobile" => mobile_image.url(:landing_page_mobile_1x),
"mobile_2x" => mobile_image.url(:landing_page_mobile_2x),
}
end

def find_image(image_expression)
match = IMAGE_PATTERN.match(image_expression)
return if match.nil?

image_id = match.captures.first
images.find { _1.filename == image_id }
end
end
end
2 changes: 1 addition & 1 deletion app/models/landing_page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def self.access_limited_by_default?
end

def permitted_image_kinds
super + Whitehall.image_kinds.values.select { _1.permits?("hero") }
super + Whitehall.image_kinds.values.select { _1.permitted_uses.intersect?(%w[hero landing_page]) }
end

def landing_page_body
Expand Down
4 changes: 3 additions & 1 deletion app/models/landing_page/block_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ def self.build(block, images)
in { type: "card", card_content: { blocks: } }
LandingPage::CompoundBlock.new(block, images, "card_content", blocks)
in { type: "featured", featured_content: { blocks: } }
LandingPage::CompoundBlock.new(block, images, "featured_content", blocks)
LandingPage::FeaturedBlock.new(block, images, blocks)
in { type: "hero", hero_content: { blocks: } }
LandingPage::HeroBlock.new(block, images, blocks)
in { type: "image" }
LandingPage::ImageBlock.new(block, images)
in { type: String, blocks: Array }
LandingPage::ParentBlock.new(block, images)
else
Expand Down
18 changes: 18 additions & 0 deletions app/models/landing_page/featured_block.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
class LandingPage::FeaturedBlock < LandingPage::CompoundBlock
include ActiveModel::API
include LandingPageImageBlock

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

image_sources = @source.dig("image", "sources") || {}
@desktop_image = find_image(image_sources["desktop"])
@tablet_image = find_image(image_sources["tablet"])
@mobile_image = find_image(image_sources["mobile"])
@alt = @source.dig("image", "alt") || ""
end

def present_for_publishing_api
super.merge(present_image)
end
end
18 changes: 18 additions & 0 deletions app/models/landing_page/image_block.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
class LandingPage::ImageBlock < LandingPage::BaseBlock
include ActiveModel::API
include LandingPageImageBlock

def initialize(source, images)
super(source, images)

image_sources = @source.dig("image", "sources") || {}
@desktop_image = find_image(image_sources["desktop"])
@tablet_image = find_image(image_sources["tablet"])
@mobile_image = find_image(image_sources["mobile"])
@alt = @source.dig("image", "alt") || ""
end

def present_for_publishing_api
super.merge(present_image)
end
end
33 changes: 33 additions & 0 deletions config/image_kinds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,39 @@ default:
width: 216
height: 140
from_version: s960
landing_page_image:
display_name: "Landing page image (2x pixel density) (1260 by 944)"
valid_width: 1416
valid_height: 1062
permitted_uses:
- landing_page
versions:
# Note - tablet landing page images are bigger than desktop
# because of the way they're laid out, so the tablet version
# comes first and is the parent of all others.
- name: landing_page_tablet_2x
width: 1416
height: 1062
- name: landing_page_tablet_1x
width: 708
height: 531
from_version: landing_page_tablet_2x
- name: landing_page_desktop_2x
width: 1260
height: 944
from_version: landing_page_tablet_2x
- name: landing_page_desktop_1x
width: 630
height: 472
from_version: landing_page_tablet_2x
- name: landing_page_mobile_2x
width: 1220
height: 914
from_version: landing_page_tablet_2x
- name: landing_page_mobile_1x
width: 610
height: 457
from_version: landing_page_tablet_2x
hero_mobile:
display_name: "Mobile hero image (2x pixel density) (1280 by 960)"
valid_width: 1280
Expand Down
20 changes: 20 additions & 0 deletions test/factories/image_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,26 @@ def image_data.file_url(variant)
end
end

factory :landing_page_image_data, class: ImageData do
file { image_fixture_file }

image_kind { "landing_page_image" }

after(:build) do |image_data|
image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: image_data.filename)
variants = %w[landing_page_desktop_2x landing_page_desktop_1x landing_page_tablet_2x landing_page_tablet_1x landing_page_mobile_2x landing_page_mobile_1x]
variants.each do |variant|
image_data.assets << build(:asset, asset_manager_id: "asset_manager_id_#{variant}", variant: Asset.variants[variant], filename: "#{variant}_#{image_data.filename}")
end

# Defining this method is a bit of a hack, but with FactoryBot created model,
# the file_url method just returns nil, which makes it less useful for testing
def image_data.file_url(variant)
"http://asset-manager/#{variant}"
end
end
end

factory :image_data, parent: :generic_image_data, traits: [:jpg]
factory :image_data_for_svg, parent: :generic_image_data, traits: [:svg]
factory :image_data_with_no_assets, parent: :generic_image_data
Expand Down
Binary file added test/fixtures/landing_page_image.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
83 changes: 83 additions & 0 deletions test/unit/app/models/landing_page/featured_block_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
require "test_helper"

class FeaturedBlockTest < ActiveSupport::TestCase
setup do
@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" => {
"alt" => "some alt text",
"sources" => {
"desktop" => "[Image: landing_page_image.png]",
"tablet" => "[Image: landing_page_image.png]",
"mobile" => "[Image: landing_page_image.png]",
},
},
"featured_content" => { "blocks" => @valid_featured_content_blocks },
}
end

test "valid when given correct params" do
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, @valid_featured_images, @valid_featured_content_blocks)
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)
expected_result = {
"type" => "featured",
"image" => {
"alt" => "some alt text",
"sources" => {
"desktop" => "http://asset-manager/landing_page_desktop_1x",
"desktop_2x" => "http://asset-manager/landing_page_desktop_2x",
"tablet" => "http://asset-manager/landing_page_tablet_1x",
"tablet_2x" => "http://asset-manager/landing_page_tablet_2x",
"mobile" => "http://asset-manager/landing_page_mobile_1x",
"mobile_2x" => "http://asset-manager/landing_page_mobile_2x",
},
},
"featured_content" => {
"blocks" => [{ "type" => "some-block-type" }],
},
}
assert_equal(expected_result, subject.present_for_publishing_api)
end

test "invalid when missing images" do
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config.except("image"), @valid_featured_images, @valid_featured_content_blocks)
assert subject.invalid?
assert_equal [
"Desktop image can't be blank",
"Tablet image can't be blank",
"Mobile image can't be blank",
], subject.errors.to_a
end

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)
assert subject.invalid?
assert_equal [
"Desktop image can't be blank",
"Tablet image can't be blank",
"Mobile image can't be blank",
], subject.errors.to_a
end

test "invalid when missing featured content blocks" do
subject = LandingPage::FeaturedBlock.new(@valid_featured_block_config, @valid_featured_images, nil)
assert subject.invalid?
assert_equal ["Content blocks can't be blank"], subject.errors.to_a
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)
assert subject.invalid?
assert_equal ["Type can't be blank"], subject.errors.to_a
end
end
64 changes: 64 additions & 0 deletions test/unit/app/models/landing_page/image_block_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
require "test_helper"

class ImageBlockTest < ActiveSupport::TestCase
setup do
@valid_landing_page_images = [
build(:image, image_data: build(:landing_page_image_data, file: upload_fixture("landing_page_image.png", "image/png"))),
]
@valid_image_block_config = {
"type" => "image",
"image" => {
"sources" => {
"desktop" => "[Image: landing_page_image.png]",
"tablet" => "[Image: landing_page_image.png]",
"mobile" => "[Image: landing_page_image.png]",
},
},
}
end

test "valid when given correct params" do
subject = LandingPage::ImageBlock.new(@valid_image_block_config, @valid_landing_page_images)
assert subject.valid?
end

test "presents hero blocks to publishing api" do
subject = LandingPage::ImageBlock.new(@valid_image_block_config, @valid_landing_page_images)
expected_result = {
"type" => "image",
"image" => {
"alt" => "",
"sources" => {
"desktop" => "http://asset-manager/landing_page_desktop_1x",
"desktop_2x" => "http://asset-manager/landing_page_desktop_2x",
"tablet" => "http://asset-manager/landing_page_tablet_1x",
"tablet_2x" => "http://asset-manager/landing_page_tablet_2x",
"mobile" => "http://asset-manager/landing_page_mobile_1x",
"mobile_2x" => "http://asset-manager/landing_page_mobile_2x",
},
},
}
assert_equal(expected_result, subject.present_for_publishing_api)
end

test "invalid when missing images" do
subject = LandingPage::ImageBlock.new(@valid_image_block_config.except("image"), @valid_landing_page_images)
assert subject.invalid?
assert_equal [
"Desktop image can't be blank",
"Tablet image can't be blank",
"Mobile image can't be blank",
], subject.errors.to_a
end

test "invalid when image expressions are not found" do
no_images = []
subject = LandingPage::ImageBlock.new(@valid_image_block_config, no_images)
assert subject.invalid?
assert_equal [
"Desktop image can't be blank",
"Tablet image can't be blank",
"Mobile image can't be blank",
], subject.errors.to_a
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,12 @@ class PublishingApi::LandingPagePresenterTest < ActiveSupport::TestCase
mobile: "[Image: hero_image_mobile_2x.png]"
hero_content:
blocks:
- type: govspeak
content: "some content"
- type: image
image:
sources:
desktop: "[Image: landing_page_image.png]"
tablet: "[Image: landing_page_image.png]"
mobile: "[Image: landing_page_image.png]"
YAML

landing_page = create(
Expand All @@ -156,6 +160,7 @@ class PublishingApi::LandingPagePresenterTest < ActiveSupport::TestCase
build(:image, image_data: build(:hero_image_data, image_kind: "hero_desktop", file: upload_fixture("hero_image_desktop_2x.png", "image/png"))),
build(:image, image_data: build(:hero_image_data, image_kind: "hero_tablet", file: upload_fixture("hero_image_tablet_2x.png", "image/png"))),
build(:image, image_data: build(:hero_image_data, image_kind: "hero_mobile", file: upload_fixture("hero_image_mobile_2x.png", "image/png"))),
build(:image, image_data: build(:landing_page_image_data, file: upload_fixture("landing_page_image.png", "image/png"))),
],
)

Expand Down Expand Up @@ -196,7 +201,19 @@ class PublishingApi::LandingPagePresenterTest < ActiveSupport::TestCase
}
},
hero_content: {
blocks: [ { type: "govspeak", content: String } ]
blocks: [{
type: "image",
image: {
sources: {
desktop_2x: "http://asset-manager/landing_page_desktop_2x",
desktop: "http://asset-manager/landing_page_desktop_1x",
tablet_2x: "http://asset-manager/landing_page_tablet_2x",
tablet: "http://asset-manager/landing_page_tablet_1x",
mobile_2x: "http://asset-manager/landing_page_mobile_2x",
mobile: "http://asset-manager/landing_page_mobile_1x",
}
},
}]
}
}],
},
Expand Down

0 comments on commit 5e9bf71

Please sign in to comment.