Imagine that you need to query for the payments received the “week before” of a given date.

How would you expose this feature in your model?

In this article I want to share some ways of doing it, by telling the story of what we did in a similar case…

Note: I am changing the names a little but the problem was mostly the same

Why do we need that query?

We needed that query because we wanted to send a notification to all users that received a payment in the last week. That notification will be sent each week.

Our first approach…

The first thing that we tried was to expose a class method/scope in the Payment record, like this…

class Payment
  def self.received_this_week
    #...
  end
end

We wrote some tests…

RSpec.describe Payment do
  describe ".received_a_week_before" do
    example "payment received on last second" do
      payment = create :project_payment, received_at: Time.zone.local(2020, 02, 13, 23, 59)
      result = described_class.received_a_week_before(Date.new(2020, 2, 14))
      expect(result).to eq([payment])
    end

    example "payment received one second late" do
      payment = create :project_payment, received_at: Time.zone.local(2020, 02, 14, 00, 00)
      result = described_class.received_a_week_before(Date.new(2020, 2, 14))
      expect(result).to be_empty
    end

    example "payment received in the first second" do
      payment = create :project_payment, received_at: Time.zone.local(2020, 02, 7, 00, 00)
      result = described_class.received_a_week_before(Date.new(2020, 2, 14))
      expect(result).to eq([payment])
    end

    example "payment received on previous week" do
      payment = create :project_payment, received_at: Time.zone.local(2020, 02, 6, 23, 59)
      result = described_class.received_a_week_before(Date.new(2020, 2, 14))
      expect(result).to be_empty
    end
  end
end

To end with an implementation like this…

class Payment < ApplicationRecord
  def self.received_this_week(date = Date.current)
    range = (date - 7.days).beginning_of_day..(date - 1.day).end_of_day
    where(received_at: range)
  end
end

Note: if you want to know about how to build the range, you can see Building a DateTime range to query for the records created a “week before”

This was ok, but…

This was ok and it was working, but the name was not really that expressive, at least for us (on that moment…).

As I said, we were implementing this method because we wanted to send a notification that was going to be sent each week.

Maybe we could have changed the name to something like Payment.for_weekly_notification but we started to think that this behavior, should not be on the Payment class, instead it could belong to a more specific object for this specific use case…

A second a approach…

As we were working to create a “weekly notification with the received payments”…

… We thought, that we could move the construction of the range to a new object and then ask Payment for the payments received between that range.

So we end with something like this…

range = Payment::WeeklyNotificationRange.for_date(Date.current)
Payment.received_between(range)

And later we add a for_today method…

range = Payment::WeeklyNotificationRange.for_today
Payment.received_between(range)

This is more verbose, but at least for us, the intent was a little more clear.

So we wrote some tests for the WeeklyNotificationRange…

RSpec.describe Payment::WeeklyNotificationRange do
  describe ".for_date" do
    example "on friday" do
      result = described_class.for_date(Date.new(2020, 02, 14))
      expect(result).to eq(Time.zone.local(2020, 02, 7).beginning_of_day..Time.zone.local(2020, 02, 13).end_of_day)
    end

    example "on thursday" do
      result = described_class.for_date(Date.new(2020, 02, 13))
      expect(result).to eq(Time.zone.local(2020, 02, 6).beginning_of_day..Time.zone.local(2020, 02, 12).end_of_day)
    end
  end

  describe ".for_today" do
    it "returns range for current day" do
      result = described_class.for_today
      expect(result).to eq(described_class.for_date(Date.current))
    end
  end
end

Extracting this method help us to express more clearly in the specs how the range will change if we ask for a different date. I think that this was a win!

Then we wrote the implementation (Well, we just moved it)…

class Payment::WeeklyNotificationRange
  def self.for_date(date)
    (date - 7.days).beginning_of_day..(date - 1.day).end_of_day
  end

  def self.for_today
    for_date(Date.current)
  end
end

We changed the Payment specs…

RSpec.describe Payment do
  describe ".received_between" do
    let(:range) do
      Time.zone.local(2020, 02, 7).beginning_of_day..Time.zone.local(2020, 02, 13).end_of_day
    end

    example "payment received on last second" do
      payment = create :project_payment, received_at: Time.zone.local(2020, 02, 13, 23, 59)
      result = described_class.received_between(range)
      expect(result).to eq([payment])
    end

    example "payment received one second late" do
      payment = create :project_payment, received_at: Time.zone.local(2020, 02, 14, 00, 00)
      result = described_class.received_between(range)
      expect(result).to be_empty
    end

    example "payment received in the first second" do
      payment = create :project_payment, received_at: Time.zone.local(2020, 02, 7, 00, 00)
      result = described_class.received_between(range)
      expect(result).to eq([payment])
    end

    example "payment received on previous week" do
      payment = create :project_payment, received_at: Time.zone.local(2020, 02, 6, 23, 59)
      result = described_class.received_between(range)
      expect(result).to be_empty
    end
  end
end

And the implementation…

class Payment < ApplicationRecord
  def self.received_between(range)
    where(received_at: range)
  end
end

We stopped here…

We decided to let the code like this, because we thought it was clear enough…

We used this code to run a rake task that sends an email to each user that received a payment in the last week…

if Date.current.friday?
  range = Payment::WeeklyNotificationRange.for_today

  Payment.received_between(range).extract_users.find_each do |user|
    UserMailer.received_payments_notification(user).deliver_later
  end
end

And in the mailer we did something like this…

class UserMailer < ApplicationMailer
  def received_payments_notification(user)
    range = Payment::WeeklyNotificationRange.for_today
    @payments = Payment.for_user(user).received_between(range)
    # mail(...)
  end
end

But we could have abstracted a little more…

Again, for us this was enough, but…

I want to share with you how you can encapsulate something like this a little more, because maybe it could be useful in some circumstances…

We can add a Payment::WeeklyNotification to handle the tasks related with this use case…

For example in our rake task we can do something like this…

if Date.current.friday?
  notification = Payment::WeeklyNotification.new

  notification.users.find_each do |user|
    UserMailer.received_payments_notification(user).deliver_later
  end
end

Or…

if Date.current.friday?
  notification = Payment::WeeklyNotification.new

  notification.users.find_each do |user|
    notification.email_for(user).deliver_later
  end
end

Or maybe just…

if Date.current.friday?
  Payment::WeeklyNotification.new.notify_users
end

And in the mailer we can do something like this…

class UserMailer < ApplicationMailer
  def received_payments_notification(user)
    @payments = Payment::WeeklyNotification.new.payments_for(user)
    # mail(...)
  end
end

And move the logic to that object…

class WeeklyNotification
  def users
    Payment.received_between(date_range).extract_users
  end

  def notify_users
    users.find_each do |user|
      email_for(user).deliver_later
    end
  end

  def email_for(user)
    UserMailer.received_payments_notification(user)
  end

  def payments_for(user)
    Payment.for_user(user).received_between(date_range)
  end

  private

  def date_range
    Payment::WeeklyNotificationRange.for_today
  end
end

And, of course add some tests… Or move them…

But you should stop somewhere…

But you should stop somewhere, and there are trade-offs in each decision…

No solution is perfect… Each abstraction requires maintenance (tests, documentation, etc…), but if you avoid them the intent of the code could be less clear.

How could you know where to stop?

Is really hard to know, but I like the BeckDesignRules …

Kent Beck came up with his four rules of simple design while he was developing ExtremeProgramming in the late 1990’s. And Martin Fowler express them like this:

• Passes the tests
• Reveals intention
• No duplication
• Fewest elements

The rules are in priority order, so “passes the tests” takes priority over “reveals intention”.

You can use this rules as a guide =)

Related articles

• What is an n+1 queries problem?
• What is the difference between includes, preload, eager_load and joins in ActiveRecord?
• Preloading associations cheatsheet
• Specific preloading with scoped associations