diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ceeb2cb..a159aae8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,9 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). ### Added -- None +- [#961](https://github.com/airblade/paper_trail/issues/961) - Instead of + crashing when misconfigured Custom Version Classes are used, an error will be + raised earlier, with a much more helpful message. ### Fixed diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb index 3d6b096f..216e4722 100644 --- a/lib/paper_trail/model_config.rb +++ b/lib/paper_trail/model_config.rb @@ -12,6 +12,16 @@ module PaperTrail or disable belongs_to_required_by_default. STR + E_HPT_ABSTRACT_CLASS = <<~STR.squish.freeze + An application model (%s) has been configured to use PaperTrail (via + `has_paper_trail`), but the version model it has been told to use (%s) is + an `abstract_class`. This could happen when an advanced feature called + Custom Version Classes (http://bit.ly/2G4ch0G) is misconfigured. When all + version classes are custom, PaperTrail::Version is configured to be an + `abstract_class`. This is fine, but all application models must be + configured to use concrete (not abstract) version models. + STR + def initialize(model_class) @model_class = model_class end @@ -98,6 +108,14 @@ module PaperTrail Gem::Version.new(ActiveRecord::VERSION::STRING) end + # Raises an error if the provided class is an `abstract_class`. + # @api private + def assert_concrete_activerecord_class(class_name) + if class_name.constantize.abstract_class? + raise format(E_HPT_ABSTRACT_CLASS, @model_class, class_name) + end + end + def cannot_record_after_destroy? Gem::Version.new(ActiveRecord::VERSION::STRING).release >= Gem::Version.new("5") && ::ActiveRecord::Base.belongs_to_required_by_default @@ -123,6 +141,8 @@ module PaperTrail @model_class.send :attr_accessor, :paper_trail_event + assert_concrete_activerecord_class(@model_class.version_class_name) + @model_class.has_many( @model_class.versions_association_name, -> { order(model.timestamp_sort_order) }, diff --git a/spec/dummy_app/app/versions/abstract_version.rb b/spec/dummy_app/app/versions/abstract_version.rb new file mode 100644 index 00000000..a55e07ca --- /dev/null +++ b/spec/dummy_app/app/versions/abstract_version.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class AbstractVersion < ActiveRecord::Base + include PaperTrail::VersionConcern + self.abstract_class = true +end diff --git a/spec/paper_trail/model_config_spec.rb b/spec/paper_trail/model_config_spec.rb new file mode 100644 index 00000000..d512e0f4 --- /dev/null +++ b/spec/paper_trail/model_config_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require "spec_helper" + +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" + end + }.to raise_error( + /use concrete \(not abstract\) version models/ + ) + end + end + end +end