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

Introduce Date#all_day #24930

Merged
merged 1 commit into from May 16, 2016
Merged

Introduce Date#all_day #24930

merged 1 commit into from May 16, 2016

Conversation

henrik
Copy link
Contributor

@henrik henrik commented May 9, 2016

Useful for queries like:

Item.where(created_at: Date.current.all_day)

There was already a Time#all_day with the same behaviour, so you can already do

Item.where(created_at: Date.current.to_time.all_day)

but arguably having the method on Date as well makes it both read better, and more convenient to write.

@rails-bot
Copy link

r? @senny

(@rails-bot has picked a reviewer for you, use r? to override)

@@ -85,6 +85,11 @@ def end_of_day
end
alias :at_end_of_day :end_of_day

# Returns a Range representing the whole day of the current date.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't love the phrasing "current date", but this is based on the docs for the Time method, which say "current time". In both cases, we mean the instance's date/time.

@egilburg
Copy link
Contributor

egilburg commented May 9, 2016

Wouldn't it make more sense to use just Date.current and have the ORM convert it to the DateTime range clause if used on a Datetime column?

@henrik
Copy link
Contributor Author

henrik commented May 9, 2016

Oops, will look into the CI failures.

@egilburg Hm, possibly. I don't mind the explicitness of the all_day (indeed, I kind of like it), and I like the symmetry with all_week and friends. And that it makes all_day shared between Date and Time like those other methods.

Useful for queries like:

    Item.where(created_at: Date.current.all_day)

There was already a Time#all_day with the same behaviour, but for
queries like the above, Date is more convenient.
@henrik
Copy link
Contributor Author

henrik commented May 11, 2016

Seems Senny might be unavailable – let's try this:

r?

@senny
Copy link
Member

senny commented May 12, 2016

r? @pixeltrix might be able to help out.

@rails-bot rails-bot assigned pixeltrix and unassigned senny May 12, 2016
@sgrif sgrif merged commit 3d4e712 into rails:master May 16, 2016
@henrik
Copy link
Contributor Author

henrik commented May 16, 2016

@sgrif Thank you, appreciate it!

@henrik henrik deleted the date-all-day branch May 16, 2016 18:21
jsugarman added a commit to ministryofjustice/Claim-for-Crown-Court-Defence that referenced this pull request Jul 16, 2021
Simplify code such as
```
 Date.today.beginning_of_day..Date.today.end_of_day
```
to
```
Date.today.all_day
```

Raised by rubocop.

[Introduced here](rails/rails#24930)
see [this article form more](https://www.bigbinary.com/blog/rails-5-1-has-introduced-date-all_day-helper)
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

7 participants