Skip to content

Commit

Permalink
Merge pull request #2143 from tvdeyen/only-crop-if-enabled
Browse files Browse the repository at this point in the history 
Only crop image if cropping is enabled
  • Loading branch information
@tvdeyen
tvdeyen authored Jun 28, 2021
2 parents fccc048 + 5474ff4 commit f4f9b58
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 81 deletions.
20 changes: 8 additions & 12 deletions app/models/concerns/alchemy/picture_thumbnails.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ def picture_url(options = {})
def picture_url_options
return {} if picture.nil?

crop = crop_values_present? || settings[:crop]
crop = !!settings[:crop]

{
format: picture.default_render_format,
crop: !!crop,
crop_from: crop_from.presence,
crop_size: crop_size.presence,
crop: crop,
crop_from: crop && crop_from.presence || nil,
crop_size: crop && crop_size.presence || nil,
size: settings[:size],
}.with_indifferent_access
end
Expand All @@ -76,13 +76,13 @@ def thumbnail_url
#
# @return [HashWithIndifferentAccess]
def thumbnail_url_options
crop = crop_values_present? || settings[:crop]
crop = !!settings[:crop]

{
size: "160x120",
crop: !!crop,
crop_from: crop_from.presence || default_crop_from&.join("x"),
crop_size: crop_size.presence || default_crop_size&.join("x"),
crop: crop,
crop_from: crop && crop_from.presence || default_crop_from&.join("x"),
crop_size: crop && crop_size.presence || default_crop_size&.join("x"),
flatten: true,
format: picture&.image_file_format || "jpg",
}
Expand Down Expand Up @@ -111,10 +111,6 @@ def allow_image_cropping?

private

def crop_values_present?
crop_from.present? && crop_size.present?
end

def default_crop_size
return nil unless settings[:crop] && settings[:size]

Expand Down
203 changes: 134 additions & 69 deletions lib/alchemy/test_support/having_picture_thumbnails_examples.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,24 +51,30 @@
allow(record).to receive(:crop_size) { "200x200" }
end

it "passes these crop values to the picture's url method." do
expect(picture).to receive(:url).with(
hash_including(crop_from: "10x10", crop_size: "200x200"),
)
picture_url
end

context "but with crop values in the options" do
let(:options) do
{ crop_from: "30x30", crop_size: "75x75" }
context "if cropping is enabled" do
before do
allow(record).to receive(:settings) { { crop: true } }
end

it "passes these crop values instead." do
it "passes these crop values to the picture's url method." do
expect(picture).to receive(:url).with(
hash_including(crop_from: "30x30", crop_size: "75x75"),
hash_including(crop_from: "10x10", crop_size: "200x200"),
)
picture_url
end

context "but with crop values in the options" do
let(:options) do
{ crop_from: "30x30", crop_size: "75x75" }
end

it "passes these crop values instead." do
expect(picture).to receive(:url).with(
hash_including(crop_from: "30x30", crop_size: "75x75"),
)
picture_url
end
end
end
end

Expand Down Expand Up @@ -121,30 +127,39 @@
allow(record).to receive(:crop_size) { "200x200" }
end

it "includes these crop values.", :aggregate_failures do
expect(picture_url_options[:crop_from]).to eq "10x10"
expect(picture_url_options[:crop_size]).to eq "200x200"
end
context "with cropping enabled" do
before do
allow(record).to receive(:settings) { { crop: true } }
end

it "includes {crop: true}" do
expect(picture_url_options[:crop]).to be true
it "includes these crop values.", :aggregate_failures do
expect(picture_url_options[:crop_from]).to eq "10x10"
expect(picture_url_options[:crop_size]).to eq "200x200"
end
end
end

# Regression spec for issue #1279
context "with crop values being empty strings" do
before do
allow(record).to receive(:crop_from) { "" }
allow(record).to receive(:crop_size) { "" }
end
context "with cropping disabled" do
before do
allow(record).to receive(:settings) { { crop: nil } }
end

it "does not include these crop values.", :aggregate_failures do
expect(picture_url_options[:crop_from]).to be_nil
expect(picture_url_options[:crop_size]).to be_nil
it "does not include these crop values.", :aggregate_failures do
expect(picture_url_options[:crop_from]).to be_nil
expect(picture_url_options[:crop_size]).to be_nil
end
end

it "includes {crop: false}" do
expect(picture_url_options[:crop]).to be false
# Regression spec for issue #1279
context "with crop values being empty strings" do
before do
allow(record).to receive(:crop_from) { "" }
allow(record).to receive(:crop_size) { "" }
end

it "does not include these crop values.", :aggregate_failures do
expect(picture_url_options[:crop_from]).to be_nil
expect(picture_url_options[:crop_size]).to be_nil
end
end
end

Expand Down Expand Up @@ -197,36 +212,61 @@
thumbnail_url
end

context "when crop sizes are present" do
before do
allow(record).to receive(:crop_size).and_return("200x200")
allow(record).to receive(:crop_from).and_return("10x10")
context "when crop is enabled in the settings" do
let(:settings) do
{ crop: true }
end

it "passes these crop sizes to the picture's url method." do
expect(picture).to receive(:url).with(
hash_including(crop_from: "10x10", crop_size: "200x200", crop: true),
)
thumbnail_url
context "and crop sizes are present" do
before do
allow(record).to receive(:crop_size).and_return("200x200")
allow(record).to receive(:crop_from).and_return("10x10")
end

it "passes these crop sizes to the picture's url method." do
expect(picture).to receive(:url).with(
hash_including(
crop_from: "10x10",
crop_size: "200x200",
crop: true,
),
)
thumbnail_url
end
end

context "when no crop sizes are present" do
it "it does not pass crop sizes to the picture's url method and enables center cropping." do
expect(picture).to receive(:url).with(
hash_including(
crop_from: nil,
crop_size: nil,
crop: true,
),
)
thumbnail_url
end
end
end

context "when no crop sizes are present" do
it "it does not pass crop sizes to the picture's url method and disables cropping." do
expect(picture).to receive(:url).with(
hash_including(crop_from: nil, crop_size: nil, crop: false),
)
thumbnail_url
context "when cropping is disabled in the settings" do
let(:settings) do
{ crop: false }
end

context "when crop is explicitely enabled in the settings" do
let(:settings) do
{ crop: true }
context "but crop sizes are present" do
before do
allow(record).to receive(:crop_size).and_return("200x200")
allow(record).to receive(:crop_from).and_return("10x10")
end

it "it enables cropping." do
it "it disables cropping." do
expect(picture).to receive(:url).with(
hash_including(crop: true),
hash_including(
crop_size: nil,
crop_from: nil,
crop: false,
),
)
thumbnail_url
end
Expand Down Expand Up @@ -274,34 +314,59 @@
end
end

context "when crop values are present" do
before do
expect(record).to receive(:crop_size).at_least(:once) { "200x200" }
expect(record).to receive(:crop_from).at_least(:once) { "10x10" }
context "when cropping is enabled in settings" do
let(:settings) do
{ crop: true }
end

it "includes these crop values" do
expect(thumbnail_url_options).to match(
hash_including(crop_from: "10x10", crop_size: "200x200", crop: true)
)
context "and crop values are present" do
before do
expect(record).to receive(:crop_size).at_least(:once) { "200x200" }
expect(record).to receive(:crop_from).at_least(:once) { "10x10" }
end

it "includes these crop values" do
expect(thumbnail_url_options).to match(
hash_including(
crop_from: "10x10",
crop_size: "200x200",
crop: true,
)
)
end
end

context "and no crop values are present" do
it "does not include crop values but enables center cropping" do
expect(thumbnail_url_options).to match(
hash_including(
crop_from: nil,
crop_size: nil,
crop: true,
)
)
end
end
end

context "when no crop values are present" do
it "does not include these crop values" do
expect(thumbnail_url_options).to_not match(
hash_including(crop_from: "10x10", crop_size: "200x200", crop: true)
)
context "when cropping is disabled in settings" do
let(:settings) do
{ crop: false }
end

context "when crop is explicitely enabled in the settings" do
let(:settings) do
{ crop: true }
context "but crop values are present" do
before do
allow(record).to receive(:crop_size) { "200x200" }
allow(record).to receive(:crop_from) { "10x10" }
end

it "it enables cropping." do
it "does not include crop values" do
expect(thumbnail_url_options).to match(
hash_including(crop: true),
hash_including(
crop_from: nil,
crop_size: nil,
crop: false,
)
)
end
end
Expand Down Expand Up @@ -367,7 +432,7 @@
end
end

context "with no sizes in settngs" do
context "with no sizes in settings" do
it "sets sizes to zero" do
expect(subject[:min_size]).to eq([0, 0])
end
Expand Down

0 comments on commit f4f9b58

Please sign in to comment.