diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 3dba2bd123..0e9cdb3975 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -441,7 +441,6 @@ Metrics/AbcSize: - 'lib/reporting/reports/payments/payments_report.rb' - 'lib/reporting/reports/sales_tax/sales_tax_report.rb' - 'lib/spree/core/controller_helpers/order.rb' - - 'lib/spree/core/s3_support.rb' - 'lib/tasks/enterprises.rake' - 'spec/services/order_checkout_restart_spec.rb' diff --git a/Gemfile b/Gemfile index a6ec2e5a78..1725e5e0d1 100644 --- a/Gemfile +++ b/Gemfile @@ -86,7 +86,6 @@ gem 'bootsnap', require: false gem 'geocoder' gem 'gmaps4rails' gem 'mimemagic', '> 0.3.5' -gem 'paperclip', '~> 3.4.1' gem 'paper_trail', '~> 12.1.0' gem 'rack-rewrite' gem 'rack-ssl', require: 'rack/ssl' diff --git a/Gemfile.lock b/Gemfile.lock index 13fb10770e..9001b74101 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -219,10 +219,7 @@ GEM rubyzip (>= 1.3.0, < 3) childprocess (4.1.0) chronic (0.10.2) - climate_control (0.2.0) cliver (0.3.2) - cocaine (0.5.8) - climate_control (>= 0.0.3, < 1.0) coderay (1.1.3) coffee-rails (5.0.0) coffee-script (>= 2.2.0) @@ -392,9 +389,6 @@ GEM marcel (1.0.2) matrix (0.4.2) method_source (1.0.0) - mime-types (3.3.1) - mime-types-data (~> 3.2015) - mime-types-data (3.2021.0225) mimemagic (0.4.3) nokogiri (~> 1) rake @@ -427,12 +421,6 @@ GEM paper_trail (12.1.0) activerecord (>= 5.2) request_store (~> 1.1) - paperclip (3.4.2) - activemodel (>= 3.0.0) - activerecord (>= 3.0.0) - activesupport (>= 3.0.0) - cocaine (~> 0.5.0) - mime-types parallel (1.21.0) paranoia (2.4.3) activerecord (>= 4.0, < 6.2) @@ -785,7 +773,6 @@ DEPENDENCIES order_management! pagy (~> 5.1) paper_trail (~> 12.1.0) - paperclip (~> 3.4.1) paranoia (~> 2.4) paypal-sdk-merchant (= 1.117.2) pdf-reader diff --git a/app/controllers/admin/contents_controller.rb b/app/controllers/admin/contents_controller.rb index 5841fa84eb..0a41cc389a 100644 --- a/app/controllers/admin/contents_controller.rb +++ b/app/controllers/admin/contents_controller.rb @@ -10,14 +10,14 @@ module Admin def update params.each do |name, value| - if ContentConfig.has_preference?(name) || ContentConfig.has_attachment?(name) - ContentConfig.public_send("#{name}=", value) + if value.is_a?(ActionDispatch::Http::UploadedFile) + blob = store_file(value) + update_preference("#{name}_blob_id", blob.id) + else + update_preference(name, value) end end - # Save any uploaded images - ContentConfig.save - flash[:success] = t(:successfully_updated, resource: I18n.t('admin.contents.edit.your_content')) @@ -26,6 +26,22 @@ module Admin private + def store_file(attachable) + ActiveStorage::Blob.create_and_upload!( + io: attachable.open, + filename: attachable.original_filename, + content_type: attachable.content_type, + service_name: :local, + identify: false, + ) + end + + def update_preference(name, value) + return unless ContentConfig.has_preference?(name) + + ContentConfig.public_send("#{name}=", value) + end + def preference_sections [ PreferenceSections::HeaderSection.new, diff --git a/app/controllers/api/v0/enterprise_attachment_controller.rb b/app/controllers/api/v0/enterprise_attachment_controller.rb index 6e183371a1..06555b277b 100644 --- a/app/controllers/api/v0/enterprise_attachment_controller.rb +++ b/app/controllers/api/v0/enterprise_attachment_controller.rb @@ -14,7 +14,7 @@ module Api respond_to :json def destroy - unless @enterprise.public_send("#{attachment_name}?") + unless @enterprise.public_send(attachment_name).attached? return respond_with_conflict(error: destroy_attachment_does_not_exist_error_message) end diff --git a/app/controllers/api/v0/enterprises_controller.rb b/app/controllers/api/v0/enterprises_controller.rb index 9dd4737a5d..5a92439001 100644 --- a/app/controllers/api/v0/enterprises_controller.rb +++ b/app/controllers/api/v0/enterprises_controller.rb @@ -44,9 +44,9 @@ module Api authorize! :update, @enterprise if params[:logo] && @enterprise.update( logo: params[:logo] ) - render html: @enterprise.logo.url(:medium), status: :ok - elsif params[:promo] && @enterprise.update( promo_image: params[:promo] ) - render html: @enterprise.promo_image.url(:medium), status: :ok + render(html: @enterprise.logo_url(:medium), status: :ok) + elsif params[:promo] && @enterprise.update!( promo_image: params[:promo] ) + render(html: @enterprise.promo_image_url(:medium), status: :ok) else invalid_resource!(@enterprise) end diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 9aae19e015..e81e846629 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -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 diff --git a/app/models/concerns/file_preferences.rb b/app/models/concerns/file_preferences.rb new file mode 100644 index 0000000000..68682b3263 --- /dev/null +++ b/app/models/concerns/file_preferences.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module FilePreferences + extend ActiveSupport::Concern + + included do + @default_urls = {} + end + + class_methods do + def file_preference(name, default_url: nil) + preference "#{name}_blob_id", :integer + @default_urls[name] = default_url if default_url + end + + def default_url(name) + @default_urls[name] + end + end + + def preference_type(key) + if has_preference?("#{key}_blob_id") + :file + else + super(key) + end + end + + def url_for(name) + blob = blob_for(name) + + if blob + Rails.application.routes.url_helpers.url_for(blob) + else + self.class.default_url(name) + end + end + + def blob_for(name) + blob_id = get_preference("#{name}_blob_id") + ActiveStorage::Blob.find_by(id: blob_id) if blob_id + end +end diff --git a/app/models/concerns/has_migrating_file.rb b/app/models/concerns/has_migrating_file.rb deleted file mode 100644 index 923ee59ba3..0000000000 --- a/app/models/concerns/has_migrating_file.rb +++ /dev/null @@ -1,67 +0,0 @@ -# frozen_string_literal: true - -module HasMigratingFile - extend ActiveSupport::Concern - - @migrating_models = [] - - def self.migrating_models - @migrating_models - end - - included do - HasMigratingFile.migrating_models.push(name) - end - - class_methods do - def has_one_migrating(name, paperclip_options = {}) - # Active Storage declaration - has_one_attached name - - # Backup Active Storage methods before they get overridden by Paperclip. - alias_method "active_storage_#{name}", name - alias_method "active_storage_#{name}=", "#{name}=" - - # Paperclip declaration - # - # This will define the `name` and `name=` methods as well. - has_attached_file name, paperclip_options - - # Paperclip callback to duplicate file with Active Storage - # - # We store files with Paperclip *and* Active Storage while we migrate - # old Paperclip files to Active Storage. This enables availability - # during the migration. - public_send("after_#{name}_post_process") do - path = processed_local_file_path(name) - if public_send(name).errors.blank? && path.present? - attach_file(name, File.open(path)) - end - end - end - end - - def attach_file(name, io) - attachable = { - io: io, - filename: public_send("#{name}_file_name"), - content_type: public_send("#{name}_content_type"), - identify: false, - } - public_send("active_storage_#{name}=", attachable) - end - - private - - def processed_local_file_path(name) - attachment = public_send(name) - - temporary = attachment.queued_for_write[:original] - - if temporary&.path.present? - temporary.path - else - attachment.path - end - end -end diff --git a/app/models/content_configuration.rb b/app/models/content_configuration.rb index 0f34598cd7..d0160407a4 100644 --- a/app/models/content_configuration.rb +++ b/app/models/content_configuration.rb @@ -1,23 +1,17 @@ # frozen_string_literal: true -require 'open_food_network/paperclippable' - -class ContentConfiguration < Spree::Preferences::FileConfiguration - include OpenFoodNetwork::Paperclippable +class ContentConfiguration < Spree::Preferences::Configuration + include FilePreferences # Header - preference :logo, :file - preference :logo_mobile, :file - preference :logo_mobile_svg, :file - has_attached_file :logo, default_url: "/default_images/ofn-logo.png" - has_attached_file :logo_mobile - has_attached_file :logo_mobile_svg, default_url: "/default_images/ofn-logo-mobile.svg" + file_preference :logo, default_url: "/default_images/ofn-logo.png" + file_preference :logo_mobile + file_preference :logo_mobile_svg, default_url: "/default_images/ofn-logo-mobile.svg" # Home page preference :home_page_alert_html, :text - preference :home_hero, :file + file_preference :home_hero, default_url: "/default_images/home.jpg" preference :home_show_stats, :boolean, default: true - has_attached_file :home_hero, default_url: "/default_images/home.jpg" # Map preference :open_street_map_enabled, :boolean, default: false @@ -66,8 +60,7 @@ class ContentConfiguration < Spree::Preferences::FileConfiguration preference :menu_7_icon_name, :string, default: "ofn-i_013-help" # Footer - preference :footer_logo, :file - has_attached_file :footer_logo, default_url: "/default_images/ofn-logo-footer.png" + file_preference :footer_logo, default_url: "/default_images/ofn-logo-footer.png" # Other preference :footer_facebook_url, :string, default: "https://www.facebook.com/OpenFoodNet" diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 9b9e587bed..461f08f919 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -1,13 +1,20 @@ # frozen_string_literal: false -require 'spree/core/s3_support' - class Enterprise < ApplicationRecord - include HasMigratingFile - include Spree::Core::S3Support - SELLS = %w(unspecified none own any).freeze ENTERPRISE_SEARCH_RADIUS = 100 + # The next Rails version will have named variants but we need to store them + # ourselves for now. + LOGO_SIZES = { + thumb: { resize_to_limit: [100, 100] }, + small: { resize_to_limit: [180, 180] }, + medium: { resize_to_limit: [300, 300] }, + }.freeze + PROMO_IMAGE_SIZES = { + thumb: { resize_to_limit: [100, 100] }, + medium: { resize_to_fill: [720, 156] }, + large: { resize_to_fill: [1200, 260] }, + }.freeze searchable_attributes :sells, :is_primary_producer searchable_associations :properties @@ -73,31 +80,16 @@ class Enterprise < ApplicationRecord tag_rule[:preferred_customer_tags].blank? } - has_one_migrating :logo, - styles: { medium: "300x300>", small: "180x180>", thumb: "100x100>" }, - url: '/images/enterprises/logos/:id/:style/:basename.:extension', - path: 'public/images/enterprises/logos/:id/:style/:basename.:extension' + has_one_attached :logo + has_one_attached :promo_image + has_one_attached :terms_and_conditions - has_one_migrating :promo_image, - styles: { - large: ["1200x260#", :jpg], - medium: ["720x156#", :jpg], - thumb: ["100x100>", :jpg] - }, - url: '/images/enterprises/promo_images/:id/:style/:basename.:extension', - path: 'public/images/enterprises/promo_images/:id/:style/:basename.:extension' - validates_attachment_content_type :logo, content_type: %r{\Aimage/.*\Z} - validates_attachment_content_type :promo_image, content_type: %r{\Aimage/.*\Z} - - has_one_migrating :terms_and_conditions, - url: '/files/enterprises/terms_and_conditions/:id/:basename.:extension', - path: 'public/files/enterprises/terms_and_conditions/:id/:basename.:extension' - validates_attachment_content_type :terms_and_conditions, - content_type: "application/pdf", - message: I18n.t(:enterprise_terms_and_conditions_type_error) - - supports_s3 :logo - supports_s3 :promo_image + validates :logo, content_type: %r{\Aimage/.*\Z} + validates :promo_image, content_type: %r{\Aimage/.*\Z} + validates :terms_and_conditions, content_type: { + in: "application/pdf", + message: I18n.t(:enterprise_terms_and_conditions_type_error), + } validates :name, presence: true validate :name_is_unique @@ -284,6 +276,22 @@ class Enterprise < ApplicationRecord relatives_including_self.is_primary_producer end + def logo_url(name) + return unless logo.variable? + + Rails.application.routes.url_helpers.url_for( + logo.variant(LOGO_SIZES[name]) + ) + end + + def promo_image_url(name) + return unless promo_image.variable? + + Rails.application.routes.url_helpers.url_for( + promo_image.variant(PROMO_IMAGE_SIZES[name]) + ) + end + def website strip_url self[:website] end diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index 8b2c908006..dcce2ab192 100644 --- a/app/models/enterprise_group.rb +++ b/app/models/enterprise_group.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true require 'open_food_network/locking' -require 'spree/core/s3_support' class EnterpriseGroup < ApplicationRecord - include HasMigratingFile include PermalinkGenerator - include Spree::Core::S3Support acts_as_list @@ -28,21 +25,11 @@ class EnterpriseGroup < ApplicationRecord delegate :phone, :address1, :address2, :city, :zipcode, :state, :country, to: :address - has_one_migrating :logo, - styles: { medium: "100x100" }, - url: '/images/enterprise_groups/logos/:id/:style/:basename.:extension', - path: 'public/images/enterprise_groups/logos/:id/:style/:basename.:extension' + has_one_attached :logo + has_one_attached :promo_image - has_one_migrating :promo_image, - styles: { large: ["1200x260#", :jpg] }, - url: '/images/enterprise_groups/promo_images/:id/:style/:basename.:extension', - path: 'public/images/enterprise_groups/promo_images/:id/:style/:basename.:extension' - - validates_attachment_content_type :logo, content_type: %r{\Aimage/.*\Z} - validates_attachment_content_type :promo_image, content_type: %r{\Aimage/.*\Z} - - supports_s3 :logo - supports_s3 :promo_image + validates :logo, content_type: %r{\Aimage/.*\Z} + validates :promo_image, content_type: %r{\Aimage/.*\Z} scope :by_position, -> { order('position ASC') } scope :on_front_page, -> { where(on_front_page: true) } diff --git a/app/models/spree/image.rb b/app/models/spree/image.rb index 996067ec6f..da48229f80 100644 --- a/app/models/spree/image.rb +++ b/app/models/spree/image.rb @@ -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 diff --git a/app/models/spree/preferences/file_configuration.rb b/app/models/spree/preferences/file_configuration.rb deleted file mode 100644 index 421484993f..0000000000 --- a/app/models/spree/preferences/file_configuration.rb +++ /dev/null @@ -1,62 +0,0 @@ -# frozen_string_literal: true - -module Spree - module Preferences - class FileConfiguration < Configuration - def self.preference(name, type, *args) - if type == :file - # Active Storage blob id: - super "#{name}_blob_id", :integer, *args - - # Paperclip attachment attributes: - super "#{name}_file_name", :string, *args - super "#{name}_content_type", :string, *args - super "#{name}_file_size", :integer, *args - super "#{name}_updated_at", :string, *args - - else - super name, type, *args - end - end - - def get_preference(key) - if !has_preference?(key) && has_attachment?(key) - # Call Paperclip's attachment method: - public_send key - elsif key.ends_with?("_blob") - # Find referenced Active Storage blob: - blob_id = super("#{key}_id") - ActiveStorage::Blob.find_by(id: blob_id) - else - super key - end - end - alias :[] :get_preference - - def preference_type(name) - if has_attachment? name - :file - else - super name - end - end - - # Spree's Configuration responds to preference methods via method_missing, but doesn't - # override respond_to?, which consequently reports those methods as unavailable. Paperclip - # errors if respond_to? isn't correct, so we override it here. - def respond_to?(method, include_all = false) - name = method.to_s.delete('=') - reference_name = "#{name}_id" - - super(self.class.preference_getter_method(name), include_all) || - super(reference_name, include_all) || - super(method, include_all) - end - - def has_attachment?(name) - self.class.respond_to?(:attachment_definitions) && - self.class.attachment_definitions.key?(name.to_sym) - end - end - end -end diff --git a/app/models/terms_of_service_file.rb b/app/models/terms_of_service_file.rb index d652c37f7e..2fb35aab3c 100644 --- a/app/models/terms_of_service_file.rb +++ b/app/models/terms_of_service_file.rb @@ -1,11 +1,9 @@ # frozen_string_literal: true class TermsOfServiceFile < ApplicationRecord - include HasMigratingFile + has_one_attached :attachment - has_one_migrating :attachment - - validates :attachment, presence: true + validates :attachment, attached: true # The most recently uploaded file is the current one. def self.current @@ -13,7 +11,11 @@ class TermsOfServiceFile < ApplicationRecord end def self.current_url - current&.attachment&.url || Spree::Config.footer_tos_url + if current + Rails.application.routes.url_helpers.url_for(current.attachment) + else + Spree::Config.footer_tos_url + end end # If no file has been uploaded, we don't know when the old terms have @@ -21,9 +23,4 @@ class TermsOfServiceFile < ApplicationRecord def self.updated_at current&.updated_at || Time.zone.now end - - def touch(_) - # Ignore Active Storage changing the timestamp during migrations. - # This can be removed once we got rid of Paperclip. - end end diff --git a/app/serializers/api/admin/enterprise_serializer.rb b/app/serializers/api/admin/enterprise_serializer.rb index 019250f634..2071024f3e 100644 --- a/app/serializers/api/admin/enterprise_serializer.rb +++ b/app/serializers/api/admin/enterprise_serializer.rb @@ -22,21 +22,26 @@ module Api has_one :business_address, serializer: Api::AddressSerializer def logo - attachment_urls(object.logo, [:thumb, :small, :medium]) + attachment_urls(object.logo, Enterprise::LOGO_SIZES) end def promo_image - attachment_urls(object.promo_image, [:thumb, :medium, :large]) + attachment_urls(object.promo_image, Enterprise::PROMO_IMAGE_SIZES) end def terms_and_conditions - return unless object.terms_and_conditions.file? + return unless object.terms_and_conditions.attached? - object.terms_and_conditions.url + Rails.application.routes.url_helpers. + url_for(object.terms_and_conditions) + end + + def terms_and_conditions_file_name + object.terms_and_conditions_blob&.filename end def terms_and_conditions_updated_at - object.terms_and_conditions_updated_at&.to_s + object.terms_and_conditions_blob&.created_at&.to_s end def tag_groups @@ -76,19 +81,19 @@ module Api # Returns a hash of URLs for specified versions of an attachment. # - # Example: + # Example result: # - # attachment_urls(object.logo, [:thumb, :small, :medium]) - # # { - # # thumb: LOGO_THUMB_URL, - # # small: LOGO_SMALL_URL, - # # medium: LOGO_MEDIUM_URL - # # } - def attachment_urls(attachment, versions) - return unless attachment.file? + # { + # thumb: LOGO_THUMB_URL, + # small: LOGO_SMALL_URL, + # medium: LOGO_MEDIUM_URL + # } + def attachment_urls(attachment, styles) + return unless attachment.variable? - versions.each_with_object({}) do |version, urls| - urls[version] = attachment.url(version) + styles.transform_values do |transformation| + Rails.application.routes.url_helpers. + url_for(attachment.variant(transformation)) end end end diff --git a/app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb b/app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb index f5ed38c76a..fa0c9e7365 100644 --- a/app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb +++ b/app/serializers/api/admin/for_order_cycle/supplied_product_serializer.rb @@ -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 diff --git a/app/serializers/api/admin/product_serializer.rb b/app/serializers/api/admin/product_serializer.rb index 6805d81d1f..d65909a521 100644 --- a/app/serializers/api/admin/product_serializer.rb +++ b/app/serializers/api/admin/product_serializer.rb @@ -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 diff --git a/app/serializers/api/admin/variant_serializer.rb b/app/serializers/api/admin/variant_serializer.rb index 1be17e1c19..062ce4ceee 100644 --- a/app/serializers/api/admin/variant_serializer.rb +++ b/app/serializers/api/admin/variant_serializer.rb @@ -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 diff --git a/app/serializers/api/cached_enterprise_serializer.rb b/app/serializers/api/cached_enterprise_serializer.rb index ede433065b..ef6b37c9cf 100644 --- a/app/serializers/api/cached_enterprise_serializer.rb +++ b/app/serializers/api/cached_enterprise_serializer.rb @@ -44,11 +44,11 @@ module Api end def logo - enterprise.logo(:medium) if enterprise.logo? + enterprise.logo_url(:medium) end def promo_image - enterprise.promo_image(:large) if enterprise.promo_image? + enterprise.promo_image_url(:large) end def path diff --git a/app/serializers/api/enterprise_shopfront_serializer.rb b/app/serializers/api/enterprise_shopfront_serializer.rb index cee6cc5145..894152778f 100644 --- a/app/serializers/api/enterprise_shopfront_serializer.rb +++ b/app/serializers/api/enterprise_shopfront_serializer.rb @@ -41,11 +41,11 @@ module Api end def logo - enterprise.logo(:medium) if enterprise.logo? + enterprise.logo_url(:medium) end def promo_image - enterprise.promo_image(:large) if enterprise.promo_image? + enterprise.promo_image_url(:large) end def path diff --git a/app/serializers/api/image_serializer.rb b/app/serializers/api/image_serializer.rb index ead54697d6..57d44ef74e 100644 --- a/app/serializers/api/image_serializer.rb +++ b/app/serializers/api/image_serializer.rb @@ -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 diff --git a/app/serializers/api/shop_for_orders_serializer.rb b/app/serializers/api/shop_for_orders_serializer.rb index a8dc799012..b7b19cb5fc 100644 --- a/app/serializers/api/shop_for_orders_serializer.rb +++ b/app/serializers/api/shop_for_orders_serializer.rb @@ -9,7 +9,7 @@ module Api end def logo - object.logo(:small) if object.logo? + object.logo_url(:small) end end end diff --git a/app/serializers/api/variant_serializer.rb b/app/serializers/api/variant_serializer.rb index 524d940656..f58810c52b 100644 --- a/app/serializers/api/variant_serializer.rb +++ b/app/serializers/api/variant_serializer.rb @@ -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 diff --git a/app/services/image_importer.rb b/app/services/image_importer.rb index 8fc243e755..706de935a0 100644 --- a/app/services/image_importer.rb +++ b/app/services/image_importer.rb @@ -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, ) diff --git a/app/services/terms_of_service.rb b/app/services/terms_of_service.rb index 95cbf661d6..5a55010272 100644 --- a/app/services/terms_of_service.rb +++ b/app/services/terms_of_service.rb @@ -5,7 +5,7 @@ class TermsOfService return false unless accepted_at = customer&.terms_and_conditions_accepted_at accepted_at > if distributor - distributor.terms_and_conditions_updated_at + distributor.terms_and_conditions_blob.created_at else TermsOfServiceFile.updated_at end @@ -20,6 +20,6 @@ class TermsOfService end def self.distributor_terms_required?(distributor) - distributor.terms_and_conditions.file? + distributor.terms_and_conditions.attached? end end diff --git a/app/views/admin/contents/_fieldset.html.haml b/app/views/admin/contents/_fieldset.html.haml index 509bf49f43..0821e4e837 100644 --- a/app/views/admin/contents/_fieldset.html.haml +++ b/app/views/admin/contents/_fieldset.html.haml @@ -8,5 +8,8 @@ - text = t(key) .field = label_tag(key, text + ': ') + tag(:br) if type != :boolean - = preference_field_tag(key, ContentConfig[key], :type => type) + - if type == :file + = file_field_tag(key, type: type) + - else + = preference_field_tag(key, ContentConfig[key], type: type) = label_tag(key, text) + tag(:br) if type == :boolean diff --git a/app/views/admin/enterprise_groups/_form_images.html.haml b/app/views/admin/enterprise_groups/_form_images.html.haml index aa31913a20..a86d4abf05 100644 --- a/app/views/admin/enterprise_groups/_form_images.html.haml +++ b/app/views/admin/enterprise_groups/_form_images.html.haml @@ -6,13 +6,13 @@ %div{'ofn-with-tip' => t('admin_enterprise_groups_data_powertip_logo')} %a= t 'admin.whats_this' .omega.eight.columns - = image_tag @object.logo.url if @object.logo.present? - = f.file_field :logo + = image_tag @object.logo if @object.logo.attached? + = f.file_field :logo, accept: "image/*" .row .alpha.three.columns = f.label :promo_image, 'ofn-with-tip' => t(:admin_enterprise_groups_data_powertip_promo_image) %div{'ofn-with-tip' => t('admin_enterprise_groups_data_powertip_promo_image')} %a= t 'admin.whats_this' .omega.eight.columns - = image_tag @object.promo_image.url if @object.promo_image.present? - = f.file_field :promo_image + = image_tag @object.promo_image if @object.promo_image.attached? + = f.file_field :promo_image, accept: "image/*" diff --git a/app/views/admin/terms_of_service_files/show.html.haml b/app/views/admin/terms_of_service_files/show.html.haml index 94b7633c52..fd78d4e7ee 100644 --- a/app/views/admin/terms_of_service_files/show.html.haml +++ b/app/views/admin/terms_of_service_files/show.html.haml @@ -6,7 +6,7 @@ .admin-current-terms-of-service - if @current_file - %p= t(".current_terms_html", tos_link: link_to(t(".terms_of_service"), @current_file.attachment.url), datetime: @current_file.updated_at) + %p= t(".current_terms_html", tos_link: link_to(t(".terms_of_service"), @current_file.attachment), datetime: @current_file.updated_at) %p= link_to t(".delete"), main_app.admin_terms_of_service_files_path, method: "delete", data: { confirm: t(".confirm_delete") } - else %p diff --git a/app/views/checkout/_all_terms_and_conditions.html.haml b/app/views/checkout/_all_terms_and_conditions.html.haml index 5a90b15cfa..6af5326284 100644 --- a/app/views/checkout/_all_terms_and_conditions.html.haml +++ b/app/views/checkout/_all_terms_and_conditions.html.haml @@ -1,4 +1,4 @@ %p %input{ type: 'checkbox', id: 'accept_terms', ng: { model: "terms_and_conditions_accepted", init: "terms_and_conditions_accepted = #{all_terms_and_conditions_already_accepted?}" } } %label.small{for: "accept_terms"} - = t('.message_html', terms_and_conditions_link: link_to( t(".terms_and_conditions"), current_order.distributor.terms_and_conditions.url, target: '_blank'), tos_link: link_to_platform_terms) + = t('.message_html', terms_and_conditions_link: link_to( t(".terms_and_conditions"), current_order.distributor.terms_and_conditions, target: '_blank'), tos_link: link_to_platform_terms) diff --git a/app/views/checkout/_terms_and_conditions.html.haml b/app/views/checkout/_terms_and_conditions.html.haml index e84b4b9c81..608a0fd66b 100644 --- a/app/views/checkout/_terms_and_conditions.html.haml +++ b/app/views/checkout/_terms_and_conditions.html.haml @@ -1,3 +1,3 @@ %p %input{ type: 'checkbox', id: 'accept_terms', ng: { model: "terms_and_conditions_accepted", init: "terms_and_conditions_accepted=#{terms_and_conditions_already_accepted?}" } } - %label.small{for: "accept_terms"}= t('.message_html', terms_and_conditions_link: link_to( t( '.link_text' ), current_order.distributor.terms_and_conditions.url, target: '_blank')) + %label.small{for: "accept_terms"}= t('.message_html', terms_and_conditions_link: link_to( t( '.link_text' ), current_order.distributor.terms_and_conditions, target: '_blank')) diff --git a/app/views/enterprises/shop.html.haml b/app/views/enterprises/shop.html.haml index 150452a4e9..628f4b8630 100644 --- a/app/views/enterprises/shop.html.haml +++ b/app/views/enterprises/shop.html.haml @@ -3,7 +3,7 @@ - content_for(:description) do = current_distributor.description - content_for(:image) do - = current_distributor.logo.url + = url_for(current_distributor.logo) if current_distributor.logo.attached? - content_for :injection_data do - cache(*CacheService::FragmentCaching.ams_shop(@enterprise)) do diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 0116a5d4d1..13966cba3f 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -3,7 +3,7 @@ - content_for(:description) do = @group.description - content_for(:image) do - = @group.logo.url + = url_for(@group.logo) if @group.logo.attached? - content_for :scripts do = render partial: "shared/google_maps_js" @@ -22,14 +22,13 @@ %header .row .small-12.columns - - if @group.promo_image.present? - %img{"src" => @group.promo_image} + = image_tag @group.promo_image.variant(resize_to_limit: [1200, 260]) if @group.promo_image.variable? .row .small-12.columns.group-header.pad-top - - if @group.logo.present? - %img.group-logo{"src" => @group.logo} + - if @group.logo.variable? + = image_tag @group.logo.variant(resize_to_limit: [100, 100]), class: "group-logo" - else - %img.group-logo{"src" => '/noimage/group.png' } + = image_tag "/noimage/group.png", class: "group-logo" %h2.group-name= @group.name %p= @group.description diff --git a/app/views/home/index.html.haml b/app/views/home/index.html.haml index 49da911e20..899e19ca72 100644 --- a/app/views/home/index.html.haml +++ b/app/views/home/index.html.haml @@ -1,5 +1,5 @@ :css - #tagline:before { background-image: url("#{ContentConfig.home_hero.url}") } + #tagline:before { background-image: url("#{ContentConfig.url_for(:home_hero)}") } - content_for :page_alert do diff --git a/app/views/layouts/darkswarm.html.haml b/app/views/layouts/darkswarm.html.haml index 0607ec2cb5..b213f52c07 100644 --- a/app/views/layouts/darkswarm.html.haml +++ b/app/views/layouts/darkswarm.html.haml @@ -4,7 +4,7 @@ %meta{name: 'viewport', content: "width=device-width,initial-scale=1.0"}/ %meta{property: "og:title", content: content_for?(:title) ? yield(:title) : t(:title)} %meta{property: "og:description", content: content_for?(:description) ? yield(:description) : t(:site_meta_description)} - %meta{property: "og:image", content: content_for?(:image) ? yield(:image) : ContentConfig.logo.url} + %meta{property: "og:image", content: content_for?(:image) ? yield(:image) : ContentConfig.url_for(:logo)} - if !Rails.env.production? || @noindex_meta_tag %meta{name: "robots", content: "noindex"} %title= content_for?(:title) ? "#{yield(:title)} - #{t(:title)}".html_safe : "#{t(:welcome_to)} #{t(:title)}" diff --git a/app/views/layouts/mailer.html.haml b/app/views/layouts/mailer.html.haml index 2536308b57..0160dfd727 100644 --- a/app/views/layouts/mailer.html.haml +++ b/app/views/layouts/mailer.html.haml @@ -15,7 +15,7 @@ %table{:bgcolor => "#f2f2f2"} %tr %td - %img{src: ContentConfig.footer_logo.url, width: "144", height: "50"}/ + %img{src: ContentConfig.url_for(:footer_logo), width: "144", height: "50"}/ %td{:align => "right"} %h6.collapse = Spree::Config[:site_name] diff --git a/app/views/shared/_footer.html.haml b/app/views/shared/_footer.html.haml index 4e7a5d05e1..4d6723892d 100644 --- a/app/views/shared/_footer.html.haml +++ b/app/views/shared/_footer.html.haml @@ -96,7 +96,7 @@ .row.legal .small-12.medium-3.medium-offset-2.columns.text-left %a{href: main_app.root_path} - %img{src: ContentConfig.footer_logo.url, width: "220"} + %img{src: ContentConfig.url_for(:footer_logo), width: "220"} .small-12.medium-5.columns.text-left %p.text-small = t '.footer_legal_call' diff --git a/app/views/shared/menu/_large_menu.html.haml b/app/views/shared/menu/_large_menu.html.haml index 1b48aa6a72..d8cb828ca6 100644 --- a/app/views/shared/menu/_large_menu.html.haml +++ b/app/views/shared/menu/_large_menu.html.haml @@ -3,7 +3,7 @@ %ul.nav-logo %li.ofn-logo %a{href: main_app.root_path} - %img{src: ContentConfig.logo.url} + %img{src: ContentConfig.url_for(:logo)} %li.powered-by %img{src: '/favicon.ico'} %span diff --git a/app/views/shared/menu/_mobile_menu.html.haml b/app/views/shared/menu/_mobile_menu.html.haml index 83c3d1c44d..aca1fa70d6 100644 --- a/app/views/shared/menu/_mobile_menu.html.haml +++ b/app/views/shared/menu/_mobile_menu.html.haml @@ -6,7 +6,7 @@ %section.left .ofn-logo %a{href: main_app.root_path} - %img{src: ContentConfig.logo_mobile.url, srcset: ContentConfig.logo_mobile_svg.url, width: "75", height: "26"} + %img{src: ContentConfig.url_for(:logo_mobile), srcset: ContentConfig.url_for(:logo_mobile_svg), width: "75", height: "26"} %section.right{"ng-cloak" => true} %span.cart-span{"ng-class" => "{ dirty: Cart.dirty || Cart.empty(), 'pure-dirty': Cart.dirty }"} diff --git a/app/views/shared/menu/_offcanvas_menu.html.haml b/app/views/shared/menu/_offcanvas_menu.html.haml index b6e553bc8c..ca8b907cf7 100644 --- a/app/views/shared/menu/_offcanvas_menu.html.haml +++ b/app/views/shared/menu/_offcanvas_menu.html.haml @@ -2,7 +2,7 @@ %ul.off-canvas-list %li.ofn-logo %a{href: main_app.root_path} - %img{src: ContentConfig.logo_mobile.url, srcset: ContentConfig.logo_mobile_svg.url, width: "75", height: "26"} + %img{src: ContentConfig.url_for(:logo_mobile), srcset: ContentConfig.url_for(:logo_mobile_svg), width: "75", height: "26"} - [*1..7].each do |menu_number| - menu_name = "menu_#{menu_number}" - if ContentConfig[menu_name].present? diff --git a/app/views/shopping_shared/_header.html.haml b/app/views/shopping_shared/_header.html.haml index 643ba2912f..144c3fc427 100644 --- a/app/views/shopping_shared/_header.html.haml +++ b/app/views/shopping_shared/_header.html.haml @@ -4,8 +4,8 @@ %distributor.details.row .small-12.medium-12.large-8.columns #distributor_title - - if distributor.logo? - %img.left{src: distributor.logo.url(:thumb)} + - if distributor.logo.variable? + = image_tag distributor.logo_url(:thumb), class: "left" = render DistributorTitleComponent.new(name: distributor.name) %location= distributor.address.city diff --git a/app/views/split_checkout/_terms_and_conditions.html.haml b/app/views/split_checkout/_terms_and_conditions.html.haml index 0acf25c899..6463df2053 100644 --- a/app/views/split_checkout/_terms_and_conditions.html.haml +++ b/app/views/split_checkout/_terms_and_conditions.html.haml @@ -3,7 +3,7 @@ - if platform_terms_required? && distributor_terms_required? = f.check_box :accept_terms, { name: "accept_terms", checked: all_terms_and_conditions_already_accepted? }, 1, nil = f.label :accept_terms do - = t('split_checkout.step3.all_terms_and_conditions.message_html', terms_and_conditions_link: link_to( t("split_checkout.step3.terms_and_conditions.link_text"), @order.distributor.terms_and_conditions.url, target: '_blank'), tos_link: link_to_platform_terms) + = t('split_checkout.step3.all_terms_and_conditions.message_html', terms_and_conditions_link: link_to( t("split_checkout.step3.terms_and_conditions.link_text"), @order.distributor.terms_and_conditions, target: '_blank'), tos_link: link_to_platform_terms) - elsif platform_terms_required? = f.check_box :accept_terms, { name: "accept_terms", checked: platform_tos_already_accepted? }, 1, nil = f.label :accept_terms do @@ -11,7 +11,7 @@ - elsif distributor_terms_required? = f.check_box :accept_terms, { name: "accept_terms", checked: terms_and_conditions_already_accepted? }, 1, nil = f.label :accept_terms do - = t('split_checkout.step3.terms_and_conditions.message_html', terms_and_conditions_link: link_to( t("split_checkout.step3.terms_and_conditions.link_text"), @order.distributor.terms_and_conditions.url, target: '_blank')) + = t('split_checkout.step3.terms_and_conditions.message_html', terms_and_conditions_link: link_to( t("split_checkout.step3.terms_and_conditions.link_text"), @order.distributor.terms_and_conditions, target: '_blank')) = f.error_message_on :terms_and_conditions, standalone: true diff --git a/app/views/spree/admin/images/edit.html.haml b/app/views/spree/admin/images/edit.html.haml index 49d1361daf..970f67d132 100644 --- a/app/views/spree/admin/images/edit.html.haml +++ b/app/views/spree/admin/images/edit.html.haml @@ -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 diff --git a/app/views/spree/admin/images/index.html.haml b/app/views/spree/admin/images/index.html.haml index 7f777c573a..978de27092 100644 --- a/app/views/spree/admin/images/index.html.haml +++ b/app/views/spree/admin/images/index.html.haml @@ -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 diff --git a/app/views/spree/admin/orders/invoice2.html.haml b/app/views/spree/admin/orders/invoice2.html.haml index 689909d856..be43fb767c 100644 --- a/app/views/spree/admin/orders/invoice2.html.haml +++ b/app/views/spree/admin/orders/invoice2.html.haml @@ -6,9 +6,9 @@ %td{ :align => "left" } %h4 = t :tax_invoice - - if @order.distributor.display_invoice_logo? && @order.distributor.logo.present? + - if @order.distributor.display_invoice_logo? && @order.distributor.logo.variable? %td{ :align => "right", rowspan: 2 } - = wicked_pdf_image_tag @order.distributor.logo(:small), width: 150, height: 150 + = wicked_pdf_image_tag @order.distributor.logo.variant(resize_to_limit: [150, 150]) %tr{ valign: "top" } %td{ :align => "left" } - if @order.distributor.business_address.blank? diff --git a/app/views/spree/order_mailer/cancel_email.html.haml b/app/views/spree/order_mailer/cancel_email.html.haml index 0c46511c3d..edfc66f162 100755 --- a/app/views/spree/order_mailer/cancel_email.html.haml +++ b/app/views/spree/order_mailer/cancel_email.html.haml @@ -6,7 +6,8 @@ = t(".customer_greeting", name: @order.bill_address.firstname) %h4 = t(".instructions_html", distributor: @order.distributor.name ) - %img{src: "#{@order.distributor.logo.url(:medium)}"} + - if @order.distributor.logo.variable? + = image_tag @order.distributor.logo_url(:medium) %p.callout = t(".dont_cancel", email: @order.distributor.contact.email) diff --git a/app/views/spree/order_mailer/confirm_email_for_customer.html.haml b/app/views/spree/order_mailer/confirm_email_for_customer.html.haml index 42ed2d9cc1..8894805243 100644 --- a/app/views/spree/order_mailer/confirm_email_for_customer.html.haml +++ b/app/views/spree/order_mailer/confirm_email_for_customer.html.haml @@ -11,7 +11,8 @@ %table.column{:align => "left"} %tr %td{:align => "right"} - %img.float-right{:src => "#{@order.distributor.logo.url(:medium)}"}/ + - if @order.distributor.logo.variable? + = image_tag @order.distributor.logo_url(:medium), class: "float-right" %span.clear %p   diff --git a/app/views/spree/order_mailer/confirm_email_for_shop.html.haml b/app/views/spree/order_mailer/confirm_email_for_shop.html.haml index 90ec5386d1..0e5b4510bf 100644 --- a/app/views/spree/order_mailer/confirm_email_for_shop.html.haml +++ b/app/views/spree/order_mailer/confirm_email_for_shop.html.haml @@ -11,7 +11,8 @@ %table.column{:align => "left"} %tr %td{:align => "right"} - %img.float-right{:src => "#{@order.distributor.logo.url(:medium)}"}/ + - if @order.distributor.logo.variable? + = image_tag @order.distributor.logo_url(:medium), class: "float-right" %span.clear %p   diff --git a/app/views/spree/shared/_variant_thumbnail.html.haml b/app/views/spree/shared/_variant_thumbnail.html.haml index 719725c927..d92e205a0b 100644 --- a/app/views/spree/shared/_variant_thumbnail.html.haml +++ b/app/views/spree/shared/_variant_thumbnail.html.haml @@ -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))) diff --git a/app/views/subscription_mailer/_header.html.haml b/app/views/subscription_mailer/_header.html.haml index cb6f2d8dfe..6f7a593669 100644 --- a/app/views/subscription_mailer/_header.html.haml +++ b/app/views/subscription_mailer/_header.html.haml @@ -11,5 +11,6 @@ %table.column{:align => "left"} %tr %td{:align => "right"} - %img.float-right{:src => "#{@order.distributor.logo.url(:medium)}"}/ + - if @order.distributor.logo.variable? + = image_tag @order.distributor.logo_url(:medium), class: "float-right" %span.clear diff --git a/app/views/subscription_mailer/confirmation_summary_email.html.haml b/app/views/subscription_mailer/confirmation_summary_email.html.haml index 861df53454..4cdefdffe1 100644 --- a/app/views/subscription_mailer/confirmation_summary_email.html.haml +++ b/app/views/subscription_mailer/confirmation_summary_email.html.haml @@ -11,7 +11,8 @@ %table.column{:align => "left"} %tr %td{:align => "right"} - %img.float-right{:src => "#{@shop.logo.url(:medium)}"}/ + - if @shop.logo.variable? + = image_tag @shop.logo_url(:medium), class: "float-right" %span.clear = render 'summary_overview', summary: @summary diff --git a/app/views/subscription_mailer/placement_summary_email.html.haml b/app/views/subscription_mailer/placement_summary_email.html.haml index 861df53454..3c8a21baca 100644 --- a/app/views/subscription_mailer/placement_summary_email.html.haml +++ b/app/views/subscription_mailer/placement_summary_email.html.haml @@ -11,7 +11,8 @@ %table.column{:align => "left"} %tr %td{:align => "right"} - %img.float-right{:src => "#{@shop.logo.url(:medium)}"}/ + - if @shop.logo.variable? + = image_tag @shop.logo_variant(:medium), class: "float-right" %span.clear = render 'summary_overview', summary: @summary diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb deleted file mode 100644 index a58c6bdf9d..0000000000 --- a/config/initializers/paperclip.rb +++ /dev/null @@ -1,46 +0,0 @@ -Paperclip::Attachment.default_options[:source_file_options] = { - all: "-auto-orient" -} - -url_adapters = [ - "Paperclip::UriAdapter", - "Paperclip::HttpUrlProxyAdapter", - "Paperclip::DataUriAdapter" -] - -# Remove Paperclip URL adapters from registered handlers -Paperclip.io_adapters.registered_handlers.delete_if do |_proc, adapter_class| - url_adapters.include? adapter_class.to_s -end - -if Paperclip::VERSION.to_f < 3.5 - if Rails::VERSION::MAJOR > 4 - # Patches an error for missing method #silence_stream with Rails 5.0 - # Can be removed after Paperclip is upgraded to 3.5+ - module Paperclip - class GeometryDetector - def silence_stream(_stream, &block) - yield - end - end - end - end -else - Rails.logger.warn "The Paperclip::GeometryDetector patch can now be removed." -end - -module UpdatedUrlGenerator - def escape_url(url) - (url.respond_to?(:escape) ? url.escape : URI::Parser.new.escape(url)). - gsub(/(\/.+)\?(.+\.)/, '\1%3F\2') - end -end - -module PaperclipImageErrors - def mark_invalid(record, attribute, types) - record.errors.add attribute, :invalid, **options.merge(:types => types.join(', ')) - end -end - -Paperclip::UrlGenerator.prepend(UpdatedUrlGenerator) -Paperclip::Validators::AttachmentPresenceValidator.prepend(PaperclipImageErrors) diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb index 145b585335..c3712a72f9 100644 --- a/config/initializers/spree.rb +++ b/config/initializers/spree.rb @@ -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? diff --git a/config/locales/en.yml b/config/locales/en.yml index ade845a073..3820f0c536 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3903,7 +3903,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" diff --git a/db/migrate/20220602013938_compute_checksum_for_big_files.rb b/db/migrate/20220602013938_compute_checksum_for_big_files.rb new file mode 100644 index 0000000000..697f731e5d --- /dev/null +++ b/db/migrate/20220602013938_compute_checksum_for_big_files.rb @@ -0,0 +1,17 @@ +# When migrating to Active Storage, we used Amazon's ETag for the blob +# checksum. But big files have been uploaded in chunks and then the checksum +# differs. We need to recalculate the checksum for large files. +class ComputeChecksumForBigFiles < ActiveRecord::Migration[6.1] + def up + blobs_with_incorrect_checksum.find_each do |blob| + md5 = Digest::MD5.base64digest(blob.download) + blob.update(checksum: md5) + end + end + + def blobs_with_incorrect_checksum + ActiveStorage::Blob. + where(service_name: "amazon"). + where("byte_size >= 20000000") + end +end diff --git a/db/schema.rb b/db/schema.rb index 0ad1f12db3..010059c3d6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_04_10_162955) do +ActiveRecord::Schema.define(version: 2022_06_02_013938) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/lib/open_food_network/paperclippable.rb b/lib/open_food_network/paperclippable.rb deleted file mode 100644 index e510c6af82..0000000000 --- a/lib/open_food_network/paperclippable.rb +++ /dev/null @@ -1,51 +0,0 @@ -# frozen_string_literal: true - -# Allow use of Paperclip's has_attached_file on non-ActiveRecord classes -# https://gist.github.com/basgys/5712426 - -module OpenFoodNetwork - module Paperclippable - def self.included(base) - base.extend(ActiveModel::Naming) - base.extend(ActiveModel::Callbacks) - base.include(ActiveModel::Validations) - base.include(Paperclip::Glue) - - # Paperclip required callbacks - base.define_model_callbacks(:save, only: [:after]) - base.define_model_callbacks(:commit, only: [:after]) - base.define_model_callbacks(:destroy, only: [:before, :after]) - - # Initialise an ID - base.__send__(:attr_accessor, :id) - base.instance_variable_set :@id, 1 - end - - # ActiveModel requirements - def to_model - self - end - - def valid?() true end - - def new_record?() true end - - def destroyed?() true end - - def save - run_callbacks :save do - end - true - end - - def errors - obj = Object.new - def obj.[](_key) [] end - - def obj.full_messages() [] end - - def obj.any?() false end - obj - end - end -end diff --git a/lib/spree/core.rb b/lib/spree/core.rb index 900da7c45b..3daa196700 100644 --- a/lib/spree/core.rb +++ b/lib/spree/core.rb @@ -6,7 +6,6 @@ require 'awesome_nested_set' require 'cancan' require 'pagy' require 'mail' -require 'paperclip' require 'paranoia' require 'ransack' require 'state_machines' diff --git a/lib/spree/core/product_duplicator.rb b/lib/spree/core/product_duplicator.rb index f4b778432f..659d2c5c6d 100644 --- a/lib/spree/core/product_duplicator.rb +++ b/lib/spree/core/product_duplicator.rb @@ -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 diff --git a/lib/spree/core/s3_support.rb b/lib/spree/core/s3_support.rb deleted file mode 100644 index 92229c15f5..0000000000 --- a/lib/spree/core/s3_support.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -module Spree - module Core - # This module exists to reduce duplication in S3 settings between - # the Image and Taxon models in Spree - module S3Support - extend ActiveSupport::Concern - - included do - def self.supports_s3(field) - # Load user defined paperclip settings - config = Spree::Config - return unless config[:use_s3] - - s3_creds = { access_key_id: config[:s3_access_key], - secret_access_key: config[:s3_secret], - bucket: config[:s3_bucket] } - attachment_definitions[field][:storage] = :s3 - attachment_definitions[field][:s3_credentials] = s3_creds - attachment_definitions[field][:s3_headers] = ActiveSupport::JSON. - decode(config[:s3_headers]) - attachment_definitions[field][:bucket] = config[:s3_bucket] - if config[:s3_protocol].present? - attachment_definitions[field][:s3_protocol] = config[:s3_protocol].downcase - end - - return if config[:s3_host_alias].blank? - - attachment_definitions[field][:s3_host_alias] = config[:s3_host_alias] - end - end - end - end -end diff --git a/lib/tasks/from_paperclip_to_active_storage.rake b/lib/tasks/from_paperclip_to_active_storage.rake deleted file mode 100644 index edd0a2e2ee..0000000000 --- a/lib/tasks/from_paperclip_to_active_storage.rake +++ /dev/null @@ -1,136 +0,0 @@ -# frozen_string_literal: true - -namespace :from_paperclip_to_active_storage do - # This migration can't be a pure database migration because we need to know - # the location of current files which is computed by Paperclip depending on - # the `url` option. - desc "Copy data to Active Storage tables referencing Paperclip files" - task migrate: :environment do - Rails.application.eager_load! - - HasMigratingFile.migrating_models.each do |model_name| - puts "Migrating #{model_name}" - migrate_model(model_name.constantize) - end - end - - # We have a special class called ContentConfiguration which is not a model - # and therfore can't use the normal Active Storage magic. - # - # It uses `Spree::Preference`s to store all the Paperclip attributes. These - # files are stored locally and we can replace them with preferences pointing - # to an Active Storage blob. - desc "Associate ContentConfig to ActiveStorage blobs" - task copy_content_config: :environment do - [ - :logo, - :logo_mobile, - :logo_mobile_svg, - :home_hero, - :footer_logo, - ].each do |name| - migrate_content_config_file(name) - end - end - - def migrate_model(model) - duplicated_attachment_names(model).each do |name| - migrate_attachment(model, name) - end - end - - def migrate_attachment(model, name) - records_to_migrate = missing_active_storage_attachment(model, name) - - print " - #{name} (#{records_to_migrate.count}) " - - records_to_migrate.find_each do |record| - attach_paperclip(name, record) - end - - puts "" - end - - def attach_paperclip(name, record) - paperclip = record.public_send(name) - - if paperclip.respond_to?(:s3_object) - attachment = storage_record_for(name, paperclip) - record.public_send("#{name}_attachment=", attachment) - print "." - elsif File.exist?(paperclip.path) - record.attach_file(name, File.open(paperclip.path)) - record.save! - print "." - else - print "x" - end - rescue StandardError => e - puts "x" - puts e.message - end - - # Creates an Active Storage record pointing to the same file Paperclip - # stored on AWS S3. Getting the checksum requires a HEAD request. - # In my tests, I could process 100 records per minute this way. - def storage_record_for(name, paperclip) - checksum = hex_to_base64_digest(paperclip.s3_object(:original).etag.delete('"')) - - blob = ActiveStorage::Blob.new( - key: paperclip.path(:original), - filename: paperclip.original_filename, - content_type: paperclip.content_type, - metadata: {}, - byte_size: paperclip.size, - checksum: checksum, - created_at: paperclip.updated_at, - ) - ActiveStorage::Attachment.new( - name: name, - blob: blob, - created_at: paperclip.updated_at, - ) - end - - def migrate_content_config_file(name) - paperclip = ContentConfig.public_send(name) - - return if ContentConfig.public_send("#{name}_blob") - return if paperclip.path.blank? || !paperclip.exists? - - blob = ActiveStorage::Blob.create_and_upload!( - io: File.open(paperclip.path), - filename: paperclip.original_filename, - content_type: paperclip.content_type, - identify: false, - ) - - ContentConfig.public_send("#{name}_blob_id=", blob.id) - puts "Copied #{name}" - end - - def duplicated_attachment_names(model) - paperclip_attachments = model.attachment_definitions.keys.map(&:to_s) - active_storage_attachments = model.attachment_reflections.keys - - only_paperclip = paperclip_attachments - active_storage_attachments - only_active_storage = active_storage_attachments - paperclip_attachments - both = paperclip_attachments & active_storage_attachments - - puts "WARNING: not migrating #{only_paperclip}" if only_paperclip.present? - puts "WARNING: no source for #{only_active_storage}" if only_active_storage.present? - - both - end - - # Records with Paperclip but without an Active storage attachment yet - def missing_active_storage_attachment(model, attachment) - model.where.not("#{attachment}_file_name" => [nil, ""]). - left_outer_joins("#{attachment}_attachment".to_sym). - where(active_storage_attachments: { id: nil }) - end - - def hex_to_base64_digest(hexdigest) - [[hexdigest].pack("H*")].pack("m0") - end -end diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index 617ed81a84..5b865822de 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -14,7 +14,6 @@ require 'rspec/rails' require 'capybara' require 'rspec/retry' require 'paper_trail/frameworks/rspec' -require "paperclip/matchers" require "factory_bot_rails" require 'shoulda/matchers' @@ -126,7 +125,6 @@ RSpec.configure do |config| config.infer_spec_type_from_file_location! config.include FactoryBot::Syntax::Methods - config.include Paperclip::Shoulda::Matchers config.include JsonSpec::Helpers config.include Rails.application.routes.url_helpers diff --git a/spec/controllers/api/v0/enterprises_controller_spec.rb b/spec/controllers/api/v0/enterprises_controller_spec.rb index ce9cf59254..2eb0127eeb 100644 --- a/spec/controllers/api/v0/enterprises_controller_spec.rb +++ b/spec/controllers/api/v0/enterprises_controller_spec.rb @@ -85,14 +85,14 @@ describe Api::V0::EnterprisesController, type: :controller do api_post :update_image, logo: logo, id: enterprise.id expect(response.status).to eq 200 expect(response.content_type).to eq "text/html" - expect(response.body).to match %r{/images/enterprises/logos/\d*/medium/logo\.png\?\d*} + expect(response.body).to match /logo\.png$/ end it "I can update enterprise promo image" do api_post :update_image, promo: logo, id: enterprise.id expect(response.status).to eq 200 expect(response.content_type).to eq "text/html" - expect(response.body).to match %r{/images/enterprises/promo_images/\d*/medium/logo\.jpg\?\d*} + expect(response.body).to match /logo\.png$/ end end end diff --git a/spec/controllers/api/v0/logos_controller_spec.rb b/spec/controllers/api/v0/logos_controller_spec.rb index 4d82469775..58d6e410a5 100644 --- a/spec/controllers/api/v0/logos_controller_spec.rb +++ b/spec/controllers/api/v0/logos_controller_spec.rb @@ -35,7 +35,7 @@ module Api expect(response.status).to eq 200 expect(json_response["id"]).to eq enterprise.id enterprise.reload - expect(enterprise.logo?).to be false + expect(enterprise.logo).to_not be_attached end context "when logo does not exist" do @@ -75,7 +75,7 @@ module Api spree_delete :destroy, enterprise_id: enterprise expect(response.status).to eq(401) enterprise.reload - expect(enterprise.logo?).to be true + expect(enterprise.logo).to be_attached end end @@ -86,7 +86,7 @@ module Api spree_delete :destroy, enterprise_id: enterprise expect(response.status).to eq(401) enterprise.reload - expect(enterprise.logo?).to be true + expect(enterprise.logo).to be_attached end end end diff --git a/spec/controllers/api/v0/promo_images_controller_spec.rb b/spec/controllers/api/v0/promo_images_controller_spec.rb index 22296172d1..dd31be66dc 100644 --- a/spec/controllers/api/v0/promo_images_controller_spec.rb +++ b/spec/controllers/api/v0/promo_images_controller_spec.rb @@ -35,7 +35,7 @@ module Api expect(response.status).to eq 200 expect(json_response["id"]).to eq enterprise.id enterprise.reload - expect(enterprise.promo_image?).to be false + expect(enterprise.promo_image).to_not be_attached end context "when promo image does not exist" do @@ -75,7 +75,7 @@ module Api spree_delete :destroy, enterprise_id: enterprise expect(response.status).to eq(401) enterprise.reload - expect(enterprise.promo_image?).to be true + expect(enterprise.promo_image).to be_attached end end @@ -86,7 +86,7 @@ module Api spree_delete :destroy, enterprise_id: enterprise expect(response.status).to eq(401) enterprise.reload - expect(enterprise.promo_image?).to be true + expect(enterprise.promo_image).to be_attached end end end diff --git a/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb b/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb index 6a8fc673a5..f99c75b5ae 100644 --- a/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb +++ b/spec/controllers/api/v0/terms_and_conditions_controller_spec.rb @@ -12,15 +12,15 @@ module Api let(:enterprise_manager) { create(:user, enterprises: [enterprise]) } describe "removing terms and conditions file" do - let(:fake_terms_file_path) { black_logo_file } + let(:terms_file_path) { Rails.root.join("public/Terms-of-service.pdf") } let(:terms_and_conditions_file) { - Rack::Test::UploadedFile.new(fake_terms_file_path, "application/pdf") + Rack::Test::UploadedFile.new(terms_file_path, "application/pdf") } let(:enterprise) { create(:enterprise, owner: enterprise_owner) } before do allow(controller).to receive(:spree_current_user) { current_user } - enterprise.update terms_and_conditions: terms_and_conditions_file + enterprise.update!(terms_and_conditions: terms_and_conditions_file) end context "as manager" do @@ -32,7 +32,7 @@ module Api expect(response.status).to eq 200 expect(json_response["id"]).to eq enterprise.id enterprise.reload - expect(enterprise.terms_and_conditions?).to be false + expect(enterprise.terms_and_conditions).to_not be_attached end context "when terms and conditions file does not exist" do diff --git a/spec/controllers/spree/admin/products_controller_spec.rb b/spec/controllers/spree/admin/products_controller_spec.rb index 7f2177494d..5474e5edad 100644 --- a/spec/controllers/spree/admin/products_controller_spec.rb +++ b/spec/controllers/spree/admin/products_controller_spec.rb @@ -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 diff --git a/spec/factories/order_cycle_factory.rb b/spec/factories/order_cycle_factory.rb index 2b1c50ddeb..8dbfcc6acd 100644 --- a/spec/factories/order_cycle_factory.rb +++ b/spec/factories/order_cycle_factory.rb @@ -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 ) diff --git a/spec/lib/spree/core/product_duplicator_spec.rb b/spec/lib/spree/core/product_duplicator_spec.rb index 4026c01c8f..05813f104d 100644 --- a/spec/lib/spree/core/product_duplicator_spec.rb +++ b/spec/lib/spree/core/product_duplicator_spec.rb @@ -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) diff --git a/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb b/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb deleted file mode 100644 index 210607034a..0000000000 --- a/spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb +++ /dev/null @@ -1,117 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" -require "rake" - -describe "from_paperclip_to_active_storage.rake" do - include FileHelper - - let(:file) { Rack::Test::UploadedFile.new(black_logo_file, 'image/png') } - 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(:all) do - Rake.application.rake_require "tasks/from_paperclip_to_active_storage" - Rake::Task.define_task(:environment) - end - - describe ":migrate" do - it "creates Active Storage records for existing images on disk" do - image = Spree::Image.create!(attachment: file) - image.attachment_attachment.delete - image.attachment_blob.delete - - expect { - run_task "from_paperclip_to_active_storage:migrate" - }.to change { - image.reload.active_storage_attachment.attached? - }.to(true) - end - - it "creates Active Storage records for images on AWS S3" 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) - - stub_request(:put, /amazonaws/).to_return(status: 200, body: "", headers: {}) - stub_request(:head, /amazonaws/).to_return( - status: 200, body: "", - headers: { - "ETag" => '"87b0a401e077485a078c0a15ceb7eb39"' - } - ) - stub_request(:put, /amazonaws/).to_return(status: 200, body: "", headers: {}) - - image = Spree::Image.create!(attachment: file) - image.attachment_attachment.delete - image.attachment_blob.delete - - expect { - run_task "from_paperclip_to_active_storage:migrate" - }.to change { - image.reload.active_storage_attachment.attached? - }.to(true) - - # The checksum can be computed with Active Storage: - # - # ActiveStorage::Blob.build_after_unfurling( - # io: file, identify: false, - # filename: "logo-black.png", - # content_type: "image/png", - # ).checksum - expect(image.attachment_blob.checksum).to eq "h7CkAeB3SFoHjAoVzrfrOQ==" - end - end - - describe ":copy_content_config" do - it "doesn't copy default images" do - run_task "from_paperclip_to_active_storage:copy_content_config" - - expect(ContentConfig.logo_blob).to eq nil - end - - it "copies uploaded images" do - ContentConfig.logo = file - ContentConfig.logo.save - - run_task "from_paperclip_to_active_storage:copy_content_config" - - expect(ContentConfig.logo_blob).to be_a ActiveStorage::Blob - end - - it "doesn't copy twice" do - ContentConfig.logo = file - ContentConfig.logo.save - - expect { - run_task "from_paperclip_to_active_storage:copy_content_config" - run_task "from_paperclip_to_active_storage:copy_content_config" - }.to change { - ActiveStorage::Blob.count - }.by(1) - end - end - - def run_task(name) - Rake::Task[name].reenable - Rake.application.invoke_task(name) - end -end diff --git a/spec/models/content_configuration_spec.rb b/spec/models/content_configuration_spec.rb index 8d2cc5a8d0..141d4111cf 100644 --- a/spec/models/content_configuration_spec.rb +++ b/spec/models/content_configuration_spec.rb @@ -5,10 +5,10 @@ require 'spec_helper' describe ContentConfiguration do describe "default logos and home_hero" do it "sets a default url with existing image" do - expect(image_exist?(ContentConfig.logo.options[:default_url])).to be true - expect(image_exist?(ContentConfig.logo_mobile_svg.options[:default_url])).to be true - expect(image_exist?(ContentConfig.home_hero.options[:default_url])).to be true - expect(image_exist?(ContentConfig.footer_logo.options[:default_url])).to be true + expect(image_exist?(ContentConfig.url_for(:logo))).to be true + expect(image_exist?(ContentConfig.url_for(:logo_mobile_svg))).to be true + expect(image_exist?(ContentConfig.url_for(:home_hero))).to be true + expect(image_exist?(ContentConfig.url_for(:footer_logo))).to be true end def image_exist?(default_url) diff --git a/spec/models/enterprise_group_spec.rb b/spec/models/enterprise_group_spec.rb index 6c443a25e1..2d62bce69c 100644 --- a/spec/models/enterprise_group_spec.rb +++ b/spec/models/enterprise_group_spec.rb @@ -39,8 +39,8 @@ describe EnterpriseGroup do e = build(:enterprise_group, description: '') end - it { is_expected.to have_attached_file :promo_image } - it { is_expected.to have_attached_file :logo } + it { is_expected.to have_one_attached :promo_image } + it { is_expected.to have_one_attached :logo } end describe "relations" do @@ -50,13 +50,6 @@ describe EnterpriseGroup do eg.enterprises << e expect(eg.reload.enterprises).to eq([e]) end - - # it "can have an image" do - # eg = create(:enterprise_group) - # image_file = File.open(File.expand_path('../../../app/assets/images/logo-white.png', __FILE__)) - # image = Spree::Image.create(viewable_id: eg.id, viewable_type: 'EnterpriseGroup', attachment: image_file) - # eg.reload.image.should == image - # end end describe "scopes" do diff --git a/spec/models/spree/image_spec.rb b/spec/models/spree/image_spec.rb index 06de9c955b..52debc535a 100644 --- a/spec/models/spree/image_spec.rb +++ b/spec/models/spree/image_spec.rb @@ -6,92 +6,54 @@ module Spree describe Image do include FileHelper - let(:file) { Rack::Test::UploadedFile.new(black_logo_file, 'image/png') } + 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: 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: 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 diff --git a/spec/models/spree/preferences/file_configuration_spec.rb b/spec/models/spree/preferences/file_configuration_spec.rb deleted file mode 100644 index b68eeb270e..0000000000 --- a/spec/models/spree/preferences/file_configuration_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -module Spree - module Preferences - class TestConfiguration < FileConfiguration - preference :name, :string - - include OpenFoodNetwork::Paperclippable - preference :logo, :file - has_attached_file :logo - end - - describe FileConfiguration do - let(:c) { TestConfiguration.new } - - describe "getting preferences" do - it "returns regular preferences" do - c.name = 'foo' - expect(c.get_preference(:name)).to eq('foo') - end - - it "returns file preferences" do - expect(c.get_preference(:logo)).to be_a Paperclip::Attachment - end - - it "returns regular preferences via []" do - c.name = 'foo' - expect(c[:name]).to eq('foo') - end - - it "returns file preferences via []" do - expect(c[:logo]).to be_a Paperclip::Attachment - end - end - - describe "getting preference types" do - it "returns regular preference types" do - expect(c.preference_type(:name)).to eq(:string) - end - - it "returns file preference types" do - expect(c.preference_type(:logo)).to eq(:file) - end - end - - describe "respond_to?" do - it "responds to preference getters" do - expect(c.respond_to?(:name)).to be true - end - - it "responds to preference setters" do - expect(c.respond_to?(:name=)).to be true - end - end - end - end -end diff --git a/spec/models/terms_of_service_file_spec.rb b/spec/models/terms_of_service_file_spec.rb index 7f638d0d69..74a28697a9 100644 --- a/spec/models/terms_of_service_file_spec.rb +++ b/spec/models/terms_of_service_file_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe TermsOfServiceFile do let(:pdf) { File.open(Rails.root.join("public/Terms-of-service.pdf")) } + let(:upload) { Rack::Test::UploadedFile.new(pdf, "application/pdf") } describe ".current" do it "returns nil" do @@ -12,8 +13,8 @@ describe TermsOfServiceFile do it "returns the last one" do existing = [ - TermsOfServiceFile.create!(attachment: pdf), - TermsOfServiceFile.create!(attachment: pdf), + TermsOfServiceFile.create!(attachment: upload), + TermsOfServiceFile.create!(attachment: upload), ] expect(TermsOfServiceFile.current).to eq existing.last @@ -27,10 +28,10 @@ describe TermsOfServiceFile do expect(subject).to eq "/Terms-of-service.pdf" end - it "points to the last uploaded file with timestamp parameter" do - file = TermsOfServiceFile.create!(attachment: pdf) + it "points to a stored file" do + file = TermsOfServiceFile.create!(attachment: upload) - expect(subject).to match %r{^/system/terms_of_service_files/attachments.*Terms-of-service\.pdf\?\d+$} + expect(subject).to match /active_storage.*Terms-of-service\.pdf$/ end end @@ -45,7 +46,8 @@ describe TermsOfServiceFile do it "returns the time when the terms were last updated" do update_time = 1.day.ago - file = TermsOfServiceFile.create!(attachment: pdf, updated_at: update_time) + file = TermsOfServiceFile.create!(attachment: upload) + file.update(updated_at: update_time) # The database isn't as precise as Ruby's time and rounds. expect(subject).to be_within(0.001).of(update_time) diff --git a/spec/serializers/api/admin/enterprise_serializer_spec.rb b/spec/serializers/api/admin/enterprise_serializer_spec.rb index 1c4b2a1127..9c3e6b86d1 100644 --- a/spec/serializers/api/admin/enterprise_serializer_spec.rb +++ b/spec/serializers/api/admin/enterprise_serializer_spec.rb @@ -16,8 +16,7 @@ describe Api::Admin::EnterpriseSerializer do context "when there is a logo" do let(:image) do - image = white_logo_file - Rack::Test::UploadedFile.new(image, "image/png") + black_logo_file end it "includes URLs of image versions" do @@ -42,14 +41,13 @@ describe Api::Admin::EnterpriseSerializer do context "when there is a promo image" do let(:image) do - image = black_logo_file - Rack::Test::UploadedFile.new(image, "image/png") + black_logo_file end it "includes URLs of image versions" do serializer = Api::Admin::EnterpriseSerializer.new(enterprise) expect(serializer.as_json[:promo_image]).to_not be_blank - expect(serializer.as_json[:promo_image][:medium]).to match(/logo-black.jpg/) + expect(serializer.as_json[:promo_image][:medium]).to match(/logo-black\.png$/) end end diff --git a/spec/services/image_importer_spec.rb b/spec/services/image_importer_spec.rb index 88ba329786..9ec390d25f 100644 --- a/spec/services/image_importer_spec.rb +++ b/spec/services/image_importer_spec.rb @@ -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 diff --git a/spec/services/terms_of_service_spec.rb b/spec/services/terms_of_service_spec.rb index 5d1e169529..514f9dfb50 100644 --- a/spec/services/terms_of_service_spec.rb +++ b/spec/services/terms_of_service_spec.rb @@ -34,12 +34,17 @@ describe TermsOfService do context "a customer has accepted the distributor terms of service" do before do allow(customer).to receive(:terms_and_conditions_accepted_at) { Time.zone.now - 1.week } - allow(distributor).to receive(:terms_and_conditions_updated_at) { Time.zone.now - 2.weeks } + allow(distributor).to receive(:terms_and_conditions_blob) { + ActiveStorage::Blob.new(created_at: Time.zone.now - 2.weeks) + } end it "should reflect whether the platform TOS have been accepted since the last update" do expect { allow(distributor).to receive(:terms_and_conditions_updated_at) { Time.zone.now } + allow(distributor).to receive(:terms_and_conditions_blob) { + ActiveStorage::Blob.new(created_at: Time.zone.now) + } }.to change { TermsOfService.tos_accepted?(customer, distributor) }.from(true).to(false) diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb index 37d1d6a4d1..7632c90c86 100644 --- a/spec/support/api_helper.rb +++ b/spec/support/api_helper.rb @@ -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 diff --git a/spec/support/file_helper.rb b/spec/support/file_helper.rb index 290467905e..8f4ead848b 100644 --- a/spec/support/file_helper.rb +++ b/spec/support/file_helper.rb @@ -2,11 +2,11 @@ module FileHelper def black_logo_file - File.open(black_logo_path) + Rack::Test::UploadedFile.new(black_logo_path) end def white_logo_file - File.open(black_logo_path) + Rack::Test::UploadedFile.new(white_logo_path) end def black_logo_path diff --git a/spec/system/admin/enterprises/images_spec.rb b/spec/system/admin/enterprises/images_spec.rb index 11d9e2e09b..5c9d483893 100644 --- a/spec/system/admin/enterprises/images_spec.rb +++ b/spec/system/admin/enterprises/images_spec.rb @@ -80,7 +80,7 @@ describe "Managing enterprise images" do go_to_images within ".page-admin-enterprises-form__promo-image-field-group" do - expect_preview_image "logo-white.jpg" + expect_preview_image "logo-white.png" end # Replacing image @@ -91,7 +91,7 @@ describe "Managing enterprise images" do go_to_images within ".page-admin-enterprises-form__promo-image-field-group" do - expect_preview_image "logo-black.jpg" + expect_preview_image "logo-black.png" end # Removing image diff --git a/spec/system/admin/enterprises/terms_and_conditions_spec.rb b/spec/system/admin/enterprises/terms_and_conditions_spec.rb index f5e11e3657..d15f286162 100644 --- a/spec/system/admin/enterprises/terms_and_conditions_spec.rb +++ b/spec/system/admin/enterprises/terms_and_conditions_spec.rb @@ -24,48 +24,36 @@ describe "Uploading Terms and Conditions PDF" do end end - let(:white_pdf_file_name) { Rails.root.join("app/webpacker/images/logo-white.pdf") } - let(:black_pdf_file_name) { Rails.root.join("app/webpacker/images/logo-black.pdf") } - - around do |example| - # Create fake PDFs from PNG images - FileUtils.cp(white_logo_path, white_pdf_file_name) - FileUtils.cp(black_logo_path, black_pdf_file_name) - - example.run - - # Delete fake PDFs - FileUtils.rm_f(white_pdf_file_name) - FileUtils.rm_f(black_pdf_file_name) - end + let(:original_terms) { Rails.root.join("public/Terms-of-service.pdf") } + let(:updated_terms) { Rails.root.join("public/Terms-of-ServiceUK.pdf") } it "uploading terms and conditions" do go_to_business_details # Add PDF - attach_file "enterprise[terms_and_conditions]", white_pdf_file_name + attach_file "enterprise[terms_and_conditions]", original_terms time = Time.zone.local(2002, 4, 13, 0, 0, 0) Timecop.freeze(run_time = time) do click_button "Update" - expect(distributor.reload.terms_and_conditions_updated_at).to eq run_time + expect(distributor.reload.terms_and_conditions_blob.created_at).to eq run_time end expect(page). to have_content "Enterprise \"#{distributor.name}\" has been successfully updated!" go_to_business_details - expect(page).to have_selector "a[href*='logo-white.pdf'][target=\"_blank\"]" + expect(page).to have_selector "a[href*='Terms-of-service.pdf'][target=\"_blank\"]" expect(page).to have_content time.strftime("%F %T") # Replace PDF - attach_file "enterprise[terms_and_conditions]", black_pdf_file_name + attach_file "enterprise[terms_and_conditions]", updated_terms click_button "Update" expect(page). to have_content "Enterprise \"#{distributor.name}\" has been successfully updated!" - expect(distributor.reload.terms_and_conditions_updated_at).to_not eq run_time + expect(distributor.reload.terms_and_conditions_blob.created_at).to_not eq run_time go_to_business_details - expect(page).to have_selector "a[href*='logo-black.pdf']" + expect(page).to have_selector "a[href*='Terms-of-ServiceUK.pdf']" end end end diff --git a/spec/system/consumer/shopping/checkout_spec.rb b/spec/system/consumer/shopping/checkout_spec.rb index 3ab016e3a4..22e8b4e623 100644 --- a/spec/system/consumer/shopping/checkout_spec.rb +++ b/spec/system/consumer/shopping/checkout_spec.rb @@ -88,6 +88,12 @@ describe "As a consumer I want to check out my cart", js: true do context 'login in as user' do let(:user) { create(:user) } + let(:pdf_upload) { + Rack::Test::UploadedFile.new( + Rails.root.join("public/Terms-of-service.pdf"), + "application/pdf" + ) + } before do login_as(user) @@ -158,21 +164,16 @@ describe "As a consumer I want to check out my cart", js: true do end context "when distributor has T&Cs" do - let(:fake_terms_and_conditions_path) { white_logo_path } - let(:terms_and_conditions_file) { - Rack::Test::UploadedFile.new(fake_terms_and_conditions_path, "application/pdf") - } - before do - order.distributor.terms_and_conditions = terms_and_conditions_file - order.distributor.save + order.distributor.terms_and_conditions = pdf_upload + order.distributor.save! end describe "when customer has not accepted T&Cs before" do it "shows a link to the T&Cs and disables checkout button until terms are accepted" do visit checkout_path - expect(page).to have_link("Terms and Conditions", - href: order.distributor.terms_and_conditions.url) + + expect(page).to have_link("Terms and Conditions") expect(page).to have_button("Place order now", disabled: true) @@ -193,7 +194,7 @@ describe "As a consumer I want to check out my cart", js: true do end describe "but afterwards the enterprise has uploaded a new T&Cs file" do - before { order.distributor.update terms_and_conditions_updated_at: Time.zone.now } + before { order.distributor.terms_and_conditions.attach(pdf_upload) } it "disables checkout button until terms are accepted" do visit checkout_path @@ -230,7 +231,7 @@ describe "As a consumer I want to check out my cart", js: true do context "when the terms have been accepted in the past" do before do TermsOfServiceFile.create!( - attachment: File.open(Rails.root.join("public/Terms-of-service.pdf")), + attachment: pdf_upload, updated_at: 1.day.ago, ) customer = create(:customer, enterprise: order.distributor, user: user) @@ -255,14 +256,10 @@ describe "As a consumer I want to check out my cart", js: true do end context "when the seller's terms and the platform's terms have to be accepted" do - let(:fake_terms_and_conditions_path) { white_logo_path } - let(:terms_and_conditions_file) { - Rack::Test::UploadedFile.new(fake_terms_and_conditions_path, "application/pdf") - } let(:tos_url) { "https://example.org/tos" } before do - order.distributor.terms_and_conditions = terms_and_conditions_file + order.distributor.terms_and_conditions = pdf_upload order.distributor.save! allow(Spree::Config).to receive(:shoppers_require_tos).and_return(true) @@ -273,8 +270,7 @@ describe "As a consumer I want to check out my cart", js: true do visit checkout_path within "#checkout_form" do - expect(page).to have_link("Terms and Conditions", - href: order.distributor.terms_and_conditions.url) + expect(page).to have_link("Terms and Conditions") expect(page).to have_link("Terms of service", href: tos_url) expect(page).to have_button("Place order now", disabled: true) end diff --git a/spec/system/consumer/shopping/shopping_spec.rb b/spec/system/consumer/shopping/shopping_spec.rb index 1fdc0b756c..b2ea899b9c 100644 --- a/spec/system/consumer/shopping/shopping_spec.rb +++ b/spec/system/consumer/shopping/shopping_spec.rb @@ -30,8 +30,7 @@ describe "As a consumer I want to shop with a distributor", js: true do it "shows a distributor with images" do # Given the distributor has a logo - distributor.logo = File.new(white_logo_path) - distributor.save! + distributor.update!(logo: white_logo_file) # Then we should see the distributor and its logo visit shop_path @@ -39,7 +38,7 @@ describe "As a consumer I want to shop with a distributor", js: true do within ".tab-buttons" do click_link "About" end - expect(first("distributor img")['src']).to include distributor.logo.url(:thumb) + expect(first("distributor img")['src']).to include "logo-white.png" end it "shows the producers for a distributor" do diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb index cbc5284519..f6a544a369 100644 --- a/spec/system/consumer/split_checkout_spec.rb +++ b/spec/system/consumer/split_checkout_spec.rb @@ -747,6 +747,15 @@ describe "As a consumer, I want to checkout my order", js: true do end describe "terms and conditions" do + let(:system_terms_path) { Rails.root.join("public/Terms-of-service.pdf") } + let(:shop_terms_path) { Rails.root.join("public/Terms-of-ServiceUK.pdf") } + let(:system_terms) { + Rack::Test::UploadedFile.new(system_terms_path, "application/pdf") + } + let(:shop_terms) { + Rack::Test::UploadedFile.new(shop_terms_path, "application/pdf") + } + context "when none are required" do it "doesn't show checkbox or links" do visit checkout_step_path(:summary) @@ -760,21 +769,14 @@ describe "As a consumer, I want to checkout my order", js: true do end context "when distributor has T&Cs" do - let(:fake_terms_and_conditions_path) { white_logo_path } - let(:terms_and_conditions_file) { - Rack::Test::UploadedFile.new(fake_terms_and_conditions_path, "application/pdf") - } - let(:terms_url) { order.distributor.terms_and_conditions.url } - before do - order.distributor.terms_and_conditions = terms_and_conditions_file - order.distributor.save + order.distributor.update!(terms_and_conditions: shop_terms) end describe "when customer has not accepted T&Cs before" do it "shows a link to the T&Cs and disables checkout button until terms are accepted" do visit checkout_step_path(:summary) - expect(page).to have_link "Terms and Conditions", href: terms_url + expect(page).to have_link "Terms and Conditions", href: /#{shop_terms_path.basename}$/ expect(page).to have_field "order_accept_terms", checked: false end end @@ -791,7 +793,7 @@ describe "As a consumer, I want to checkout my order", js: true do end describe "but afterwards the enterprise has uploaded a new T&Cs file" do - before { order.distributor.update terms_and_conditions_updated_at: Time.zone.now } + before { order.distributor.update!(terms_and_conditions: shop_terms) } it "disables checkout button until terms are accepted" do visit checkout_step_path(:summary) @@ -820,7 +822,7 @@ describe "As a consumer, I want to checkout my order", js: true do context "when the terms have been accepted in the past" do before do TermsOfServiceFile.create!( - attachment: File.open(Rails.root.join("public/Terms-of-service.pdf")), + attachment: system_terms, updated_at: 1.day.ago, ) customer = create(:customer, enterprise: order.distributor, user: user) @@ -837,16 +839,10 @@ describe "As a consumer, I want to checkout my order", js: true do end context "when the seller's terms and the platform's terms have to be accepted" do - let(:fake_terms_and_conditions_path) { white_logo_path } - let(:terms_and_conditions_file) { - Rack::Test::UploadedFile.new(fake_terms_and_conditions_path, "application/pdf") - } let(:tos_url) { "https://example.org/tos" } - let(:terms_url) { order.distributor.terms_and_conditions.url } before do - order.distributor.terms_and_conditions = terms_and_conditions_file - order.distributor.save! + order.distributor.update!(terms_and_conditions: shop_terms) allow(Spree::Config).to receive(:shoppers_require_tos).and_return(true) allow(Spree::Config).to receive(:footer_tos_url).and_return(tos_url) @@ -855,7 +851,7 @@ describe "As a consumer, I want to checkout my order", js: true do it "shows links to both terms and all need accepting" do visit checkout_step_path(:summary) - expect(page).to have_link "Terms and Conditions", href: terms_url + expect(page).to have_link "Terms and Conditions", href: /#{shop_terms_path.basename}$/ expect(page).to have_link "Terms of service", href: tos_url expect(page).to have_field "order_accept_terms", checked: false end