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

Date.to_secs does not take timezone into account. #24

Closed
ideal-knee opened this issue Nov 26, 2014 · 8 comments
Closed

Date.to_secs does not take timezone into account. #24

ideal-knee opened this issue Nov 26, 2014 · 8 comments
Milestone

Comments

@ideal-knee
Copy link

Example symptom:

iex(38)> Date.convert(Date.from({2014, 11, 17}, "CST"), :secs)
1416182400                                                    
iex(39)> Date.convert(Date.from({2014, 11, 17}), :secs)       
1416182400                                                    

Current implementation of Date.to_secs (

timex/lib/date/date.ex

Lines 377 to 379 in b7b7a79

def to_secs(%DateTime{:year => y, :month => m, :day => d, :hour => h, :minute => min, :second => s}, :zero) do
:calendar.datetime_to_gregorian_seconds({{y, m, d}, {h, min, s}})
end
) is not handling timezone before sending off to Erlang.

@bitwalker
Copy link
Owner

For anyone that may want to help tackle this, if you find yourself making any code changes to the Timezone.* modules, ping me first.

@bbense
Copy link
Contributor

bbense commented Jan 5, 2015

Looking at the code this probably is also a bug in to_days, although you'd need to look a lot harder to find a specific example.

@bbense
Copy link
Contributor

bbense commented Jan 6, 2015

I made a first pass at this problem by writing a test that fails and a simple solution, however it seems to be causing problems in other code. Date.to_secs is fundamental to many other functions and I may be running into chicken and egg issues.

The code is here if you want to look, will keep plugging away as I have time.

https://github.com/bbense/timex/tree/Date_to_secs_bug_24

@bbense
Copy link
Contributor

bbense commented Jan 9, 2015

This patch resolves the bug.

#32

No changes to the Timezone code were required, but other methods in Date required some reworking to adapt to the changes in to_secs.

@bitwalker
Copy link
Owner

Merged and published as part of the 0.13.3 release. Thanks! Do you happen to know which functions specifically required rework? I can dig through and find out, but if you know offhand, then I can open a new issue to track that.

@bbense
Copy link
Contributor

bbense commented Jan 10, 2015

Date.shift and Date.compare

On Fri, Jan 9, 2015 at 4:28 PM, Paul Schoenfelder notifications@github.com
wrote:

Merged and published as part of the 0.13.3 release. Thanks! Do you happen
to know which functions specifically required rework? I can dig through and
find out, but if you know offhand, then I can open a new issue to track
that.


Reply to this email directly or view it on GitHub
#24 (comment).

@bbense
Copy link
Contributor

bbense commented Jan 10, 2015

Oh and Date.diff

On Fri, Jan 9, 2015 at 4:37 PM, Booker Bense bbense@gmail.com wrote:

Date.shift and Date.compare

On Fri, Jan 9, 2015 at 4:28 PM, Paul Schoenfelder <
notifications@github.com> wrote:

Merged and published as part of the 0.13.3 release. Thanks! Do you happen
to know which functions specifically required rework? I can dig through and
find out, but if you know offhand, then I can open a new issue to track
that.


Reply to this email directly or view it on GitHub
#24 (comment).

@bitwalker
Copy link
Owner

@bbense Excellent, thank you sir! Might be a good place to start if you are looking for another issue to tackle. I'll cc you on it when I create it, you can decide whether you want to work that one or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants