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

Deprecate delegating to `arel` in `Relation` #29619

Merged
merged 1 commit into from Jun 29, 2017

Conversation

Projects
None yet
5 participants
@kamipo
Member

kamipo commented Jun 28, 2017

Active Record doesn't rely delegating to arel in the internal since
425f2ca. The delegation is a lower priority than delegating to klass,
so it is pretty unclear which method is delegated to arel.

For example, bind_values method was removed at b06f64c (a series of
changes 79f71d3...b06f64c). But a
relation still could respond to the method because arel also have the
same named method (#28976).

Removing the delegation will achieve predictable behavior.

activerecord/lib/active_record/relation/delegation.rb Outdated
@@ -89,6 +89,9 @@ def method_missing(method, *args, &block)
self.class.delegate_to_scoped_klass(method)
scoping { @klass.public_send(method, *args, &block) }
elsif arel.respond_to?(method)
ActiveSupport::Deprecation.warn \
"Delegating #{method} to arel is deprecated and will be removed in Rails 5.3. " \
"Use relation.arel.#{method} instead."

This comment has been minimized.

@kaspth

kaspth Jun 29, 2017

Member

Isn't arel still private API? So the delegation makes sense?

This comment has been minimized.

@rafaelfranca

rafaelfranca Jun 29, 2017

Member

Right. Better to say that is going to be removed with no replacement. Also the next version of Rails after 5.2 is 6.0

Deprecate delegating to `arel` in `Relation`
Active Record doesn't rely delegating to `arel` in the internal since
425f2ca. The delegation is a lower priority than delegating to `klass`,
so it is pretty unclear which method is delegated to `arel`.

For example, `bind_values` method was removed at b06f64c (a series of
changes 79f71d3...b06f64c). But a
relation still could respond to the method because `arel` also have the
same named method (#28976).

Removing the delegation will achieve predictable behavior.

@kaspth kaspth merged commit a317af9 into rails:master Jun 29, 2017

1 check passed

codeclimate no new or fixed issues
Details
@kaspth

This comment has been minimized.

Member

kaspth commented Jun 29, 2017

Great!

@kamipo

This comment has been minimized.

Member

kamipo commented Jun 29, 2017

Thanks!

@kamipo kamipo deleted the kamipo:deprecate_delegating_to_arel_in_relation branch Jun 29, 2017

kamipo added a commit to kamipo/rails that referenced this pull request Jul 1, 2017

@samstickland

This comment has been minimized.

samstickland commented May 28, 2018

Not sure if this is the right place for this question, but I'm unsure how to move this to non-deprecated calls (short of manually generating SQL):

class TimesheetBlock
  scope :with_entries, -> {
    where(TimesheetEntry.where('timesheet_block_id = timesheet_blocks.id').exists)
  }
end

This generates SQL like this:

SELECT "timesheet_blocks".* FROM "timesheet_blocks" WHERE EXISTS (SELECT "timesheet_entries".* FROM "timesheet_entries" WHERE (timesheet_block_id = timesheet_blocks.id))

But gives this deprecation warning:

Delegating exists to arel is deprecated and will be removed in Rails 6.0

@kamipo

This comment has been minimized.

Member

kamipo commented May 28, 2018

Please use .arel.exists instead.

@samstickland

This comment has been minimized.

samstickland commented May 28, 2018

@kamipo Thanks!

koic added a commit to koic/24pullrequests that referenced this pull request Sep 5, 2018

Suppress "Delegating exists to arel is deprecated" warning
This commit suppresses the following Active Record warnings.

```console
DEPRECATION WARNING: Delegating exists to arel is deprecated and will be
removed in Rails 6.0. (called from pull_request_filter at
/Users/koic/src/github.com/24pullrequests/24pullrequests/app/models/aggregation_filter.rb:7)

DEPRECATION WARNING: Delegating with to arel is deprecated and will be
removed in Rails 6.0. (called from block (4 levels) in <top (required)>
at /Users/koic/src/github.com/24pullrequests/24pullrequests/spec/controllers/users_controller_spec.rb:16)
```

This patch is based on the following comment.
rails/rails#29619 (comment)

andrew added a commit to 24pullrequests/24pullrequests that referenced this pull request Sep 5, 2018

Suppress "Delegating exists to arel is deprecated" warning
This commit suppresses the following Active Record warnings.

```console
DEPRECATION WARNING: Delegating exists to arel is deprecated and will be
removed in Rails 6.0. (called from pull_request_filter at
/Users/koic/src/github.com/24pullrequests/24pullrequests/app/models/aggregation_filter.rb:7)

DEPRECATION WARNING: Delegating with to arel is deprecated and will be
removed in Rails 6.0. (called from block (4 levels) in <top (required)>
at /Users/koic/src/github.com/24pullrequests/24pullrequests/spec/controllers/users_controller_spec.rb:16)
```

This patch is based on the following comment.
rails/rails#29619 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment