Fix for issue paper-trail-gem#594, reifying sub-classed models that u…

…se STI

CHANGELOG.md

@@ -11,11 +11,16 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
 
 ### Added
 
-- None
+- [#1108](https://github.com/paper-trail-gem/paper_trail/pull/1108) -
+  Generator `update_sti` creates a migration that updates existing `version` entries such
+  that `item_type` then refers to the specific class name instead of base_class. See
+  [5.c. Generators](https://github.com/paper-trail-gem/paper_trail/blob/master/README.md#5c-generators)
 
 ### Fixed
 
-- None
+- [#1108](https://github.com/paper-trail-gem/paper_trail/pull/1108) -
+  Properly reifying a version of a subclassed STI model requires `item_type` to represent
+  the specific class name instead of base_class. Fixes [Issue #594](https://github.com/paper-trail-gem/paper_trail/issues/594)
 
 ## 9.2.0 (2018-06-09)
 
@@ -37,6 +42,8 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).
   object_changes column. An example of this implementation using the hashdiff
   gem can be found here:
   [paper_trail-hashdiff](https://github.com/hashwin/paper_trail-hashdiff)
+- [#1108](https://github.com/paper-trail-gem/paper_trail/pull/1108) -
+  Generator `update_sti` builds a migration to update `item_type` for existing versions.
 
 ### Fixed
 

README.md

@@ -941,8 +941,13 @@ class Banana < Fruit
 end
 ```
 
-However, there is a known issue when reifying [associations](#associations),
-see https://github.com/paper-trail-gem/paper_trail/issues/594
+A change in what `item_type` stores for subclassed models was introduced in [PR#1108](https://github.com/paper-trail-gem/paper_trail/pull/1108),
+recording the actual class name instead of refering to the base class. This simplifies
+reifying through associations, and allows for a change in PT-AT that fixes [issue 594](https://github.com/paper-trail-gem/paper_trail/issues/594).
+
+For those that have existing version data from STI models, `item_type` can be updated by using
+a generator, `rails generate paper_trail:update_sti`.  More information is found in section
+[5.c. Generators](#5c-generators).
 
 ### 5.b. Configuring the `versions` Association
 
@@ -962,12 +967,13 @@ Overriding associations is not recommended in general.
 
 ### 5.c. Generators
 
-PaperTrail has one generator, `paper_trail:install`. It writes, but does not
-run, a migration file.  It also creates a PaperTrail configuration initializer.
-The migration adds (at least) the `versions` table. The
-most up-to-date documentation for this generator can be found by running `rails
-generate paper_trail:install --help`, but a copy is included here for
-convenience.
+##### paper_trail:install
+
+PaperTrail has two generators. The first, `paper_trail:install` writes, but does not
+run, a migration file. It also creates a PaperTrail configuration initializer.
+The migration adds (at least) the `versions` table. The most up-to-date documentation
+for this generator can be found by running `rails generate paper_trail:install --help`,
+but a copy is included here for convenience.
 
 ```
 Usage:
@@ -985,6 +991,57 @@ Runtime options:
 Generates (but does not run) a migration to add a versions table.  Also generates an initializer file for configuring PaperTrail
 ```
 
+##### paper_trail:update_sti
+
+The second generator, `paper_trail:update_sti`, also writes but does not run a migration file
+which scans existing data in the versions table to identify all use of STI. Upon finding each
+entry which refers to an object from a subclassed model, `item_type` is updated to reflect
+the subclass name, providing full compatibility with how the `versions` association works
+after PR#1108. In order to more quickly update a large volume of version records, updates are
+batched to affect 100 records at a time.
+
+Hints can be used in rare cases when the STI structure is modified over time such as
+when establishing additional intermediary inheritance structure, pointing to a new
+base class, abandoning STI entirely, or changing the name of the inheritance_column. The
+vast majority of users will probably never need to supply hints when using this generator.
+
+Let's give an example where the inheritance_column is changed, say originally there is an
+Animal class that uses the default `type` column, but after generating 500 Bird and Cat
+objects is modified to instead use the `species` column with a line like this:
+```
+Animal.inheritance_column = "species"
+```
+With this change a hint can be used to build the migration in a way that indicates how to
+treat the older records. For those first 500 Animal objects, the `item_type` should be
+derived from `type`, and for the rest, from `species`. We would need to research the
+ID numbers for versions that utilise `type` in the `object` and `object_changes` data.
+In this example let's say that those first 500 Animals had version IDs between 249 and 1124.
+These objects would have been created having an `item_type` of Animal prior to PR#1108.
+Of course versions representing all manner of other changes would also be intermingled along
+with creates and updates for Animal objects. Perhaps another STI model exists for Car that
+inherits from Vehicle, and 50 Car objects were also built around the same timeframe as Bird
+and Cat objects, with various creates and updates for those being referened in versions with
+IDs from 382 to 516. For Vehicle let's say that initially the inheritance_column was set to
+`kind`, but then it changed to something else after ID 516 in the versions table. In this
+situation, to properly use the generator then these hints can be supplied:
+
+`rails generate update_sti Animal(type):249..1124 Vehicle(kind):382..516`
+
+The resulting migration will include these lines near the top:
+
+```
+# Versions of item_type "Animal" with IDs between 249 and 1124 will be updated based on `type`
+# Versions of item_type "Vehicle" with IDs between 382 and 516 will be updated based on `kind`
+hints = {"Animal"=>{249..1124=>"type"}, "Vehicle"=>{382..516=>"kind"}}
+```
+
+It is important to note that the IDs are not those of the Bird, Cat, or Car objects, but
+rather the IDs from the create, update, and destroy entries in the versions table. As well,
+these hints are only needed in situations where the inheritance_column has changed at some
+point in time, or in cases where the entire STI structure is modified or is abandoned. It
+ultimately facilitates better reporting for historic subclassed items, and allows PT-AT to
+properly reify these historic objects through associations.
+
 ### 5.d. Protected Attributes
 
 As of version 6, PT no longer supports rails 3 or the [protected_attributes][17]

lib/generators/paper_trail/USAGE → lib/generators/paper_trail/install/USAGE

@@ -1,2 +1,2 @@
 Description:
-  Generates (but does not run) a migration to add a versions table.
+  Generates (but does not run) a migration to add a versions table. See section 5.c. Generators in README.md for more information.

lib/generators/paper_trail/install_generator.rb → lib/generators/paper_trail/install/install_generator.rb

@@ -1,13 +1,10 @@
 # frozen_string_literal: true
 
-require "rails/generators"
-require "rails/generators/active_record"
+require_relative "../migration_generator"
 
 module PaperTrail
   # Installs PaperTrail in a rails app.
-  class InstallGenerator < ::Rails::Generators::Base
-    include ::Rails::Generators::Migration
-
+  class InstallGenerator < MigrationGenerator
     # Class names of MySQL adapters.
     # - `MysqlAdapter` - Used by gems: `mysql`, `activerecord-jdbcmysql-adapter`.
     # - `Mysql2Adapter` - Used by `mysql2` gem.
@@ -25,34 +22,16 @@ class InstallGenerator < ::Rails::Generators::Base
     )
 
     desc "Generates (but does not run) a migration to add a versions table." \
-         "  Also generates an initializer file for configuring PaperTrail"
+         "  Also generates an initializer file for configuring PaperTrail." \
+         "  See section 5.c. Generators in README.md for more information."
 
     def create_migration_file
-      add_paper_trail_migration("create_versions")
+      add_paper_trail_migration("create_versions",
+        item_type_options: item_type_options,
+        versions_table_options: versions_table_options)
       add_paper_trail_migration("add_object_changes_to_versions") if options.with_changes?
     end
 
-    def self.next_migration_number(dirname)
-      ::ActiveRecord::Generators::Base.next_migration_number(dirname)
-    end
-
-    protected
-
-    def add_paper_trail_migration(template)
-      migration_dir = File.expand_path("db/migrate")
-      if self.class.migration_exists?(migration_dir, template)
-        ::Kernel.warn "Migration already exists: #{template}"
-      else
-        migration_template(
-          "#{template}.rb.erb",
-          "db/migrate/#{template}.rb",
-          item_type_options: item_type_options,
-          migration_version: migration_version,
-          versions_table_options: versions_table_options
-        )
-      end
-    end
-
     private
 
     # MySQL 5.6 utf8mb4 limit is 191 chars for keys used in indexes.
@@ -63,13 +42,6 @@ def item_type_options
       ", #{opt}"
     end
 
-    def migration_version
-      major = ActiveRecord::VERSION::MAJOR
-      if major >= 5
-        "[#{major}.#{ActiveRecord::VERSION::MINOR}]"
-      end
-    end
-
     def mysql?
       MYSQL_ADAPTERS.include?(ActiveRecord::Base.connection.class.name)
     end

lib/generators/paper_trail/templates/create_versions.rb.erb → lib/generators/paper_trail/install/templates/create_versions.rb.erb

@@ -25,7 +25,7 @@ class CreateVersions < ActiveRecord::Migration<%= migration_version %>
       # the `created_at` column.
       # (https://dev.mysql.com/doc/refman/5.6/en/fractional-seconds.html)
       #
-      # MySQL users should also upgrade to rails 4.2, which is the first
+      # MySQL users should also upgrade to at least rails 4.2, which is the first
       # version of ActiveRecord with support for fractional seconds in MySQL.
       # (https://github.com/rails/rails/pull/14359)
       #

lib/generators/paper_trail/migration_generator.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require "rails/generators"
+require "rails/generators/active_record"
+
+module PaperTrail
+  # Basic structure to support a generator that builds a migration
+  class MigrationGenerator < ::Rails::Generators::Base
+    include ::Rails::Generators::Migration
+
+    def self.next_migration_number(dirname)
+      ::ActiveRecord::Generators::Base.next_migration_number(dirname)
+    end
+
+    protected
+
+    def add_paper_trail_migration(template, extra_options = {})
+      migration_dir = File.expand_path("db/migrate")
+      if self.class.migration_exists?(migration_dir, template)
+        ::Kernel.warn "Migration already exists: #{template}"
+      else
+        migration_template(
+          "#{template}.rb.erb",
+          "db/migrate/#{template}.rb",
+          { migration_version: migration_version }.merge(extra_options)
+        )
+      end
+    end
+
+    def migration_version
+      major = ActiveRecord::VERSION::MAJOR
+      if major >= 5
+        "[#{major}.#{ActiveRecord::VERSION::MINOR}]"
+      end
+    end
+  end
+end

lib/generators/paper_trail/update_sti/USAGE

@@ -0,0 +1,2 @@
+Description:
+  Generates (but does not run) a migration to update item_type for STI entries in an existing versions table. See section 5.c. Generators in README.md for more information.

lib/generators/paper_trail/update_sti/templates/update_versions_for_sti.rb.erb

@@ -0,0 +1,85 @@
+# This migration updates existing `versions` that have `item_type` that refers to
+# the base_class, and changes them to refer to the subclass instead.
+class UpdateVersionsForSti < ActiveRecord::Migration<%= migration_version %>
+  include ActionView::Helpers::TextHelper
+  def up
+<%=
+  # Returns class, column, range
+  def self.parse_custom_entry(text)
+    parts = text.split("):")
+    range = parts.last.split("..").map(&:to_i)
+    range = Range.new(range.first, range.last)
+    parts.first.split("(") + [range]
+  end
+  # Running:
+  #   rails g paper_trail:update_sti Animal(species):1..4 Plant(genus):42..1337
+  # results in:
+  #   # Versions of item_type "Animal" with IDs between 1 and 4 will be updated based on `species`
+  #   # Versions of item_type "Plant" with IDs between 42 and 1337 will be updated based on `genus`
+  #   hints = {"Animal"=>{1..4=>"species"}, "Plant"=>{42..1337=>"genus"}}
+  hint_descriptions = ""
+  hints = args.inject(Hash.new{|h, k| h[k] = {}}) do |s, v|
+    klass, column, range = parse_custom_entry(v)
+    hint_descriptions << "    # Versions of item_type \"#{klass}\" with IDs between #{
+      range.first} and #{range.last} will be updated based on \`#{column}\`\n"
+    s[klass][range] = column
+    s
+  end
+
+  unless hints.empty?
+    "#{hint_descriptions}    hints = #{hints.inspect}\n"
+  end
+%>
+    # Find all ActiveRecord models mentioned in existing versions
+    changes = Hash.new { |h, k| h[k] = [] }
+    model_names = PaperTrail::Version.select(:item_type).distinct
+    model_names.map(&:item_type).each do |model_name|
+      hint = hints[model_name] if defined?(hints)
+      begin
+        klass = model_name.constantize
+        # Actually implements an inheritance_column?  (Usually "type")
+        has_inheritance_column = klass.columns.map(&:name).include?(klass.inheritance_column)
+        # Find domain of types stored in PaperTrail versions
+        PaperTrail::Version.where(item_type: model_name).select(:id, :object, :object_changes).each do |obj|
+          if (object_detail = PaperTrail.serializer.load(obj.object || obj.object_changes))
+            is_found = false
+            type_name = nil
+            hint&.each do |k, v|
+              if k === obj.id && (type_name = object_detail[v])
+                break
+              end
+            end
+            if type_name.nil? && has_inheritance_column
+              type_name = object_detail[klass.inheritance_column]
+            end
+            if type_name
+              type_name = type_name.last if type_name.is_a?(Array)
+              if type_name != model_name
+                changes[type_name] << obj.id
+              end
+            end
+          end
+        end
+      rescue NameError => ex
+        say "Skipping reference to #{model_name}", subitem: true
+      end
+    end
+    changes.each do |k, v|
+      # Update in blocks of up to 100 at a time
+      block_of_ids = []
+      id_count = 0
+      num_updated = 0
+      v.sort.each do |id|
+        block_of_ids << id
+        if (id_count += 1) % 100 == 0
+          num_updated += PaperTrail::Version.where(id: block_of_ids).update_all(item_type: k)
+          block_of_ids = []
+        end
+      end
+      num_updated += PaperTrail::Version.where(id: block_of_ids).update_all(item_type: k)
+      if num_updated > 0
+        say "Associated #{pluralize(num_updated, 'record')} to #{k}", subitem: true
+      end
+    end
+  end
+end

lib/generators/paper_trail/update_sti/update_sti_generator.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require_relative "../migration_generator"
+
+module PaperTrail
+  # Updates STI entries for PaperTrail
+  class UpdateStiGenerator < MigrationGenerator
+    source_root File.expand_path("templates", __dir__)
+
+    desc "Generates (but does not run) a migration to update item_type for STI entries in an "\
+      "existing versions table. See section 5.c. Generators in README.md for more information."
+
+    def create_migration_file
+      add_paper_trail_migration("update_versions_for_sti", sti_type_options: options)
+    end
+  end
+end

lib/paper_trail/model_config.rb

@@ -168,6 +168,32 @@ def cannot_record_after_destroy?
         ::ActiveRecord::Base.belongs_to_required_by_default
     end
 
+    def setup_versions_association(klass)
+      klass.has_many(
+        klass.versions_association_name,
+        lambda do |object|
+          # Support Single Table Inheritance (STI) with custom inheritance columns.
+          #
+          # For example, imagine a `version` whose `item_type` is "Animal". The
+          # `animals` table is an STI table (it has cats and dogs) and it has a
+          # custom inheritance column, `species`. If `attrs["species"]` is "Dog",
+          # type_name is set to `Dog`. If `attrs["species"]` is blank, type_name
+          # is set to `Animal`. You can see this particular example in action in
+          # `spec/models/animal_spec.rb`.
+          item_type = object.paper_trail.versions_association_item_type
+          if item_type.blank?
+            order(model.timestamp_sort_order)
+          else
+            unscope(where: :item_type).
+            where(item_type: item_type).
+            order(model.timestamp_sort_order)
+          end
+        end,
+        class_name: klass.version_class_name,
+        as: :item
+      )
+    end
+
     def setup_associations(options)
       @model_class.class_attribute :version_association_name
       @model_class.version_association_name = options[:version] || :version
@@ -185,12 +211,7 @@ def setup_associations(options)
 
       assert_concrete_activerecord_class(@model_class.version_class_name)
 
-      @model_class.has_many(
-        @model_class.versions_association_name,
-        -> { order(model.timestamp_sort_order) },
-        class_name: @model_class.version_class_name,
-        as: :item
-      )
+      setup_versions_association(@model_class)
     end
 
     def setup_callbacks_from_options(options_on = [])