Skip to content

Commit

Permalink
An idea to combat model pollution
Browse files Browse the repository at this point in the history 
Problem
-------

`has_paper_trail` adds too many methods to the ActiveRecord model.

> If I'm counting correctly, installing the paper_trail gem adds 36 instance
> methods and 10 class methods to all of your active record models. Of those
> 46, 13 have a prefix, either "pt_" or "paper_trail_". I don't know what the
> best way to deal with this is. Ideally, we'd add far fewer methods to
> people's models. If we were able to add fewer methods to models, then I
> wouldn't mind prefixing all of them.
> #703

Solution
--------

Add only two methods to the AR model.

1. An instance method `#paper_trail`
2. A class method `.paper_trail`

The instance method would return a `RecordTrail` and the class method would
return a `ClassTrail`. Those names are totally up for debate.

Advantages
----------

- Plain ruby, easy to understand
- Adding new methods to e.g. the `RecordTrail` does not add any methods to
  the AR model.
- Methods privacy is more strongly enforced.
- Enables isolated testing of e.g. `RecordTrail`; it can be constructed with a
  mock AR instance.

Disadvantages
-------------

- Two new classes, though they are simple.
  • Loading branch information
@jaredbeck
jaredbeck committed May 20, 2016
1 parent 8ad601b commit 46d9398
Show file tree
Hide file tree
Showing 17 changed files with 429 additions and 290 deletions.
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Metrics/CyclomaticComplexity:
Max: 13 # Goal: 6

Metrics/ModuleLength:
Max: 313
Max: 317

Metrics/PerceivedComplexity:
Max: 16 # Goal: 7
232 changes: 76 additions & 156 deletions lib/paper_trail/has_paper_trail.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
require "paper_trail/attribute_serializers/legacy_active_record_shim"
require "paper_trail/attribute_serializers/object_attribute"
require "paper_trail/attribute_serializers/object_changes_attribute"
require "paper_trail/model_config"
require "paper_trail/record_trail"

module PaperTrail
# Extensions to `ActiveRecord::Base`. See `frameworks/active_record.rb`.
Expand Down Expand Up @@ -64,8 +66,7 @@ def has_paper_trail(options = {})
# symbol to be passed in.
options[:on] = Array(options[:on])

setup_model_for_paper_trail(options)

paper_trail.setup(options)
setup_callbacks_from_options options[:on]

setup_callbacks_for_habtm options[:join_tables]
Expand Down Expand Up @@ -105,50 +106,19 @@ def setup_callbacks_for_habtm(join_tables)
end
end

# Installs callbacks, associations, "class attributes", and more.
# For details of how "class attributes" work, see the activesupport docs.
# @api private
def setup_model_for_paper_trail(options = {})
# Lazily include the instance methods so we don't clutter up
# any more ActiveRecord models than we have to.
send :include, InstanceMethods

if ::ActiveRecord::VERSION::STRING < "4.2"
send :extend, AttributeSerializers::LegacyActiveRecordShim
end

class_attribute :version_association_name
self.version_association_name = options[:version] || :version

# The version this instance was reified from.
attr_accessor version_association_name

class_attribute :version_class_name
self.version_class_name = options[:class_name] || "PaperTrail::Version"

setup_paper_trail_options(options)

class_attribute :versions_association_name
self.versions_association_name = options[:versions] || :versions

attr_accessor :paper_trail_event
def paper_trail
::PaperTrail::ModelConfig.new(self)
end

# `has_many` syntax for specifying order uses a lambda in Rails 4
if ::ActiveRecord::VERSION::MAJOR >= 4
has_many versions_association_name,
-> { order(model.timestamp_sort_order) },
class_name: version_class_name, as: :item
else
has_many versions_association_name,
class_name: version_class_name,
as: :item,
order: paper_trail_version_class.timestamp_sort_order
end
def paper_trail_deprecate(new_method, old_method = nil)
old = old_method.nil? ? new_method : old_method
msg = format("Use paper_trail.%s instead of %s", new_method, old)
::ActiveSupport::Deprecation.warn(msg, caller(2))
end

# Reset the transaction id when the transaction is closed.
after_commit :reset_transaction_id
after_rollback :reset_transaction_id
after_rollback :clear_rolled_back_versions
def setup_model_for_paper_trail(*args)
paper_trail_deprecate "setup", "setup_model_for_paper_trail"
paper_trail.setup(*args)
end

# Given `options`, populates `paper_trail_options`.
Expand All @@ -168,65 +138,39 @@ def setup_paper_trail_options(options)
end

def setup_callbacks_from_options(options_on = [])
options_on.each do |option|
send "paper_trail_on_#{option}"
options_on.each do |event|
paper_trail.public_send("on_#{event}")
end
end

