Skip to content

Commit

Permalink
closes spree#9171 - removing paperclip
Browse files Browse the repository at this point in the history
  • Loading branch information
amalrik committed Jan 30, 2019
1 parent 4cc2c29 commit 20fddb1
Show file tree
Hide file tree
Showing 19 changed files with 24 additions and 187 deletions.
9 changes: 0 additions & 9 deletions .circleci/config.yml
Expand Up @@ -69,12 +69,6 @@ jobs:
environment:
POSTGRES_USER: postgres

run_tests_postgres_paperclip:
<<: *run_tests_postgres
environment:
<<: *postgres_environment
SPREE_USE_PAPERCLIP: true

run_tests_mysql:
<<: *run_tests
environment:
Expand Down Expand Up @@ -157,9 +151,6 @@ workflows:
- run_tests_mysql:
requires:
- bundle_install
- run_tests_postgres_paperclip:
requires:
- run_tests_postgres
- send_test_coverage:
requires:
- run_tests_mysql
2 changes: 1 addition & 1 deletion .ruby-version
@@ -1 +1 @@
2.5.1
2.5.3
3 changes: 0 additions & 3 deletions backend/spec/features/admin/products/edit/images_spec.rb
Expand Up @@ -11,13 +11,10 @@
# undefined method `processors' for \"48x48>\
Spree::Image.styles.symbolize_keys!
allow(ENV).to receive(:[]).and_call_original
allow(ENV).to receive(:[]).with(:SPREE_USE_PAPERCLIP).and_return(true)
end

context 'uploading, editing, and deleting an image' do
it 'allows an admin to upload and edit an image for a product' do
Spree::Image.attachment_definitions[:attachment].delete :storage if Rails.application.config.use_paperclip

create(:product)

visit spree.admin_products_path
Expand Down
13 changes: 1 addition & 12 deletions core/app/models/spree/asset.rb
@@ -1,17 +1,6 @@
module Spree
if Rails.application.config.use_paperclip
ActiveSupport::Deprecation.warn(<<-EOS, caller)
Paperclip support is deprecated, and will be removed in Spree 4.0.
Please migrate to ActiveStorage, to avoid problems after update
https://github.com/thoughtbot/paperclip/blob/master/MIGRATING.md
EOS
Paperclip.interpolates :viewable_id do |attachment, _style|
attachment.instance.viewable_id
end
end

class Asset < Spree::Base
include Support::ActiveStorage unless Rails.application.config.use_paperclip
include Support::ActiveStorage

belongs_to :viewable, polymorphic: true, touch: true
acts_as_list scope: [:viewable_id, :viewable_type]
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/image.rb
@@ -1,6 +1,6 @@
module Spree
class Image < Asset
include Rails.application.config.use_paperclip ? Configuration::Paperclip : Configuration::ActiveStorage
include Configuration::ActiveStorage
include Rails.application.routes.url_helpers

def styles
Expand Down
64 changes: 0 additions & 64 deletions core/app/models/spree/image/configuration/paperclip.rb

This file was deleted.

2 changes: 1 addition & 1 deletion core/app/models/spree/taxon_image.rb
@@ -1,6 +1,6 @@
module Spree
class TaxonImage < Asset
include Rails.application.config.use_paperclip ? Configuration::Paperclip : Configuration::ActiveStorage
include Configuration::ActiveStorage
include Rails.application.routes.url_helpers

def styles
Expand Down
27 changes: 0 additions & 27 deletions core/app/models/spree/taxon_image/configuration/paperclip.rb

This file was deleted.

3 changes: 0 additions & 3 deletions core/config/initializers/use_paperclip.rb

This file was deleted.

1 change: 0 additions & 1 deletion core/lib/spree/core.rb
Expand Up @@ -7,7 +7,6 @@
require 'kaminari'
require 'mail'
require 'monetize'
require 'paperclip'
require 'paranoia'
require 'mini_magick'
require 'premailer/rails'
Expand Down
6 changes: 1 addition & 5 deletions core/lib/spree/core/product_duplicator.rb
Expand Up @@ -58,11 +58,7 @@ def duplicate_variant(variant)

def duplicate_image(image)
new_image = image.dup
if Rails.application.config.use_paperclip
new_image.assign_attributes(attachment: image.attachment.clone)
else
new_image.attachment.attach(image.attachment.blob)
end
new_image.attachment.attach(image.attachment.blob)
new_image.save!
new_image
end
Expand Down
8 changes: 2 additions & 6 deletions core/lib/spree/testing_support/factories/image_factory.rb
@@ -1,11 +1,7 @@
FactoryBot.define do
factory :image, class: Spree::Image do
if Rails.application.config.use_paperclip
attachment { File.new(Spree::Core::Engine.root + 'spec/fixtures' + 'thinking-cat.jpg') }
else
before(:create) do |image|
image.attachment.attach(io: File.new(Spree::Core::Engine.root + 'spec/fixtures' + 'thinking-cat.jpg'), filename: 'thinking-cat.jpg')
end
before(:create) do |image|
image.attachment.attach(io: File.new(Spree::Core::Engine.root + 'spec/fixtures' + 'thinking-cat.jpg'), filename: 'thinking-cat.jpg')
end
end
end
16 changes: 5 additions & 11 deletions core/lib/spree/testing_support/image_helpers.rb
Expand Up @@ -2,17 +2,11 @@ module Spree
module TestingSupport
module ImageHelpers
def create_image(attachable, file)
# user paperclip to attach an image
if Rails.application.config.use_paperclip
attachable.images.create!(attachment: file)
# use ActiveStorage (default)
else
image = attachable.images.new
image.attachment.attach(io: file, filename: File.basename(file))
image.save!
file.rewind
image
end
image = attachable.images.new
image.attachment.attach(io: file, filename: File.basename(file))
image.save!
file.rewind
image
end
end
end
Expand Down
26 changes: 5 additions & 21 deletions core/spec/models/spree/image_spec.rb
Expand Up @@ -7,38 +7,22 @@
let(:text_file) { File.open(Spree::Core::Engine.root + 'spec/fixtures' + 'text-file.txt') }

it 'has attachment present' do
if Rails.application.config.use_paperclip
spree_image.attachment = image_file
else
spree_image.attachment.attach(io: image_file, filename: 'thinking-cat.jpg')
end
spree_image.attachment.attach(io: image_file, filename: 'thinking-cat.jpg')
expect(spree_image).to be_valid
end

it 'has attachment absent' do
if Rails.application.config.use_paperclip
spree_image.attachment = nil
else
spree_image.attachment.attach(nil)
end
spree_image.attachment.attach(nil)
expect(spree_image).not_to be_valid
end

it 'has allowed attachment content type' do
if Rails.application.config.use_paperclip
spree_image.attachment = image_file
else
spree_image.attachment.attach(io: image_file, filename: 'thinking-cat.jpg', content_type: 'image/jpeg')
end
it 'has allowed attachment content type' do
spree_image.attachment.attach(io: image_file, filename: 'thinking-cat.jpg', content_type: 'image/jpeg')
expect(spree_image).to be_valid
end

it 'has no allowed attachment content type' do
if Rails.application.config.use_paperclip
spree_image.attachment = text_file
else
spree_image.attachment.attach(io: text_file, filename: 'text-file.txt', content_type: 'text/plain')
end
spree_image.attachment.attach(io: text_file, filename: 'text-file.txt', content_type: 'text/plain')
expect(spree_image).not_to be_valid
end
end
Expand Down
4 changes: 1 addition & 3 deletions core/spec/models/spree/product_duplicator_spec.rb
Expand Up @@ -18,9 +18,7 @@ module Spree

before do
new_image = Spree::Image.new(params)
unless Rails.application.config.use_paperclip
new_image.attachment.attach(io: file, filename: File.basename(file))
end
new_image.attachment.attach(io: file, filename: File.basename(file))
new_image.save!
end

Expand Down
11 changes: 4 additions & 7 deletions core/spec/models/spree/product_spec.rb
Expand Up @@ -487,13 +487,10 @@ def build_option_type_with_values(name, values)
Spree::Image.create(params.merge(alt: 'position 1', position: 1)),
Spree::Image.create(params.merge(viewable_type: 'ThirdParty::Extension', alt: 'position 1', position: 2))
]
# fix for ActiveStorage
unless Rails.application.config.use_paperclip
images.each_with_index do |image, index|
image.attachment.attach(io: file, filename: "thinking-cat-#{index + 1}.jpg", content_type: 'image/jpeg')
image.save!
file.rewind # we need to do this to avoid `ActiveStorage::IntegrityError`
end
images.each_with_index do |image, index|
image.attachment.attach(io: file, filename: "thinking-cat-#{index + 1}.jpg", content_type: 'image/jpeg')
image.save!
file.rewind # we need to do this to avoid `ActiveStorage::IntegrityError`
end
end

Expand Down
12 changes: 2 additions & 10 deletions core/spec/models/spree/taxon_image_spec.rb
Expand Up @@ -7,20 +7,12 @@
let(:text_file) { File.open(Spree::Core::Engine.root + 'spec/fixtures' + 'text-file.txt') }

it 'has allowed attachment content type' do
if Rails.application.config.use_paperclip
spree_image.attachment = image_file
else
spree_image.attachment.attach(io: image_file, filename: 'thinking-cat.jpg', content_type: 'image/jpeg')
end
spree_image.attachment.attach(io: image_file, filename: 'thinking-cat.jpg', content_type: 'image/jpeg')
expect(spree_image).to be_valid
end

it 'has no allowed attachment content type' do
if Rails.application.config.use_paperclip
spree_image.attachment = text_file
else
spree_image.attachment.attach(io: text_file, filename: 'text-file.txt', content_type: 'text/plain')
end
spree_image.attachment.attach(io: text_file, filename: 'text-file.txt', content_type: 'text/plain')
expect(spree_image).not_to be_valid
end
end
Expand Down
1 change: 0 additions & 1 deletion core/spree_core.gemspec
Expand Up @@ -32,7 +32,6 @@ Gem::Specification.new do |s|
s.add_dependency 'kaminari', '~> 1.0.1'
s.add_dependency 'money', '~> 6.13'
s.add_dependency 'monetize', '~> 1.9'
s.add_dependency 'paperclip', '~> 6.1.0'
s.add_dependency 'paranoia', '~> 2.4.1'
s.add_dependency 'premailer-rails'
s.add_dependency 'acts-as-taggable-on', '~> 6.0'
Expand Down
1 change: 0 additions & 1 deletion frontend/spec/features/products_spec.rb
Expand Up @@ -10,7 +10,6 @@
before do
visit spree.root_path
allow(ENV).to receive(:[]).and_call_original
allow(ENV).to receive(:[]).with(:SPREE_USE_PAPERCLIP).and_return(true)
end

it 'is able to show the shopping cart after adding a product to it', js: true do
Expand Down

0 comments on commit 20fddb1

Please sign in to comment.