Why do form_helpers look directly into the database?

Hi all,

I have a small testing RailsApp with a simple model, which has a float attribute called price:

In the model I have overridden the price() method:

class Product < ActiveRecord::Base   def price     return "bla"   end end

In the console, it behaves as expected: p = Product.new p.price = 12.12 p.price

"bla"

However, Using the form helper methods, it appears they are not using the instance methods:

for instance:

<%= form.text_field :price %>

displays: 12.12 in my view, and ignores the overridden AR instance method.

Why do the form_helpers work this way? Is this a bug? Or is this approach chosen on purpose?

Regards,

Harm de Laat Kabisa ICT

This issue already has a thread: “Ticket #2322”. Here is the actual ticket: http://dev.rubyonrails.org/ticket/2322

There are actually several tickets that show this behavior (bug?). I believe the OP is writing to ask whether this is the desired behavior, or whether it should be treated as a bug and we should be pushing to commit patches relating to it.

Kev

It seems the ticket is closed? Any reason why? Should we reopen it?

Feel free to reopen it, but as this discussion covers multiple tickets (and intended behavior), it should take place here.

At least to me, it ‘feel’s’ like kinda odd behavior. It seems inconsistent: IMHO product.price() should display the exact same value as form.text_field :price. Also the method header is: text_field(object_name, method, options = {}), which imho implies that ‘method’ is going to be called on ‘object_name’.

Cheers, harm!

There must have been some reason for this behavior.

There must have been some reason for this behavior.

validates_numericality_of

if you have an integer field, and the user types in "a12345" instead of "12345". When displaying error messages you want to display the value before it was typecast, otherwise it'd just be set to 0. So the field helper doesn't look directly in the database, it uses price_before_type_cast, if it exists.

So the behaviour is intentional, if somewhat opaque. Perhaps what's needed is a way to make it easier to wrap attributes with custom accessors and mutators?

Ah lame I totally just patched this.

Maybe we could fix validates_numericality_of? Rather than changing every field helper because of one case? I suppose it is more difficult that way around however.

The docs really do make it seem like it calls the method. So we'd need to remember to change those :slight_smile:

Ah lame I totally just patched this.

Maybe we could fix validates_numericality_of? Rather than changing every field helper because of one case? I suppose it is more difficult that way around however.

It's also totally random behaviour to sometimes get a String from an otherwise numeric attribute. I'm not sure that there's a really simple fix that'll magically solve every case we could come across, but I do believe that if we find the cases where this behaviour is biting people, we'll probably be able to solve those cases nicely.

My understanding is that the most common case is something along the lines of "I want to store the number of cents instead of a dollar amount"? What else breaks?

It seems the real problem is two fold:

1) it's kinda surprising that overriding the accessors doesn't automatically work 2) The docs outright lie and tell you that it will work.

The docs really do make it seem like it calls the method. So we'd need to remember to change those :slight_smile:

Indeed.

What about:

  1) If an overridden accessor is there, (i.e you've defined Product#price) then that gets called    2) Otherwise, we call price_before_type_cast

(i.e the opposite of the current logic :))

And we amend the documents.

That’s still no good. You want to be able to display to the user exactly what he entered, unconditionally. Doing what you suggested would make the behavior even more inconsistent.

How about this for a solution.

The real problem seems to be that people want to format attributes before they display them (credit card, phone number). So, how about we allow that. Could be as simple as:

<%= text_field :person, :name, :formatter => :random -%>

class Person < ActiveRecord::Base

   def random(value)       value.upcase * 4    end

end

Thoughts?

That takes care of the transformation in one direction, but not the
other. I suppose you can just handle that the usual way with a
before_validation callback. Sorry if that's silly, I haven't been
following this thread in detail.

--josh

I think the only way you want to format it is for display to the end user, store it in the database in a different format.

In the example I was needing this for: We need to allow people to enter prices in dutch format, i.e.: 12,25 (notice the comma). Before storing this into the database we would like to transform this value to: 12.25 (notice the dot). When displaying the value in the view we need to do the opposite, retrieve the value from the database and replace dots by comma’s and display the value to the end user.

We solved this easily using a virtual attribute, however, it would be nice if it were just possible to override getters and setters in AR. It just seems awkward that this is not possible for helper methods.

Why is this so hard? Why not save the original value typed in by the end user, and if validation goes wrong just display the original entered value, else just use the overridden setter?

I might be missing something though…

        > > How about this for a solution.         > >         > > The real problem seems to be that people want to format attributes         > > before they display them (credit card, phone number). So, how about we         > > allow that. Could be as simple as:         > >         > > <%= text_field :person, :name, :formatter => :random -%>         > >

+1

I would love for this to be in core. I have been feeling the pain of hacking around this for sometime.

Nik, will you do the patch or should I?

Koz, do you think this would be applied?

I'm happy to put together a patch if everyone is happy with that soution...?

Sometimes (or perhaps generally) form builders and objects are too closely tied -- surely you do not want to pollute your model with a lot of user interface stuff. Instead, a level of indirection is useful, such as a form model:

class ProductForm def initialize(object)    @object = object end

def dutch_price    ('%.2f' % @object.price).gsub('.', ',') end

def dutch_price=(price)    @object.price = price.to_s.gsub(',', '.') end

def method_missing(name, *args, &block)    @object.send(name, *args, &block) end end

Now you can do:

<% form_for :product, @product do |form| %> <%= form.text_field :title %> <%= form.text_field :dutch_price %> <% end %>

where @product could be initialized thus:

  @product = ProductForm.new(Product.find(params[:id]))

Alexander.

I'm happy to put together a patch if everyone is happy with that soution...?

Another thing to check is if it's feasible to make the form helpers only use _before_type_cast if the field is invalid? Perhaps that'll allow for a relatively low-impact solution?