Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for time type columns with invalid time value #7337

Merged
merged 2 commits into from Sep 5, 2012

Conversation

adzap
Copy link
Contributor

@adzap adzap commented Aug 12, 2012

The string_to_dummy_time method was blindly parsing the dummy time string
with Date._parse which returns a hash for the date part regardless
of whether the time part is an invalid time string.

@danbeaulieu
Copy link

+1. Is there a work around in the meantime?

@rafaelfranca
Copy link
Member

What is the current behavior? What is the expected behavior? And why? Please send more information about the issue, it will make easier to review and merge.

@danbeaulieu
Copy link

@rafaelfranca,
I have an open question on SO about this: http://stackoverflow.com/questions/12263135/time-type-field-getting-bad-default-datetime-if-it-is-invalid

Current behavior is if you have a :time type attribute on a model and you set it to some invalid time, like 'foo', ActiveRecord happily parses it into a valid datetime object.

Expected behavior would be similar to :date and :datetime where if the input is invalid, nil is the result.

Essentially what happens with :time is it comes down to Date._parse("2000-01-01 foo") which will parse into a hash with some values, but since this is specifically a :time type, it doesn't validate the the hash has at least a value for :hour.

Hope this helps.

@rafaelfranca
Copy link
Member

Make sense. @adzap lets move this forward to merge. Could you add a CHANGELOG entry?

@adzap
Copy link
Contributor Author

adzap commented Sep 4, 2012

@rafaelfranca will do.

@danbeaulieu thanks for putting in the explanation.

The string_to_dummy_time method was blindly parsing the dummy time string
with Date._parse which returns a hash for the date part regardless
of whether the time part is an invalid time string.
@adzap
Copy link
Contributor Author

adzap commented Sep 5, 2012

@rafaelfranca Changelog updated. This change can/should be back ported to v3 branches too.

rafaelfranca added a commit that referenced this pull request Sep 5, 2012
Fix for time type columns with invalid time value
@rafaelfranca rafaelfranca merged commit 91183ba into rails:master Sep 5, 2012
rafaelfranca added a commit that referenced this pull request Sep 5, 2012
Fix for time type columns with invalid time value
Conflicts:
	activerecord/CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants