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

File fields conflict with CSV #694

Closed
ghaydarov opened this issue Feb 6, 2017 · 23 comments
Closed

File fields conflict with CSV #694

ghaydarov opened this issue Feb 6, 2017 · 23 comments
Milestone

Comments

@ghaydarov
Copy link

ghaydarov commented Feb 6, 2017

I use this gem in just one form in my app and I only load its js file in that forms layout and initializer file content on that forms model.

My problem is, I have different layout with some other forms that I do not want to use client side validation, and these forms use carrierwave. Now this gem automatically loaded everywhere even if I don't want to use this everywhere. With the forms that I don't use this gem, I get error with carrierwave, it just fails to save the form and get translation missing errors when i check the source code. Otherwise I just get cant be blank error.

I hope I explained my situation understandably.

Shortly, I want to use this gem with only one form, but it still shows up in all forms regardless of the layout.

Any ideas? Thanks!

@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

Hi,

From the README

In your FormBuilder you only need to enable validations:

<%= form_for @user, validate: true do |f| %>
  ...

That should be enough to get you going.

So you should simply skip validate: true if you don't want CSV

Hope it helps

Closing here

@tagliala tagliala closed this as completed Feb 6, 2017
@ghaydarov
Copy link
Author

Thanks for the reply. That should be expected behavior, I know, but it doesn't just disable it for some reason.

@ghaydarov
Copy link
Author

ghaydarov commented Feb 6, 2017

I have this in initializer, could this be loading everywhere, regardless of validate true?

ActionView::Base.field_error_proc = proc do |html_tag, instance|
  if html_tag =~ /^<label/
    %(<div class="field_with_errors">#{html_tag}<label for="#{instance.send(:tag_id)}" class="message">#{instance.error_message.first}</label></div>).html_safe
  else
  	 %(<div class="field_with_errors">#{html_tag}</div>).html_safe
  end
end

@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

The above initalizer is supposed to render errors on the client side when the server side validations fail and it should not affect forms without errors

Also, the suggested field_error_proc should be:

ActionView::Base.field_error_proc = proc do |html_tag, instance|
  if html_tag =~ /^<label/
    %(<div class="field_with_errors">#{html_tag}</div>).html_safe
  else
    %(<div class="field_with_errors">#{html_tag}<label for="#{instance.send(:tag_id)}" class="message">#{instance.error_message.first}</label></div>).html_safe
  end
end

It looks like that yours invertsif and else branches.

@ghaydarov
Copy link
Author

It also should not affect forms without validate: true right?
and field_error_proc I have affect the html where errors appear, and that was intended.

@ghaydarov
Copy link
Author

Only time this problem goes away is when I remove the gem from gemfile.

@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

It also should not affect forms without validate: true right?

Nope, the above code affects all forms

@ghaydarov
Copy link
Author

Ok I see, so now I have form without validate true, and this form includes carrierwave field, where I upload image. If I have CSV gem, for image upload field I get 'cant be blank' error. If I remove the CSV gem, error disappears and and everything is good. Would this be something about CW or CSV?

@tagliala tagliala reopened this Feb 6, 2017
@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

Ok I see, so now I have form without validate true, and this form includes carrierwave field, where I upload image.

validate: true means that you want to add client side validations. If you don't want CSV for that form, you should not add validate: true to the form_for method parameters.

validate: true when CSV is not installed is useless.

Also, you could skip validations on a specific field with validate: false but I'm giving a look with carrierwave installed because I've changed a lot of stuff

@ghaydarov
Copy link
Author

ghaydarov commented Feb 6, 2017

The thing is I am NOT (sorry mistyped) using CSV with that form and i don't want to. and I dont have validate: true. I get cant be blank error and no other errors. May be there is a conflict between the gems. Not sure.

@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

@ghaydarov
Copy link
Author

ghaydarov commented Feb 6, 2017

Thank you. For the sake of reproduction, I scaffolded new rails app. I added Post model with image field. Added gems CSV and CW. I get the same error. Please note that I dont want to use CSV in this form/model. I want to disable it for this form. When I want to use it, i can use it without any problems. I can't disable it for this form. Thats the problem. (btw, if it helps, I get them same behavior regardless of turbolink's presence. )

Using client_side_validations 9.0.0
Using rails 5.0.1
Using carrierwave 1.0.0

Code snippet from my model of the validations

class Post < ApplicationRecord
	mount_uploader :image, ImageUploader
	validates :image, presence: true
end

The form code from my template

<%= form_for(post) do |f| %>
  <% if post.errors.any? %>
    <div id="error_explanation">
      <h2><%= pluralize(post.errors.count, "error") %> prohibited this post from being saved:</h2>

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

  <div class="field">
    <%= f.label :title %>
    <%= f.text_field :title %>
  </div>

  <div class="field">
    <%= f.label :image %>
    <%= f.file_field :image %>
  </div>

  <div class="actions">
    <%= f.submit %>
  </div>
<% end %>

The resulting HTML along with the script tag Layout file

<!DOCTYPE html>
<html>
  <head>
    <title>Testapp</title>
    <%= csrf_meta_tags %>

    <%= stylesheet_link_tag    'application', media: 'all', 'data-turbolinks-track': 'reload' %>
    <%= javascript_include_tag 'application', 'data-turbolinks-track': 'reload' %>
  </head>

  <body>
    <%= yield %>
  </body>
</html>

My application.js file

//
//= require jquery
//= require jquery_ujs
//= require turbolinks
//= require rails.validations
//= require_tree .

@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

The resulting HTML along with the script tag Layout file

This is the .html file generated from your .erb template (right click -> view html source)

<%= form_for(post) do |f| %>
  <% if post.errors.any? %>
...

I would expect something like

<form action="" ...>
</form>

@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

Sorry, cannot replicate.

Please take a look at: https://github.com/tagliala/csv-test-carrierwave

This is the resulting HTML

<form class="new_post" id="new_post" action="/posts" accept-charset="UTF-8" method="post"><input name="utf8" type="hidden" value="&#x2713;" /><input type="hidden" name="authenticity_token" value="h9iSqgL6AFlTBQk4IYAvWo7a8j0XrXk7RNJDUSxFZRwSmX21k56Ei8awFo43iF6QWtudB5X7Au7MNip8KfNikQ==" />

  <div class="field">
    <label for="post_title">Title</label>
    <input type="text" name="post[title]" id="post_title" />
  </div>

  <div class="field">
    <label for="post_image">Image</label>
    <input type="file" name="post[image]" id="post_image" />
  </div>

  <div class="actions">
    <input type="submit" name="commit" value="Create Post" data-disable-with="Create Post" />
  </div>
</form>

Please note that there is no data-client-side-validations attribute which is required to enable CSV on this form.

Please also keep in mind that CSV doesn't skip server side validations.

This means that if you don't allow image to be passed to the model, you will see "image can't be blank" message after submitting the post.

Eg:

Posts Controller:

class PostsController < ApplicationController
  def new
    @post = Post.new
  end

  def create
    # I'm not permitting `:image` on purpose
    @post = Post.new(params.require(:post).permit(:title))
    if @post.save
      redirect_to :new
    else
      render :new
    end
  end
end

CSV Initializer (not a default, I've manually uncommented the field_error_proc):

# ClientSideValidations Initializer

# Disabled validators
# ClientSideValidations::Config.disabled_validators = []

# Uncomment to validate number format with current I18n locale
# ClientSideValidations::Config.number_format_with_locale = true

# Uncomment the following block if you want each input field to have the validation messages attached.
#
# Note: client_side_validation requires the error to be encapsulated within
# <label for="#{instance.send(:tag_id)}" class="message"></label>
#
ActionView::Base.field_error_proc = proc do |html_tag, instance|
  if html_tag =~ /^<label/
    %(<div class="field_with_errors">#{html_tag}</div>).html_safe
  else
    %(<div class="field_with_errors">#{html_tag}<label for="#{instance.send(:tag_id)}" class="message">#{instance.error_message.first}</label></div>).html_safe
  end
end

Result after submitting the form (Server side validations)

image

@tagliala tagliala closed this as completed Feb 6, 2017
@ghaydarov
Copy link
Author

Thanks for your time on this.
I tried to run yours, but it does not have all actions.

Here is my repo, can you please check this.
https://github.com/ghaydarov/CSV-CW

I have allowed image like this : params.require(:post).permit(:title, :image)

@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

Cannot replicate the issue on your repo.

Could you please write step to step to reproduce?

@ghaydarov
Copy link
Author

ghaydarov commented Feb 6, 2017

  1. yes, just go to http://localhost:3000/posts/new
  2. try to create new post. It will fail.
  3. go to gemfile and remove CSV gem and bundle and restart.
    now repeat the process again. It will work.

@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

Finally got it

It seems a CSV bug

@ghaydarov
Copy link
Author

Glad you could replicate. I tried to find it in the source, but wasn't able to focus much.

@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

It looks like CSV is swallowing enctype="multipart/form-data" attribute for the form, I'm investigating

@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

Wow, this never worked on rails 5!

tagliala added a commit that referenced this issue Feb 6, 2017
File fields automatically add `enctype="multipart/form-data"` attribute
to `<form>` tags. CSV was treating `file_field` as all other fields, so
`self.multipart = true` was skipped

Fix: #694
@tagliala tagliala added this to the 9.0.1 milestone Feb 6, 2017
@tagliala tagliala changed the title Disable client_side_validations gem in some forms. File fields conflict with CSV Feb 6, 2017
@tagliala
Copy link
Contributor

tagliala commented Feb 6, 2017

@ghaydarov thanks, please give a try to the just released 9.0.1 version

@ghaydarov
Copy link
Author

Hey @tagliala, great job on this. Thank you! I upgraded and it is working as it is supposed to.

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

No branches or pull requests

2 participants