stimulusreflex / stimulus_reflex

Build reactive applications with the Rails tooling you already know and love.
https://docs.stimulusreflex.com
MIT License
2.28k stars 172 forks source link

Reflex on form submit does not get parameter from input[type="file"] #277

Closed paulbernhard closed 4 years ago

paulbernhard commented 4 years ago

Bug Report

First of all, thanks for building this, Reflex looks very (very) promising to me.

Describe the bug

I am building a form based on the Working with HTML Forms from the StimulusReflex docs. This form should accept a file input on the parameter file_data for further file uploading. The form triggers a reflex called submit in UploadReflex which should take care of building or updating, and saving the record.

When the reflex receives the params from the form, the file_data parameter which was set by a file_field is always empty. When the file_data parameter is provided by a text_field instead of file_field, the file_data in the reflex' params is not empty. Equally, when the form is submitted to the corresponding controller without using a reflex, the file_data parameter in params corresponds to the file_field input.

Thus it seems, the reflex is ignoring parameters provided by a file input.

To Reproduce

# app/reflexes/upload_reflex.rb
class UploadReflex < ApplicationReflex
  before_reflex :set_upload

  def submit
    byebug
    @upload.assign_attributes(upload_params)
    @upload.save
  end

  private

    def set_upload
      if element.dataset.signed_id.nil?
        @upload = Upload.new
      else
        @upload = GlobalID::Locator.locate_signed(element.dataset.signed_id)
      end
    end

    def upload_params
      params.require(:upload).permit(:caption, :file_data)
    end
end
# app/controllers/uploads_controller.rb
class UploadsController < ApplicationController
  before_action :set_upload, only: [:show, :edit, :update, :destroy]

  def new
    @upload ||= Upload.new
  end

  def edit
  end

  def create
    byebug
    @upload = Upload.new(upload_params)

    respond_to do |format|
      if @upload.save
        redirect_to @upload
      else
        render :new
      end
  end

  def update
      if @upload.update(upload_params)
        redirect_to @upload
      else
        render :edit
      end
  end

  private
    def set_upload
      @upload ||= Upload.find(params[:id])
    end

    def upload_params
      params.require(:upload).permit(:caption, :file_data)
    end
end
# app/views/uploads/_form.html.erb
<%= form_with(model: upload, local: true, multipart: true, data: { reflex: "submit->UploadReflex#submit", signed_id: upload.persisted? ? upload.to_sgid.to_s : nil }) do |form| %>

  <% if upload.errors.any? %>
    <%= pluralize(upload.errors.count, "error") %> prohibited this upload from being saved:

    <ul>
      <% upload.errors.full_messages.each do |message| %>
        <li><%= message %></li>
      <% end %>
    </ul>
  <% end %>

  <%= form.label :file_data, "file_data" %>
  <%= form.file_field :file_data %>

  <%= form.label :caption, "Caption" %>
  <%= form.text_field :caption %>

  <%= form.submit %>
<% end %>

When the form is submitted using submit->UploadReflex#submit the method upload_params in UploadReflex#submit returns:

<ActionController::Parameters {"caption"=>"This is a caption…"} permitted: true>

When the same form is submitted without the reflex, the method upload_params in UploadsController#create returns:

<ActionController::Parameters {"caption"=>"This is a caption…", "file_data"=>#<ActionDispatch::Http::UploadedFile:0x00007f9e484b3348 @tempfile=#<Tempfile:/var/folders/zj/x1_t9wss5mlcsj62qcj35js40000gn/T/RackMultipart20200720-13302-ldvz0s.jpg>, @original_filename="IMG-20191231-WA0015.jpg", @content_type="image/jpeg", @headers="Content-Disposition: form-data; name=\"upload[file_data]\"; filename=\"IMG-20191231-WA0015.jpg\"\r\nContent-Type: image/jpeg\r\n">} permitted: true>

Expected behavior

The upload_params in UploadReflex#submit should read the file_data parameter provided by a input[type="file"].

Versions

StimulusReflex

External tools

Browser

hopsoft commented 4 years ago

We'll need to dive deeper into possibly adding support for file inputs.

For the time being, I think the best course of action is to consider file inputs as unsupported and mark them as data-reflex-permanent so you don't lose any state when reflexes are triggered.

paulbernhard commented 4 years ago

Hi and thanks for the response, i am looking forward to hear and see more of StimulusReflex.

It might not be a solution to the actual "problem" of Stimulus Reflex not supporting file inputs, but for the people using the Shrine gem to handle file uploads, there might be a "frontend workaround":

Shrine provides a Upload Endpoint plugin in order to use JS upload libraries (e.g. Uppy). This endpoint accepts a post request with the uploaded file and returns a JSON response.

Using a JS uploader and the upload endpoint we can avoid using a input[type="file"], instead populate a input[type="hidden"] with the necessary JSON data and pass it to a reflex.

An example implementation of Uppy with Shrine and the Upload endpoint can be found here: Better File Uploads with Shrine: Direct Uploads.

leastbad commented 4 years ago

Hey Paul! (Are you on Discord? If not, please consider joining us!)

This sounds like a potentially viable workaround, at least for Shrine users. If you are able to try this out, verify it and help extract the important bits, I'd love to work with you to add an example of this into the documentation because you're probably not the last person to ask about uploads.

kieranklaassen commented 4 years ago

Yeah running into the same issue here! I'll try shrine or some other things. Following thread

paulbernhard commented 4 years ago

Hey @leastbad, i am sorry for the delayed answer. I already made a first tryout which worked and i could work out an example. Right now i am too occupied to do this but i will have to work further with async file uploads in an upcoming project and i am planning to hook them to Stimulus Reflex. For anybody in need right now, i think the approach described above (https://github.com/hopsoft/stimulus_reflex/issues/277#issuecomment-661726137) is a good start.

As soon as i'd have something to share, i'll post it here as i am not on Discord.

paulbernhard commented 4 years ago

Hi ( @leastbad ), following the question to circumvent the problem of using input type="file" fields in order to submit files through forms using StimulusReflex, i just wrote a small dummy app which makes use of StimulusReflex in combination with Shrine which can be found here: https://gitlab.com/swrs/stimulus-reflex-shrine.

The application is obviously rough and uses Shrine's Upload endpoint together with the frontend Uppy uploader, submitting a form upon file upload. I included some file validations for mime-types (image/jpeg image/jpg image/png image/gif application/pdf) and file size (< 3MB) in order to test error behaviour. As the form is submitted after every file upload, it is possible to do batch uploads of multiple files.

The docs mention: if you're thinking of replacing UJS remote forms with StimulusReflex form handling without a specific reason for doing so... just stick with Rails! which seems to be quite true, but for file uploads it could make sense to pass a form through a reflex in order to facilitate validations, a file thumb partial, … .

Some short notes on how it works:

uploader_controller.js initializes the Uppy dashboard by clicking data-target="uploader.button". – Upon selecting a file, Uppy uploads files to Shrine's upload endpoint which responds with the data to populate the record's file attribute. – Upon a successful upload, the response is stored in an input type="hidden" and the form is submitted via the reflex Upload#submit.