From 42de9e73e48dfdb0b70c426abec76fbdb5c8d604 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Tue, 13 Dec 2016 22:23:30 -0500 Subject: [PATCH 1/4] Extract private methods out of merge_metadata --- .rubocop_todo.yml | 2 +- lib/paper_trail/record_trail.rb | 65 +++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 169345e7..5b3e565e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -8,7 +8,7 @@ Metrics/CyclomaticComplexity: Max: 8 # Goal: 6 Metrics/PerceivedComplexity: - Max: 10 # Goal: 7 + Max: 9 # Goal: 7 Style/FrozenStringLiteralComment: Enabled: false diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb index 9004705e..20c89654 100644 --- a/lib/paper_trail/record_trail.rb +++ b/lib/paper_trail/record_trail.rb @@ -118,32 +118,49 @@ module PaperTrail source_version.nil? end + # Updates `data` from the model's `meta` option and from `controller_info`. # @api private - def merge_metadata(data) - # First we merge the model-level metadata in `meta`. - @record.paper_trail_options[:meta].each do |k, v| - data[k] = - if v.respond_to?(:call) - v.call(@record) - elsif v.is_a?(Symbol) && @record.respond_to?(v, true) - # If it is an attribute that is changing in an existing object, - # be sure to grab the current version. - if @record.has_attribute?(v) && - attribute_changed_in_latest_version?(v) && - data[:event] != "create" - attribute_in_previous_version(v) - else - @record.send(v) - end - else - v - end - end + def merge_metadata_into(data) + merge_metadata_from_model_into(data) + merge_metadata_from_controller_into(data) + end - # Second we merge any extra data from the controller (if available). + # Updates `data` from `controller_info`. + # @api private + def merge_metadata_from_controller_into(data) data.merge(PaperTrail.controller_info || {}) end + # Updates `data` from the model's `meta` option. + # @api private + def merge_metadata_from_model_into(data) + @record.paper_trail_options[:meta].each do |k, v| + data[k] = model_metadatum(v, data[:event]) + end + end + + # Given a `value` from the model's `meta` option, returns an object to be + # persisted. The `value` can be a simple scalar value, but it can also + # be a symbol that names a model method, or even a Proc. + # @api private + def model_metadatum(value, event) + if value.respond_to?(:call) + value.call(@record) + elsif value.is_a?(Symbol) && @record.respond_to?(value, true) + # If it is an attribute that is changing in an existing object, + # be sure to grab the current version. + if event != "create" && + @record.has_attribute?(value) && + attribute_changed_in_latest_version?(value) + attribute_in_previous_version(value) + else + @record.send(value) + end + else + value + end + 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? @@ -210,7 +227,7 @@ module PaperTrail data[:object_changes] = recordable_object_changes end add_transaction_id_to(data) - merge_metadata(data) + merge_metadata_into(data) end def record_destroy @@ -238,7 +255,7 @@ module PaperTrail whodunnit: PaperTrail.whodunnit } add_transaction_id_to(data) - merge_metadata(data) + merge_metadata_into(data) end # Returns a boolean indicating whether to store serialized version diffs @@ -280,7 +297,7 @@ module PaperTrail data[:object_changes] = recordable_object_changes end add_transaction_id_to(data) - merge_metadata(data) + merge_metadata_into(data) end # Returns an object which can be assigned to the `object` attribute of a From b4149864f4c6561a191b6b447c6d9da27eb94f8a Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Tue, 13 Dec 2016 22:27:29 -0500 Subject: [PATCH 2/4] Remove rails 3 branch of association definition --- lib/paper_trail/model_config.rb | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/lib/paper_trail/model_config.rb b/lib/paper_trail/model_config.rb index ab44fc26..61edc973 100644 --- a/lib/paper_trail/model_config.rb +++ b/lib/paper_trail/model_config.rb @@ -124,22 +124,12 @@ module PaperTrail @model_class.send :attr_accessor, :paper_trail_event - # In rails 4, the `has_many` syntax for specifying order uses a lambda. - if ::ActiveRecord::VERSION::MAJOR >= 4 - @model_class.has_many( - @model_class.versions_association_name, - -> { order(model.timestamp_sort_order) }, - class_name: @model_class.version_class_name, - as: :item - ) - else - @model_class.has_many( - @model_class.versions_association_name, - class_name: @model_class.version_class_name, - as: :item, - order: @model_class.paper_trail.version_class.timestamp_sort_order - ) - end + @model_class.has_many( + @model_class.versions_association_name, + -> { order(model.timestamp_sort_order) }, + class_name: @model_class.version_class_name, + as: :item + ) end # Adds callbacks to record changes to habtm associations such that on save From 14eee417a703a672edb798f0a17f38626b074d40 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Tue, 13 Dec 2016 22:32:11 -0500 Subject: [PATCH 3/4] Extract private method save_bt_association --- lib/paper_trail/record_trail.rb | 51 +++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/lib/paper_trail/record_trail.rb b/lib/paper_trail/record_trail.rb index 20c89654..cec29cfe 100644 --- a/lib/paper_trail/record_trail.rb +++ b/lib/paper_trail/record_trail.rb @@ -346,29 +346,15 @@ module PaperTrail # Saves associations if the join table for `VersionAssociation` exists. def save_associations(version) return unless PaperTrail.config.track_associations? - save_associations_belongs_to(version) - save_associations_habtm(version) + save_bt_associations(version) + save_habtm_associations(version) end - def save_associations_belongs_to(version) + # Save all `belongs_to` associations. + # @api private + def save_bt_associations(version) @record.class.reflect_on_all_associations(:belongs_to).each do |assoc| - assoc_version_args = { - version_id: version.id, - foreign_key_name: assoc.foreign_key - } - - if assoc.options[:polymorphic] - associated_record = @record.send(assoc.name) if @record.send(assoc.foreign_type) - 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? - assoc_version_args[:foreign_key_id] = @record.send(assoc.foreign_key) - end - - if assoc_version_args.key?(:foreign_key_id) - PaperTrail::VersionAssociation.create(assoc_version_args) - end + save_bt_association(assoc, version) end end @@ -376,7 +362,8 @@ module PaperTrail # HABTM associations looked like before any changes were made, by using # the `paper_trail_habtm` data structure. Then, we create # `VersionAssociation` records for each of the associated records. - def save_associations_habtm(version) + # @api private + def save_habtm_associations(version) @record.class.reflect_on_all_associations(:has_and_belongs_to_many).each do |a| next unless save_habtm_association?(a) habtm_assoc_ids(a).each do |id| @@ -525,6 +512,28 @@ module PaperTrail ) end + # Save a single `belongs_to` association. + # @api private + def save_bt_association(assoc, version) + assoc_version_args = { + version_id: version.id, + foreign_key_name: assoc.foreign_key + } + + if assoc.options[:polymorphic] + associated_record = @record.send(assoc.name) if @record.send(assoc.foreign_type) + 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? + assoc_version_args[:foreign_key_id] = @record.send(assoc.foreign_key) + end + + if assoc_version_args.key?(:foreign_key_id) + PaperTrail::VersionAssociation.create(assoc_version_args) + end + end + # Returns true if the given HABTM association should be saved. # @api private def save_habtm_association?(assoc) From 9ddff8d3582e0200b7594c3de24e7ad50604cd65 Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Tue, 13 Dec 2016 22:41:09 -0500 Subject: [PATCH 4/4] Extract private method load_versions_for_hmt_association --- .rubocop_todo.yml | 2 +- lib/paper_trail/reifier.rb | 38 +++++++++++++++++++++++--------------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 5b3e565e..cadc7cd3 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -2,7 +2,7 @@ # one by one as the offenses are removed from the code base. Metrics/AbcSize: - Max: 24 # Goal: 15 + Max: 22 # Goal: 15 Metrics/CyclomaticComplexity: Max: 8 # Goal: 6 diff --git a/lib/paper_trail/reifier.rb b/lib/paper_trail/reifier.rb index 53dc1b23..09ca5795 100644 --- a/lib/paper_trail/reifier.rb +++ b/lib/paper_trail/reifier.rb @@ -117,23 +117,12 @@ module PaperTrail end # @api private - def hmt_collection_through_belongs_to(through_collection, assoc, options, transaction_id) - collection_keys = through_collection.map { |through_model| + def hmt_collection_through_belongs_to(through_collection, assoc, options, tx_id) + ids = through_collection.map { |through_model| through_model.send(assoc.source_reflection.foreign_key) } - version_id_subquery = assoc.klass.paper_trail.version_class. - select("MIN(id)"). - where("item_type = ?", assoc.class_name). - where("item_id IN (?)", collection_keys). - where( - "created_at >= ? OR transaction_id = ?", - options[:version_at], - transaction_id - ). - group("item_id"). - to_sql - versions = versions_by_id(assoc.klass, version_id_subquery) - collection = Array.new assoc.klass.where(assoc.klass.primary_key => collection_keys) + versions = load_versions_for_hmt_association(assoc, ids, tx_id, options[:version_at]) + collection = Array.new assoc.klass.where(assoc.klass.primary_key => ids) prepare_array_for_has_many(collection, options, versions) collection end @@ -217,6 +206,25 @@ module PaperTrail versions_by_id(model.class, version_id_subquery) end + # Given a `has_many(through:)` association and an array of `ids`, return + # the version records from the point in time identified by `tx_id` or + # `version_at`. + # @api private + def load_versions_for_hmt_association(assoc, ids, tx_id, version_at) + version_id_subquery = assoc.klass.paper_trail.version_class. + select("MIN(id)"). + where("item_type = ?", assoc.class_name). + where("item_id IN (?)", ids). + where( + "created_at >= ? OR transaction_id = ?", + version_at, + tx_id + ). + group("item_id"). + to_sql + versions_by_id(assoc.klass, version_id_subquery) + end + # Set all the attributes in this version on the model. def reify_attributes(model, version, attrs) enums = model.class.respond_to?(:defined_enums) ? model.class.defined_enums : {}