Skip to content
This repository has been archived by the owner on May 4, 2019. It is now read-only.

Renaming dropbox url's #36

Open
izelnakri opened this issue Mar 19, 2014 · 5 comments
Open

Renaming dropbox url's #36

izelnakri opened this issue Mar 19, 2014 · 5 comments

Comments

@izelnakri
Copy link

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
Copy link
Owner

janko commented Mar 19, 2014

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
Copy link
Author

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
  • Proc is not evaluated every time the URL is requested, at least not in my case (paperclip-dropbox v1.2.1).
  • Thanks for the path options suggestion, it works when individually applied to the has_attachments however when I want to create a default path in config/application.rb like these:
    config.paperclip_defaults = {
        storage: :dropbox, 
        dropbox_credentials: Rails.root.join("config/dropbox.yml"),
        dropbox_visibility: 'public',
        path: ":id/:attachment" }

then default path doesnt work.

  • When I use a customized path with :path without a :filename, the format(in this case .png) gets ommitted in the url.
  • Default dropbox path behavior is simply ":filename". However this is very bad. Imagine different users upload different images with the same name(e.g "image.jpeg"). Then the second uploaded image would show up as image(1).jpeg in the dropbox. Even worse is that its url is "http://(dropbox_app_folder)/image.jpeg", which probably means the first upload is attached to the second upload.
  • Lastly is there any way to write a one-line default path for the models to clean up the code duplication. So that my paths would be id_(picture_id), (e.g picture_2 of the item.id 5 = "http://(dropbox_app_folder)/5_2.png")

@janko
Copy link
Owner

janko commented Mar 19, 2014

I agree with everything you said.

  • The defaults are not working because I had to maintain backwards compatibility with these proc versions of paths. I didn't want to, but I didn't want to also break existing code.
  • Wow, I did not know that the extension disappears in that case.
  • :filename indeed is a bad default, I'm sorry for that.
  • I think something like this could be ok:
[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
Copy link
Author

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
Copy link
Owner

janko commented Mar 20, 2014

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants