westfieldlabs / datetime_helper Goto Github PK

iso8601 on a date-time field causes it to be formatted with a time also. by using strftime('%F') only the date will be extracted from the date-time field.

Without an anchor, a datetime value can pass as a date.

People writing their own serialisers need a simple way to enforce that date-time fields are rendered out in correct Zulu Time format.

On a case-by-case basis we have been doing this:

  %w(updated_at deleted_at).each do |attribute|
    define_method(attribute) do
      object.send(attribute).iso8601 unless object.send(attribute).blank?
    end
  end

I feel that could be simplified to

  %w(updated_at deleted_at).each { |attribute| in_zulu_time(attribute) }

From Simon on the Rails Oceania list.

Nice work; I actually find myself just redoing this sort of thing over and over. My only query would be how it handles leap seconds -- your regex seems to limit seconds to values 0-59.

The regex in question is

ZULU_TIME_PATTERN = /^\d{4}-[0-1][0-9]-[0-3]\d{1}T[0-2]\d{1}:[0-5]\d{1}:[0-5]\d{1}Z$/

I see no reason not to change it to

/^(\d{4})-([0-1][0-9])-([0-3]\d{1})T([0-2]\d{1}):([0-5]\d{1}):([0-5]\d{1})(\.[0-9]{1,3})?Z$/

This has the advantage of allowing the user to parse the string into chunks without going to all the trouble of using Time.parse :-) Such win!

In the readme it calls the serialiser method in_zulu_time but it's actually enforce_zulu_time

also

require 'datetime_helper/active_model_serialisers' ought to be require 'datetime_helper/active_model_serialiser'

@leondewey I'd like you to give this all one final nod before I bump the version to 1.0.0 and we ship it as an actual gem on Rubygems. @mcinerney can wire up Travis to do that automatically.

Review for

• spelling
• module naming
• license

something like

RSpec::Matchers.define :be_an_iso_formatted_time do |expected|
  match do |time|
    !(time =~ /\d{2}:\d{2}/).nil?
  end
end

RSpec::Matchers.define :be_an_iso_formatted_date do |expected|
  match do |date|
    !(date =~ /\d{4}\-\d{2}\-\d{2}/).nil?
  end
end

Dates do not respond to utc so the enforce_zulu_time cannot be used to ensure iso8601 format. The default format from Rails looks like Fri, 15 May 2015 when it needs to be 2015-05-15.

What are your thoughts about changing the usage

From:
validates :updated_at, zulu_time: true

To: Something along the lines of ...

validates :updated_at, datetime_format: :zulu

I think it would make the gem a bit more flexible in the sense of using the same param to support different formats if we ever add any other date or time based validations in the future

@davesag

Fields stored in a database as a timestamp instead of a datetime don't get properly converted to zulu time.

If the local time zone is UTC+0 we currently skip the tests that check our handling of local time. This is patently inadequate and ought to be fixed.

Without forcing a dependency on ActiveSupport I'd like to be able to change the local timezone for selected tests. A wrapper like the following provides some guidance on a way to do this.

def with_time_zone(tz_name)
  prev_tz = ENV['TZ']
  ENV['TZ'] = tz_name
  yield
ensure
  ENV['TZ'] = prev_tz
end

The is_zulu_time? method accepts a time object, a datetime object and a string time, but when we pass a string time, it fails to validate datetime strings.
This PR makes the is_zulu_time? method accept datetime strings

API attribute names don't always match the database column names. Having the ability in the serializer to create a formatted attribute for a column of a different name is needed.

As a developer trying to unit test an Model (which subclasses ActiveModel::Base)
I'd like to be able to test the following

expect(updated_at).to be_zulu_time

where updated_at could be

• a String (i.e. it's a Zulu Time formatted string), or a
• DateTime (ie `.zone == "+00:00"), or a
• Time (i.e. .utc? == true)

Currently the be_zulu_time matcher is only useful for Strings.

We use the ActiveModel::SerializerSupport module which don't exist anymore on the newer versions of the active_model_serializers gem, so we have to use the 0.9.0 version.

From Simon on the RoRo list.

It's really just a minor detail, as NTP kind of hides leap seconds most of the time (because really, they're a pretty horrible hack), but technically speaking, sometimes the second field can contain "60" -- i.e. the minute contains 61 seconds. There's one this year, in June.

So, depending on how your system implements leap seconds, you might see:
2015-06-30T23:59:60Z