rubocop / rails-style-guide

A community-driven Ruby on Rails style guide
http://rails.rubystyle.guide
6.48k stars 1.06k forks source link

Add new "Prefer `all_(day|week|month|quarter|year)` over range of date/time" rule #319

Closed koic closed 2 years ago

koic commented 2 years ago

Follow up https://github.com/rubocop/rubocop-rails/pull/730.

This PR adds new "Prefer all_(day|week|month|quarter|year) over range of date/time" rule.

# bad
date.beginning_of_day..date.end_of_day
date.beginning_of_week..date.end_of_week
date.beginning_of_month..date.end_of_month
date.beginning_of_quarter..date.end_of_quarter
date.beginning_of_year..date.end_of_year

# good
date.all_day
date.all_week
date.all_month
date.all_quarter
date.all_year
pirj commented 2 years ago

I ran rg --no-ignore --hidden "\.all_(day|week|year|quarter|month){1}" over real-world-rspec and found quite a few usages:

canvas-lms/lib/dates_overridable.rb
226:      hash[:all_day] = assignment.all_day
227:      hash[:all_day_date] = assignment.all_day_date # ?????

# a dozen more usages in the same repo, but there's `def all_day`, so it's not the Rails one

loomio/vue/src/components/poll/meeting/form.vue
19:          {text: @$t('common.all_day'), value: null}

mastodon/app/helpers/admin/announcements_helper.rb
5:    if announcement.all_day?

mastodon/app/models/announcement.rb
89:    self.all_day = false if starts_at.blank? || ends_at.blank?

# Only two real usages:

gitlabhq/spec/models/user_custom_attribute_spec.rb
31:      subject { UserCustomAttribute.by_updated_at(Date.today.all_day) }

gitlabhq/app/models/user_custom_attribute.rb
36:      by_key('blocked_at').by_updated_at(Date.yesterday.all_day)

I love the API, really. But should we recommend something that is not widespread? Maybe start with the cop and see if this good practice spreads?

koic commented 2 years ago

These are APIs that are fully compatible with bad cases and are a smart solution to avoid redundant writing. So it can start with cop, but I think it's useful to mention it in the style guide as well. While it may be less known than I think, but it's useful to know this API.

Especially seen in the totaling up of the data of Rails applications, rather this may not be seen much in OSS :-)