Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with form for uploading file in the AA #3476

Closed
betasve opened this issue Oct 7, 2014 · 13 comments
Closed

Problem with form for uploading file in the AA #3476

betasve opened this issue Oct 7, 2014 · 13 comments

Comments

@betasve
Copy link
Contributor

betasve commented Oct 7, 2014

Hello,

Could you please, assist me with information about an issue I have. I've decided to implement my own file upload functionality. It worked fine when (for test) implemented on the front-end. When I decided to take it back-end in AA, a strange error started popping. I have suspicion it might be of some expectation on the AA side that
f.input :location, :as => :file is expecting field that contains file in the DB and not a string field that is containing path to the file on the storage place.

More detailed explanation and example of the problem could be found here>
http://stackoverflow.com/questions/26215068/cant-cast-actiondispatchhttpuploadedfile-to-string-in-activeadmin?noredirect=1#comment41113413_26215068

Thank you in advance for your help.

Best Regards

@timoschilling
Copy link
Member

You need to write a wrapper or alias method like this:

def uploaded_file= file
  self.location = file.read
end

or you can do this (not recommended):

def location= file
  super file.read
end

Thats because ActionDispatch don't give you the file content, it gives you a ActionDispatch::Http::UploadedFile which behaves like a File instance.

@betasve
Copy link
Contributor Author

betasve commented Oct 7, 2014

Thanks for your immediate input, @timoschilling. Appreciate it.

Just a couple of follow up questions:

  1. Did you see the upload method in my model? I am trying to get the already uploaded file's location into the self.location, not the file itself.
  2. As far as I understood your suggestoin, you mean to replace the @uploaded_file with a method that's holding the file reference and the rest of the info?

@timoschilling
Copy link
Member

I know what do you want to do with your #upload method, but this is not that a good way. Sure it works in a Handwritten controller, but let me explain that for you. Try to use skinny controllers and fat models!

You should have a controller method like this:

def create
  @document = Document.new params # example without permitted params
  # redirect or render
end

This is that what the AA #create does!

And a model like this:

model Document < ActiveRecord::Base
  STORE_DIR = "documents"
  STORE_PATH = File.join("public", STORE_DIR)
  Dir.mkdir(STORE_PATH) unless Dir.exists?(STORE_PATH) # this should not be done here, move it into the server setup

  def file= file
    self.doc_type = file.original_filename.split('.').last
    self.location = File.join(STORE_DIR, file.original_filename)
    File.open(File.join(STORE_PATH, file.original_filename), "wb") { |f| f.write(file.read) }
  end
end

And this is what you form looks like:

form :html => { :enctype => "multipart/form-data" } do |f|
    f.inputs  :multipart => true do 
      f.input :name
      f.input :file, :as => :file
      f.input :static_page_id, :label => 'Parent Static Page', :as => :select, :collection => ->{StaticPage.pluck(:title, :id) }
    end
    f.actions
  end

@betasve
Copy link
Contributor Author

betasve commented Oct 7, 2014

Thank you for your detailed solution. You are very right about the slim controller and fat model. I will try to make those that way from now on.

I've implemented your suggestion for file but there is a strange error I can't seem to understand. Tried a few different solutions and it's still there. It says:

"undefined method `original_filename' for "documents/docname.pdf":String"

for
self.doc_type = file.original_filename.split('.').last

I've read that this may be because of missing ":multipart => true" in the form but mine is there.

@timoschilling
Copy link
Member

I think the :multipart => true needs to be on form not on f.inputs. Try that, if it don't work, please post all of your related code again.

@betasve
Copy link
Contributor Author

betasve commented Oct 7, 2014

It was on the inputs but I had
:html => { :enctype => "multipart/form-data" }
on the form. And it's still not working. So I am posting the related code here

Controller:

def create
 @document = Document.new params
 redirect_to admin_documents_path
end

Model:

    STORE_DIR = 'documents'
STORE_PATH = File.join("public", STORE_DIR)
Dir.mkdir(STORE_PATH) unless Dir.exists?(STORE_PATH)

belongs_to :static_page

validates :name, presence: true, uniqueness: true
validates :location, presence: true, uniqueness: true
validates :doc_type, presence: true
validates :static_page, presence: true

def location= file
    self.doc_type = file.original_filename.split('.').last
    File.open(File.join(STORE_PATH, file.original_filename), "wb") { |f| f.write(file.read) }
    self.location = File.join(STORE_DIR, file.original_filename)
end

**Active Admin form:

form :multipart => true do |f| #:html => { :enctype => "multipart/form-data" } 
    f.inputs do #:multipart => true
      f.input :name
      f.input :location, :as => :file
      f.input :static_page_id, :label => 'Parent Static Page', :as => :select, :collection => StaticPage.all.map{|sp| ["#{sp.title}", sp.id]}
   end
   f.actions
 end

@timoschilling
Copy link
Member

You can't (re)use the location location method, for your 'upload' method.
The reason is, that #update_attributes cast the ActionDispatch::Http::UploadedFile to a String (the type of location), so you get "documents/docname.pdf" instead of a UploadedFile instance.
You need to use a different name!
Use my version:

  def file= file
    self.doc_type = file.original_filename.split('.').last
    self.location = File.join(STORE_DIR, file.original_filename)
    File.open(File.join(STORE_PATH, file.original_filename), "wb") { |f| f.write(file.read) }
  end

@betasve
Copy link
Contributor Author

betasve commented Oct 7, 2014

Ok, that's totally my mistake. I though :file in the form of your example is a field that needs to persist in the database and replaced it with :location not thinking about the fact I cant use self.location = in the same function as my intention was.

Now (after moving :multipart => true to the form), it already works.

Thanks a whole lot for your valuable help. I wouldn't of been able to solve this on my own even spending a week more trying to figure it out.

Best Regards,
Sly

@timoschilling
Copy link
Member

Ok, you are right, I don't have see the real problem ;)

I'm glad to have helped you!

@betasve
Copy link
Contributor Author

betasve commented Oct 7, 2014

Hey, sorry for continuously bothering you, but I was trying to implement your advice on moving the line for creating uploads/documents.
Tried to google for the good practices about this, but can't find something on this matter. Could you, please advise me on where / how shoud I structure this:

Dir.mkdir(STORE_PATH) unless Dir.exists?(STORE_PATH)

Thank you in advance for your precious input.

@timoschilling
Copy link
Member

You can place it in a capistrano setup, or if you don't have a server setup tool, than place it in a initializer

@betasve
Copy link
Contributor Author

betasve commented Oct 7, 2014

Cool, thanks again. I guess it will be in initializer then.

@etonline
Copy link

Just want to share my finding that :multipart => true in the form for active admin was not needed before commit 6afaf58.

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

No branches or pull requests

3 participants