From 6c0ea6c87fddf7e6a4e07d5599efedfb6ef8c325 Mon Sep 17 00:00:00 2001
From: Chris Gunther <chris@room118solutions.com>
Date: Tue, 11 Feb 2025 19:52:30 -0500
Subject: [PATCH] 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.
---
 CHANGELOG.md                               |  5 ++++
 lib/draper/decoratable.rb                  |  2 +-
 lib/draper/decoratable/collection_proxy.rb | 15 ----------
 lib/draper/decoratable/relation.rb         | 15 ++++++++++
 lib/draper/railtie.rb                      |  2 +-
 spec/dummy/spec/models/post_spec.rb        | 34 +++++++++++++++++++++-
 6 files changed, 55 insertions(+), 18 deletions(-)
 delete mode 100644 lib/draper/decoratable/collection_proxy.rb
 create mode 100644 lib/draper/decoratable/relation.rb

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0b362dc4..a4228a92 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,10 @@
 # Draper Changelog
 
+## Unreleased
+
+### Fixes
+* Fix decoration of AR relations [#944](https://github.com/drapergem/draper/pull/944)
+
 ## 4.0.4 - 2025-01-28
 
 ### Fixes
diff --git a/lib/draper/decoratable.rb b/lib/draper/decoratable.rb
index f8ecde81..e6e98028 100644
--- a/lib/draper/decoratable.rb
+++ b/lib/draper/decoratable.rb
@@ -12,7 +12,7 @@ module Decoratable
     extend ActiveSupport::Concern
     include Draper::Decoratable::Equality
 
-    autoload :CollectionProxy, 'draper/decoratable/collection_proxy'
+    autoload :Relation, 'draper/decoratable/relation'
 
     included do
       prepend Draper::Compatibility::Broadcastable if defined? Turbo::Broadcastable
diff --git a/lib/draper/decoratable/collection_proxy.rb b/lib/draper/decoratable/collection_proxy.rb
deleted file mode 100644
index 4a8873f5..00000000
--- a/lib/draper/decoratable/collection_proxy.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-module Draper
-  module Decoratable
-    module CollectionProxy
-      # Decorates a collection of objects. Used at the end of a scope chain.
-      #
-      # @example
-      #   company.products.popular.decorate
-      # @param [Hash] options
-      #   see {Decorator.decorate_collection}.
-      def decorate(options = {})
-        decorator_class.decorate_collection(load_target, options.reverse_merge(with: nil))
-      end
-    end
-  end
-end
diff --git a/lib/draper/decoratable/relation.rb b/lib/draper/decoratable/relation.rb
new file mode 100644
index 00000000..ee29e304
--- /dev/null
+++ b/lib/draper/decoratable/relation.rb
@@ -0,0 +1,15 @@
+module Draper
+  module Decoratable
+    module Relation
+      # Decorates a relation of objects. Used at the end of a scope chain.
+      #
+      # @example
+      #   Product.popular.decorate
+      # @param [Hash] options
+      #   see {Decorator.decorate_collection}.
+      def decorate(options = {})
+        decorator_class.decorate_collection(self, options.reverse_merge(with: nil))
+      end
+    end
+  end
+end
diff --git a/lib/draper/railtie.rb b/lib/draper/railtie.rb
index f5d25e0e..3ff089d2 100644
--- a/lib/draper/railtie.rb
+++ b/lib/draper/railtie.rb
@@ -36,7 +36,7 @@ class Railtie < Rails::Railtie
       ActiveSupport.on_load :active_record do
         include Draper::Decoratable
 
-        ActiveRecord::Associations::CollectionProxy.include Draper::Decoratable::CollectionProxy
+        ActiveRecord::Relation.include Draper::Decoratable::Relation
       end
 
       ActiveSupport.on_load :mongoid do
diff --git a/spec/dummy/spec/models/post_spec.rb b/spec/dummy/spec/models/post_spec.rb
index 51ce22a0..4de3f6ba 100644
--- a/spec/dummy/spec/models/post_spec.rb
+++ b/spec/dummy/spec/models/post_spec.rb
@@ -21,6 +21,38 @@
     end
   end if defined? Turbo::Broadcastable
 
+  describe 'relations' do
+    context 'when decorated' do
+      subject { relation.decorate }
+
+      let(:relation)   { described_class.where('1=1') }
+      let(:persisted)  { relation.create! [{}] * rand(0..2) }
+
+      before { persisted } # should exist
+
+      it 'returns a decorated collection' do
+        is_expected.to match_array persisted
+        is_expected.to be_all &:decorated?
+      end
+
+      it 'uses cached records' do
+        expect(relation).not_to be_loaded
+
+        relation.load
+
+        expect { subject.to_a }.to execute.exactly(0).queries
+      end
+
+      it 'caches records' do
+        expect(relation).not_to be_loaded
+
+        relation.decorate.to_a
+
+        expect { subject.to_a; relation.load }.to execute.exactly(0).queries
+      end
+    end
+  end
+
   describe 'associations' do
     context 'when decorated' do
       subject { associated.decorate }
@@ -47,7 +79,7 @@
       it 'caches records' do
         expect(associated).not_to be_loaded
 
-        associated.decorate
+        associated.decorate.to_a
 
         expect { subject.to_a; associated.load }.to execute.exactly(0).queries
       end