default_string_options limits integer input on MySQL to byte size (4) #19

Closed
robertwahler opened this Issue Jun 9, 2009 · 9 comments

Projects

None yet

4 participants

@robertwahler

:maxlength and :size is set to the byte size as stored in the DB, i.e. MySQL = 4. Suggest using html_options rather than setting any defaults, even for strings.

Work around is to pass in :maxlength and :size in html_options or use the code at this commit:
http://github.com/robertwahler/formtastic/commit/435f52ee1b5d47f67a222a7e2c459d3f322482b7

Thanks for the great plugin!
-robert

@jimneath

I've just come across the same problem.

You can fix it by changing the following in default_string_options (1107):

{ :maxlength => column.limit, :size => [column.limit, @@default_text_field_size].min }

With something like the following:

maxlength = column.type.eql?(:integer) ? ((256 ** column.limit) / 2).to_s.length - 1 : column.limit
{ :maxlength => maxlength, :size => [maxlength, @@default_text_field_size].min }

The following:

((256 ** column.limit) / 2)

Calculates the maximum signed number that you can have for the column type.

((256 ** column.limit) / 2).to_s.length

Gets the number of digits in the number, which gives us the maximum length. So for an INT the limit would be 4, which would give us a range of numbers between 2147483647 and -2147483648.

I've subtracted 1 from the result, because whats the point in having a maxlength of 10 for an integer when the maximum you can go to is 2147483647 and thus losing over a quarter of the numbers.

Hope all that makes sense. I would have created a patch but by the time I go home I'll have forgotten :)

@robertwahler

Really nice fix Jim, thanks.

Still, it isn't obvious to me how setting columns limits may change the input size on other numeric types. I have not checked myself. I guess I'm not sold on allowing the plugin to decide what the limits should be.

I prefer allowing the application user to type in whatever they want. I'll inform the user if there is a problem via model validations. I prefer that to having the user not understand why their browser is preventing them from typing in the number they want. It caught me out. I had to examine the source to find out why I couldn't type in a 5 digit integer. I would have preferred to get a validation message when I exceed a limit rather than have my additional input blocked by my browser. Just my 2 cents.

-robert

@justinfrench
Owner

Robert,

The plugin doesn't decide what the limit and size is, it guesses what you probably want based on the table information. What's the difference? The difference is that you can still specify the size and maxlength attributes on a per-input basis with the :input_html option, but if you don't supply them, Formtastic does it's best to pick suitable defaults.

This lets you be quite specific, and me let Formtastic to most of the work :) It lets both your approach (do it all with validations) and my approach (do it with limits as well as validations so that I don't get so frustrated when I get errors back) work side-by-side.

If we removed the functionality altogether as per your patch, there would only be your way :)

Having said all that, there's obviously a bug here with numeric inputs which I'm very thankful you raised, and I'm going to beg Jim to put together a real patch with specs because this is obviously an issue before we hit 1.0.

@justinfrench
Owner

Jim, can we convince you to make a patch? ;)

@jimneath

If it can wait til the weekend I'll knock up a patch with some specs :)

@justinfrench
Owner

That'd be awesome, thanks!

@robertwahler

Justin,

Thanks for the explanation, it has helped me see the value in having Formtastic set the upper bounds on input. As I understand, if the maxlength/size can be reliably determined from any supported database, then model validations will never clash with the limits the plugin assigns in the view since the plugin will assign the max that can be stored for any datatype.

-robert

@justinfrench
Owner

Yup, that was definitely the intent, and hopefully Jim has the fix!

@josevalim

It's quite strange to set maxlenght to 256 ** 4. It's better just not to set the maxlength in cases where the input is :numeric. #fixed

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment