add an on_load callback to simply_versioned

refs CNVS-6869

When a version is loaded from the database, the on_load callback
gives the model an opportunity to process or modify the data if
necessary. For example, if current information on the model should
override a cached attribute, this would be the place to do it.

Test Plan:
- in the console, create a submission, e.g.
  >> s = Submission.create :assignment => a1, :user => u1
- add an on_load callback, e.g.
  >> submission.simply_versioned_options[:on_load] =
     lambda{ |model, version| model.grade = 'A' }
- now, no matter what grade you set (and save) in a versioned copy of
  the submission, you should get 'A' back

Change-Id: I9dde0118e2833c72e6209f3d632d9503db1e1eb3
Reviewed-on: https://gerrit.instructure.com/22472
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
Product-Review: Duane Johnson <duane@instructure.com>

vendor/plugins/simply_versioned/lib/simply_versioned.rb

@@ -25,7 +25,8 @@ module SoftwareHeretics
         # callbacks
         :when => nil,
         :on_create => nil,
-        :on_update => nil
+        :on_update => nil,
+        :on_load => nil
       }
       
       module ClassMethods
@@ -51,6 +52,8 @@ module SoftwareHeretics
         #               that's about to be saved.
         # +on_update+ - callback to allow additional changes to an updated (see
         #               +explicit+ parameter) version that's about to be saved.
+        # +on_load+   - callback to allow processing or changes after loading
+        #               (finding) the version from the database.
         #
         # To save the record without creating a version either set +versioning_enabled+ to false
         # on the model before calling save or, alternatively, use +without_versioning+ and save
@@ -64,9 +67,13 @@ module SoftwareHeretics
           options.reverse_merge!(DEFAULTS)
           options[:exclude] = Array( options[ :exclude ] ).map( &:to_s )
           
-          has_many :versions, :order => 'number DESC', :as => :versionable, :dependent => :destroy, :extend => VersionsProxyMethods
+          has_many :versions, :order => 'number DESC', :as => :versionable,
+                   :dependent => :destroy,
+                   :inverse_of => :versionable,
+                   :extend => VersionsProxyMethods
           # INSTRUCTURE: Added to allow quick access to the most recent version
-          has_one :current_version, :class_name => 'Version', :order => 'number DESC', :as => :versionable, :dependent => :destroy, :extend => VersionsProxyMethods
+          # See 'current_version' below for the common use of current_version_unidirectional
+          has_one :current_version_unidirectional, :class_name => 'Version', :order => 'number DESC', :as => :versionable, :dependent => :destroy, :extend => VersionsProxyMethods
           # INSTRUCTURE: Lets us ignore certain things when deciding whether to store a new version
           before_save :check_if_changes_are_worth_versioning
           after_save :simply_versioned_create_version
@@ -175,6 +182,14 @@ module SoftwareHeretics
           !@simply_versioned_version_number
         end
 
+        # Create a bi-directional current_version association so we don't need
+        # to reload the 'versionable' object each time we access the model
+        def current_version
+          current_version_unidirectional.tap do |version|
+            version.versionable = self
+          end
+        end
+
         protected
         
         # INSTRUCTURE: If defined on a method, allow a check
@@ -212,22 +227,49 @@ module SoftwareHeretics
       end
 
       module VersionsProxyMethods
+        # Anything that returns a Version should have its versionable pre-
+        # populated. This is basically a way of getting around the fact that
+        # ActiveRecord doesn't have a polymorphic :inverse_of option.
+        def method_missing(method, *a, &b)
+          case method
+          when :minimum, :maximum, :exists?, :all, :find_all, :each then
+            populate_versionables(super)
+          when :find then
+            case a.first
+            when :all          then populate_versionables(super)
+            when :first, :last then populate_versionable(super)
+            else super
+            end
+          else
+            super
+          end 
+        end
+
+        def populate_versionables(versions)
+          versions.each{ |v| populate_versionable(v) } if versions.is_a?(Array)
+          versions
+        end
+
+        def populate_versionable(version)
+          version.versionable = proxy_owner if version && !version.frozen?
+          version
+        end
         
         # Get the Version instance corresponding to this models for the specified version number.
         def get_version( number )
-          find_by_number( number )
+          populate_versionable find_by_number( number )
         end
         alias_method :get, :get_version
         
         # Get the first Version corresponding to this model.
         def first_version
-          reorder( 'number ASC' ).first
+          populate_versionable reorder( 'number ASC' ).first
         end
         alias_method :first, :first_version
 
         # Get the current Version corresponding to this model.
         def current_version
-          reorder( 'number DESC' ).first
+          populate_versionable reorder( 'number DESC' ).first
         end
         alias_method :current, :current_version
         
@@ -241,13 +283,13 @@ module SoftwareHeretics
         
         # Return the Version for this model with the next higher version
         def next_version( number )
-          reorder( 'number ASC' ).where( "number > ?", number ).first
+          populate_versionable reorder( 'number ASC' ).where( "number > ?", number ).first
         end
         alias_method :next, :next_version
         
         # Return the Version for this model with the next lower version
         def previous_version( number )
-          reorder( 'number DESC' ).where( "number < ?", number ).first
+          populate_versionable reorder( 'number DESC' ).where( "number < ?", number ).first
         end
         alias_method :previous, :previous_version
       end

vendor/plugins/simply_versioned/lib/version.rb

@@ -23,6 +23,7 @@ class Version < ActiveRecord::Base #:nodoc:
       # INSTRUCTURE:  added if... so that if a column is removed in a migration after this was versioned it doesen't die with NoMethodError: undefined method `some_column_name=' for ...
       obj.__send__( "#{var_name}=", var_value ) if obj.respond_to?("#{var_name}=")
     end
+    obj.simply_versioned_options[:on_load].try(:call, obj, self)
     # INSTRUCTURE: Added to allow model instances pulled out
     # of versions to still know their version number
     obj.simply_versioned_version_model = true

vendor/plugins/simply_versioned/spec_canvas/simply_versioned_spec.rb

@@ -1,6 +1,6 @@
 require File.expand_path(File.dirname(__FILE__)+'/../../../../spec/apis/api_spec_helper')
 
-class Woozel< ActiveRecord::Base
+class Woozel < ActiveRecord::Base
   simply_versioned :explicit => true
 end
 
@@ -17,6 +17,7 @@ describe 'simply_versioned' do
   end
 
   describe "explicit versions" do
+    let(:woozel) { Woozel.create!(:name => 'Eeyore') }
     it "should create the first version on save" do
       woozel = Woozel.new(:name => 'Eeyore')
       woozel.should_not be_versioned
@@ -27,7 +28,6 @@ describe 'simply_versioned' do
     end
 
     it "should keep the last version up to date for each save" do
-      woozel = Woozel.create!(:name => 'Eeyore')
       woozel.should be_versioned
       woozel.versions.length.should eql(1)
       woozel.versions.current.model.name.should eql('Eeyore')
@@ -38,7 +38,6 @@ describe 'simply_versioned' do
     end
 
     it "should create a new version when asked to" do
-      woozel = Woozel.create!(:name => 'Eeyore')
       woozel.name = 'Piglet'
       woozel.with_versioning(:explicit => true, &:save!)
       woozel.versions.length.should eql(2)
@@ -47,7 +46,6 @@ describe 'simply_versioned' do
     end
 
     it 'should not create a new version when not explicitly asked to' do
-      woozel = Woozel.create!(:name => 'Eeyore')
       woozel.name = 'Piglet'
       woozel.with_versioning(&:save!)
       woozel.versions.length.should eql(1)
@@ -55,12 +53,31 @@ describe 'simply_versioned' do
     end
 
     it 'should not update the last version when not versioning' do
-      woozel = Woozel.create!(:name => 'Eeyore')
       woozel.name = 'Piglet'
       woozel.without_versioning(&:save!)
       woozel.versions.length.should eql(1)
       woozel.versions.current.model.name.should eql('Eeyore')
     end
+
+    it 'should not reload one versionable association from the database' do
+      woozel.name = 'Piglet'
+      woozel.with_versioning(&:save!)
+      woozel.versions.loaded?.should == false
+      first = woozel.versions.first
+      Woozel.connection.expects(:select_all).never
+      first.versionable.should == woozel
+    end
+
+    it 'should not reload any versionable associations from the database' do
+      woozel.name = 'Piglet'
+      woozel.with_versioning(&:save!)
+      woozel.versions.loaded?.should == false
+      all = woozel.versions.all
+      Woozel.connection.expects(:select_all).never
+      all.each do |version|
+        version.versionable.should == woozel
+      end
+    end
   end
 
   describe "#current_version?" do
@@ -83,4 +100,21 @@ describe 'simply_versioned' do
       @woozel.versions.map { |v| v.model.current_version? }.should == [false, false]
     end
   end
+
+  context "callbacks" do
+    let(:woozel) { Woozel.create!( name: 'test' ) }
+    context "on_load" do
+      let(:on_load) do
+        lambda { |model, version| model.name = 'test override' }
+      end
+      before do
+        woozel.simply_versioned_options[:on_load] = on_load
+        woozel.reload
+      end
+      it "can modify a version after loading" do
+        YAML::load(woozel.current_version.yaml)['name'].should == 'test'
+        woozel.current_version.model.name.should == 'test override'
+      end
+    end
+  end
 end