Skip to content
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

Removes item_subtype requirement for model-specific limits. #1309

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 0 additions & 16 deletions lib/paper_trail/model_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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?
jaredbeck marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
2 changes: 0 additions & 2 deletions lib/paper_trail/reifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
26 changes: 14 additions & 12 deletions lib/paper_trail/version_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
jaredbeck marked this conversation as resolved.
Show resolved Hide resolved
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
11 changes: 0 additions & 11 deletions spec/paper_trail/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 15 additions & 0 deletions spec/paper_trail/version_limit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down