# Record version before or after "destroy" event
def paper_trail_on_destroy(recording_order = "before")
unless %w(after before).include?(recording_order.to_s)
raise ArgumentError, 'recording order can only be "after" or "before"'
end

if recording_order == "after" &&
Gem::Version.new(ActiveRecord::VERSION::STRING) >= Gem::Version.new("5")
if ::ActiveRecord::Base.belongs_to_required_by_default
::ActiveSupport::Deprecation.warn(
"paper_trail_on_destroy(:after) is incompatible with ActiveRecord " +
"belongs_to_required_by_default and has no effect. Please use :before " +
"or disable belongs_to_required_by_default."
)
end
end

send "#{recording_order}_destroy", :record_destroy, if: :save_version?

return if paper_trail_options[:on].include?(:destroy)
paper_trail_options[:on] << :destroy
def paper_trail_on_destroy(*args)
paper_trail_deprecate "on_destroy", "paper_trail_on_destroy"
paper_trail_on_destroy(*args)
end

# Record version after "update" event
def paper_trail_on_update
before_save :reset_timestamp_attrs_for_update_if_needed!, on: :update
after_update :record_update, if: :save_version?
after_update :clear_version_instance!

return if paper_trail_options[:on].include?(:update)
paper_trail_options[:on] << :update
paper_trail_deprecate "on_update", "paper_trail_on_update"
paper_trail.on_update
end

# Record version after "create" event
def paper_trail_on_create
after_create :record_create, if: :save_version?

return if paper_trail_options[:on].include?(:create)
paper_trail_options[:on] << :create
paper_trail_deprecate "on_create", "paper_trail_on_create"
paper_trail.on_create
end

# Switches PaperTrail off for this class.
def paper_trail_off!
PaperTrail.enabled_for_model(self, false)
paper_trail_deprecate "disable", "paper_trail_off!"
paper_trail.disable
end

# Switches PaperTrail on for this class.
def paper_trail_on!
PaperTrail.enabled_for_model(self, true)
paper_trail_deprecate "enable", "paper_trail_on!"
paper_trail.enable
end

def paper_trail_enabled_for_model?
return false unless include?(PaperTrail::Model::InstanceMethods)
PaperTrail.enabled_for_model?(self)
paper_trail_deprecate "enabled?", "paper_trail_enabled_for_model?"
paper_trail.enabled?
end

def paper_trail_version_class
Expand All @@ -237,79 +181,77 @@ def paper_trail_version_class
# Wrap the following methods in a module so we can include them only in the
# ActiveRecord models that declare `has_paper_trail`.
module InstanceMethods
# Returns true if this instance is the current, live one;
# returns false if this instance came from a previous version.
def paper_trail
::PaperTrail::RecordTrail.new(self)
end

def live?
source_version.nil?
self.class.paper_trail_deprecate "live?"
paper_trail.live?
end

# Returns who put the object into its current state.
def paper_trail_originator
(source_version || send(self.class.versions_association_name).last).try(:whodunnit)
self.class.paper_trail_deprecate "originator", "paper_trail_originator"
paper_trail.originator
end

def originator
::ActiveSupport::Deprecation.warn "Use paper_trail_originator instead of originator."
paper_trail_originator
self.class.paper_trail_deprecate "originator"
paper_trail.originator
end

# Invoked after rollbacks to ensure versions records are not created
# for changes that never actually took place.
# Optimization: Use lazy `reset` instead of eager `reload` because, in
# many use cases, the association will not be used.
def clear_rolled_back_versions
send(self.class.versions_association_name).reset
self.class.paper_trail_deprecate "clear_rolled_back_versions"
paper_trail.clear_rolled_back_versions
end

def source_version
self.class.paper_trail_deprecate "source_version"
paper_trail.source_version
end

# Returns the object (not a Version) as it was at the given timestamp.
def version_at(timestamp, reify_options = {})
# Because a version stores how its object looked *before* the change,
# we need to look for the first version created *after* the timestamp.
v = send(self.class.versions_association_name).subsequent(timestamp, true).first
return v.reify(reify_options) if v
self unless destroyed?
def version_at(*args)
self.class.paper_trail_deprecate "version_at"
paper_trail.version_at(*args)
end

# Returns the objects (not Versions) as they were between the given times.
# TODO: Either add support for the third argument, `_reify_options`, or
# add a deprecation warning if someone tries to use it.
def versions_between(start_time, end_time, _reify_options = {})
versions = send(self.class.versions_association_name).between(start_time, end_time)
versions.collect { |version| version_at(version.send(PaperTrail.timestamp_field)) }
self.class.paper_trail_deprecate "versions_between"
paper_trail.versions_between(start_time, end_time)
end

# Returns the object (not a Version) as it was most recently.
def previous_version
previous =
if source_version
source_version.previous
else
send(self.class.versions_association_name).last
end
previous.try(:reify)
self.class.paper_trail_deprecate "previous_version"
paper_trail.previous_version
end

# Returns the object (not a Version) as it became next.
# NOTE: if self (the item) was not reified from a version, i.e. it is the
# "live" item, we return nil. Perhaps we should return self instead?
def next_version
subsequent_version = source_version.next
subsequent_version ? subsequent_version.reify : self.class.find(id)
rescue
nil
self.class.paper_trail_deprecate "next_version"
paper_trail.next_version
end

def paper_trail_enabled_for_model?
self.class.paper_trail_enabled_for_model?
self.class.paper_trail_deprecate "enabled_for_model?", "paper_trail_enabled_for_model?"
paper_trail.enabled_for_model?
end

# Executes the given method or block without creating a new version.
def without_versioning(method = nil)
paper_trail_was_enabled = paper_trail_enabled_for_model?
self.class.paper_trail_off!
method ? method.to_proc.call(self) : yield(self)
paper_trail_was_enabled = paper_trail.enabled_for_model?
self.class.paper_trail.disable
if method
if paper_trail.respond_to?(method)
paper_trail.public_send(method)
else
method.to_proc.call(self)
end
else
yield(self)
end
ensure
self.class.paper_trail_on! if paper_trail_was_enabled
self.class.paper_trail.enable if paper_trail_was_enabled
end

# Utility method for reifying. Anything executed inside the block will
Expand All @@ -334,40 +276,18 @@ def whodunnit(value)
PaperTrail.whodunnit = current_whodunnit
end

# Mimics the `touch` method from `ActiveRecord::Persistence`, but also
# creates a version. A version is created regardless of options such as
# `:on`, `:if`, or `:unless`.
#
# TODO: look into leveraging the `after_touch` callback from
# `ActiveRecord` to allow the regular `touch` method to generate a version
# as normal. May make sense to switch the `record_update` method to
# leverage an `after_update` callback anyways (likely for v4.0.0)
def touch_with_version(name = nil)
raise ActiveRecordError, "can not touch on a new record object" unless persisted?

attributes = timestamp_attributes_for_update_in_model
attributes << name if name
current_time = current_time_from_proper_timezone

attributes.each { |column| write_attribute(column, current_time) }

record_update(true) unless will_record_after_update?
save!(validate: false)
self.class.paper_trail_deprecate "touch_with_version"
paper_trail.touch_with_version(name)
end

private

# Returns true if `save` will cause `record_update`
# to be called via the `after_update` callback.
def will_record_after_update?
on = paper_trail_options[:on]
on.nil? || on.include?(:update)
end

def source_version
send self.class.version_association_name
end

def record_create
if paper_trail_switched_on?
data = {
Expand Down Expand Up @@ -464,7 +384,7 @@ def clear_version_instance!
# Invoked via callback when a user attempts to persist a reified
# `Version`.
def reset_timestamp_attrs_for_update_if_needed!
return if live?
return if paper_trail.live?
timestamp_attributes_for_update_in_model.each do |column|
# ActiveRecord 4.2 deprecated `reset_column!` in favor of
# `restore_column!`.
Expand Down Expand Up @@ -514,10 +434,10 @@ def save_associations_belongs_to(version)

if assoc.options[:polymorphic]
associated_record = send(assoc.name) if send(assoc.foreign_type)
if associated_record && associated_record.class.paper_trail_enabled_for_model?
if associated_record && associated_record.class.paper_trail.enabled?
assoc_version_args[:foreign_key_id] = associated_record.id
end
elsif assoc.klass.paper_trail_enabled_for_model?
elsif assoc.klass.paper_trail.enabled?
assoc_version_args[:foreign_key_id] = send(assoc.foreign_key)
end

Expand All @@ -533,7 +453,7 @@ def save_associations_has_and_belongs_to_many(version)
self.class.reflect_on_all_associations(:has_and_belongs_to_many).each do |a|
next unless
self.class.paper_trail_save_join_tables.include?(a.name) ||
a.klass.paper_trail_enabled_for_model?
a.klass.paper_trail.enabled?
assoc_version_args = {
version_id: version.transaction_id,
foreign_key_name: a.name
Expand Down Expand Up @@ -638,7 +558,7 @@ def changed_and_not_ignored
def paper_trail_switched_on?
PaperTrail.enabled? &&
PaperTrail.enabled_for_controller? &&
paper_trail_enabled_for_model?
paper_trail.enabled_for_model?
end

def save_version?
Expand Down

0 comments on commit 46d9398

Please sign in to comment.