rails model scope doesnt select good query

Viewed 231

I would like to select my Offers bookable, I compare arrival and departure dates search params with bookings confirmed dates of each offer in a scope.

Offer model:

has_many :bookings

scope :booking_available, -> (arrival_date, departure_date) {
  includes(:bookings)
    .references(:bookings)
    .where.not(
      'bookings.arrival_date <= ? AND
      bookings.departure_date >= ? AND
      bookings.status != ?',
      arrival_date,
      departure_date,
      0
    )
}

Booking model:

belongs_to :offer

And call it in my search function in controller.

if offer_params[:arrival_date].present? && offer_params[:departure_date].present?
  @offers = @offers.booking_available(
    offer_params[:arrival_date],
    offer_params[:departure_date]
  )
end

But the result give me only offers not available, I think .not in my query doesn't work and I don't know how fix it.

3 Answers

Considering the complexity, I think you need to break this apart, so that you can test the pieces individually.

You describe the business logic as "offers who don't have bookings where... (conditions)".

This suggests a query like:

Offer.joins("left outer join bookings on bookings.offer_id = offers.id").
      select("offers.*, count(bookings.id)").
      group("offers.id").
      having("count(bookings.id) = 0")

# yeah, I didn't write it as a scope, b/c it's a bit more readable
# you can make it a scope if you wish

write a test and make sure this gives you the correct result. It does not yet select "bookings available". So now we add the bookings available criterion:

available_bookings = Booking.available(ar_date, dep_date).to_sql

joins = "left outer join (#{available_bookings}) as available_bookings on available_bookings.offer_id = offers.id"

Offer.joins(joins).
      select("offers.*, count(available_bookings.id)").
      group("offers.id").
      having("count(available_bookings.id) = 0")

By delegating the booking availability criteria to the Booking model, we can test it separately. Let me see if I have correctly understood what you mean by an available booking:

# booking.rb

scope :available, ->(arrival_date, departure_date){ 
       outside_of(arrival_date, departure_date).not_pending(arrival_date, departure_date) 
}

scope :outside_of, ->(arrival_date, departure_date){ 
       where("arrival_date <= ? and departure_date >= ?", arrival_date, departure_date) 
}

scope :not_pending, ->(arrival_date, departure_date){ 
       where.not("arrival_date >= ? and departure_date <= ? and status != ?",arrival_date, departure_date, 0)
}

This gives you three scopes on Booking that you can test separately.

Testing the components is critical here, to make sure each one works as intended.

Stylistically, too, it's good practice (maybe even best practice) to keep the scopes for bookings in the Booking model, and not to have them in the Offer model. Testability being one of the reasons.

Let us know how this works out.

As I understand, you need to check if:

  • given a date interval P
  • list all sets A that do not intersect P

A intersects P if:

  • A.arrival_date <= P.departure_date, and
  • A.deperture_date >= P.arrival_date

Hence, just swapping the order of the parameters should suffice:

has_many :bookings

scope :booking_available, -> (arrival_date, departure_date) {
  includes(:bookings)
    .references(:bookings)
    .where.not(
      'bookings.arrival_date <= ? AND
      bookings.departure_date >= ? AND
      bookings.status != ?',
      departure_date,
      arrival_date,
      0
    )
}

Your scope condition

...
    .where.not(
      'bookings.arrival_date <= ? AND
      bookings.departure_date >= ? AND
      bookings.status != ?',
      arrival_date,
      departure_date,
      0
    )
...

will generate a WHERE query like this:

...
  WHERE NOT (bookings.arrival_date <= 'arrival_date'
    AND bookings.departure_date >= 'departure_date'
    AND bookings.status != 0)

which the same to

...
  WHERE NOT bookings.arrival_date <= 'arrival_date'
    OR NOT bookings.departure_date >= 'departure_date'
    OR NOT bookings.status != 0

Or simpler:

...
  WHERE bookings.arrival_date > 'arrival_date'
    OR bookings.departure_date < 'departure_date'
    OR bookings.status == 0

This, I guess, queries nearly all the data you have. Because this is logic operation, not filtering. You should remove .not and write it in positive:

...
    .where(
      'bookings.arrival_date > ? AND
      bookings.departure_date < ? AND
      bookings.status == ?',
      arrival_date,
      departure_date,
      0
    )
...

Does it express correct your business logic:~

Available bookings are bookings have:

  • arrival date after <arrival_date> and
  • departure date before <departure_date> and
  • status is 0

It's a bit complex but you could read the logic out of query below:

# query
arrival_date = "2021-07-10 01:00:00"
departure_date = "2021-07-20 01:00:00"

# query
select_query = "offers.*, " +
  "SUM(CASE WHEN arrival_date <= '#{arrival_date}' THEN 1 ELSE 0 END) AS arrival_date_bound, " +
  "SUM(CASE WHEN departure_date >= '#{departure_date}' THEN 1 ELSE 0 END) AS departure_date_bound, " +
  "COUNT(*) as total"
Offer.select(select_query)
  .left_outer_joins(:bookings)       # supported from rails 5+
  .where.not(bookings: {status: 0})
  .group("offers.id, offers.name")
  .having("arrival_bound + departure_bound = 0 AND total > 0")

This query joins offers with bookings, and group by offer. For each offer, we count numbers of pending bookings has arrival_date before a specific arrival_date, called arrival_bound. And count numbers of pending bookings has departure_date after a specific departure_date. And just get offers don't have these kind of bookings. I add an extra total condition to filter out offers haven't booking.

I tried it with tests below. Only o1 and o5 have all bookings, which have both arrival_date and departure_date within range t1->t4

t0 = Time.new(2021,7, 5, 1, 0,0)
t1 = Time.new(2021,7,10, 1, 0,0) # arrival_date to query
t2 = Time.new(2021,7,13, 1, 0,0)
t3 = Time.new(2021,7,18, 1, 0,0)
t4 = Time.new(2021,7,20, 1, 0,0) # departure_date to query
t5 = Time.new(2021,7,25, 1, 0,0)

# offers have 1 booking
o1 = Offer.create!(name: "test 1")
Booking.create!(offer: o1, arrival_date: t2, departure_date: t3, status: 1)

o2 = Offer.create!(name: "test 2")
Booking.create!(offer: o2, arrival_date: t0, departure_date: t3, status: 1)

o3 = Offer.create!(name: "test 3")
Booking.create!(offer: o3, arrival_date: t2, departure_date: t5, status: 1)

o4 = Offer.create!(name: "test 4")
Booking.create!(offer: o4, arrival_date: t0, departure_date: t5, status: 1)

# offers have more than 1 booking
o5 = Offer.create!(name: "test 5")
Booking.create!(offer: o5, arrival_date: t2, departure_date: t3, status: 1)
Booking.create!(offer: o5, arrival_date: t2, departure_date: t3, status: 1)

o6 = Offer.create!(name: "test 6")
Booking.create!(offer: o6, arrival_date: t0, departure_date: t5, status: 1)
Booking.create!(offer: o6, arrival_date: t0, departure_date: t5, status: 1)

o7 = Offer.create!(name: "test 7")
Booking.create!(offer: o7, arrival_date: t2, departure_date: t3, status: 1)
Booking.create!(offer: o7, arrival_date: t2, departure_date: t5, status: 1)

o8 = Offer.create!(name: "test 8")
Booking.create!(offer: o8, arrival_date: t2, departure_date: t3, status: 1)
Booking.create!(offer: o8, arrival_date: t0, departure_date: t3, status: 1)
Related