This project is archived and is in readonly mode.
Multiparameter attributes should be set to nil when positional parameters are missing
Reported by James Le Cuirot | September 4th, 2008 @ 11:03 PM | in 2.3.10
There is a problem with multiparameter attributes. When a positional parameter is empty, it is simply not added to the call. This particularly causes trouble when you have a date selector that allows blanks. If you select the day and month but not the year, or if you have the day discarded and blank the month and year, then things like this tend to happen.
>> Date.new(6, 1)
=> Fri, 01 Jan 0006
>> Date.new(1)
=> Sat, 01 Jan 0001
Not only is that clearly wrong but it also makes a mess of the date selector if it gets redisplayed. For example, you may get a year selector starting at -4 and ending at 2008.
This can all be avoided if you set the attribute to nil when positional parameters are missing. For dates, the parameters are 1i, 2i and 3i for the year, month and day respectively. If just 1i and 2i are present then that is okay, but if just 2i and 3i are present then we set to nil because 1i is missing. This will ensure that parameters are always passed in their correct positions and should make sense for any use of multiparameters.
I have written a patch for edge and 2.1. A new test is included and no existing tests fail as a result.
Comments and changes to this ticket
-
James Le Cuirot September 4th, 2008 @ 11:04 PM
- no changes were found...
-
grant November 25th, 2008 @ 11:14 AM
+1 for fixing this. The current behaviour makes for confusing errors. If a user enters 12 for the day, October for the month, and leaves the year empty, they'll get a validation error. If they instead enter 13 for the day, they'll get a 500 because MultiparameterAssignmentErrors was raised.
I like your patch but it doesn't cover the case of the smallest part of the multiparameter being empty. If a user leaves the day unselected, it will get switched to '1'. To be this fair, this seems to be the expected behaviour in Rails -- in that it's included in a couple of tests -- but I don't know why. Setting parts automatically makes sense if you are hiding them (say, showing only the month and year) but not otherwise. If a user hasn't selected a value, it shouldn't suddenly change on them.
Related is this rails-core discussion of empty string coercion: http://groups.google.com/group/r...
The multiparameter code was coercing the empty string to 0 by calling to_i on everything.
I don't think I've included a full fix. The date selects should be kept at their user-set values across requests, even if it doesn't form a valid date. I started poking at date_or_time_select and backed off... not enough time!
-
Pratik March 6th, 2009 @ 07:44 PM
- Assigned user set to “Michael Koziarski”
-
-
Michael Koziarski June 9th, 2009 @ 09:42 AM
- Milestone changed from “2.x” to “2.3.4”
-
Chris Hapgood August 5th, 2009 @ 10:23 PM
+1 for fixing this. The behavior of date_select currently defies expectations.
-
Jeremy Kemper September 11th, 2009 @ 11:04 PM
- Milestone changed from “2.3.4” to “2.3.6”
[milestone:id#50064 bulk edit command]
-
sob October 22nd, 2009 @ 05:14 AM
+1 for having this fixed. specific example of where this is useful: the rails documentation for date_select mentions a use case of having a users birthday but dropping the year (an hr requirement for some companies). I was overriding the year value to always use the current year to store the date but hiding the year prevents the date from ever being set to nil, say if the user wants to remove their birthday from their profile.
-
Rizwan Reza May 16th, 2010 @ 02:41 AM
- Tag changed from “2.1, activerecord, bug, edge, multiparameter, patch” to “2.1, activerecord, bug, bugmash, edge, multiparameter, patch”
-
-
-
-
Jeremy Kemper August 30th, 2010 @ 02:28 AM
- Milestone changed from “2.3.9” to “2.3.10”
- Importance changed from “” to “High”
-
Aditya Sanghi October 17th, 2010 @ 07:40 PM
- State changed from “new” to “duplicate”
Please see ticket 4346 where we're still discussing this issue after 2 years. We've provided a patch with many more tests and comprehensively covers date/time/datetime issues as well as positional parameters among other issues.
Marking this as duplicate. Please provide your comments and review on #4346.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
Aditya Sanghi
James Le Cuirot
Jeremy Kemper
Michael Koziarski
Pratik