spree-contrib / spree_editor

Rich text editor for Spree with Image and File uploading in-place.
http://guides.spreecommerce.org
BSD 3-Clause "New" or "Revised" License
112 stars 183 forks source link

Causes the wrong attachment path to exist in production environment #4

Closed gommo closed 12 years ago

gommo commented 13 years ago

See the thread here - https://groups.google.com/d/topic/spree-user/Fn9znqrWXG8/discussion

This breaks images on production instances if this extension is installed

mikz commented 12 years ago

I can confirm this. Took me a while.

Its strange because paperclip uses class_inhertitable_attribute - https://github.com/thoughtbot/paperclip/blob/9c83611ba757e56f143e34d90a36217ac036d4cf/lib/paperclip.rb#L280

fabien commented 12 years ago

I can confirm this as well. I think it might have something to do on how classes are (re)loaded. I've noticed that (even with reload_classes = false) *_decorator.rb files were loaded twice (const already defined popped up for example, for reguler CONST defines). I also have an image_decorator.rb for custom image sizes, maybe this is tripping things up in loading the code. Do any of you guys have such a decorator in place as well?

mikz commented 12 years ago

I don't have any custom decorator. But there are plenty in gems I use (spree_social, spree_blue_theme, ...).

mikz commented 12 years ago

I have put some messages to Image and ContentImage and they are loaded just once.

but it can be easily replicated like this in production console:

class Tester < Asset
  has_attached_file :attachment,
                             :styles => { :mini => '48x48>', :small => '100x100>', :product => '240x240>', :large => '600x600>' }
end

class ContentTester < Tester
  has_attached_file :attachment,
                             :styles => { :mini => '48x48>'}
end

Tester.attachment_definitions # => {:attachment=>{:validations=>[], :styles=>{:mini=>"48x48>"}}}

I'll try to look deeper into paperclip.

mikz commented 12 years ago

I see. Its fixed in paperclip master - https://github.com/thoughtbot/paperclip/blob/master/lib/paperclip.rb#L320

Unfortunataly spree_core has fixed paperclip dependency.

You can use core from https://github.com/mikz/spree/tree/0-70-stable and then latest paperclip master. This fixed the issue. Also this issue is not related to spree_editor but to paperclip (inheritance problem) and spree itself (version dependency).

peterberkenbosch commented 12 years ago

Thanks guys for getting the issue.. closing this one now.