@@ -1,5 +1,7 @@ acts_as_audited ChangeLog ------------------------------------------------------------------------------- +* 2009-01-27 - Store old and new values for updates, and store all attributes on destroy. + Refactored revisioning methods to work as expected * 2008-10-10 - changed to make it work in development mode * 2008-04-19 - refactored to make compatible with dirty tracking in edge rails and to stop storing both old and new values in a single audit CHANGELOG @@ -73,7 +73,7 @@ module CollectiveIdea #:nodoc: except |= Array(options[:except]).collect(&:to_s) if options[:except] write_inheritable_attribute :non_audited_columns, except - has_many :audits, :as => :auditable, :order => "#{Audit.quoted_table_name}.version desc" + has_many :audits, :as => :auditable, :order => "#{Audit.quoted_table_name}.version" attr_protected :audit_ids if options[:protect] Audit.audited_classes << self @@ -115,47 +115,32 @@ module CollectiveIdea #:nodoc: # end # def revisions(from_version = 1) - changes_from(from_version) {|attributes| revision_with(attributes) } + revision = audits.find_by_version(from_version).revision + audits = self.audits.find(:all, :conditions => ['version >= ?', from_version]) + Audit.reconstruct_attributes(audits) {|attrs| revision.revision_with(attrs) } end - # Get a specific revision + # Get a specific revision specified by the version number, or +:previous+ def revision(version) - revision_with changes_from(version) + revision_with Audit.reconstruct_attributes(audits_to(version)) end def revision_at(date_or_time) - audit = audits.find(:first, :conditions => ["created_at <= ?", date_or_time], - :order => "created_at DESC") - revision_with changes_from(audit.version) if audit + audits = self.audits.find(:all, :conditions => ["created_at <= ?", date_or_time]) + revision_with Audit.reconstruct_attributes(audits) unless audits.empty? end - private - - def audited_changes - changed_attributes.except(*non_audited_columns).inject({}) do |changes,(attr, old_value)| - changes[attr] = [old_value, self[attr]] - changes - end + def audited_attributes + attributes.except(*non_audited_columns) end - def changes_from(version = 1, &block) - if version == :previous - version = if self.version - self.version - 1 - else - last_audit = audits.find(:first, :offset => 1) - last_audit ? last_audit.version : 1 - end - end - revisions = audits.find(:all, :conditions => ['version >= ?', version]) - Audit.reconstruct_attributes(revisions, &block) - end + protected def revision_with(attributes) returning self.dup do |revision| revision.send :instance_variable_set, '@attributes', self.attributes_before_type_cast revision.attributes = attributes.reject {|attr,_| !respond_to?("#{attr}=") } - + # Remove any association proxies so that they will be recreated # and reference the correct object for this revision. The only way # to determine if an instance variable is a proxy object is to @@ -170,8 +155,30 @@ module CollectiveIdea #:nodoc: end end + private + + def audited_changes + changed_attributes.except(*non_audited_columns).inject({}) do |changes,(attr, old_value)| + changes[attr] = [old_value, self[attr]] + changes + end + end + + def audits_to(version = nil) + if version == :previous + version = if self.version + self.version - 1 + else + previous = audits.find(:first, :offset => 1, + :order => "#{Audit.quoted_table_name}.version DESC") + previous ? previous.version : 1 + end + end + audits.find(:all, :conditions => ['version <= ?', version]) + end + def audit_create(user = nil) - write_audit(:action => 'create', :changes => audited_changes, :user => user) + write_audit(:action => 'create', :changes => audited_attributes, :user => user) end def audit_update(user = nil) @@ -181,7 +188,7 @@ module CollectiveIdea #:nodoc: end def audit_destroy(user = nil) - write_audit(:action => 'destroy', :user => user) + write_audit(:action => 'destroy', :user => user, :changes => audited_attributes) end def write_audit(attrs) lib/acts_as_audited.rb @@ -10,11 +10,6 @@ describe CollectiveIdea::Acts::Audited do User.should be_kind_of(CollectiveIdea::Acts::Audited::SingletonMethods) end - it "should know which attributes are audited" do - attrs = {'name' => nil, 'username' => nil, 'logins' => 0, 'activated' => nil} - User.new.audited_attributes.should == attrs - end - ['created_at', 'updated_at', 'lock_version', 'id', 'password'].each do |column| it "should not audit #{column}" do User.non_audited_columns.should include(column) @@ -38,8 +33,8 @@ describe CollectiveIdea::Acts::Audited do end it "should store all the audited attributes" do - audit = User.create(:name => 'Brandon').audits.first - audit.changes.should == {'name' => [nil, 'Brandon']} + user = User.create(:name => 'Brandon') + user.audits.first.changes.should == user.audited_attributes end end @@ -59,12 +54,12 @@ describe CollectiveIdea::Acts::Audited do it "should set the action to 'update'" do @user.update_attributes :name => 'Changed' - @user.audits.first.action.should == 'update' + @user.audits.last.action.should == 'update' end it "should store the changed attributes" do @user.update_attributes :name => 'Changed' - @user.audits.first.changes.should == {'name' => ['Brandon', 'Changed']} + @user.audits.last.changes.should == {'name' => ['Brandon', 'Changed']} end it "should not save an audit if the value doesn't change after type casting" do @@ -89,12 +84,19 @@ describe CollectiveIdea::Acts::Audited do it "should set the action to 'destroy'" do @user.destroy - @user.audits.first.action.should == 'destroy' + @user.audits.last.action.should == 'destroy' end - it "should not store any changes" do + it "should store all of the audited attributes" do @user.destroy - @user.audits.first.changes.should be_nil + @user.audits.last.changes.should == @user.audited_attributes + end + + it "should be able to reconstruct destroyed record without history" do + @user.audits.delete_all + @user.destroy + revision = @user.audits.first.revision + revision.name.should == @user.name end end @@ -150,16 +152,33 @@ describe CollectiveIdea::Acts::Audited do u.update_attributes :name => 'Awesome', :username => 'keepers' u.revisions.size.should == 3 - u.revisions[0].name.should == 'Awesome' - u.revisions[0].username.should == 'keepers' + + u.revisions[0].name.should == 'Brandon' + u.revisions[0].username.should == 'brandon' u.revisions[1].name.should == 'Foobar' u.revisions[1].username.should == 'brandon' - u.revisions[2].name.should == 'Brandon' - u.revisions[2].username.should == 'brandon' + u.revisions[2].name.should == 'Awesome' + u.revisions[2].username.should == 'keepers' end + it "should access to only recent revisions" do + u = User.create(:name => 'Brandon', :username => 'brandon') + u.update_attributes :name => 'Foobar' + u.update_attributes :name => 'Awesome', :username => 'keepers' + + u.revisions(2).size.should == 2 + + u.revisions(2)[0].name.should == 'Foobar' + u.revisions(2)[0].username.should == 'brandon' + + u.revisions(2)[1].name.should == 'Awesome' + u.revisions(2)[1].username.should == 'keepers' + end + + + it "should ignore attributes that have been deleted" do @user.audits.last.update_attributes :changes => {:old_attribute => 'old value'} lambda { @user.revisions }.should_not raise_error(ActiveRecord::UnknownAttributeError) @@ -195,6 +214,21 @@ describe CollectiveIdea::Acts::Audited do previous.revision(:previous).version.should == 3 end + it "should set the attributes for each revision" do + u = User.create(:name => 'Brandon', :username => 'brandon') + u.update_attributes :name => 'Foobar' + u.update_attributes :name => 'Awesome', :username => 'keepers' + + u.revision(3).name.should == 'Awesome' + u.revision(3).username.should == 'keepers' + + u.revision(2).name.should == 'Foobar' + u.revision(2).username.should == 'brandon' + + u.revision(1).name.should == 'Brandon' + u.revision(1).username.should == 'brandon' + end + it "should not raise an error when no previous audits exist" do @user.audits.destroy_all lambda{ @user.revision(:previous) }.should_not raise_error spec/acts_as_audited_spec.rb @@ -65,10 +65,10 @@ describe Audit do user = User.create! :name => "Set Version Number" user.audits.first.version.should == 1 user.update_attribute :name, "Set to 2" - user.audits(true).first.version.should == 2 - user.audits(true).last.version.should == 1 + user.audits(true).first.version.should == 1 + user.audits(true).last.version.should == 2 user.destroy - user.audits(true).first.version.should == 3 + user.audits(true).last.version.should == 3 end describe "reconstruct_attributes" do spec/audit_spec.rb 4c598c200e98e24d3c8ae52040682130c9ed11c4 Brandon Keepers brandon@collectiveidea.com http://github.com/collectiveidea/acts_as_audited/commit/938f3d9b6a92f8c73fe0971ac4441205821114d4 938f3d9b6a92f8c73fe0971ac4441205821114d4 2009-01-27T19:23:47-08:00 2009-01-27T19:23:47-08:00 Store all attributes on create and destroy, and only store old and new values for updates. Fixed bugs in revisioning methods 6c8c950a894b3f0b79729c6a68ab25c823e56368 Brandon Keepers brandon@collectiveidea.com