raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-01-24 20:36:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Replace Paperclip on Spree::Image

Browse Source
This commit is contained in:
Maikel Linke
2022-04-15 16:35:52 +10:00
parent 4a0ed99919
commit b7efa1b018
20 changed files with 73 additions and 204 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
app/controllers/spree/admin/products_controller.rb
View File

@@ -39,9 +39,6 @@ module Spree
render :new
end
end
rescue Paperclip::Errors::NotIdentifiedByImageMagickError
@object.errors.add(:base, t('spree.admin.products.image_upload_error'))
respond_with(@object)
end
def show

98
app/models/spree/image.rb
View File

@@ -1,103 +1,39 @@
# frozen_string_literal: true
require 'spree/core/s3_support'
module Spree
class Image < Asset
include HasMigratingFile
SIZES = {
mini: { resize_to_fill: [48, 48] },
small: { resize_to_fill: [227, 227] },
product: { resize_to_limit: [240, 240] },
large: { resize_to_limit: [600, 600] },
}.freeze
validates_attachment_presence :attachment
has_one_attached :attachment
validates :attachment, attached: true, content_type: %r{\Aimage/.*\Z}
validate :no_attachment_errors
# This is where the styles are used in the app:
# - mini: used in the BackOffice: Bulk Product Edit page and Order Cycle edit page
# - small: used in the FrontOffice: Product List page
# - product: used in the BackOffice: Product Image upload modal in the Bulk Product Edit page
# and Product image edit page
# - large: used in the FrontOffice: product modal
has_one_migrating :attachment,
styles: { mini: "48x48#", small: "227x227#",
product: "240x240>", large: "600x600>" },
default_style: :product,
url: '/spree/products/:id/:style/:basename.:extension',
path: ':rails_root/public/spree/products/:id/:style/:basename.:extension',
convert_options: { all: '-strip -auto-orient -colorspace sRGB' }
# save the w,h of the original image (from which others can be calculated)
# we need to look at the write-queue for images which have not been saved yet
after_post_process :find_dimensions
include Spree::Core::S3Support
supports_s3 :attachment
# used by admin products autocomplete
def mini_url
attachment.url(:mini, false)
def variant(name)
attachment.variant(SIZES[name])
end
def find_dimensions
return if attachment.errors.present?
def url(size)
return unless attachment.variable?
geometry = Paperclip::Geometry.from_file(local_filename_of_original)
self.attachment_width = geometry.width
self.attachment_height = geometry.height
end
def local_filename_of_original
temporary = attachment.queued_for_write[:original]
if temporary&.path.present?
temporary.path
else
attachment.path
end
Rails.application.routes.url_helpers.url_for(variant(size))
end
# if there are errors from the plugin, then add a more meaningful message
def no_attachment_errors
return if attachment.errors.empty?
return if errors[:attachment].empty?
if errors.all? { |e| e.type == "Paperclip::Errors::NotIdentifiedByImageMagickError" }
if errors.all? { |e| e.type == :content_type_invalid }
attachment.errors.clear
errors.add :base, I18n.t('spree.admin.products.image_upload_error')
else
errors.add :attachment,
I18n.t('spree.admin.products.paperclip_image_error', attachment_file_name: attachment_file_name)
end
false
end
def self.set_attachment_attribute(attribute_name, attribute_value)
attachment_definitions[:attachment][attribute_name] = attribute_value
end
def self.set_storage_attachment_attributes
if Spree::Config[:use_s3]
set_s3_attachment_attributes
else
attachment_definitions[:attachment].delete(:storage)
end
end
def self.set_s3_attachment_attributes
set_attachment_attribute(:storage, :s3)
set_attachment_attribute(:s3_credentials, s3_credentials)
set_attachment_attribute(:s3_headers,
ActiveSupport::JSON.decode(Spree::Config[:s3_headers]))
set_attachment_attribute(:bucket, Spree::Config[:s3_bucket])
# We use :s3_alias_url (virtual host url style) and set the URL on property s3_host_alias
set_attachment_attribute(:s3_host_alias, attachment_definitions[:attachment][:url])
set_attachment_attribute(:url, ":s3_alias_url")
end
private_class_method :set_s3_attachment_attributes
def self.s3_credentials
{ access_key_id: Spree::Config[:s3_access_key],
secret_access_key: Spree::Config[:s3_secret],
bucket: Spree::Config[:s3_bucket] }
end
private_class_method :s3_credentials
end
end

2
app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb
View File

@@ -11,7 +11,7 @@ module Api
end
def image_url
object.images.present? ? object.images.first.attachment.url(:mini) : nil
object.images.first&.url(:mini)
end
def master_id

12
app/serializers/api/admin/product_serializer.rb
View File

@@ -13,19 +13,11 @@ module Api
has_one :master, serializer: Api::Admin::VariantSerializer
def image_url
if object.images.present?
object.images.first.attachment.url(:product)
else
"/noimage/product.png"
end
object.images.first&.url(:product) || "/noimage/product.png"
end
def thumb_url
if object.images.present?
object.images.first.attachment.url(:mini)
else
"/noimage/mini.png"
end
object.images.first&.url(:mini) || "/noimage/mini.png"
end
def on_hand

4
app/serializers/api/admin/variant_serializer.rb
View File

@@ -33,9 +33,7 @@ module Api
end
def image
return if object.product.images.empty?
object.product.images.first.mini_url
object.product.images.first&.url(:mini)
end
def in_stock

8
app/serializers/api/image_serializer.rb
View File

@@ -4,18 +4,18 @@ class Api::ImageSerializer < ActiveModel::Serializer
attributes :id, :alt, :thumb_url, :small_url, :image_url, :large_url
def thumb_url
object.attachment.url(:mini, false)
object.url(:mini)
end
def small_url
object.attachment.url(:small, false)
object.url(:small)
end
def image_url
object.attachment.url(:product, false)
object.url(:product)
end
def large_url
object.attachment.url(:large, false)
object.url(:large)
end
end

6
app/serializers/api/variant_serializer.rb
View File

@@ -35,11 +35,7 @@ class Api::VariantSerializer < ActiveModel::Serializer
end
def thumb_url
if object.product.images.present?
object.product.images.first.attachment.url(:mini)
else
"/noimage/mini.png"
end
object.product.images.first&.url(:mini) || "/noimage/mini.png"
end
def unit_price_price

17
app/services/image_importer.rb
View File

@@ -2,21 +2,12 @@
class ImageImporter
def import(url, product)
attach(download(url), product)
end
valid_url = URI.parse(url)
file = open(valid_url.to_s)
filename = File.basename(valid_url.path)
private
def download(url)
local_file = Tempfile.new
remote_file = open(url)
IO.copy_stream(remote_file, local_file)
local_file
end
def attach(file, product)
Spree::Image.create(
attachment: file,
attachment: { io: file, filename: filename },
viewable_id: product.master.id,
viewable_type: Spree::Variant,
)

4
app/views/spree/admin/images/edit.html.haml
View File

@@ -11,7 +11,9 @@
.field.alpha.three.columns.align-center
= f.label t('spree.thumbnail')
%br/
= link_to image_tag(@image.attachment.url(:small)), @image.attachment.url(:product)
- # A Rails bug makes it necessary to call `main_app.url_for` here.
- # https://github.com/rails/rails/issues/31325
= link_to image_tag(main_app.url_for(@image.variant(:small))), main_app.url_for(@image.variant(:product))
.nine.columns.omega
= render partial: 'form', locals: { f: f }
.clear

4
app/views/spree/admin/images/index.html.haml
View File

@@ -32,7 +32,9 @@
%td.no-border
%span.handle
%td
= link_to image_tag(image.attachment.url(:mini)), image.attachment.url(:product)
- # A Rails bug makes it necessary to call `main_app.url_for` here.
- # https://github.com/rails/rails/issues/31325
= link_to image_tag(main_app.url_for(image.variant(:mini))), main_app.url_for(image.variant(:product))
%td= options_text_for(image)
%td= image.alt
%td.actions

4
app/views/spree/shared/_variant_thumbnail.html.haml
View File

@@ -1,4 +1,6 @@
- if variant.product.images.length == 0
= image_tag("/noimage/mini.png")
- else
= image_tag(variant.product.images.first.attachment.url(:mini))
- # A Rails bug makes it necessary to call `main_app.url_for` here.
- # https://github.com/rails/rails/issues/31325
= image_tag(main_app.url_for(variant.product.images.first.variant(:mini)))

5
config/initializers/spree.rb
View File

@@ -29,11 +29,6 @@ Rails.application.reloader.to_prepare do
# applied correctly in Spree::Config.
MailConfiguration.apply!
# Attachments settings
Spree::Image.set_attachment_attribute(:path, ENV['ATTACHMENT_PATH']) if ENV['ATTACHMENT_PATH']
Spree::Image.set_attachment_attribute(:url, ENV['ATTACHMENT_URL']) if ENV['ATTACHMENT_URL']
Spree::Image.set_storage_attachment_attributes
# TODO Work out why this is necessary
# Seems like classes within OFN module become 'uninitialized' when server reloads
# unless the empty module is explicity 'registered' here. Something to do with autoloading?

1
config/locales/en.yml
View File

@@ -3902,7 +3902,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using
no_payment_via_admin_backend: Paypal payments cannot be captured in the Backoffice
products:
image_upload_error: "The product image was not recognised. Please upload an image in PNG or JPG format."
paperclip_image_error: "Paperclip returned errors for file '%{attachment_file_name}' - check ImageMagick installation or image source file."
new:
title: "New Product"
new_product: "New Product"

2
lib/spree/core/product_duplicator.rb
View File

@@ -46,7 +46,7 @@ module Spree
def duplicate_image(image)
new_image = image.dup
new_image.assign_attributes(attachment: image.attachment.clone)
new_image.attachment.attach(image.attachment_blob)
new_image
end

5
spec/controllers/spree/admin/products_controller_spec.rb
View File

@@ -163,9 +163,8 @@ describe Spree::Admin::ProductsController, type: :controller do
}
)
expect do
spree_put :create, product: product_attrs_with_image
end.not_to raise_error Paperclip::Errors::NotIdentifiedByImageMagickError
spree_put :create, product: product_attrs_with_image
expect(response.status).to eq 200
end
end

2
spec/factories/order_cycle_factory.rb
View File

@@ -35,7 +35,7 @@ FactoryBot.define do
viewable_id: product.master.id,
viewable_type: 'Spree::Variant',
alt: "position 1",
attachment: white_logo_file,
attachment: Rack::Test::UploadedFile.new(white_logo_path),
position: 1
)

7
spec/lib/spree/core/product_duplicator_spec.rb
View File

@@ -70,11 +70,8 @@ describe Spree::Core::ProductDuplicator do
expect(new_variant).to receive(:price=).with(variant.price)
expect(new_variant).to receive(:currency=).with(variant.currency)
expect(image.attachment).to receive(:clone).and_return(image.attachment)
expect(new_image).to receive(:assign_attributes).
with(attachment: image.attachment).
and_return(new_image)
expect(image).to receive(:attachment_blob)
expect(new_image).to receive_message_chain(:attachment, :attach)
expect(new_property).to receive(:created_at=).with(nil)
expect(new_property).to receive(:updated_at=).with(nil)

89
spec/models/spree/image_spec.rb
View File

@@ -6,91 +6,54 @@ module Spree
describe Image do
include FileHelper
subject {
Spree::Image.create!(
attachment: black_logo_file,
viewable: product.master,
)
}
let(:product) { create(:product) }
describe "#url" do
it "returns URLs for different sizes" do
expect(subject.url(:small)).to match(
%r|^http://test\.host/rails/active_storage/representations/redirect/.+/logo-black\.png$|
)
end
it "returns nil for unsupported formats" do
subject.attachment_blob.update_columns(
content_type: "application/octet-stream"
)
expect(subject.url(:small)).to eq nil
end
end
describe "using local storage" do
it "stores a new image" do
image = Spree::Image.create!(
attachment: black_logo_file,
viewable: product.master,
)
attachment = subject.attachment
expect(attachment.attached?).to eq true
attachment = image.attachment
expect(attachment.exists?).to eq true
expect(attachment.file?).to eq true
expect(attachment.url).to match %r"^/spree/products/[0-9]+/product/logo-black\.png\?[0-9]+$"
end
it "duplicates the image with Active Storage" do
image = Spree::Image.create!(
attachment: file,
viewable: product.master,
)
attachment = image.active_storage_attachment
url = Rails.application.routes.url_helpers.url_for(attachment)
expect(url).to match %r|^http://test\.host/rails/active_storage/blobs/redirect/[[:alnum:]-]+/logo-black\.png$|
end
end
describe "using AWS S3" do
let(:s3_config) {
{
url: ":s3_alias_url",
storage: :s3,
s3_credentials: {
access_key_id: "A...A",
secret_access_key: "H...H",
},
s3_headers: { "Cache-Control" => "max-age=31557600" },
bucket: "ofn",
s3_protocol: "https",
s3_host_alias: "ofn.s3.us-east-1.amazonaws.com",
# This is for easier testing:
path: "/:id/:style/:basename.:extension",
}
}
before do
attachment_definition = Spree::Image.attachment_definitions[:attachment]
allow(Spree::Image).to receive(:attachment_definitions).and_return(
attachment: attachment_definition.merge(s3_config)
)
allow(Rails.application.config.active_storage).
to receive(:service).and_return(:test_amazon)
end
it "saves a new image when none is present" do
# Paperclip requests
upload_pattern = %r"^https://ofn.s3.amazonaws.com/[0-9]+/(original|mini|small|product|large)/logo-black.png$"
download_pattern = %r"^https://ofn.s3.amazonaws.com/[0-9]+/product/logo-black.png$"
public_url_pattern = %r"^https://ofn.s3.us-east-1.amazonaws.com/[0-9]+/product/logo-black.png\?[0-9]+$"
stub_request(:put, upload_pattern).to_return(status: 200, body: "", headers: {})
stub_request(:head, download_pattern).to_return(status: 200, body: "", headers: {})
# Active Storage requests
as_upload_pattern = %r"^https://ofn.s3.amazonaws.com/[[:alnum:]]+$"
stub_request(:put, as_upload_pattern).to_return(status: 200, body: "", headers: {})
image = Spree::Image.create!(
attachment: black_logo_file,
viewable: product.master,
)
# Paperclip
attachment = image.attachment
expect(attachment.exists?).to eq true
expect(attachment.file?).to eq true
expect(attachment.url).to match public_url_pattern
# Active Storage
attachment = image.active_storage_attachment
expect(attachment).to be_attached
expect(Rails.application.routes.url_helpers.url_for(attachment)).
expect(subject.attachment).to be_attached
expect(Rails.application.routes.url_helpers.url_for(subject.attachment)).
to match %r"^http://test\.host/rails/active_storage/blobs/redirect/[[:alnum:]-]+/logo-black\.png"
end
end

2
spec/services/image_importer_spec.rb
View File

@@ -15,7 +15,7 @@ describe ImageImporter do
}.by(1)
expect(product.images.count).to eq 1
expect(product.images.first.attachment.size).to eq 6274
expect(product.images.first.attachment_blob.byte_size).to eq 6274
end
end
end

2
spec/support/api_helper.rb
View File

@@ -28,7 +28,7 @@ module OpenFoodNetwork
end
def image(filename)
File.open(Rails.root + "spec/support/fixtures" + filename)
Rack::Test::UploadedFile.new(Rails.root + "spec/support/fixtures" + filename)
end
end
end