janko / paperclip-dropbox

[OBSOLETE] Extends Paperclip with Dropbox storage.
MIT License
148 stars 36 forks source link

Renaming dropbox url's #36

Open izelnakri opened 10 years ago

izelnakri commented 10 years ago

It's very difficult at the moment to change the Model.attachment.url's(dropbox url's).

Example:

class Item < ActiveRecord::Base
    has_attached_file :picture_1, { dropbox_options: { path: proc { "#{self.id}_1" }  }, default_url: "default.png" }
    has_attached_file :picture_2, { dropbox_options: { path: proc { "#{self.id}_2" }  }}
    has_attached_file :picture_3, { dropbox_options: { path: proc { "#{self.id}_3" }  }}

    validates_attachment_content_type :picture_1, content_type: /\Aimage\/.*\Z/
    validates_attachment_content_type :picture_2, content_type: /\Aimage\/.*\Z/
    validates_attachment_content_type :picture_3, content_type: /\Aimage\/.*\Z/

end

Also I have to manually add a callback when the models referred to self.id changes.(eg. if we change the id of an item that has an attachment, dropbox url wont change automatically so samemodel.picture_1.url would cause an error).

I think we need a better API + handling for dropbox uploads.

janko commented 10 years ago

This proc is evaluated every time an URL is requested, it should change when the Item#id changes. Can you show me an isolated example where it doesn't change?

Also, you might consider using Paperclip's standard :path option (I've added support for it some time ago), not this one with proc which I invented (I have no idea why I implemented that :)).

izelnakri commented 10 years ago

When I type these into rails console with the code above:

item = Item.first
item.picture_1 = File.open("#{Rails.root}/public/abc.png")
item.save
item.id = 909 #different id 
item.save
item.picture_1.url #returns path not found
    config.paperclip_defaults = {
        storage: :dropbox, 
        dropbox_credentials: Rails.root.join("config/dropbox.yml"),
        dropbox_visibility: 'public',
        path: ":id/:attachment" }

then default path doesnt work.

janko commented 10 years ago

I agree with everything you said.

[1, 2, 3].each do |n|
  has_attached_file :"picture_#{n}", path: ":id_#{n}.:extension"
end

The reason why these things are still here is that I don't use Dropbox uploads, nor Paperclip. CarrierWave also has a carrierwave-dropbox extension gem, so that could be an alternative. I find CarrierWave much more polished and it has a much better interface (uploading logic is held in uploaders, much more object-oriented and respecting SRP).

izelnakri commented 10 years ago

Thanks for the quick response I'll give CarrierWave a look. I'd really appreciate if we can have a fix for these issues as well.

janko commented 10 years ago

Since I don't use Paperclip anymore, I don't have the time/will to fix these things. I have tried to keep the code pretty and maintainable (although tests could be better), so I will happily accept pull requests.