This project is archived and is in readonly mode.
AR:Base mangles dates when user submits only part of the fields
Reported by Martijn Vos | January 9th, 2009 @ 07:05 AM
When using date_select, it's possible for the user to submit only month and day, leaving year empty. How to handle that issue is up to the developer of the site, but his job is made a lot harder by ActiveRecord::Base which throws empty values out of the hash before assigning the composite values to a field.
The offending code:
def extract_callstack_for_multiparameter_attributes(pairs)
attributes = { }
for pair in pairs
multiparameter_name, value = pair
attribute_name = multiparameter_name.split("(").first
attributes[attribute_name] = [] unless attributes.include?(attribute_name)
unless value.empty?
attributes[attribute_name] <<
[ find_parameter_position(multiparameter_name), type_cast_attribute_value(multiparameter_name, value) ]
end
end
attributes.each { |name, values| attributes[name] = values.sort_by{ |v| v.first }.collect { |v| v.last } }
end
This means that if the user submits something like:
{"date(1i)"=>"", "date(2i)"=>"6", "date(3i)"=>"5"}
The date becomes "0005-06-01" instead of "0000-06-05".
Both are meaningless dates, but in the latter case, at least it's obvious what the problem is. The developer can easily validate this and return a meaningful error. In the former case, this is not so easy.
Comments and changes to this ticket
-
Martijn Vos January 9th, 2009 @ 07:38 AM
This patch fixes the problem. For me at least -- it's likely there are use cases I'm not aware of.
The patch doesn't add or update any tests because I couldn't find any relevant ones, and I'm not very familiar with these tests yet. This is my first Rails patch. Please check carefully.
The patch is also in http://github.com/mcv/rails/comm...
-
Pratik March 12th, 2009 @ 03:47 PM
- State changed from new to incomplete
- Assigned user set to Michael Koziarski
Patch is missing tests.
Thanks.
-
CancelProfileIsBroken August 5th, 2009 @ 03:30 PM
- Tag changed from activecord, bug, date to activecord, bug, bugmash, date
-
Hugo Peixoto August 9th, 2009 @ 07:35 AM
-1 for the patch, as it breaks two of the current tests.
Throws an ActiveRecord::MultiparameterAssignmentErrors exception when the day is set "", because it tries to execute Date.new(2004, 12, 0), which is invalid. It also breaks when every parameter is set to "", because it executes Date.new(0, 0, 0), which is also invalid.
-
Hugo Peixoto August 9th, 2009 @ 08:22 AM
Actually, according to date_select,
NOTE: Discarded selects will default to 1. So if no month select is available, January will be assumed.
So I've attached a patch that respects this behaviour, with additional tests, checking many combinations of empty values.
-
Rizwan Reza August 9th, 2009 @ 04:06 PM
verified
This patch applies cleanly to 2-3-stable. All tests pass.
-
Elad Meidar August 9th, 2009 @ 05:40 PM
+1 Verified, +1 patch. applies on 2-3-stable and master with all tests pass
-
Repository August 9th, 2009 @ 09:43 PM
- State changed from incomplete to committed
(from [2c4f4a8734b4137adc331186bb2255fb6a38c31e]) With multiparameter date attributes, the behaviour when empty fields are present is now coherent with the one described in the date_select documentation.
[#1715 state:committed]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/2c4f4a8734b4137adc331186bb2255... -
Repository August 9th, 2009 @ 09:43 PM
(from [870750ed4b1e0b0e574aaec86db3e2cdf94b1190]) With multiparameter date attributes, the behaviour when empty fields are present is now coherent with the one described in the date_select documentation.
[#1715 state:committed]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/870750ed4b1e0b0e574aaec86db3e2... -
CancelProfileIsBroken August 9th, 2009 @ 10:05 PM
- Assigned user cleared.
- Tag changed from activecord, bug, bugmash, date to activecord, bug, date
- Milestone cleared.
-
Raimonds Simanovskis August 10th, 2009 @ 02:43 PM
These new tests fail on Oracle database (using oracle_enhanced adapter) because Oracle adapter by default returns date values as Time values and for Time values execute_callstack_for_multiparameter_attributes method does not replace missing values with 1.
-
Pratik August 10th, 2009 @ 03:34 PM
Hey Raimonds,
If you have a patch ready, I'll be happy to push.
Thanks.
-
Raimonds Simanovskis August 10th, 2009 @ 08:26 PM
I modified the test so that using Oracle enhanced adapter attribute type is forced to Date and now tests are passing: http://github.com/rsim/rails/commit/416d6cc51a6c67a89a564c86de8fede...
But the question remains is it necessary to substitute missing values with 1 when using datetime_select to assign values to Time attribute. Currently it seems that in such case result is unpredictable but in this case nothing is told in API documentation :)
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>
People watching this ticket
Attachments
Tags
Referenced by
- 1715 AR:Base mangles dates when user submits only part of the fields [#1715 state:committed]
- 1715 AR:Base mangles dates when user submits only part of the fields [#1715 state:committed]
- 2561 Weird years list when year not selected I think this was fixed in https://rails.lighthouseapp.com...