diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d1c2f05..2b086661 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ This project follows [semver 2.0.0](http://semver.org/spec/v2.0.0.html) and the recommendations of [keepachangelog.com](http://keepachangelog.com/). ## Unreleased - +- [#1309](https://github.com/paper-trail-gem/paper_trail/pull/1309) - + Removes `item_subtype` requirement when specifying model-specific limits. ### Breaking Changes - None diff --git a/README.md b/README.md index 9fdc7a34..f7a1562b 100644 --- a/README.md +++ b/README.md @@ -613,10 +613,6 @@ has_paper_trail limit: 2 has_paper_trail limit: nil ``` -To use a per-model limit, your `versions` table must have an -`item_subtype` column. See [Section -4.b.1](https://github.com/paper-trail-gem/paper_trail#4b1-the-optional-item_subtype-column). - ## 3. Working With Versions ### 3.a. Reverting And Undeleting A Model diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb index c7fc8b21..b976bd5b 100644 --- a/lib/paper_trail/model_config.rb +++ b/lib/paper_trail/model_config.rb @@ -18,11 +18,6 @@ class ModelConfig `abstract_class`. This is fine, but all application models must be configured to use concrete (not abstract) version models. STR - E_MODEL_LIMIT_REQUIRES_ITEM_SUBTYPE = <<~STR.squish.freeze - To use PaperTrail's per-model limit in your %s model, you must have an - item_subtype column in your versions table. See documentation sections - 2.e.1 Per-model limit, and 4.b.1 The optional item_subtype column. - 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. @@ -124,7 +119,6 @@ def setup(options = {}) @model_class.send :include, ::PaperTrail::Model::InstanceMethods setup_options(options) setup_associations(options) - check_presence_of_item_subtype_column(options) @model_class.after_rollback { paper_trail.clear_rolled_back_versions } setup_callbacks_from_options options[:on] end @@ -151,16 +145,6 @@ def cannot_record_after_destroy? ::ActiveRecord::Base.belongs_to_required_by_default end - # Some options require the presence of the `item_subtype` column. Currently - # only `limit`, but in the future there may be others. - # - # @api private - def check_presence_of_item_subtype_column(options) - return unless options.key?(:limit) - return if version_class.item_subtype_column_present? - raise Error, format(E_MODEL_LIMIT_REQUIRES_ITEM_SUBTYPE, @model_class.name) - end - def check_version_class_name(options) # @api private - `version_class_name` @model_class.class_attribute :version_class_name diff --git a/lib/paper_trail/reifier.rb b/lib/paper_trail/reifier.rb index fbb587fb..e7d192de 100644 --- a/lib/paper_trail/reifier.rb +++ b/lib/paper_trail/reifier.rb @@ -117,8 +117,6 @@ def reify_attributes(model, version, attrs) # this method returns the constant `Dog`. If `attrs["species"]` is blank, # this method returns the constant `Animal`. You can see this particular # example in action in `spec/models/animal_spec.rb`. - # - # TODO: Duplication: similar `constantize` in VersionConcern#version_limit def version_reification_class(version, attrs) inheritance_column_name = version.item_type.constantize.inheritance_column inher_col_value = attrs[inheritance_column_name] diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 398cdac2..edd39997 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -23,10 +23,6 @@ module VersionConcern # :nodoc: module ClassMethods - def item_subtype_column_present? - column_names.include?("item_subtype") - end - def with_item_keys(item_type, item_id) where item_type: item_type, item_id: item_id end @@ -379,16 +375,22 @@ def sibling_versions # The version limit can be global or per-model. # # @api private - # - # TODO: Duplication: similar `constantize` in Reifier#version_reification_class def version_limit - if self.class.item_subtype_column_present? - klass = (item_subtype || item_type).constantize - if klass&.paper_trail_options&.key?(:limit) - return klass.paper_trail_options[:limit] - end + if limit_option?(item.class) + item.class.paper_trail_options[:limit] + elsif base_class_limit_option?(item.class) + item.class.base_class.paper_trail_options[:limit] + else + PaperTrail.config.version_limit end - PaperTrail.config.version_limit + end + + def limit_option?(klass) + klass.respond_to?(:paper_trail_options) && klass.paper_trail_options.key?(:limit) + end + + def base_class_limit_option?(klass) + klass.respond_to?(:base_class) && limit_option?(klass.base_class) end end end diff --git a/spec/paper_trail/config_spec.rb b/spec/paper_trail/config_spec.rb index f91d1ba7..c2e2eafa 100644 --- a/spec/paper_trail/config_spec.rb +++ b/spec/paper_trail/config_spec.rb @@ -52,17 +52,6 @@ module PaperTrail limited_bike.save assert_equal 2, limited_bike.versions.length end - - context "when item_subtype column is absent" do - it "uses global version_limit" do - PaperTrail.config.version_limit = 6 - names = PaperTrail::Version.column_names - ["item_subtype"] - allow(PaperTrail::Version).to receive(:column_names).and_return(names) - bike = LimitedBicycle.create!(name: "My Bike") # has_paper_trail limit: 3 - 10.times { bike.update(name: SecureRandom.hex(8)) } - assert_equal 7, bike.versions.length - end - end end end end diff --git a/spec/paper_trail/version_limit_spec.rb b/spec/paper_trail/version_limit_spec.rb index f981b8af..4899ff5e 100644 --- a/spec/paper_trail/version_limit_spec.rb +++ b/spec/paper_trail/version_limit_spec.rb @@ -21,6 +21,21 @@ module PaperTrail # 4 versions = 3 updates + 1 create. end + it "cleans up old versions with limit specified on base class" do + PaperTrail.config.version_limit = 10 + + Animal.paper_trail_options[:limit] = 5 + Dog.paper_trail_options = Animal.paper_trail_options.without(:limit) + + dog = Dog.create(name: "Fluffy") # Dog specified has_paper_trail with no limit option + + 15.times do |i| + dog.update(name: "Name #{i}") + end + expect(Dog.find(dog.id).versions.count).to eq(6) # Dog uses limit option on base class, Animal + # 6 versions = 5 updates + 1 create. + end + it "cleans up old versions" do PaperTrail.config.version_limit = 10 widget = Widget.create