What do we need to smooth out file uploads?

[wout]

I'm now focussing on file uploads as this is something I'll need personally for three Lucky projects. I've seen some examples around from other Luckiers, which illustrate some shortcomings. Here is a reasonably complete example by @fernandes:

https://gist.github.com/fernandes/31c56b5aa1841643958031dc24c6a86b

But before starting on the real work, I want to paint a bigger picture of the requirements. To be honest, I've only briefly touched file uploads in Lucky, so I'm still a bit wet behind the ears. ☺️

A Shrine.cr adapter for Avram will be fantastic to have, and @igor-alexandrov is already working on that. I want to make sure Lucky and Avram have the tools to make the implementation as smooth as possible.

So I'd love to hear what you are all thinking about what's required and awesome to have. Obviously #1145, #1147 and #1192 will be attended to.

The first thing I noticed is that operations should be able to deal with uploads. So this:

upload = params.nested_file?(:image)["file"]

SaveImage.create(params, upload) do |operation, image|
  ...
end

Can become this:

SaveImage.create(params) do |operation, image|
  ...
end

I'm sure I'll run into more of those in the following days and I'll post my findings here.

[igor-alexandrov]

I'm sure, that

SaveImage.create(params) do |operation, image|
  ...
end

should be the only way to work with uploads. There is absolutely no reason to split params and multipart data (as it works now).

[jwoertink]

Thanks for taking this initiative, @wout. I haven't really looked much in to the file upload stuff. The main thing I noticed when I did try was that testing them was really difficult #1145

They should definitely be seamless with operations. You shouldn't have to test if you have a file to decide what method to call. Maybe link to this discussion in the chat? There's a few people that have run in to some issues around file uploads.

[fernandes]

@wout thank you for kickstarting it.. I could figure out how to make it work, I'm pretty sure there's room for improvement, I'm available to help whatever you guys need. what's the plan? :D

[wout]

Great, I will let you know for sure if I need a hand. Thanks! The idea is to polish away all rough edges and make file uploads as intuitive as possible. Even if someone wants to do vanilla uploads.

This weekend and next week I'll be working on this so @igor-alexandrov can continue with the Avram adapter for Shrine. I'll also contribute some more useful plugins to Shrine once that's all done.

[oneiros]

I dabbled with this a long time ago. I am not 100% sure my recollection is accurate, so feel free to ignore me if this is no longer valid:

I think it would be helpful if Avram recognized Lucky::UploadedFile as a valid "attribute".

[wout]

Thanks! And yes, that's the most expected behavior.

[paulcsmith]

I think since file uploads come from a different "part" of the params, we should instead have a different method for defining file attributes called file_attribute :avatar. This would use params.nested_file(:user)[:avatar] as the Avram::PermittedAttribute#value. Since this would now be a regular avram attribute it can be used in forms as expected, you can add errors to it like normal, and you can operate on the UploadedFile by calling avatar.value

This should allow people to do whatever they need with file uploads, and people could build macros on top of it if they want (but I'd probably use a regular method in a before_save or after_save since methods are a lot more flexible.

Does this make sense?

class User < BaseModel
  table do
    column name : String
    column avatar_path : String
  end
end

class SaveUser < User::SaveOperation
  permit_columns name
  file_attribute :avatar

  before_save do
    avatar.add_error("Must be a .png image") unless avatar.value.name.ends_with?(".png")
    avatar.value.try do |file|
      if avatar.valid?
        s3_path = upload_file_to_s3(file)
        avatar_path.value = s3_path
      end
    end
  end
end

This is a super simple example, but hopefully shows that it gives you the power to do whatever you want since you have full access to the errors on the attribute, and full access to the uploaded file to get metadata, name, whatever.

[wout]

Thanks Paul, that makes a lot of sense! Exactly what I needed as a guide.

[wout]

There is now a PR for this luckyframework/avram#404.