diff --git a/app/models/alchemy/essence_picture.rb b/app/models/alchemy/essence_picture.rb index 0ee9521d70..8296149a8e 100644 --- a/app/models/alchemy/essence_picture.rb +++ b/app/models/alchemy/essence_picture.rb @@ -56,23 +56,35 @@ class EssencePicture < ActiveRecord::Base # @option options crop [Boolean] # If set to true the picture will be cropped to fit the size value. # + # @return [String] def picture_url(options = {}) return if picture.nil? - options = { - format: picture.default_render_format, - crop_from: crop_from, - crop_size: crop_size - }.merge(options) + picture.url picture_url_options.merge(options) + end - picture.url(options) + # Picture rendering options + # + # Returns the +default_render_format+ of the associated +Alchemy::Picture+ + # together with the +crop_from+ and +crop_size+ values + # + # @return [HashWithIndifferentAccess] + def picture_url_options + return {} if picture.nil? + + { + format: picture.default_render_format, + crop_from: crop_from.presence, + crop_size: crop_size.presence + }.with_indifferent_access end - # Renders a thumbnail representation of the assigned image + # Returns an url for the thumbnail representation of the assigned picture # # It takes cropping values into account, so it always represents the current # image displayed in the frontend. # + # @return [String] def thumbnail_url(options = {}) return if picture.nil? @@ -96,6 +108,7 @@ def thumbnail_url(options = {}) # @param max [Integer] # The maximum length of the text returned. # + # @return [String] def preview_text(max = 30) return "" if picture.nil? picture.name.to_s[0..max - 1] @@ -103,6 +116,7 @@ def preview_text(max = 30) # A Hash of coordinates suitable for the graphical image cropper. # + # @return [Hash] def cropping_mask return if crop_from.blank? || crop_size.blank? crop_from = point_from_string(read_attribute(:crop_from)) @@ -112,6 +126,8 @@ def cropping_mask end # Returns a serialized ingredient value for json api + # + # @return [String] def serialized_ingredient picture_url(content.settings) end @@ -132,13 +148,15 @@ def crop_values_present? private def fix_crop_values - %w(crop_from crop_size).each do |crop_value| - write_attribute crop_value, normalize_crop_value(crop_value) + %i(crop_from crop_size).each do |crop_value| + if self[crop_value].is_a?(String) + write_attribute crop_value, normalize_crop_value(crop_value) + end end end def normalize_crop_value(crop_value) - send(crop_value).to_s.split('x').map { |n| normalize_number(n) }.join('x') + self[crop_value].split('x').map { |n| normalize_number(n) }.join('x') end def normalize_number(number) diff --git a/app/models/alchemy/essence_picture_view.rb b/app/models/alchemy/essence_picture_view.rb index f2ae2ad2f5..e5c5348b10 100644 --- a/app/models/alchemy/essence_picture_view.rb +++ b/app/models/alchemy/essence_picture_view.rb @@ -14,7 +14,7 @@ class EssencePictureView disable_link: false, srcset: [], sizes: [] - } + }.with_indifferent_access def initialize(content, options = {}, html_options = {}) @content = content diff --git a/app/models/alchemy/picture/transformations.rb b/app/models/alchemy/picture/transformations.rb index d01e9793e5..64f3c4a63b 100644 --- a/app/models/alchemy/picture/transformations.rb +++ b/app/models/alchemy/picture/transformations.rb @@ -111,12 +111,11 @@ def image_size end # An Image smaller than dimensions - # can not be cropped to string - unless upsample is true. + # can not be cropped to given size - unless upsample is true. # def can_be_cropped_to(string, upsample = false) - dimensions = sizes_from_string(string) return true if upsample - is_bigger_than(dimensions) + is_bigger_than sizes_from_string(string) end # Returns true if the class we're included in has a meaningful render_size attribute diff --git a/app/models/alchemy/picture/url.rb b/app/models/alchemy/picture/url.rb index 57560ce0e3..02f58da0fa 100644 --- a/app/models/alchemy/picture/url.rb +++ b/app/models/alchemy/picture/url.rb @@ -46,7 +46,7 @@ def processed_image(image, options = {}) return image unless size.present? && has_convertible_format? - if options[:crop_size].present? && options[:crop_from].present? || options[:crop].present? + if options[:crop] crop(size, options[:crop_from], options[:crop_size], upsample) else resize(size, upsample) diff --git a/spec/models/alchemy/essence_picture_spec.rb b/spec/models/alchemy/essence_picture_spec.rb index 1b48f44446..5b7858ca5b 100644 --- a/spec/models/alchemy/essence_picture_spec.rb +++ b/spec/models/alchemy/essence_picture_spec.rb @@ -25,6 +25,13 @@ module Alchemy expect(essence.crop_size).to eq("100x203") end + it "should not store empty strings for nil crop values" do + essence = EssencePicture.new(crop_from: nil, crop_size: nil) + essence.save! + expect(essence.crop_from).to eq(nil) + expect(essence.crop_size).to eq(nil) + end + it "should convert newlines in caption into
s" do essence = EssencePicture.new(caption: "hello\nkitty") essence.save! @@ -94,6 +101,51 @@ module Alchemy end end + describe '#picture_url_options' do + subject(:picture_url_options) { essence.picture_url_options } + + let(:picture) { build_stubbed(:alchemy_picture) } + let(:essence) { build_stubbed(:alchemy_essence_picture, picture: picture) } + + it { is_expected.to be_a(HashWithIndifferentAccess) } + + it "includes the pictures default render format." do + expect(picture).to receive(:default_render_format) { 'img' } + expect(picture_url_options[:format]).to eq('img') + end + + context 'with crop sizes present' do + before do + expect(essence).to receive(:crop_size) { '200x200' } + expect(essence).to receive(:crop_from) { '10x10' } + end + + it "includes these crop sizes.", :aggregate_failures do + expect(picture_url_options[:crop_from]).to eq '10x10' + expect(picture_url_options[:crop_size]).to eq '200x200' + end + end + + # Regression spec for issue #1279 + context 'with crop sizes being empty strings' do + before do + expect(essence).to receive(:crop_size) { '' } + expect(essence).to receive(:crop_from) { '' } + end + + it "does not include these crop sizes.", :aggregate_failures do + expect(picture_url_options[:crop_from]).to be_nil + expect(picture_url_options[:crop_size]).to be_nil + end + end + + context 'without picture assigned' do + let(:picture) { nil } + + it { is_expected.to be_a(Hash) } + end + end + describe '#thumbnail_url' do subject(:thumbnail_url) { essence.thumbnail_url(options) } diff --git a/spec/models/alchemy/essence_picture_view_spec.rb b/spec/models/alchemy/essence_picture_view_spec.rb index dc06ef01aa..cd5fda712d 100644 --- a/spec/models/alchemy/essence_picture_view_spec.rb +++ b/spec/models/alchemy/essence_picture_view_spec.rb @@ -32,6 +32,13 @@ allow(picture).to receive(:url) { picture_url } end + describe 'DEFAULT_OPTIONS' do + it do + expect(Alchemy::EssencePictureView::DEFAULT_OPTIONS). + to be_a(HashWithIndifferentAccess) + end + end + context "with caption" do let(:options) do {} diff --git a/spec/models/alchemy/picture_url_spec.rb b/spec/models/alchemy/picture_url_spec.rb index 537b15a31f..d729908c0b 100644 --- a/spec/models/alchemy/picture_url_spec.rb +++ b/spec/models/alchemy/picture_url_spec.rb @@ -69,32 +69,63 @@ def decode_dragon_fly_job(url) end end - context "and crop_from and crop_size is passed in" do + context "and crop is set to true" do let(:options) do { - crop_size: '123x44', - crop_from: '0x0', - size: '160x120' + size: '160x120', + crop: true } end - it "crops and resizes the picture" do + it "crops from center and resizes the picture" do job = decode_dragon_fly_job(url) - expect(job[1]).to include("-crop 123x44+0+0 -resize 160x120>") + expect(job[1]).to include("160x120#") + end + + context "and crop_from and crop_size is passed in" do + let(:options) do + { + crop_size: '123x44', + crop_from: '0x0', + size: '160x120', + crop: true + } + end + + it "crops and resizes the picture" do + job = decode_dragon_fly_job(url) + expect(job[1]).to include("-crop 123x44+0+0 -resize 160x120>") + end end end - context "and crop is set to true" do + context "and crop is set to false" do let(:options) do { size: '160x120', - crop: true + crop: false } end - it "crops from center and resizes the picture" do + it "does not crop the picture" do job = decode_dragon_fly_job(url) - expect(job[1]).to include("160x120#") + expect(job[1]).to_not include("160x120#") + end + + context "and crop_from and crop_size is passed in" do + let(:options) do + { + crop_size: '123x44', + crop_from: '0x0', + size: '160x120', + crop: false + } + end + + it "does not crop the picture" do + job = decode_dragon_fly_job(url) + expect(job[1]).to_not include("-crop 123x44+0+0") + end end end diff --git a/spec/support/transformation_examples.rb b/spec/support/transformation_examples.rb index ed5582f7df..ca0e8c1dbe 100644 --- a/spec/support/transformation_examples.rb +++ b/spec/support/transformation_examples.rb @@ -153,7 +153,7 @@ module Alchemy allow(picture).to receive(:image_file_width) { 400 } allow(picture).to receive(:image_file_height) { 300 } - expect(picture.can_be_cropped_to("200x100")).to be_truthy + expect(picture.can_be_cropped_to("200x100")).to be(true) end end @@ -162,7 +162,7 @@ module Alchemy allow(picture).to receive(:image_file_width) { 400 } allow(picture).to receive(:image_file_height) { 300 } - expect(picture.can_be_cropped_to("600x500")).to be_falsey + expect(picture.can_be_cropped_to("600x500")).to be(false) end end @@ -171,7 +171,7 @@ module Alchemy allow(picture).to receive(:image_file_width) { 400 } allow(picture).to receive(:image_file_height) { 300 } - expect(picture.can_be_cropped_to("600x500", true)).to be_truthy + expect(picture.can_be_cropped_to("600x500", true)).to be(true) end end end