Fixes image cropping

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,13 +108,15 @@ 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]
     end
 
     # 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)

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

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

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 <br/>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) }
 

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
       {}

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