Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Multiparameter attributes should be set to nil when positional parameters are missing

#972

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.

Reported by James Le Cuirot · September 4th, 2008 @ 11:03 PM

State: duplicate
Milestone: 2.3.10
Assigned to: Michael Koziarski Michael Koziarski
Importance: High

Activity

  1. James Le Cuirot
  2. grant
    grant

    +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!

    November 25th, 2008 @ 11:14 AM

  3. Pratik
    Pratik
    • Assigned user set to Michael Koziarski

    March 6th, 2009 @ 07:44 PM

  4. Brennan Dunn
    Brennan Dunn

    +1 for this. Had the same issue happen recently.

    June 5th, 2009 @ 07:35 PM

  5. Michael Koziarski
    Michael Koziarski
    • Milestone changed from 2.x to 2.3.4

    June 9th, 2009 @ 09:42 AM

  6. Chris Hapgood
    Chris Hapgood

    +1 for fixing this. The behavior of date_select currently defies expectations.

    August 5th, 2009 @ 10:23 PM

  7. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.4 to 2.3.6

    [milestone:id#50064 bulk edit command]

    September 11th, 2009 @ 11:04 PM

  8. sob
    sob

    +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.

    October 22nd, 2009 @ 05:14 AM

  9. Rizwan Reza
    Rizwan Reza
    • Tag changed from 2.1, activerecord, bug, edge, multiparameter, patch to 2.1, activerecord, bug, bugmash, edge, multiparameter, patch

    May 16th, 2010 @ 02:41 AM

  10. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.6 to 2.3.7

    May 23rd, 2010 @ 05:54 PM

  11. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.7 to 2.3.8

    May 24th, 2010 @ 09:40 AM

  12. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.8 to 2.3.9

    May 25th, 2010 @ 11:45 PM

  13. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.9 to 2.3.10
    • Importance changed from to High

    August 30th, 2010 @ 02:28 AM

  14. Aditya Sanghi
    Aditya Sanghi
    • 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 time_select not interpreting the multi-parameter correctly.

    October 17th, 2010 @ 07:40 PM