diff --git a/app/assets/javascripts/components/image-cropper.js b/app/assets/javascripts/components/image-cropper.js index c7f8001d976..17fd03188f0 100644 --- a/app/assets/javascripts/components/image-cropper.js +++ b/app/assets/javascripts/components/image-cropper.js @@ -8,8 +8,8 @@ window.GOVUK.Modules = window.GOVUK.Modules || {} this.$image = this.$imageCropper.querySelector( '.app-c-image-cropper__image' ) - this.$targetWidth = 960 - this.$targetHeight = 640 + this.$targetWidth = parseInt(this.$imageCropper.dataset.width, 10) + this.$targetHeight = parseInt(this.$imageCropper.dataset.height, 10) } ImageCropper.prototype.init = function () { @@ -65,7 +65,7 @@ window.GOVUK.Modules = window.GOVUK.Modules || {} this.cropper = new window.Cropper(this.$image, { // eslint-disable-line viewMode: 2, - aspectRatio: 3 / 2, + aspectRatio: this.$targetWidth / this.$targetHeight, autoCrop: true, autoCropArea: 1, guides: false, diff --git a/app/controllers/admin/edition_images_controller.rb b/app/controllers/admin/edition_images_controller.rb index b4978049ac4..9c340a5316f 100644 --- a/app/controllers/admin/edition_images_controller.rb +++ b/app/controllers/admin/edition_images_controller.rb @@ -37,6 +37,9 @@ def create PublishingApiDocumentRepublishingWorker.perform_async(@edition.document_id) redirect_to edit_admin_edition_image_path(@edition, @new_image.id), notice: "#{@new_image.filename} successfully uploaded" elsif new_image_needs_cropping? + image_kind_config = @new_image.image_data.image_kind_config + @valid_width = image_kind_config.valid_width + @valid_height = image_kind_config.valid_height @data_url = image_data_url render :crop else @@ -90,6 +93,6 @@ def enforce_permissions! end def image_params - params.fetch(:image, {}).permit(image_data: [:file]) + params.fetch(:image, {}).permit(image_data: %i[file image_kind]) end end diff --git a/app/helpers/govspeak_helper.rb b/app/helpers/govspeak_helper.rb index 4f35b2a9db6..4c77a25f08b 100644 --- a/app/helpers/govspeak_helper.rb +++ b/app/helpers/govspeak_helper.rb @@ -48,6 +48,7 @@ def govspeak_html_attachment_to_html(html_attachment) def prepare_images(images) images + .select { |image| image.image_data&.image_kind_config&.permits? "govspeak_embed" } .select { |image| image.image_data&.all_asset_variants_uploaded? } .map do |image| { diff --git a/app/models/asset.rb b/app/models/asset.rb index 13f705c383e..9dc40ae4cf3 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -19,11 +19,9 @@ def unique_variant_for_each_assetable enum variant: { original: "original".freeze, thumbnail: "thumbnail".freeze, - s960: "s960".freeze, - s712: "s712".freeze, - s630: "s630".freeze, - s465: "s465".freeze, - s300: "s300".freeze, - s216: "s216".freeze, - } + }.merge( + Whitehall.image_kinds.values + .flat_map(&:version_names) + .to_h { |version_name| [version_name.to_sym, version_name.freeze] }, + ) end diff --git a/app/models/concerns/edition/images.rb b/app/models/concerns/edition/images.rb index 50e6d2ca5d4..5a3c78d3d56 100644 --- a/app/models/concerns/edition/images.rb +++ b/app/models/concerns/edition/images.rb @@ -39,6 +39,10 @@ def allows_image_attachments? true end + def permitted_image_kinds + Whitehall.image_kinds.values_at("default") + end + private def no_substantive_attributes?(attrs) diff --git a/app/models/concerns/image_kind.rb b/app/models/concerns/image_kind.rb new file mode 100644 index 00000000000..1a482b59e05 --- /dev/null +++ b/app/models/concerns/image_kind.rb @@ -0,0 +1,25 @@ +module ImageKind + extend ActiveSupport::Concern + + included do + def assign_attributes(attributes) + # It's important that the image_kind attribute is assigned first, as it is intended to be used by + # carrierwave uploaders when they are mounted to work out which image versions to use. + image_kind = attributes.delete(:image_kind) + ordered_attributes = if image_kind.present? + { image_kind:, **attributes } + else + attributes + end + super ordered_attributes + end + + def image_kind + attributes.fetch("image_kind", "default") + end + + def image_kind_config + @image_kind_config ||= Whitehall.image_kinds.fetch(image_kind) + end + end +end diff --git a/app/models/featured_image_data.rb b/app/models/featured_image_data.rb index d0c7bec8be6..0cadb2d0d65 100644 --- a/app/models/featured_image_data.rb +++ b/app/models/featured_image_data.rb @@ -1,5 +1,6 @@ class FeaturedImageData < ApplicationRecord mount_uploader :file, FeaturedImageUploader, mount_on: :carrierwave_image + include ImageKind belongs_to :featured_imageable, polymorphic: true @@ -10,7 +11,7 @@ class FeaturedImageData < ApplicationRecord validates :file, presence: true validates :featured_imageable, presence: true - validates_with ImageValidator, size: [960, 640] + validates_with ImageValidator delegate :url, to: :file diff --git a/app/models/image_data.rb b/app/models/image_data.rb index a8d9e93ac97..9b0f9e05c5e 100644 --- a/app/models/image_data.rb +++ b/app/models/image_data.rb @@ -3,8 +3,8 @@ class ImageData < ApplicationRecord attr_accessor :validate_on_image - VALID_WIDTH = 960 - VALID_HEIGHT = 640 + include ImageKind + SVG_CONTENT_TYPE = "image/svg+xml".freeze has_many :images @@ -15,7 +15,7 @@ class ImageData < ApplicationRecord mount_uploader :file, ImageUploader, mount_on: :carrierwave_image validates :file, presence: { message: "cannot be uploaded. Choose a valid JPEG, PNG, SVG or GIF." } - validates_with ImageValidator, size: [VALID_WIDTH, VALID_HEIGHT], if: :image_changed? + validates_with ImageValidator, if: :image_changed? validate :filename_is_unique delegate :width, :height, to: :dimensions @@ -38,7 +38,7 @@ def bitmap? def all_asset_variants_uploaded? asset_variants = assets.map(&:variant).map(&:to_sym) - required_variants = bitmap? ? ImageUploader.versions.keys.push(:original) : [Asset.variants[:original].to_sym] + required_variants = file.active_version_names + [:original] (required_variants - asset_variants).empty? end @@ -55,10 +55,10 @@ def dimensions @dimensions ||= if valid? # Whitehall doesn't store local copies of original images. Once they've been # uploaded to Asset Manager, we can't expect them to exist locally again. - # But since every uploaded image has to be these exact dimensions, we can + # But since every uploaded image has to have valid dimensions, we can # be confident a valid image (either freshly uploaded, or already persisted) - # will be these exact dimensions. - Dimensions.new(VALID_WIDTH, VALID_HEIGHT) + # will have valid dimensions. + Dimensions.new(image_kind_config.valid_width, image_kind_config.valid_height) else image = MiniMagick::Image.open file.path Dimensions.new(image[:width], image[:height]) diff --git a/app/models/promotional_feature_item.rb b/app/models/promotional_feature_item.rb index 6e24c4e5d9a..9193712ac86 100644 --- a/app/models/promotional_feature_item.rb +++ b/app/models/promotional_feature_item.rb @@ -1,6 +1,8 @@ class PromotionalFeatureItem < ApplicationRecord VALID_YOUTUBE_URL_FORMAT = /\A(?:(?:https:\/\/youtu\.be\/)(.+)|(?:https:\/\/www\.youtube\.com\/watch\?v=)(.*?)(?:&|#|$).*)\Z/ + include ImageKind + belongs_to :promotional_feature, inverse_of: :promotional_feature_items has_one :organisation, through: :promotional_feature has_many :links, class_name: "PromotionalFeatureLink", dependent: :destroy, inverse_of: :promotional_feature_item @@ -10,7 +12,7 @@ class PromotionalFeatureItem < ApplicationRecord inverse_of: :assetable validates :summary, presence: true, length: { maximum: 500 } - validates_with ImageValidator, method: :image, size: [960, 640], if: :image_changed? + validates_with ImageValidator, method: :image, if: :image_changed? validates :title_url, uri: true, allow_blank: true validate :image_or_youtube_url_is_present validates :youtube_video_url, format: { diff --git a/app/models/topical_event_featuring_image_data.rb b/app/models/topical_event_featuring_image_data.rb index bfd5617326a..b330e6024b2 100644 --- a/app/models/topical_event_featuring_image_data.rb +++ b/app/models/topical_event_featuring_image_data.rb @@ -1,6 +1,8 @@ class TopicalEventFeaturingImageData < ApplicationRecord mount_uploader :file, FeaturedImageUploader, mount_on: :carrierwave_image + include ImageKind + has_one :topical_event_featuring, inverse_of: :image has_many :assets, as: :assetable, @@ -8,7 +10,7 @@ class TopicalEventFeaturingImageData < ApplicationRecord validates :file, presence: true - validates_with ImageValidator, size: [960, 640] + validates_with ImageValidator delegate :url, to: :file diff --git a/app/uploaders/featured_image_uploader.rb b/app/uploaders/featured_image_uploader.rb index 10d76d3b963..4a5fb3fa458 100644 --- a/app/uploaders/featured_image_uploader.rb +++ b/app/uploaders/featured_image_uploader.rb @@ -16,12 +16,11 @@ def extension_allowlist config.validate_integrity = true end - version(:s960) { process resize_to_fill: [960, 640] } - version(:s712, from_version: :s960) { process resize_to_fill: [712, 480] } - version(:s630, from_version: :s960) { process resize_to_fill: [630, 420] } - version(:s465, from_version: :s960) { process resize_to_fill: [465, 310] } - version(:s300, from_version: :s960) { process resize_to_fill: [300, 195] } - version(:s216, from_version: :s960) { process resize_to_fill: [216, 140] } + Whitehall.image_kinds.fetch("default").versions.each do |v| + version v.name, from_version: v.from_version&.to_sym do + process resize_to_fill: v.resize_to_fill + end + end def image_cache file.file.gsub("/govuk/whitehall/carrierwave-tmp/", "") if send("cache_id").present? diff --git a/app/uploaders/image_uploader.rb b/app/uploaders/image_uploader.rb index 14897a46a16..92d47ce161e 100644 --- a/app/uploaders/image_uploader.rb +++ b/app/uploaders/image_uploader.rb @@ -10,23 +10,16 @@ def extension_allowlist %w[jpg jpeg gif png svg] end - version :s960, if: :bitmap? do - process resize_to_fill: [960, 640] - end - version :s712, from_version: :s960, if: :bitmap? do - process resize_to_fill: [712, 480] - end - version :s630, from_version: :s960, if: :bitmap? do - process resize_to_fill: [630, 420] - end - version :s465, from_version: :s960, if: :bitmap? do - process resize_to_fill: [465, 310] - end - version :s300, from_version: :s960, if: :bitmap? do - process resize_to_fill: [300, 195] - end - version :s216, from_version: :s960, if: :bitmap? do - process resize_to_fill: [216, 140] + Whitehall.image_kinds.each do |image_kind, image_kind_config| + use_versions_for_this_image_kind_proc = lambda do |uploader, opts| + uploader.model.image_kind == image_kind && uploader.bitmap?(opts[:file]) + end + + image_kind_config.versions.each do |v| + version v.name, from_version: v.from_version&.to_sym, if: use_versions_for_this_image_kind_proc do + process resize_to_fill: v.resize_to_fill + end + end end def bitmap?(new_file) @@ -40,4 +33,10 @@ def image_cache file.file.gsub("/govuk/whitehall/carrierwave-tmp/", "") end end + + def active_version_names + # active_versions is protected, so it can only be called by subclasses + # it returns an array of [key, value] pairs, and we want the keys + active_versions.map(&:first) + end end diff --git a/app/validators/image_validator.rb b/app/validators/image_validator.rb index ebed308268f..1536f040f0f 100644 --- a/app/validators/image_validator.rb +++ b/app/validators/image_validator.rb @@ -8,7 +8,6 @@ class ImageValidator < ActiveModel::Validator def initialize(options = {}) super @method = options[:method] || :file - @size = options[:size] || nil @mime_types = options[:mime_types] || DEFAULT_MIME_TYPES end @@ -39,12 +38,12 @@ def validate_mime_type(record, file_path) end def validate_size(record, image) - return unless @size + return unless record.respond_to?(:image_kind_config) actual_width = image[:width] actual_height = image[:height] - target_width = @size[0] - target_height = @size[1] + target_width = record.image_kind_config.valid_width + target_height = record.image_kind_config.valid_height too_small = actual_width < target_width || actual_height < target_height too_large = actual_width > target_width || actual_height > target_height diff --git a/app/views/admin/edition_images/_image_upload.html.erb b/app/views/admin/edition_images/_image_upload.html.erb index ddd0a4ea4b0..e430e67fdb1 100644 --- a/app/views/admin/edition_images/_image_upload.html.erb +++ b/app/views/admin/edition_images/_image_upload.html.erb @@ -15,6 +15,21 @@ accept: "image/png, image/jpeg, image/gif, image/svg+xml", } %> + <% if @edition.permitted_image_kinds.size == 1 %> + <%= hidden_field_tag("image[image_data][image_kind]", @edition.permitted_image_kinds.first.name) %> + <% else %> + <%= render "govuk_publishing_components/components/radio", { + heading: "What kind of image is this?", + name: "image[image_data][image_kind]", + items: @edition.permitted_image_kinds.map do |image_kind| + { + value: image_kind.name, + text: image_kind.display_name, + } + end, + } %> + <% end %> + <%= render "govuk_publishing_components/components/details", { title: "You must use an SVG for charts and diagrams", } do %> diff --git a/app/views/admin/edition_images/crop.html.erb b/app/views/admin/edition_images/crop.html.erb index 53ddd56e979..b5a2a275f2c 100644 --- a/app/views/admin/edition_images/crop.html.erb +++ b/app/views/admin/edition_images/crop.html.erb @@ -14,11 +14,15 @@

If you need to crop your image, use your cursor or the arrow keys and “+” and “-” to select a part of the image. The part you select will be resized for GOV.UK. The shape is fixed.

+ <%= hidden_field_tag("image[image_data][image_kind]", @new_image.image_data.image_kind) %> + <%= render "components/image_cropper", { name: "image[image_data][file]", src: @data_url, filename: @new_image.filename, type: @new_image.content_type, + width: @valid_width, + height: @valid_height, } %>
diff --git a/app/views/components/_image_cropper.html.erb b/app/views/components/_image_cropper.html.erb index bd32d7bb8af..1d05e6e8f8f 100644 --- a/app/views/components/_image_cropper.html.erb +++ b/app/views/components/_image_cropper.html.erb @@ -1,4 +1,4 @@ -<%= tag.div class: "app-c-image-cropper", tabindex: "0", data: {module: "image-cropper", filename:, type:}, "aria-live" => "polite" do %> +<%= tag.div class: "app-c-image-cropper", tabindex: "0", data: { module: "image-cropper", filename:, type:, width:, height: }, "aria-live" => "polite" do %> <%= tag.div class: "app-c-image-cropper__container" do %> <%= tag.img class: "app-c-image-cropper__image", diff --git a/config/image_kinds.yml b/config/image_kinds.yml new file mode 100644 index 00000000000..a1e1c352ac0 --- /dev/null +++ b/config/image_kinds.yml @@ -0,0 +1,31 @@ +--- +default: + display_name: "Default (960 by 640)" + valid_width: 960 + valid_height: 640 + permitted_uses: + - govspeak_embed + versions: + - name: s960 + width: 960 + height: 640 + - name: s712 + width: 712 + height: 480 + from_version: s960 + - name: s630 + width: 630 + height: 420 + from_version: s960 + - name: s465 + width: 465 + height: 310 + from_version: s960 + - name: s300 + width: 300 + height: 195 + from_version: s960 + - name: s216 + width: 216 + height: 140 + from_version: s960 diff --git a/db/migrate/20241105092100_add_image_kind_to_image_data.rb b/db/migrate/20241105092100_add_image_kind_to_image_data.rb new file mode 100644 index 00000000000..cf89fbb2bd9 --- /dev/null +++ b/db/migrate/20241105092100_add_image_kind_to_image_data.rb @@ -0,0 +1,5 @@ +class AddImageKindToImageData < ActiveRecord::Migration[7.1] + def change + add_column :image_data, :image_kind, :string, default: "default", null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index dbb45a95833..f32de2533df 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -667,6 +667,7 @@ t.string "carrierwave_image" t.datetime "created_at", precision: nil t.datetime "updated_at", precision: nil + t.string "image_kind", default: "default", null: false end create_table "images", id: :integer, charset: "utf8mb3", collation: "utf8mb3_unicode_ci", force: :cascade do |t| diff --git a/lib/whitehall.rb b/lib/whitehall.rb index b6f14e934f6..1a9add51aa5 100644 --- a/lib/whitehall.rb +++ b/lib/whitehall.rb @@ -195,6 +195,10 @@ def self.worldwide_tagging_organisations YAML.load_file(Rails.root.join("config/worldwide_tagging_organisations.yml")) end + def self.image_kinds + @image_kinds ||= ImageKinds.build_image_kinds(YAML.load_file(Rails.root.join("config/image_kinds.yml"))) + end + def self.integration_or_staging? website_root = ENV.fetch("GOVUK_WEBSITE_ROOT", "") %w[integration staging].any? { |environment| website_root.include?(environment) } diff --git a/lib/whitehall/image_kinds.rb b/lib/whitehall/image_kinds.rb new file mode 100644 index 00000000000..684c4b26fd0 --- /dev/null +++ b/lib/whitehall/image_kinds.rb @@ -0,0 +1,51 @@ +module Whitehall + class ImageVersion + attr_reader :name, :width, :height, :from_version + + def initialize(version_config) + @name = version_config.fetch("name") + @width = version_config.fetch("width") + @height = version_config.fetch("height") + @from_version = version_config.fetch("from_version", nil) + end + + def deconstruct_keys(_keys) + { name:, width:, height:, from_version: } + end + + def resize_to_fill + [width, height] + end + end + + class ImageKind + attr_reader :name, :display_name, :valid_width, :valid_height, :permitted_uses, :versions + + def initialize(name, config) + @name = name + @display_name = config.fetch("display_name") + @valid_width = config.fetch("valid_width") + @valid_height = config.fetch("valid_height") + @permitted_uses = config.fetch("permitted_uses") + @versions = config.fetch("versions").map { |version_config| ImageVersion.new(version_config) }.freeze + end + + def deconstruct_keys(_keys) + { name:, display_name:, valid_width:, valid_height:, permitted_uses:, versions: } + end + + def version_names + versions.map(&:name) + end + + def permits?(use_case) + permitted_uses.include?(use_case) + end + end + + class ImageKinds + def self.build_image_kinds(hash) + hash.to_h { |name, config| [name, ImageKind.new(name, config)] }.freeze + end + end +end diff --git a/spec/javascripts/components/image-cropper-spec.js b/spec/javascripts/components/image-cropper-spec.js index 0003b15fc44..bfc47fc0540 100644 --- a/spec/javascripts/components/image-cropper-spec.js +++ b/spec/javascripts/components/image-cropper-spec.js @@ -10,6 +10,8 @@ describe('GOVUK.Modules.ImageCropper', () => { component = document.createElement('div') component.setAttribute('data-filename', 'test.png') component.setAttribute('data-type', 'image/png') + component.setAttribute('data-width', '960') + component.setAttribute('data-height', '640') component.setAttribute('class', 'app-c-image-cropper') component.innerHTML = ` diff --git a/test/components/image_cropper_test.rb b/test/components/image_cropper_test.rb index 7ab7b61f75d..12ce9f26950 100644 --- a/test/components/image_cropper_test.rb +++ b/test/components/image_cropper_test.rb @@ -17,6 +17,8 @@ def component_name src: "src", filename: "filename", type: "type", + width: 960, + height: 640, }) assert_select ".app-c-image-cropper" diff --git a/test/unit/app/helpers/govspeak_helper_test.rb b/test/unit/app/helpers/govspeak_helper_test.rb index 2a340ddb1b3..d276e542216 100644 --- a/test/unit/app/helpers/govspeak_helper_test.rb +++ b/test/unit/app/helpers/govspeak_helper_test.rb @@ -197,6 +197,25 @@ def edition_with_attachment(body:) refute_select_within_html html, ".image.embedded" end + test "should ignore images with image kinds which do not permit use in govspeak embeds" do + embed_code = "[Image: minister-of-funk.960x640.jpg]" + body = "#Heading\n\n#{embed_code}\n\n##Subheading" + image = build(:image) + image_data = image.image_data + def image_data.image_kind_config = Whitehall::ImageKind.new( + "some name", + "display_name" => "some display name", + "valid_width" => 0, + "valid_height" => 0, + "permitted_uses" => [], + "versions" => [], + ) + document = build(:published_news_article, images: [image], body:) + + html = govspeak_edition_to_html(document) + refute_select_within_html html, ".image.embedded" + end + test "should not convert documents with no block attachments" do text = "#Heading\n\n!@2" document = build(:published_detailed_guide, body: text) diff --git a/test/unit/app/models/concerns/image_kind_test.rb b/test/unit/app/models/concerns/image_kind_test.rb new file mode 100644 index 00000000000..235f59c4ed9 --- /dev/null +++ b/test/unit/app/models/concerns/image_kind_test.rb @@ -0,0 +1,34 @@ +require "test_helper" + +class ImageKindTest < ActiveSupport::TestCase + setup do + @test_instance = ( + Class.new do + include ActiveModel::Model + include ActiveModel::Attributes + include ActiveModel::AttributeAssignment + + include ImageKind + + attr_reader :assigned_attributes + + def assign_attributes(attributes) + @assigned_attributes = attributes + end + end + ).new + end + + test "adds an image_kind method with a default" do + assert_equal "default", @test_instance.image_kind + end + + test "reorders attributes so image_kind comes first" do + @test_instance.assign_attributes(file: "some file", image_kind: "some image kind") + assert_equal({ image_kind: "some image kind", file: "some file" }, @test_instance.assigned_attributes) + end + + test "loads image_kind_config" do + assert_instance_of Whitehall::ImageKind, @test_instance.image_kind_config + end +end diff --git a/test/unit/app/uploaders/featured_image_uploader_test.rb b/test/unit/app/uploaders/featured_image_uploader_test.rb index 4a660af4595..761fc2b7982 100644 --- a/test/unit/app/uploaders/featured_image_uploader_test.rb +++ b/test/unit/app/uploaders/featured_image_uploader_test.rb @@ -33,6 +33,11 @@ class FeaturedImageUploaderTest < ActiveSupport::TestCase assert_match %r{^system}, uploader.store_dir end + test "should use default versions" do + uploader = FeaturedImageUploader.new(create(:featured_image_data), "mounted-as") + assert_equal %i[s960 s712 s630 s465 s300 s216], uploader.versions.keys + end + private def assert_image_has_correct_size(asset_path) diff --git a/test/unit/app/validators/image_validator_test.rb b/test/unit/app/validators/image_validator_test.rb index 675980222cc..c3ab2d707da 100644 --- a/test/unit/app/validators/image_validator_test.rb +++ b/test/unit/app/validators/image_validator_test.rb @@ -1,7 +1,10 @@ require "test_helper" +require_relative "../../../../lib/whitehall/image_kinds" class ImageValidatorTest < ActiveSupport::TestCase - EXAMPLE_MODEL = ImageData + def setup + @example_model = ImageData + end test "should accept a good jpeg image" do assert_validates_as_valid(ImageValidator.new, "960x640_jpeg.jpg") @@ -28,22 +31,45 @@ class ImageValidatorTest < ActiveSupport::TestCase assert_validates_as_valid(subject, "960x640_gif.gif") end - test "with size option it should only accept original images of that size" do - subject = ImageValidator.new(size: [960, 640]) + test "with image kind config on the model it should only accept original images of a valid size" do + @example_model = Class.new(ImageData) do + def image_kind_config + Whitehall::ImageKind.new( + "some image kind", + "display_name" => "some image kind display name", + "permitted_uses" => [], + "valid_width" => 50, + "valid_height" => 33, + "versions" => [], + ) + end + end + + subject = ImageValidator.new + + assert_validates_as_valid(subject, "50x33_gif.gif") + assert_validates_as_invalid(subject, "960x640_jpeg.jpg") + end + + test "accepts any size if the model does not have image kind config" do + @example_model = Class.new(ImageData) do + undef_method :image_kind_config + end - assert_validates_as_invalid(subject, "50x33_gif.gif") + subject = ImageValidator.new + assert_validates_as_valid(subject, "50x33_gif.gif") assert_validates_as_valid(subject, "960x640_jpeg.jpg") end test "error type is :too_small when the image is too small" do - subject = ImageValidator.new(size: [960, 640]) + subject = ImageValidator.new too_small = build_example("50x33_gif.gif") subject.validate(too_small) assert too_small.errors.of_kind?(:file, :too_small) end test "error type is :too_large when the image is too large" do - subject = ImageValidator.new(size: [960, 640]) + subject = ImageValidator.new too_large = build_example("960x960_jpeg.jpg") subject.validate(too_large) assert too_large.errors.of_kind?(:file, :too_large) @@ -78,13 +104,13 @@ def assert_validates_as_invalid(validator, image_file_name) def build_example(file_name) if file_name.present? File.open(Rails.root.join("test/fixtures/images", file_name)) do |file| - EXAMPLE_MODEL.new(file:).tap do |image_data| + @example_model.new(file:).tap do |image_data| carrierwave_file = image_data.file.file carrierwave_file.content_type = content_type(file_name) end end else - EXAMPLE_MODEL.new + @example_model.new end end diff --git a/test/unit/lib/image_kinds_test.rb b/test/unit/lib/image_kinds_test.rb new file mode 100644 index 00000000000..e70b7e6c52a --- /dev/null +++ b/test/unit/lib/image_kinds_test.rb @@ -0,0 +1,91 @@ +require "test_helper" + +class ImageKindsTest < ActiveSupport::TestCase + test "builds image kinds when given valid config" do + result = Whitehall::ImageKinds.build_image_kinds( + "default" => { + "display_name" => "default display name", + "valid_width" => 1, + "valid_height" => 2, + "permitted_uses" => [], + "versions" => [ + { + "name" => "some name", + "width" => 3, + "height" => 4, + "from_version" => "some other name", + }, + ], + }, + ) + + assert_instance_of Hash, result + assert_pattern do + result["default"] => { + display_name: "default display name", + valid_width: 1, + valid_height: 2, + versions: [ + { + name: "some name", + width: 3, + height: 4, + from_version: "some other name", + } + ] + } + end + end + + test "gets version names" do + result = Whitehall::ImageKinds.build_image_kinds( + "default" => { + "display_name" => "default display name", + "valid_width" => 0, + "valid_height" => 0, + "permitted_uses" => [], + "versions" => %w[a b c d e f g].map do + { + "name" => _1, + "width" => 0, + "height" => 0, + } + end, + }, + ) + + assert_equal %w[a b c d e f g], result["default"].version_names + end + + test "#permits? checks if use cases are permitted" do + image_kind = Whitehall::ImageKind.new( + "default", + "display_name" => "default display name", + "valid_width" => 0, + "valid_height" => 0, + "permitted_uses" => %w[use_case_1 use_case_2 use_case_3], + "versions" => [], + ) + assert_equal(true, image_kind.permits?("use_case_1")) + assert_equal(true, image_kind.permits?("use_case_2")) + assert_equal(true, image_kind.permits?("use_case_3")) + assert_equal(false, image_kind.permits?("use_case_4")) + assert_equal(false, image_kind.permits?("use_case_5")) + end + + test "raises errors when given invalid config" do + # Missing display_name / valid_width / valid_height / versions keys + assert_raise(KeyError, "key not found") { Whitehall::ImageKinds.build_image_kinds("default" => {}) } + + # Missing versions.name / versions.width / versions.height keys + assert_raise(KeyError, "key not found") do + Whitehall::ImageKinds.build_image_kinds("default" => { + "valid_width" => 0, + "valid_height" => 0, + "versions" => [ + {}, + ], + }) + end + end +end diff --git a/test/unit/lib/whitehall_test.rb b/test/unit/lib/whitehall_test.rb index a4348c949fe..23e337c2446 100644 --- a/test/unit/lib/whitehall_test.rb +++ b/test/unit/lib/whitehall_test.rb @@ -24,6 +24,28 @@ class WhitehallTest < ActiveSupport::TestCase ENV["TEST_ENV_NUMBER"] = before end + test "Whitehall.image_kinds is populated with defaults from config" do + assert_pattern do + Whitehall.image_kinds["default"] => { + name: "default", + valid_width: 960, + valid_height: 640, + versions: [ + { name: "s960" }, + *_rest + ], + } + end + end + + test "Whitehall.image_kinds all have distinct version names" do + # It's important that version names are globally unique to ensure that + # asset variants in the database are not ambiguous + all_version_names = Whitehall.image_kinds.values.flat_map(&:version_names) + unique_version_names = all_version_names.uniq + assert_equal all_version_names.size, unique_version_names.size + end + test "Whitehall.integration_or_staging? tells us if we are in the right env" do before = ENV["GOVUK_WEBSITE_ROOT"]