Skip to content

Commit bb295d1

Browse files
committed
Relations: #decorate queries DB every time
Similar to #932 for associations, calling `#decorate` on a relation loads the records every time, regardless of whether the underlying relation has already been loaded. This is because the default of passing `all` as the collection to decorate ends up being a new relation, so even if it were previously loaded, the new relation including `all` needs to be loaded. `ActiveRecord::Associations::CollectionProxy` actually inherits from `ActiveRecord::Relation`, so by pushing this up to the relation, we can cover both scenarios. https://github.com/rails/rails/blob/57c24948eb5cc9e5f9a4cecb6f2060f53e2246e1/activerecord/lib/active_record/associations/collection_proxy.rb#L31 The documentation included in #932 (`company.products.popular.decorate`) wasn't correct as you're decorating a relation there (`popular`), not just the association (`products`). This now covers the association itself (`company.products.decorate`), as well as a relation from that association (`company.products.popular.decorate`), as well as a relation right from the class without associations (`Product.popular.decorate`). Decorating the class itself (`Product.decorate`) still calls `all` since the model class is not a relation itself.
1 parent 52ea163 commit bb295d1

File tree

6 files changed

+55
-18
lines changed

6 files changed

+55
-18
lines changed

CHANGELOG.md

+5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
# Draper Changelog
22

3+
## Unreleased
4+
5+
### Fixes
6+
* Fix decoration of AR relations [#944](https://github.com/drapergem/draper/pull/944)
7+
38
## 4.0.4 - 2025-01-28
49

510
### Fixes

lib/draper/decoratable.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ module Decoratable
1212
extend ActiveSupport::Concern
1313
include Draper::Decoratable::Equality
1414

15-
autoload :CollectionProxy, 'draper/decoratable/collection_proxy'
15+
autoload :Relation, 'draper/decoratable/relation'
1616

1717
included do
1818
prepend Draper::Compatibility::Broadcastable if defined? Turbo::Broadcastable

lib/draper/decoratable/collection_proxy.rb

-15
This file was deleted.

lib/draper/decoratable/relation.rb

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
module Draper
2+
module Decoratable
3+
module Relation
4+
# Decorates a relation of objects. Used at the end of a scope chain.
5+
#
6+
# @example
7+
# Product.popular.decorate
8+
# @param [Hash] options
9+
# see {Decorator.decorate_collection}.
10+
def decorate(options = {})
11+
decorator_class.decorate_collection(self, options.reverse_merge(with: nil))
12+
end
13+
end
14+
end
15+
end

lib/draper/railtie.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class Railtie < Rails::Railtie
3636
ActiveSupport.on_load :active_record do
3737
include Draper::Decoratable
3838

39-
ActiveRecord::Associations::CollectionProxy.include Draper::Decoratable::CollectionProxy
39+
ActiveRecord::Relation.include Draper::Decoratable::Relation
4040
end
4141

4242
ActiveSupport.on_load :mongoid do

spec/dummy/spec/models/post_spec.rb

+33-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,38 @@
2121
end
2222
end if defined? Turbo::Broadcastable
2323

24+
describe 'relations' do
25+
context 'when decorated' do
26+
subject { relation.decorate }
27+
28+
let(:relation) { described_class.where('1=1') }
29+
let(:persisted) { relation.create! [{}] * rand(0..2) }
30+
31+
before { persisted } # should exist
32+
33+
it 'returns a decorated collection' do
34+
is_expected.to match_array persisted
35+
is_expected.to be_all &:decorated?
36+
end
37+
38+
it 'uses cached records' do
39+
expect(relation).not_to be_loaded
40+
41+
relation.load
42+
43+
expect { subject.to_a }.to execute.exactly(0).queries
44+
end
45+
46+
it 'caches records' do
47+
expect(relation).not_to be_loaded
48+
49+
relation.decorate.to_a
50+
51+
expect { subject.to_a; relation.load }.to execute.exactly(0).queries
52+
end
53+
end
54+
end
55+
2456
describe 'associations' do
2557
context 'when decorated' do
2658
subject { associated.decorate }
@@ -47,7 +79,7 @@
4779
it 'caches records' do
4880
expect(associated).not_to be_loaded
4981

50-
associated.decorate
82+
associated.decorate.to_a
5183

5284
expect { subject.to_a; associated.load }.to execute.exactly(0).queries
5385
end

0 commit comments

Comments
 (0)