Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auditing of virtual attributes (e.g. tag_list) #1

Closed
ledermann opened this issue Apr 17, 2009 · 13 comments
Closed

Auditing of virtual attributes (e.g. tag_list) #1

ledermann opened this issue Apr 17, 2009 · 13 comments

Comments

@ledermann
Copy link

I would like to audit virtual attributes, which are not stored as columns in the same table. Think about a “tag_list” managed by acts_as_taggable_on. This virtual attributes are stored elsewhere, but the changes should be audited in the owner model.

Perhaps this could be configured by an additional option like :add in addition to :except.

@bkeepers
Copy link
Contributor

I don’t know if this is possible. acts_as_audited depends on Active Record’s dirty tracking, which detects changes to attributes. I wouldn’t be opposed to adding this, if you could come up with a clean implementation.

Another idea I had was to allow auditing of any arbitrary data with a record. For example, if I wanted to store the IP of the user. I haven’t come up with a good API for it yet, but here are a couple ideas:

```
@record.audit :ip => remote_address
@record.save

  1. or
    @record.save :ip => remote_address
    ```

@ledermann
Copy link
Author

Hm, after some investigation I think it would be better to tell other plugins (like acts_as_taggable etc.) to “make the model dirty”, so e.g. after setting tag_list to a new value, the “changed_attributes” hash should reflect this change.

@bridiver
Copy link

bridiver commented Nov 2, 2009

did anyone ever come up with a working solution to this? It's a problem I'm looking at right now

@bridiver
Copy link

bridiver commented Nov 2, 2009

I'm looking into the "dirty" method, but I'm wondering if it might affect partial_updates?

@bkeepers
Copy link
Contributor

bkeepers commented Nov 2, 2009

I don't know of anyone that is working on this yet.

@bridiver
Copy link

bridiver commented Nov 3, 2009

what do you think of this? Rather than try to come up a generic API, you just add whatever you need to changed_methods hash. I changed self[attr] to send(attr), it didn't break any tests and I can't see that it would cause any problems.

--- a/lib/acts_as_audited.rb
+++ b/lib/acts_as_audited.rb
@@ -97,6 +97,11 @@ module CollectiveIdea #:nodoc:

   module InstanceMethods
  •    # Add methods to be tracked
    
  •    def changed_methods
    
  •      @changed_methods ||= {}
    
  •    end
    
  •  # Temporarily turns off auditing while saving.
     def save_without_auditing
       without_auditing { save }
    

    @@ -164,8 +169,8 @@ module CollectiveIdea #:nodoc:
    private

     def audited_changes
    
  •      changed_attributes.except(*non_audited_columns).inject({}) do |changes,(attr, old_value)|
    
  •        changes[attr] = [old_value, self[attr]]
    
  •      changed_attributes.merge(changed_methods).except(*non_audited_columns).inject({}) do |changes,(attr, old_value)|
    
  •        changes[attr] = [old_value, send(attr)]
         changes
       end
     end
    

then I used this method in my class to audit the tag list

audit tag changes

def set_tag_list_on_with_audit(context,new_list, tagger=nil)
# Audit.reconstruct_attributes will send an array to repopulate this for revisions
new_list = new_list.join(',') if new_list.is_a? Array
old_value = send("#{context.to_s.singularize}_list").map(&:strip).sort
changed_methods["#{context.to_s.singularize}_list"] = old_value unless old_value == new_list.split(',').map(&:strip).sort
set_tag_list_on_without_audit(context,new_list, tagger)
end
alias_method_chain :set_tag_list_on, :audit

@bridiver
Copy link

bridiver commented Nov 3, 2009

sorry about the formatting

@kennethkalmer
Copy link
Collaborator

Doing some house cleaning and closing here.

If you guys want to take a stab at this again, we're now using Rails 3 and maybe ActiveModel makes this easier to achieve. The contribution guidelines in the README have been updated to show the new branches and suggestions for pull requests.

Thanks !

bryckbost added a commit that referenced this issue Apr 11, 2012
…cts_as_audited into nilakanta-after_audit_callback

* 'after_audit_callback' of https://github.com/nilakanta/acts_as_audited:
  reverting the gemspec bump
  bumping the gem version
  adding a callback for after_audit actions
  [#1] It should be possible to use the same audits table with Rails 2 and 3 versions of the same app.   Audits table should not raise ActiveRecord::DangerousAttributeError in Rails 3.0.   Rename audits.changes to audits.audited_changes.
  adding support for an after_audit callback
  bump the version to 1.1.2.beta1
  Version bump to 0.0.0
  comments are essential only for specified attributes and only on specified validation_options
  require comment validations apply only if auditing is enabled
@tquill
Copy link

tquill commented Mar 9, 2016

Got acts_as_taggable working correctly with this the around_audit callback like this:

def around_audit
  current_audit = yield
  if current_audit.audited_changes.has_key?("tag_list")
    current_audit.audited_changes["tag_list"][1] = self.tag_list
    current_audit.save
  end
end

@kayzee
Copy link

kayzee commented Mar 18, 2016

@tquill I found that this doesn't work with the first version of an object.
The first audits record will be a 'create' but it doesn't contain any associated (tag_list) values.
Subsequent audits records will include any changes to tag_list as planned.

I also added self.tag_list.join(',') because the previous revision wasn't "reifying" properly.

def around_audit
  current_audit = yield
  if current_audit.action.eql?('create')
    current_audit.audited_changes['tag_list'] = tag_list.join(', ')
    current_audit.save
  elsif current_audit.audited_changes.key?('tag_list')
    current_audit.audited_changes['tag_list'][1] = tag_list.join(', ')
    current_audit.save
  end
end

@kayzee
Copy link

kayzee commented Mar 20, 2016

@tquill the solution you suggested is incomplete. It only works for self.tag_list and does not keep track of self.tags.

The latter always returns the latest tags no matter what revision you are trying to reify. I was playing around with adding audited to the Tag model, but that just saves the audit trail of each Tag.

class Tag < ActiveRecord::Base
  audited
end

@johnknapp
Copy link

@kayzee Your custom callback really helped me out. Thanks! Though I'm curious why you didn't address the update action? (For the Model I'm auditing, assigned tags change occasionally during update so I needed to add that.) Anyway cheers from 2018! (with a cc to @tquill)

lzap pushed a commit to lzap/acts_as_audited that referenced this issue Oct 29, 2018
… with Rails 2 and 3 versions of the same app.

  Audits table should not raise ActiveRecord::DangerousAttributeError in Rails 3.0.
  Rename audits.changes to audits.audited_changes.
lzap pushed a commit to lzap/acts_as_audited that referenced this issue Oct 29, 2018
Rename audits.changes to audits.audited_changes
rocket-turtle pushed a commit to rocket-turtle/audited that referenced this issue Jan 4, 2023
danielmorrison pushed a commit that referenced this issue Apr 20, 2023
Use RequestStore instead of Thread.current for thread-safe requests.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants