Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only crop image if cropping is enabled #2143

Merged
merged 1 commit into from
Jun 28, 2021
Merged

Only crop image if cropping is enabled #2143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions app/models/concerns/alchemy/picture_thumbnails.rb
View file
Open in desktop
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
Open in desktop
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