beginning_of_week and end_of_week extended to work with any week start #3547

Closed
wants to merge 2 commits into
from

Projects

None yet

10 participants

@gregolsen

In rails week starts from Monday and there's no built-in way to get beginning of week, that starts from another day.
That's especially important for people in US, where week starts from Sunday. Also ruby considers Sunday as a week start.

Me and many others (http://bit.ly/v4H8RB, http://bit.ly/vclIEd) faced this problem, so I re-write beginning_of_week (and end_of_week) to take week start day symbol as an argument and return week start (end) based on that day. days_to_week_start method added (return number of days to week start) and it's used in both beginning_of_week and end_of_week.

@gregolsen gregolsen beginning_of_week extended in both Time and Date so that to return we…
…ek start based on start day that is monday by default
03f9353
@josevalim
Ruby on Rails member

/cc @fxn

@fxn
Ruby on Rails member
fxn commented Nov 7, 2011

ACK, let me think a bit about possible edge cases.

In the meantime, @gregolsen could you revise the AS core extensions guide? Also, in the API docstring note that the default value is the symbol :monday and the reader cannot infer that from the description "Monday". Please update the PR to include this if you are so kind.

@gregolsen

Concerning AS core extensions guide - I believe I did an update in the first commit in the railties/guides/source/active_support_core_extensions.textile. Is there any guides that should be updated too? Let me know.
Also I've improved API docstrings with info about that default symbol value is :monday.

Actually, in first commit I just state Monday just like in docstring for prev_week method:
"Returns a new Time representing the start of the given day in next week (default is Monday)."

@fxn
Ruby on Rails member
fxn commented Nov 7, 2011

@gregolsen oh you're right re guide, thanks for the update of the API, the other docstring should probably need to be updated too (not in this PR).

@gregolsen

I've added another PR with updates to those methods API docstrings (#3668)
Please, let me know if something else should be done for this PR (along with docstrings fix PR) to be acceped. Thanks.

@wycats
Ruby on Rails member

This looks good :)

@fxn
Ruby on Rails member

Hey I am back to this PR, hope to apply today or tomorrow after testing some edge cases.

@fxn fxn added a commit that closed this pull request Nov 25, 2011
@fxn fxn some tweaks to PR#3547. [Closes #3547] a5b362d
@fxn fxn closed this in a5b362d Nov 25, 2011
@vijaydev
Ruby on Rails member

@fxn Should/can we remove the aliases :monday and :sunday set on beginning_of_week and end_of week? With this commit, we can do obj.monday(:tuesday) and so on now :-)

@fxn
Ruby on Rails member

Hahaha.

What about defining them as methods for backwards compat?

@vijaydev
Ruby on Rails member

Yeah, that sounds good.

@taq

+1 for this. I used Rails for a long time but just today I needed to deal with calendar dates regarding the start of week, and ouch, I thought it was Sunday. :-)

@wxianfeng

yeah , i laso think , the week first day is sunday , not monday , most other languages is sunday , if i do as rails style , it's not match other language!

@taelor

Is there a way to set the "beginning of week" day on initialization so I don't have to send :monday in everywhere?

@adonespitogo

I configure my app beginning_of_week = :monday (default). Today is Sunday Aug. 24. When I do Date.today.beginning_of_week, it returns Monday Aug. 25. I was expecting a return of Monday Aug. 18 since monday is the start of the week and sunday(today) is the last day of the week bound to this week (in my presumption). I'm new to development and I need to know if returning the Aug. 18 instead of Aug. 25 does makes sense.

@matthewd
Ruby on Rails member

@adonespitogo my first guess is that you're seeing a timezone related difference... Date.today may not be what you're expecting.

Date.current is the TZ-aware version. If you can reproduce a situation where some_date.beginning_of_week(any_day) is later than some_date, please do file a new issue, describing the version you're using, and the steps needed to demonstrate it.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment