From b1ccf2ef4b2dc4261a854844bdd8cd8d7479d1be Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Fri, 1 Feb 2019 18:59:28 +0100 Subject: [PATCH 1/4] Prefer a constant over global methods It's good practice not to define methods on Object, even if the method name was unique enough not to be in conflict with anything. Also changed the constant to reflect the actual content (Rails 5.2+) rather than a consequence of it, that allowed reusing it in more places. --- core/lib/spree/testing_support/dummy_app.rb | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/core/lib/spree/testing_support/dummy_app.rb b/core/lib/spree/testing_support/dummy_app.rb index ffb4d327935..2a5250e029c 100644 --- a/core/lib/spree/testing_support/dummy_app.rb +++ b/core/lib/spree/testing_support/dummy_app.rb @@ -12,14 +12,11 @@ require 'solidus_core' -# @private -def forgery_protected_by_default? - Gem::Version.new(Rails.version) >= Gem::Version.new('5.2') -end +RAILS_52_OR_ABOVE = Gem::Version.new(Rails.version) >= Gem::Version.new('5.2') # @private class ApplicationController < ActionController::Base - if !forgery_protected_by_default? + unless RAILS_52_OR_ABOVE protect_from_forgery with: :exception end end @@ -65,12 +62,8 @@ class Application < ::Rails::Application config.active_support.deprecation = :stderr config.secret_key_base = 'SECRET_TOKEN' - if forgery_protected_by_default? + if RAILS_52_OR_ABOVE config.action_controller.default_protect_from_forgery = true - end - - if config.active_record.sqlite3 - # Rails >= 5.2 config.active_record.sqlite3.represent_boolean_as_integer = true end From d70892e26e75d1536348e620d40755f92ffce6f8 Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Mon, 21 Jan 2019 23:09:32 +0100 Subject: [PATCH 2/4] Extract Spree::Image Paperclip implementation --- core/app/models/spree/image.rb | 46 +--------------- .../spree/image/paperclip_attachment.rb | 55 +++++++++++++++++++ core/lib/spree/app_configuration.rb | 10 ++++ 3 files changed, 67 insertions(+), 44 deletions(-) create mode 100644 core/app/models/spree/image/paperclip_attachment.rb diff --git a/core/app/models/spree/image.rb b/core/app/models/spree/image.rb index 8478257758e..ff4b75e8335 100644 --- a/core/app/models/spree/image.rb +++ b/core/app/models/spree/image.rb @@ -2,55 +2,13 @@ module Spree class Image < Asset - validate :no_attachment_errors - - has_attached_file :attachment, - styles: { mini: '48x48>', small: '100x100>', product: '240x240>', large: '600x600>' }, - default_style: :product, - default_url: 'noimage/:style.png', - 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' } - validates_attachment :attachment, - presence: true, - content_type: { content_type: %w(image/jpeg image/jpg image/png image/gif) } - - # 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, if: :valid? + include ::Spree::Config.image_attachment_module def mini_url Spree::Deprecation.warn( 'Spree::Image#mini_url is DEPRECATED. Use Spree::Image#url(:mini) instead.' ) - attachment.url(:mini, false) - end - - def url(size) - attachment.url(size) - end - - def filename - attachment_file_name - end - - def find_dimensions - temporary = attachment.queued_for_write[:original] - filename = temporary.path unless temporary.nil? - filename = attachment.path if filename.blank? - geometry = Paperclip::Geometry.from_file(filename) - self.attachment_width = geometry.width - self.attachment_height = geometry.height - end - - # if there are errors from the plugin, then add a more meaningful message - def no_attachment_errors - unless attachment.errors.empty? - # uncomment this to get rid of the less-than-useful interim messages - # errors.clear - errors.add :attachment, "Paperclip returned errors for file '#{attachment_file_name}' - check ImageMagick installation or image source file." - false - end + url(:mini) end end end diff --git a/core/app/models/spree/image/paperclip_attachment.rb b/core/app/models/spree/image/paperclip_attachment.rb new file mode 100644 index 00000000000..b716824d8b6 --- /dev/null +++ b/core/app/models/spree/image/paperclip_attachment.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +module Spree::Image::PaperclipAttachment + extend ActiveSupport::Concern + + included do + validate :no_attachment_errors + + has_attached_file :attachment, + styles: { mini: '48x48>', small: '100x100>', product: '240x240>', large: '600x600>' }, + default_style: :product, + default_url: 'noimage/:style.png', + 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' } + validates_attachment :attachment, + presence: true, + content_type: { content_type: %w[image/jpeg image/jpg image/png image/gif] } + + # 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, if: :valid? + end + + def url(size) + attachment.url(size) + end + + def filename + attachment_file_name + end + + def attachment_present? + attachment.present? + end + + def find_dimensions + temporary = attachment.queued_for_write[:original] + filename = temporary.path unless temporary.nil? + filename = attachment.path if filename.blank? + geometry = Paperclip::Geometry.from_file(filename) + self.attachment_width = geometry.width + self.attachment_height = geometry.height + end + + # if there are errors from the plugin, then add a more meaningful message + def no_attachment_errors + unless attachment.errors.empty? + # uncomment this to get rid of the less-than-useful interim messages + # errors.clear + errors.add :attachment, "Paperclip returned errors for file '#{attachment_file_name}' - check ImageMagick installation or image source file." + false + end + end +end diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb index 02d1249390e..eea32ae6a94 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -423,6 +423,16 @@ def payment_canceller # Enumerable of images adhering to the present_image_class interface class_name_attribute :product_gallery_class, default: 'Spree::Gallery::ProductGallery' + # Allows switching attachment library for Image + # + # `Spree::Image::PaperclipAttachment` + # is the default and provides the classic Paperclip implementation. + # + # @!attribute [rw] image_attachment_module + # @return [Module] a module that can be included into Spree::Image to allow attachments + # Enumerable of images adhering to the present_image_class interface + class_name_attribute :image_attachment_module, default: 'Spree::Image::PaperclipAttachment' + # Allows providing your own class instance for generating order numbers. # # @!attribute [rw] order_number_generator From 54c51d30c3941417178e98998d986d0b547d749b Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Fri, 8 Feb 2019 16:18:41 +0100 Subject: [PATCH 3/4] Extract Spree::Taxon Paperclip implementation --- .../views/spree/admin/taxons/_form.html.erb | 2 +- core/app/models/spree/taxon.rb | 10 +-------- .../spree/taxon/paperclip_attachment.rb | 21 +++++++++++++++++++ core/lib/spree/app_configuration.rb | 10 +++++++++ 4 files changed, 33 insertions(+), 10 deletions(-) create mode 100644 core/app/models/spree/taxon/paperclip_attachment.rb diff --git a/backend/app/views/spree/admin/taxons/_form.html.erb b/backend/app/views/spree/admin/taxons/_form.html.erb index f3baebcd403..6c93d998402 100644 --- a/backend/app/views/spree/admin/taxons/_form.html.erb +++ b/backend/app/views/spree/admin/taxons/_form.html.erb @@ -23,7 +23,7 @@ <%= f.field_container :icon do %> <%= f.label :icon %>
<%= f.file_field :icon %> - <%= image_tag f.object.icon(:mini) if f.object.icon.present? %> + <%= image_tag f.object.icon(:mini) if f.object.icon_present? %> <% end %> diff --git a/core/app/models/spree/taxon.rb b/core/app/models/spree/taxon.rb index ecd4c475a1e..396f2a92d0e 100644 --- a/core/app/models/spree/taxon.rb +++ b/core/app/models/spree/taxon.rb @@ -25,15 +25,7 @@ class Taxon < Spree::Base after_save :touch_ancestors_and_taxonomy after_touch :touch_ancestors_and_taxonomy - has_attached_file :icon, - styles: { mini: '32x32>', normal: '128x128>' }, - default_style: :mini, - url: '/spree/taxons/:id/:style/:basename.:extension', - path: ':rails_root/public/spree/taxons/:id/:style/:basename.:extension', - default_url: '/assets/default_taxon.png' - - validates_attachment :icon, - content_type: { content_type: ["image/jpg", "image/jpeg", "image/png", "image/gif"] } + include ::Spree::Config.taxon_attachment_module self.whitelisted_ransackable_attributes = %w[name] diff --git a/core/app/models/spree/taxon/paperclip_attachment.rb b/core/app/models/spree/taxon/paperclip_attachment.rb new file mode 100644 index 00000000000..cb4a83437cc --- /dev/null +++ b/core/app/models/spree/taxon/paperclip_attachment.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Spree::Taxon::PaperclipAttachment + extend ActiveSupport::Concern + + included do + has_attached_file :icon, + styles: { mini: '32x32>', normal: '128x128>' }, + default_style: :mini, + url: '/spree/taxons/:id/:style/:basename.:extension', + path: ':rails_root/public/spree/taxons/:id/:style/:basename.:extension', + default_url: '/assets/default_taxon.png' + + validates_attachment :icon, + content_type: { content_type: %w[image/jpg image/jpeg image/png image/gif] } + end + + def icon_present? + icon.present? + end +end diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb index eea32ae6a94..14979abdc1c 100644 --- a/core/lib/spree/app_configuration.rb +++ b/core/lib/spree/app_configuration.rb @@ -433,6 +433,16 @@ def payment_canceller # Enumerable of images adhering to the present_image_class interface class_name_attribute :image_attachment_module, default: 'Spree::Image::PaperclipAttachment' + # Allows switching attachment library for Taxon + # + # `Spree::Taxon::PaperclipAttachment` + # is the default and provides the classic Paperclip implementation. + # + # @!attribute [rw] taxon_attachment_module + # @return [Module] a module that can be included into Spree::Taxon to allow attachments + # Enumerable of taxons adhering to the present_taxon_class interface + class_name_attribute :taxon_attachment_module, default: 'Spree::Taxon::PaperclipAttachment' + # Allows providing your own class instance for generating order numbers. # # @!attribute [rw] order_number_generator From 55abebe438751133231ec2bd082097e8646f6b8c Mon Sep 17 00:00:00 2001 From: Elia Schito Date: Mon, 11 Feb 2019 22:27:39 +0100 Subject: [PATCH 4/4] Set Paperclip as the default storage strategy We keep Paperclip as the default strategy for new apps. --- .../spree/install/templates/config/initializers/spree.rb.tt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt b/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt index e4d4a91d76d..5039d31dc43 100644 --- a/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt +++ b/core/lib/generators/spree/install/templates/config/initializers/spree.rb.tt @@ -18,6 +18,9 @@ Spree.config do |config| # any inventory changes. # config.inventory_cache_threshold = 3 + # Enable Paperclip adapter for attachments on images and taxons + config.image_attachment_module = 'Spree::Image::PaperclipAttachment' + config.taxon_attachment_module = 'Spree::Taxon::PaperclipAttachment' # Frontend: