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

Modify TimeWithZone#as_json to return 3DP of sub-second accuracy. #9128

Merged
merged 1 commit into from Feb 6, 2013

Conversation

@jimsynz
Copy link
Contributor

commented Jan 31, 2013

The ISO8601 spec says that decimal fractions may be added for the seconds (actually any) value. The default in recent versions of Firefox, Chrome and Safari (that I have tested) all return 3 decimal places of accuracy for seconds for their default serialisation:

(new Date()).toJSON()
"2013-01-31T01:15:22.236Z"

I ran into this in an app using rails-api/active_model_serializers, and initially intended to submit a PR there, but soon discovered that it uses ActiveSupport's default JSON encodings.

It's a very simple change, so I've updated the one test for this method and added a changelog entry. Tests pass on my machine. Please let me know if there's anything I've missed and I'll clean it up immediately.

…efault, since it's allowed by the spec and is very useful.
jeremy added a commit that referenced this pull request Feb 6, 2013
Modify TimeWithZone#as_json to return 3DP of sub-second accuracy.
@jeremy jeremy merged commit c759813 into rails:master Feb 6, 2013
@jimsynz jimsynz deleted the jimsynz:iso8601-sub-second-accuracy branch Feb 6, 2013
carlosantoniodasilva added a commit that referenced this pull request Feb 6, 2013
And improve AS changelog a bit [ci skip]
tenderlove added a commit that referenced this pull request Feb 8, 2013
* master:
  Skip schema dumper extensions test if connection does not support it
  active_record: Quote numeric values compared to string columns.
  Run schema dumper extensions without creating real extensions
  Do not print anything related to extensions when they don't none exist
  Add blank line after extensions to separate from tables in schema
  Fix indentation of extensions in schema
  Call super to use the abstract adapter implementation instead
  Add changelog entry for #9203 about schema dumper with db extensions
  add ActiveRecord::AbstractAdapter#extensions and ActiveRecord::ConnectionAdapters::PostgreSQLAdapter#extensions to allow dumping of enabled extensions to schema.rb, add ActiveRecord::SchemaDumper#extensions to dump extensions to schema.rb
  improve tests to check for existence of extensions method, and skip testing dumped extensions if they are unsupported by the database
  Add some tests to enumerate how extensions should be stored in the schema output
  Update changelog from #9128 with author name
  Update actionpack/CHANGELOG.md
  ruby constant syntax is not supported as routing `:controller` option.
  Fix article for generator name
  Update .gitignore
  Enable hstore extensions on tests if it is not enabled and database supports it
  Ignore .ruby-version
  Modify TimeWithZone#as_json to return 3DP of sub-second accuracy by default, since it's allowed by the spec and is very useful.
@ersatzryan

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2013

If the extra 3 decimal places is optional why is rails making it default? Seems like something that should be optional based on the ISO8601 spec.

@jimsynz

This comment has been minimized.

Copy link
Contributor Author

commented Sep 4, 2013

Because if the object in question has sub-second accuracy, then it would make sense to serialize it, rather than lose data.

@ersatzryan

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2013

It is odd that only TimeWithZone that has these fractional seconds. Time#as_json and DateTime#as_json do not.

https://github.com/rails/rails/blob/master/activesupport/lib/active_support/json/encoding.rb#L319
https://github.com/rails/rails/blob/master/activesupport/lib/active_support/json/encoding.rb#L339

I suggest that it be configurable and consistent across all the time types.

#12226

@palexvs

This comment has been minimized.

Copy link

commented on 28ab79d Nov 5, 2013

Is there some right way to return old behavior: exclude milliseconds?

This comment has been minimized.

Copy link
Contributor

replied Nov 5, 2013

The only way is to monkey-patch TimeWithZone to change the behavior of it's as_json method.

@chenkirk

This comment has been minimized.

Copy link

commented Jun 26, 2014

+1 to having some way of excluding the milliseconds. We have to monkey patch this as we have clients that are expecting to parse the output that doesn't have milliseconds.

@sgrif

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2014

Please do not comment on PRs that were closed months ago, or add +1 comments in general. They do not give anything to the community nor will they cause changes to occur. Feature requests should be made on the rails-core mailing list

@rails rails locked and limited conversation to collaborators Jun 26, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
8 participants
You can’t perform that action at this time.