Fix tracking subclasses with additional fields

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 * Your contribution here.
 * [#236](https://github.com/mongoid/mongoid-history/pull/236): Fix Ruby 2.7 keyword argument warnings - [@vasilysn](https://github.com/vasilysn).
+* [#237](https://github.com/mongoid/mongoid-history/pull/237): Fix tracking subclasses with additional fields - [@getaroom](https://github.com/getaroom).
 
 ### 0.8.2 (2019/12/02)
 

lib/mongoid/history/options.rb

@@ -13,45 +13,44 @@ def scope
       end
 
       def prepared
-        @prepared ||= begin
-          prepare_skipped_fields
-          prepare_formatted_fields
-          parse_tracked_fields_and_relations
-          options
-        end
+        return @prepared if @prepared
+        @prepared = options.dup
+        prepare_skipped_fields
+        prepare_formatted_fields
+        parse_tracked_fields_and_relations
+        @prepared
       end
 
       private
 
       def default_options
-        @default_options ||=
-          { on: :all,
-            except: %i[created_at updated_at],
-            tracker_class_name: nil,
-            modifier_field: :modifier,
-            version_field: :version,
-            changes_method: :changes,
-            scope: scope,
-            track_create: true,
-            track_update: true,
-            track_destroy: true,
-            format: nil }
+        { on: :all,
+          except: %i[created_at updated_at],
+          tracker_class_name: nil,
+          modifier_field: :modifier,
+          version_field: :version,
+          changes_method: :changes,
+          scope: scope,
+          track_create: true,
+          track_update: true,
+          track_destroy: true,
+          format: nil }
       end
 
       # Sets the :except attributes and relations in `options` to be an [ Array <String> ]
       # The attribute names and relations are stored by their `database_field_name`s
       # Removes the `nil` and duplicate entries from skipped attributes/relations list
       def prepare_skipped_fields
         # normalize :except fields to an array of database field strings
-        @options[:except] = Array(options[:except])
-        @options[:except] = options[:except].map { |field| trackable.database_field_name(field) }.compact.uniq
+        @prepared[:except] = Array(@prepared[:except])
+        @prepared[:except] = @prepared[:except].map { |field| trackable.database_field_name(field) }.compact.uniq
       end
 
       def prepare_formatted_fields
         formats = {}
 
-        if options[:format].class == Hash
-          options[:format].each do |field, format|
+        if @prepared[:format].class == Hash
+          @prepared[:format].each do |field, format|
             next if field.nil?
 
             field = trackable.database_field_name(field)
@@ -68,33 +67,33 @@ def prepare_formatted_fields
           end
         end
 
-        options[:format] = formats
+        @prepared[:format] = formats
       end
 
       def parse_tracked_fields_and_relations
         # case `options[:on]`
         # when `posts: [:id, :title]`, then it will convert it to `[[:posts, [:id, :title]]]`
         # when `:foo`, then `[:foo]`
         # when `[:foo, { posts: [:id, :title] }]`, then return as is
-        @options[:on] = Array(options[:on])
+        @prepared[:on] = Array(@prepared[:on])
 
-        @options[:on] = options[:on].map { |opt| opt == :all ? :fields : opt }
+        @prepared[:on] = @prepared[:on].map { |opt| opt == :all ? :fields : opt }
 
-        if options[:on].include?(:fields)
-          @options[:on] = options[:on].reject { |opt| opt == :fields }
-          @options[:on] = options[:on] | trackable.fields.keys.map(&:to_sym) - reserved_fields.map(&:to_sym)
+        if @prepared[:on].include?(:fields)
+          @prepared[:on] = @prepared[:on].reject { |opt| opt == :fields }
+          @prepared[:on] = @prepared[:on] | trackable.fields.keys.map(&:to_sym) - reserved_fields.map(&:to_sym)
         end
 
-        if options[:on].include?(:embedded_relations)
-          @options[:on] = options[:on].reject { |opt| opt == :embedded_relations }
-          @options[:on] = options[:on] | trackable.embedded_relations.keys
+        if @prepared[:on].include?(:embedded_relations)
+          @prepared[:on] = @prepared[:on].reject { |opt| opt == :embedded_relations }
+          @prepared[:on] = @prepared[:on] | trackable.embedded_relations.keys
         end
 
-        @options[:fields] = []
-        @options[:dynamic] = []
-        @options[:relations] = { embeds_one: {}, embeds_many: {} }
+        @prepared[:fields] = []
+        @prepared[:dynamic] = []
+        @prepared[:relations] = { embeds_one: {}, embeds_many: {} }
 
-        options[:on].each do |option|
+        @prepared[:on].each do |option|
           if option.is_a?(Hash)
             option.each { |k, v| split_and_categorize(k => v) }
           else
@@ -145,7 +144,7 @@ def get_field_options(option)
       # @param [ String ] field The database field name of field or relation to track
       # @param [ nil | Array <String | Symbol> ] field_options The tracked fields for embedded relations
       def categorize_tracked_option(field, field_options = nil)
-        return if options[:except].include?(field)
+        return if @prepared[:except].include?(field)
         return if reserved_fields.include?(field)
 
         field_options = Array(field_options)
@@ -155,23 +154,23 @@ def categorize_tracked_option(field, field_options = nil)
         elsif trackable.embeds_many?(field)
           track_relation(field, :embeds_many, field_options)
         elsif trackable.fields.keys.include?(field)
-          @options[:fields] << field
+          @prepared[:fields] << field
         else
-          @options[:dynamic] << field
+          @prepared[:dynamic] << field
         end
       end
 
       def track_relation(field, kind, field_options)
         relation_class = trackable.relation_class_of(field)
-        @options[:relations][kind][field] = if field_options.blank?
-                                              relation_class.fields.keys
-                                            else
-                                              %w[_id] | field_options.map { |opt| relation_class.database_field_name(opt) }
-                                               end
+        @prepared[:relations][kind][field] = if field_options.blank?
+                                               relation_class.fields.keys
+                                             else
+                                               %w[_id] | field_options.map { |opt| relation_class.database_field_name(opt) }
+                                             end
       end
 
       def reserved_fields
-        @reserved_fields ||= ['_id', '_type', options[:version_field].to_s, "#{options[:modifier_field]}_id"]
+        @reserved_fields ||= ['_id', '_type', @prepared[:version_field].to_s, "#{@prepared[:modifier_field]}_id"]
       end
     end
   end

lib/mongoid/history/trackable.rb

@@ -559,7 +559,11 @@ def clear_trackable_memoization
           @tracked_fields = nil
           @tracked_embeds_one = nil
           @tracked_embeds_many = nil
-          @obfuscated_fields = nil
+        end
+
+        def inherited(subclass)
+          super
+          subclass.mongoid_history_options = Mongoid::History::Options.new(subclass, mongoid_history_options.options)
         end
       end
     end

spec/integration/subclasses_spec.rb

@@ -7,12 +7,16 @@ class Element
       include Mongoid::Timestamps
       include Mongoid::History::Trackable
 
+      track_history
+
       field :body
 
-      track_history on: [:body]
+      # force preparation of options
+      history_trackable_options
     end
 
     class Prompt < Element
+      field :head
     end
 
     class User
@@ -31,11 +35,13 @@ class User
   it 'tracks subclass create and update' do
     prompt = Prompt.new(modifier: user)
     expect { prompt.save! }.to change(Tracker, :count).by(1)
-    expect { prompt.update_attributes!(body: 'one') }.to change(Tracker, :count).by(1)
+    expect { prompt.update_attributes!(body: 'one', head: 'two') }.to change(Tracker, :count).by(1)
     prompt.undo! user
     expect(prompt.body).to be_blank
+    expect(prompt.head).to be_blank
     prompt.redo! user, 2
     expect(prompt.body).to eq('one')
+    expect(prompt.head).to eq('two')
     expect { prompt.destroy }.to change(Tracker, :count).by(1)
   end
 end

spec/unit/options_spec.rb

@@ -84,6 +84,12 @@ class EmbFour
   end
 
   describe '#parse' do
+    it 'does not mutate the original options' do
+      original_options = service.options.dup
+      service.prepared
+      expect(service.options).to eq original_options
+    end
+
     describe '#default_options' do
       let(:expected_options) do
         {

spec/unit/trackable_spec.rb

@@ -836,4 +836,25 @@ def my_changes
       m.save!
     end
   end
+
+  context "subclassing a #{described_class}" do
+    before :each do
+      MyModel.track_history(track_destroy: false)
+
+      class MySubclassModel < MyModel
+      end
+    end
+
+    after :each do
+      Object.send(:remove_const, :MySubclassModel)
+    end
+
+    describe '.inherited' do
+      it 'creates new history options for the subclass' do
+        options = MySubclassModel.mongoid_history_options
+        expect(options.trackable).to eq MySubclassModel
+        expect(options.options).to eq MyModel.mongoid_history_options.options
+      end
+    end
+  end
 end