client_side_validations icon indicating copy to clipboard operation
client_side_validations copied to clipboard

use-associations-validations

Open MichalRemis opened this issue 4 years ago • 5 comments

I need this change to make checkboxes/radiobuttons work in CSV-simple_form.

Currently in CSV validations are added to CSV hash (via collection_check_boxes/collection_radio_buttons overrides). Events aren't attached to radiobuttons (filtered out). For checkboxes, events are attached but when JS validating, their validations are NOT applied because of special names of association checkbox inputs (e.g. user[:roles][]). Not sure if this was an intention, but it is allright because standard presence validation wouldn't work for checkboxes anyway, and as I understand they are not going to be supported in main CSV.

But to make collections work in CSV-simple_form I need to use (overridden) validations and therefore need to clean input names user[:roles][]->user[:roles]. Couldn't do it in CSV-simple_form therefore updating it here in main. Maybe you have some better idea how to achieve this.

MichalRemis avatar Apr 30 '20 13:04 MichalRemis

Coverage Status

Coverage remained the same at 100.0% when pulling d54818489a4dd7b099bd08eec5ddc03d6c342bb2 on MichalRemis:attach-correctly-associations-validations into c7468a6bdc424ed9fec1b181c428534acffe7fc1 on DavyJonesLocker:master.

coveralls avatar Apr 30 '20 13:04 coveralls

@MichalRemis can you please paste the generated HTML code of a page showing the issue, as well as the ruby template?

Maybe with both collection_check_boxes and collection_radio_buttons, so I can take a look of the actual behavior

tagliala avatar Apr 30 '20 13:04 tagliala

Sure:

ruby:

class Box < ApplicationRecord
  validates :colors, presence: true
  validates :material, presence: true

  belongs_to :material
  has_and_belongs_to_many :colors
end
<%= form_for @box, validate: true do |f| %>
    <%= f.collection_radio_buttons :material, Material.all, :id, :name %>
    <%= f.collection_check_boxes :colors, Color.all, :id, :name %>

    <%= f.submit %>
  <% end %>

HTML:

<form class="new_box" id="new_box" novalidate="novalidate" data-client-side-validations="{&quot;html_settings&quot;:{&quot;type&quot;:&quot;ActionView::Helpers::FormBuilder&quot;,&quot;input_tag&quot;:&quot;\u003cdiv class=\&quot;field_with_errors\&quot;\u003e\u003cspan id=\&quot;input_tag\&quot;\u003e\u003c/span\u003e\u003c/div\u003e&quot;,&quot;label_tag&quot;:&quot;\u003cdiv class=\&quot;field_with_errors\&quot;\u003e\u003clabel id=\&quot;label_tag\&quot;\u003e\u003c/label\u003e\u003c/div\u003e&quot;},&quot;number_format&quot;:{&quot;separator&quot;:&quot;.&quot;,&quot;delimiter&quot;:&quot;,&quot;},&quot;validators&quot;:{&quot;box[material]&quot;:{&quot;presence&quot;:[{&quot;message&quot;:&quot;can't be blank&quot;},{&quot;message&quot;:&quot;must exist&quot;}]},&quot;box[colors]&quot;:{&quot;length&quot;:[{&quot;messages&quot;:{&quot;minimum&quot;:&quot;is too short (minimum is 1 character)&quot;,&quot;maximum&quot;:&quot;is too long (maximum is 2 characters)&quot;},&quot;minimum&quot;:1,&quot;maximum&quot;:2}]}}}" action="/boxes" accept-charset="UTF-8" method="post">
  <input name="utf8" type="hidden" value="✓">
  <input type="hidden" name="authenticity_token" value="6TvRPJ5x9rYO1hE4n3kwWBraopOEOmXI9YU93cNFbXVnaFk7RuUgCv8BzP6naBoHRO5qapfWkJppgf9tIN3ITg==">
  
  <input type="hidden" name="box[material]" value="" data-validate="true">
  <input type="radio" value="1" name="box[material]" id="box_material_1" data-validate="true">
  <label for="box_material_1">Wood</label>
  <input type="radio" value="2" name="box[material]" id="box_material_2" data-validate="true">
  <label for="box_material_2">Plastic</label>
  <input type="radio" value="3" name="box[material]" id="box_material_3" data-validate="true">
  <label for="box_material_3">Glass</label>
  
  <input type="hidden" name="box[colors][]" value="">
  <input type="checkbox" value="1" name="box[colors][]" id="box_colors_1" data-validate="true">
  <label for="box_colors_1">Red</label>
  <input type="checkbox" value="2" name="box[colors][]" id="box_colors_2" data-validate="true">
  <label for="box_colors_2">Green</label>
  <input type="checkbox" value="3" name="box[colors][]" id="box_colors_3" data-validate="true">
  <label for="box_colors_3">Blue</label>

  <input type="submit" name="commit" value="Create Box" data-disable-with="Create Box">
</form>

Problem with plain Rails radio/checkbox collections is that they don't have a wrapper around them like simple_form does.

Regarding this PR. You can see validations for box[colors] are in the CSV hash but currently when JS validating checkbox input,

https://github.com/DavyJonesLocker/client_side_validations/blob/c7468a6bdc424ed9fec1b181c428534acffe7fc1/src/main.js#L63-L71

it doesn't use box[colors] JS validator because name of input is box[colors][]. This function: https://github.com/DavyJonesLocker/client_side_validations/blob/c7468a6bdc424ed9fec1b181c428534acffe7fc1/src/main.js#L83 and my PR's change in it cleans the name to box[colors] to apply validators correctly.

MichalRemis avatar Apr 30 '20 14:04 MichalRemis

Thanks

Got the need for different names

we should do three things:

  1. Move /\[(\w+_attributes)\]\[[\da-z_]+\](?=\[(?:\w+_attributes)\])/g to a self-explaining constant. I don't remember what this regexp ecaxtly do, we need to go back in history. -- SEPARATE PR/COMMIT
  2. See if we can safely omit the trailing [] when checking for validations.
  3. Find another solution (we can't check for the formbuilder's name)

tagliala avatar Apr 30 '20 15:04 tagliala

  1. Move /\[(\w+_attributes)\]\[[\da-z_]+\](?=\[(?:\w+_attributes)\])/g to a self-explaining constant. I don't remember what this regexp ecaxtly do, we need to go back in history. -- SEPARATE PR/COMMIT

Look's like it normalizes input name for nested attributes https://api.rubyonrails.org/classes/ActiveRecord/NestedAttributes/ClassMethods.html

  1. See if we can safely omit the trailing [] when checking for validations.

I don't think we may omit it safely for mainCSV, because JS validators, e.g.:

https://github.com/DavyJonesLocker/client_side_validations/blob/c7468a6bdc424ed9fec1b181c428534acffe7fc1/src/validators/local/absence_presence.js#L9-L13

aren't ready for checkboxes and would report false errors for association checkbox's inputs (not tested though). That's why I added SimpleForm::FormBuilder condition. It would be safe if you tweak validators to work with checkboxes, something like https://github.com/MichalRemis/client_side_validations-simple_form/blob/add-checkboxes-and-radios-support/src/validator_overrides/presence.js

  1. Find another solution (we can't check for the formbuilder's name)

If we can't check formbuilder's name and you have no other idea or won't implement working collection_checkboxes in main CSV, I could implement it in CSV_simple-form, but it would require a lot of code copy & pasting from main - I am not sure if that's a good idea.

MichalRemis avatar Apr 30 '20 16:04 MichalRemis