Safe-autocorrect with Rails/WhereRange does not work correctly when

[mark-young-atg]

rubocop -a does not correctly update this method with the Rails/WhereRange cop

In app/models/booking.rb

  def over_due
    joins(:events).where('end_at < ?', Time.zone.now)
  end

It's referenced and used in app/models/vehicle.rb

class Vehicle < ApplicationRecord
  has_many :bookings
  ...

  def overdue?
    bookings.over_due.where(status: %w[on_loan active]).any?
  end
end

Expected behavior

The expected corrected code is

    def over_due
      joins(:events).where(events: { end_at: ...Time.zone.now })
    end

Converted to SQL this gives

> Booking.joins(:events).where(events: {end_at: ...Time.zone.now}).to_sql
=> "SELECT \"bookings\".* FROM \"bookings\" INNER JOIN \"events\" ON \"events\".\"eventable_type\" = 'Booking' AND \"events\".\"eventable_id\" = \"bookings\".\"id\" WHERE \"events\".\"end_at\" < '2024-05-20 14:03:48.132401'"

The where clause is based on events.end_at

Either the safe-autocorrect needs to detect and cope with this situation or this needs to be handled as an unsafe-autocorrection, ideally with documentation that explains the scenario(s) where it isn't safe.

The issue with this is that the where clause attempts to reference the end_at attribute on the Booking model rather than the Event model.

Actual behavior

This is the save auto-corrected method

    def over_due
      joins(:events).where(end_at: ...Time.zone.now)
    end

Converted to SQL this gives

> Booking.joins(:events).where(end_at: ...Time.zone.now).to_sql
=> "SELECT \"bookings\".* FROM \"bookings\" INNER JOIN \"events\" ON \"events\".\"eventable_type\" = 'Booking' AND \"events\".\"eventable_id\" = \"bookings\".\"id\" WHERE \"bookings\".\"end_at\" < '2024-05-20 14:03:56.321699'"

The where clause is based on bookings.end_at

Steps to reproduce the problem

rubocop -a app/models/booking.rb

RuboCop version

$ bundle exec rubocop -V
1.63.5 (using Parser 3.3.1.0, rubocop-ast 1.31.3, running on ruby 3.3.1) [arm64-darwin23]
  - rubocop-capybara 2.20.0
  - rubocop-factory_bot 2.25.1
  - rubocop-rails 2.25.0
  - rubocop-rspec 2.29.2
  - rubocop-rspec_rails 2.28.3

[fatkodima]

Are you on MySQL? Because for PostgreSQL, the original query produces an "ambiguous column" error.
We cannot reliably detect if the autocorrect is safe, so we should mark its autocorrection as unsafe.

[mark-young-atg]

@fatkodima I'm on PostgreSQL. Apologies if my attempt to extract the details from our code base has mislead you. I agree that it should be marked as unsafe.

[fatkodima]

So are you sure that joins(:events).where('end_at < ?', Time.zone.now) worked before? If both tables have a end_at column, Postgres will raise an error.

[mark-young-atg]

@fatkodima apologies, I suspect my write up was not clear enough. There is no end_at column on the Bookings table

dev=# select end_at from bookings;
ERROR:  column "end_at" does not exist
LINE 1: select end_at from bookings;
               ^
dev=# select end_at from events;
 end_at
--------
(0 rows)

[fatkodima]

So this issue should be closed?

[mark-young-atg]

No I'm not saying that. I'm trying to clarify the situation.

The application has Bookings, which does not have an end_at column, and Events which does have end_at.

The Vehicle model has many bookings and has a method called overdue? which seeks to find a limited set of bookings by using a scoping method called overdue in the Booking model:

In app/models/vehicle.rb

class Vehicle < ApplicationRecord
  has_many :bookings
  ...

  def overdue?
    bookings.over_due.where(status: %w[on_loan active]).any?
  end
end

In app/models/booking.rb

  def over_due
    joins(:events).where('end_at < ?', Time.zone.now)
  end

With that in mind when the cop is run in safe method it modifies the over_due method in the Bookings model to be

    def over_due
      joins(:events).where(end_at: ...Time.zone.now)
    end

Which is incorrect, and when it is referenced it fails:

> Vehicle.find(103).bookings.over_due.count
(irb):in `<main>': PG::UndefinedColumn: ERROR:  column bookings.end_at does not exist (ActiveRecord::StatementInvalid)
LINE 1: ...ings"."id" WHERE "bookings"."vehicle_id" = $2 AND "bookings"...
                                                             ^
/usr/src/app/vendor/bundle/ruby/3.3.0/gems/activerecord-7.1.3.3/lib/active_record/connection_adapters/postgresql_adapter.rb:894:in `exec_params': ERROR:  column bookings.end_at does not exist (PG::UndefinedColumn)
LINE 1: ...ings"."id" WHERE "bookings"."vehicle_id" = $2 AND "bookings"...
                                                             ^

Whereas when over_due is modified to be

    def over_due
      joins(:events).where(events: { end_at: ...Time.zone.now })
    end

Then it works and returns the bookings records

> Vehicle.find(1031197).bookings.over_due.count
=> 4

[fatkodima]

Got it, thanks.