Merge pull request #1158 from TylerRick/pass_options_to_has_many

Add the ability to pass options to the has_many :versions association macro

CHANGELOG.md

@@ -9,9 +9,16 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 - None
 
+### Deprecated
+
+- [#1158](https://github.com/paper-trail-gem/paper_trail/pull/1158) — Passing association name as
+  `versions:` option or Version class name as `class_name:` options directly to `has_paper_trail`.
+  Use `has_paper_trail versions: {name: :drafts, class_name: "MyVersionModel"}` instead.
+
 ### Added
 
-- None
+- [#1158](https://github.com/paper-trail-gem/paper_trail/pull/1158) — Add the ability to pass
+  options, such as `scope` or `extend:` to the `has_many :versions` association macro.
 
 ### Fixed
 

README.md

@@ -955,7 +955,7 @@ see https://github.com/paper-trail-gem/paper_trail/issues/594
 ### 5.b. Configuring the `versions` Association
 
 You may configure the name of the `versions` association by passing
-a different name to `has_paper_trail`.
+a different name to `has_paper_trail`:
 
 ```ruby
 class Post < ActiveRecord::Base
@@ -968,6 +968,23 @@ Post.new.versions # => NoMethodError
 Overriding (instead of configuring) the `versions` method is not supported.
 Overriding associations is not recommended in general.
 
+You may pass other options for the `has_many` by passing a hash of options:
+
+```ruby
+class Post < ActiveRecord::Base
+  has_paper_trail versions: {
+    name: :drafts,
+    scope: -> { order("id desc") },
+    extend: VersionsExtensions
+  }
+end
+```
+
+Refer to
+https://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html#method-i-has_many-label-Options
+for the full list of supported options for `has_many`.
+
+
 ### 5.c. Generators
 
 PaperTrail has one generator, `paper_trail:install`. It writes, but does not

lib/paper_trail/model_config.rb

@@ -18,6 +18,15 @@ class ModelConfig
       `abstract_class`. This is fine, but all application models must be
       configured to use concrete (not abstract) version models.
     STR
+    DPR_PASSING_ASSOC_NAME_DIRECTLY_TO_VERSIONS_OPTION = <<~STR.squish
+      Passing versions association name as `has_paper_trail versions: %{versions_name}`
+      is deprecated. Use `has_paper_trail versions: {name: %{versions_name}}` instead.
+    STR
+    DPR_CLASS_NAME_OPTION = <<~STR.squish
+      Passing Version class name as `has_paper_trail class_name: %{class_name}`
+      is deprecated. Use `has_paper_trail versions: {class_name: %{class_name}}`
+      instead.
+    STR
 
     def initialize(model_class)
       @model_class = model_class
@@ -138,41 +147,84 @@ def setup_associations(options)
       # @api public
       @model_class.send :attr_accessor, @model_class.version_association_name
 
-      # @api private - `version_class_name` - However, `rails_admin` has been
-      # using it since 2014 (see `rails_admin/extensions/paper_trail/auditing_adapter.rb`,
-      # https://github.com/sferik/rails_admin/commit/959e1bd4e47e0369d264b58bbbe972ff863767cd)
-      # In PR _____ () we ask them to use `paper_trail_options` instead.
-      @model_class.class_attribute :version_class_name
-      @model_class.version_class_name = options[:class_name] || "PaperTrail::Version"
-
-      # @api private - versions_association_name
-      @model_class.class_attribute :versions_association_name
-      @model_class.versions_association_name = options[:versions] || :versions
-
       # @api public - paper_trail_event
       @model_class.send :attr_accessor, :paper_trail_event
 
-      assert_concrete_activerecord_class(@model_class.version_class_name)
+      define_has_many_versions(options)
+    end
+
+    def define_has_many_versions(options)
+      options = ensure_versions_option_is_hash(options)
+      check_version_class_name(options)
+      check_versions_association_name(options)
+      scope = get_versions_scope(options)
+      options[:versions].assert_valid_keys(valid_keys_for_has_many)
 
-      # @api public
       @model_class.has_many(
         @model_class.versions_association_name,
-        -> { order(model.timestamp_sort_order) },
+        scope,
         class_name: @model_class.version_class_name,
-        as: :item
+        as: :item,
+        **options[:versions]
       )
     end
 
+    def ensure_versions_option_is_hash(options)
+      unless options[:versions].is_a?(Hash)
+        if options[:versions]
+          ::ActiveSupport::Deprecation.warn(
+            format(
+              DPR_PASSING_ASSOC_NAME_DIRECTLY_TO_VERSIONS_OPTION,
+              versions_name: options[:versions].inspect
+            ),
+            caller(1)
+          )
+        end
+        options[:versions] = {
+          name: options[:versions]
+        }
+      end
+      options
+    end
+
+    def check_version_class_name(options)
+      # @api private - `version_class_name`
+      @model_class.class_attribute :version_class_name
+      if options[:class_name]
+        ::ActiveSupport::Deprecation.warn(
+          format(
+            DPR_CLASS_NAME_OPTION,
+            class_name: options[:class_name].inspect
+          ),
+          caller(1)
+        )
+        options[:versions][:class_name] = options.delete(:class_name)
+      end
+      @model_class.version_class_name = options[:versions][:class_name] || "PaperTrail::Version"
+      assert_concrete_activerecord_class(@model_class.version_class_name)
+    end
+
+    def check_versions_association_name(options)
+      # @api private - versions_association_name
+      @model_class.class_attribute :versions_association_name
+      @model_class.versions_association_name = options[:versions].delete(:name) || :versions
+    end
+
+    def get_versions_scope(options)
+      options[:versions].delete(:scope) || -> { order(model.timestamp_sort_order) }
+    end
+
     def setup_callbacks_from_options(options_on = [])
       options_on.each do |event|
         public_send("on_#{event}")
       end
     end
 
     def setup_options(options)
-      # @api public - paper_trail_options - Let's encourage plugins to use
-      # eg. `paper_trail_options[:class_name]` rather than `version_class_name`
-      # because the former is documented and the latter is not.
+      # @api public - paper_trail_options - Let's encourage plugins to use eg.
+      # `paper_trail_options[:versions][:class_name]` rather than
+      # `version_class_name` because the former is documented and the latter is
+      # not.
       @model_class.class_attribute :paper_trail_options
       @model_class.paper_trail_options = options.dup
 
@@ -185,5 +237,19 @@ def setup_options(options)
 
       @model_class.paper_trail_options[:meta] ||= {}
     end
+
+    # @api private
+    def valid_keys_for_has_many
+      if ActiveRecord.gem_version >= Gem::Version.new("5.0")
+        ActiveRecord::Associations::Builder::HasMany.valid_options({})
+      else
+        %i[
+          after_add after_remove anonymous_class as autosave before_add
+          before_remove class_name counter_cache dependent extend foreign_key
+          foreign_type inverse_of join_table primary_key source source_type
+          table_name through validate
+        ]
+      end
+    end
   end
 end

spec/dummy_app/app/models/concerns/prefix_versions_inspect_with_count.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+module PrefixVersionsInspectWithCount
+  def inspect
+    "#{length} versions:\n" +
+      super
+  end
+end

spec/dummy_app/app/models/custom_primary_key_record.rb

@@ -5,7 +5,7 @@
 class CustomPrimaryKeyRecord < ActiveRecord::Base
   self.primary_key = :uuid
 
-  has_paper_trail class_name: "CustomPrimaryKeyRecordVersion"
+  has_paper_trail versions: { class_name: "CustomPrimaryKeyRecordVersion" }
 
   # This default_scope is to test the case of the Version#item association
   # not returning the item due to unmatched default_scope on the model.

spec/dummy_app/app/models/document.rb

@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
-# Demonstrates a "custom versions association name". Instead of the assication
+# Demonstrates a "custom versions association name". Instead of the association
 # being named `versions`, it will be named `paper_trail_versions`.
 class Document < ActiveRecord::Base
   has_paper_trail(
-    versions: :paper_trail_versions,
+    versions: { name: :paper_trail_versions },
     on: %i[create update]
   )
 end

spec/dummy_app/app/models/fruit.rb

@@ -2,6 +2,6 @@
 
 class Fruit < ActiveRecord::Base
   if ENV["DB"] == "postgres" || JsonVersion.table_exists?
-    has_paper_trail class_name: "JsonVersion"
+    has_paper_trail versions: { class_name: "JsonVersion" }
   end
 end

spec/dummy_app/app/models/kitchen/banana.rb

@@ -2,6 +2,6 @@
 
 module Kitchen
   class Banana < ActiveRecord::Base
-    has_paper_trail class_name: "Kitchen::BananaVersion"
+    has_paper_trail versions: { class_name: "Kitchen::BananaVersion" }
   end
 end

spec/dummy_app/app/models/no_object.rb

@@ -3,7 +3,7 @@
 # Demonstrates a table that omits the `object` column.
 class NoObject < ActiveRecord::Base
   has_paper_trail(
-    class_name: "NoObjectVersion",
+    versions: { class_name: "NoObjectVersion" },
     meta: { metadatum: 42 }
   )
   validates :letter, length: { is: 1 }, presence: true

spec/dummy_app/app/models/post.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 class Post < ActiveRecord::Base
-  has_paper_trail class_name: "PostVersion"
+  has_paper_trail versions: { class_name: "PostVersion" }
 end

spec/dummy_app/app/models/thing.rb

@@ -1,7 +1,10 @@
 # frozen_string_literal: true
 
 class Thing < ActiveRecord::Base
-  has_paper_trail
+  has_paper_trail versions: {
+    extend: PrefixVersionsInspectWithCount,
+    scope: -> { order("id desc") }
+  }
 
   if ActiveRecord.gem_version >= Gem::Version.new("5.0")
     belongs_to :person, optional: true

spec/models/thing_spec.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe Thing, type: :model do
+  describe "#versions", versioning: true do
+    let(:thing) { Thing.create! }
+
+    it "applies the extend option" do
+      expect(thing.versions.singleton_class).to be < PrefixVersionsInspectWithCount
+      expect(thing.versions.inspect).to start_with("1 versions:")
+    end
+
+    it "applies the scope option" do
+      expect(Thing.reflect_on_association(:versions).scope).to be_a Proc
+      expect(thing.versions.to_sql).to end_with "ORDER BY id desc"
+    end
+  end
+end

spec/paper_trail/model_config_spec.rb

@@ -4,15 +4,94 @@
 
 module PaperTrail
   ::RSpec.describe ModelConfig do
-    describe "when has_paper_trail is called" do
-      it "raises an error" do
-        expect {
-          class MisconfiguredCVC < ActiveRecord::Base
-            has_paper_trail class_name: "AbstractVersion"
+    describe "has_paper_trail" do
+      describe "versions:" do
+        it "name can be passed instead of an options hash", :deprecated do
+          allow(::ActiveSupport::Deprecation).to receive(:warn)
+          klass = Class.new(ActiveRecord::Base) do
+            has_paper_trail versions: :drafts
           end
-        }.to raise_error(
-          /use concrete \(not abstract\) version models/
-        )
+          expect(klass.reflect_on_association(:drafts)).to be_a(
+            ActiveRecord::Reflection::HasManyReflection
+          )
+          expect(::ActiveSupport::Deprecation).to have_received(:warn).once.with(
+            %(Passing versions association name as `has_paper_trail versions: :drafts` is ) +
+            %(deprecated. Use `has_paper_trail versions: {name: :drafts}` instead.),
+            array_including(
+              /#{__FILE__}:/
+            )
+          )
+        end
+        it "name can be passed in the options hash" do
+          klass = Class.new(ActiveRecord::Base) do
+            has_paper_trail versions: { name: :drafts }
+          end
+          expect(klass.reflect_on_association(:drafts)).to be_a(
+            ActiveRecord::Reflection::HasManyReflection
+          )
+        end
+        it "class_name can be passed in the options hash" do
+          klass = Class.new(ActiveRecord::Base) do
+            has_paper_trail versions: { class_name: "NoObjectVersion" }
+          end
+          expect(klass.reflect_on_association(:versions).options[:class_name]).to eq(
+            "NoObjectVersion"
+          )
+        end
+        it "allows any option that has_many supports" do
+          klass = Class.new(ActiveRecord::Base) do
+            has_paper_trail versions: { autosave: true, validate: true }
+          end
+          expect(klass.reflect_on_association(:versions).options[:autosave]).to eq true
+          expect(klass.reflect_on_association(:versions).options[:validate]).to eq true
+        end
+        it "can even override options that PaperTrail adds to has_many" do
+          klass = Class.new(ActiveRecord::Base) do
+            has_paper_trail versions: { as: :foo }
+          end
+          expect(klass.reflect_on_association(:versions).options[:as]).to eq :foo
+        end
+        it "raises an error on unknown has_many options" do
+          expect {
+            Class.new(ActiveRecord::Base) do
+              has_paper_trail versions: { read_my_mind: true, validate: true }
+            end
+          }.to raise_error(
+            /Unknown key: :read_my_mind. Valid keys are: .*:class_name,/
+          )
+        end
+
+        describe "passing an abstract class to class_name" do
+          it "raises an error" do
+            expect {
+              Class.new(ActiveRecord::Base) do
+                has_paper_trail versions: { class_name: "AbstractVersion" }
+              end
+            }.to raise_error(
+              /use concrete \(not abstract\) version models/
+            )
+          end
+        end
+      end
+
+      describe "class_name:" do
+        it "can be used instead of versions: {class_name: ...}", :deprecated do
+          allow(::ActiveSupport::Deprecation).to receive(:warn)
+          klass = Class.new(ActiveRecord::Base) do
+            has_paper_trail class_name: "NoObjectVersion"
+          end
+          expect(klass.reflect_on_association(:versions).options[:class_name]).to eq(
+            "NoObjectVersion"
+          )
+          expect(::ActiveSupport::Deprecation).to have_received(:warn).once.with(
+            %(Passing Version class name as `has_paper_trail class_name: "NoObjectVersion"` ) +
+            %(is deprecated. Use `has_paper_trail versions: {class_name: "NoObjectVersion"}` ) +
+            %(instead.),
+            array_including(
+              /#{__FILE__}:/
+            )
+          )
+        end
       end
     end
   end