Reuse of validations from different models

[ilyakatz]

I find it that people often want to create a separate object that incapsulates model that the form is being created for, something along the lines of http://pivotallabs.com/form-backing-objects-for-fun-and-profit/

Yesterday I found myself in the situation where I wanted to use CSV for one such object and it worked with basic client side validation like presence

# Object that takes care of more complicate forms 
# related to creating a User
class UserRegistrationForm
  include ActiveModel::Validations
  validates_presence_of :email
end

However, I wanted to go further to reuse ajax-based validation

class UserRegistrationForm
  include ActiveRecord::Validations
  validates_uniqueness_of :email
end

That didn't work because it was trying to validate against UserRegistrationForm in CSV which makes sense.

I'm proposing to allow a syntax like this

class UserRegistrationForm
  validates_uniqueness_of :email, 
       client_valdation: { class: User }
end

I'd be happy to give it a shot to code it up but wanted to run it past you first.

[bcardarella]

I'd like to see the code because for local validators it should work as you expect. For remote validators like uniqueness it won't.

[ilyakatz]

That's correct, it does work for local validators, I'm proposing to make it work for remote

[bcardarella]

To make it work with remote you should be able to override the name attribute. In the case of your example, if you are trying to target the User class then you should be able to do the following with your form:

<% form_for @user_registration_form, :as => :user %>
   ..

[ilyakatz]

hm, just tried it out and it didn't work, i'll play with it a bit more and will report back

[bcardarella]

Can you show me what the resulting HTML code is? The remote validator middleware parses the model name based upon the name attribute on the input.

[ilyakatz]

Ok, looks like I found a bug:

I start with this code in the template

<% form_for(resource, as: :user %>
...

JS that is produced is this:

"validators":
{"user[email]":{
"presence":[{"message":"Required"}],
"uniqueness":[{"message":"Already in use.","case_sensitive":true,"class":"accounts/user_registration_form"}]}
}};

When I update the template to this

<% form_for(resource, as: :new_model %>
...

Validation JS is this:

"validators":
{"new_model[email]":{
"presence":[{"message":"Required"}],
"uniqueness":[{"message":"Already in use.","case_sensitive":true,"class":"accounts/user_registration_form"}]}
}};

So the "class" key in the remote validation is still the same although other references are updated

[bcardarella]

Ah, yeah that does look like a bug. Ok, I'll keep this open and try to get to it soon. I'm pretty swamped right now so maybe not immediately :(

[ilyakatz]

Cool I'll try to mock around as well

sent from aPhone
On Apr 2, 2013 11:38 AM, "Brian Cardarella" notifications@github.com
wrote:

Ah, yeah that does look like a bug. Ok, I'll keep this open and try to get
to it soon. I'm pretty swamped right now so maybe not immediately :(

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/519#issuecomment-15782810
.

[arr-ee]

It behaves that way because we set validation options in the ActiveModel (or, in case of uniqueness validator, in ActiveRecord) and don't mess with it in ActionView, where we set :as.

I think passing real model name in validator options is ok since, well, that's a wrapper object and we have already jumped through a couple of hoops making it quack like original one :) Plus, making :as change class option can possibly break things.

If my speculation makes sense, I'll hack it up in an hour (because testing it looks pretty painful)

[arr-ee]

Oh, and it fails only when wrapper class is namespaced (note accounts/user_registration_form), because we don't add class parameter to non-namespaced classes.

[ilyakatz]

thanks for the fast response @arr-ee! I was under impression that that @bcardarella didn't want to add any extra options.

[arr-ee]

@ilyakatz, yeah, I thought that too, but after a bit of playing with code I thought it would be better solution. Adding workaround for this somewhere in ActiveView seemed even more messed up.

[arr-ee]

@ilyakatz, by the way, could you please test if pull request actually resolves the problem?

[ilyakatz]

sure, will try it out. kind of agree with the implications on ActiveView as well

[ilyakatz]

@arr-ee just tried it out, the request works fine. one problem is that if I don't specify a message, I get an error

translation missing: en.activemodel.errors.models.accounts/user_registration_form.attributes.email.taken

which makes sense since it is not an AR model so it doesn't fall back to activerecord.errors localization/defaults. Not sure if it's a good idea to add fallback to the gem or just document that message is required

[arr-ee]

That's not a gem's problem, actually, your wrapper just doesn't quack hard enough.

Try adding this to you wrapper class:

  extend ActiveModel::Naming
  extend ActiveModel::Translation

  def self.model_name
    @_model_name ||= ActiveModel::Name.new(User) # or whatever your actual class is
  end

  def self.i18n_scope
    :activerecord
  end

[ilyakatz]

ok cool, in that case, i'm happy - you made me happy :) I actually don't want to add all that stuff in, so I will play around it

[arr-ee]

Great!

Anyway, those modules are strongly recommended because they make your form objects behave more like actual models (actually, in Rails 4 they are conveniently packed into ActiveModel::Model).
We're using this approach in a couple of places and it works great.

I'll wait for @bcardarella comments on this and if he's good with it, I'll merge it in upstream.

[bcardarella]

@arr-ee this is OK as it doesn't really violate semver. I look at this as fixing a bug. I will comment inline on some changes I would make before merging.

[arr-ee]

Fixed in #520.

[ilyakatz]

Thanks for the fast turnaround time, @arr-ee and @bcardarella