Correct method name for shop logo in email

Creating a seperate release off the master branch.

.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'
 

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'

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

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,

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
 

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

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

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

app/models/concerns/has_migrating_file.rb

@@ -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

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"

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

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) }

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

app/models/spree/preferences/file_configuration.rb

@@ -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

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

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

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

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

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

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

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

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

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

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

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,
     )

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

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

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/*"

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

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)

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'))

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

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
 

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

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)}"

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]

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'

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

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 }"}

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?

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
 

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
 

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

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

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?

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)

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  

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  

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)))

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

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

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_url(:medium), class: "float-right"
       %span.clear
 
 = render 'summary_overview', summary: @summary

config/initializers/paperclip.rb

@@ -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)

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?

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"

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

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"

lib/open_food_network/paperclippable.rb

@@ -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

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'

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
 

lib/spree/core/s3_support.rb

@@ -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

lib/tasks/from_paperclip_to_active_storage.rake

@@ -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

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

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

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

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

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

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

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
         )
 

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)

spec/lib/tasks/from_paperclip_to_active_storage_rake_spec.rb

@@ -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

spec/mailers/subscription_mailer_spec.rb

@@ -240,15 +240,22 @@ describe SubscriptionMailer, type: :mailer do
         allow(summary).to receive(:order_count) { 37 }
         allow(summary).to receive(:issue_count) { 0 }
         allow(summary).to receive(:issues) { {} }
-        SubscriptionMailer.placement_summary_email(summary).deliver_now
       end
 
       it "sends the email, which notifies the enterprise that all orders were successfully processed" do
+        SubscriptionMailer.placement_summary_email(summary).deliver_now
+
         expect(body).to include I18n.t("#{scope}.placement_summary_email.intro", shop: shop.name)
         expect(body).to include I18n.t("#{scope}.summary_overview.total", count: 37)
         expect(body).to include I18n.t("#{scope}.summary_overview.success_all")
         expect(body).to_not include I18n.t("#{scope}.summary_overview.issues")
       end
+
+      it "renders the shop's logo" do
+        shop.update!(logo: fixture_file_upload("logo.png", "image/png"))
+        SubscriptionMailer.placement_summary_email(summary).deliver_now
+        expect(SubscriptionMailer.deliveries.last.body).to include "logo.png"
+      end
     end
 
     context "when some issues were encountered while processing subscriptions" do

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)

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

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

spec/models/spree/preferences/file_configuration_spec.rb

@@ -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

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)

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
 

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

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)

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

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

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

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

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

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

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