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

Allow multiple associated audits #406

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/audited.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def config

require 'audited/auditor'
require 'audited/audit'
require 'audited/audit_associate'

::ActiveRecord::Base.send :include, Audited::Auditor

Expand Down
6 changes: 5 additions & 1 deletion lib/audited/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def dump(obj)
class Audit < ::ActiveRecord::Base
belongs_to :auditable, polymorphic: true
belongs_to :user, polymorphic: true
belongs_to :associated, polymorphic: true
has_many :audit_associates

before_create :set_version_number, :set_audit_user, :set_request_uuid, :set_remote_address

Expand All @@ -60,6 +60,10 @@ def ancestors
self.class.ascending.auditable_finder(auditable_id, auditable_type).to_version(version)
end

def associates
audit_associates.map(&:associated)
end

# Return an instance of what the object looked like at this revision. If
# the object has been destroyed, this will be a new record.
def revision
Expand Down
8 changes: 8 additions & 0 deletions lib/audited/audit_associate.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module Audited
class AuditAssociate < ::ActiveRecord::Base
self.table_name_prefix = "audited_"

belongs_to :audit
belongs_to :associated, polymorphic: true
end
end
19 changes: 16 additions & 3 deletions lib/audited/auditor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ def audited(options = {})
end

def has_associated_audits
has_many :associated_audits, as: :associated, class_name: Audited.audit_class.name
has_many :audit_associates, as: :associated, class_name: Audited::AuditAssociate.name
has_many :associated_audits, through: :audit_associates, source: :audit, class_name: Audited.audit_class.name
end
end

Expand Down Expand Up @@ -212,9 +213,15 @@ def audit_destroy
end

def write_audit(attrs)
attrs[:associated] = send(audit_associated_with) unless audit_associated_with.nil?
self.audit_comment = nil
run_callbacks(:audit) { audits.create(attrs) } if auditing_enabled

if auditing_enabled
run_callbacks(:audit) do
audit = audits.create(attrs)
audit.audit_associates << collect_audit_associated_with unless audit_associated_with.nil?
audit
end
end
end

def require_comment
Expand Down Expand Up @@ -242,6 +249,12 @@ def reconstruct_attributes(audits)
audits.each { |audit| attributes.merge!(audit.new_attributes) }
attributes
end

def collect_audit_associated_with
Array(audit_associated_with).map do |associated|
Audited::AuditAssociate.new(associated: send(associated))
end
end
end # InstanceMethods

module AuditedClassMethods
Expand Down
26 changes: 26 additions & 0 deletions lib/generators/audited/templates/create_audit_associates.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
class <%= migration_class_name %> < <%= migration_parent %>
def self.up
create_table :audited_audit_associates, :force => true do |t|
t.column :audit_id, :integer
t.column :associated_id, :integer
t.column :associated_type, :string
end

execute <<-SQL
INSERT INTO audited_audit_associates (audit_id, associated_id, associated_type)
SELECT id, associated_id, associated_type
FROM audits
SQL

add_index :audited_audit_associates, :audit_id, :name => 'index_audited_audit_associates_on_audit_id'
add_index :audited_audit_associates, [:associated_type, :associated_id], :name => 'index_audited_audit_associates_on_associated'

remove_index :audits, name: 'associated_index'
remove_column :audits, :associated_id
remove_column :audits, :associated_type
end

def self.down
raise ActiveRecord::IrreversibleMigration
end
end
13 changes: 10 additions & 3 deletions lib/generators/audited/templates/install.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ def self.up
create_table :audits, :force => true do |t|
t.column :auditable_id, :integer
t.column :auditable_type, :string
t.column :associated_id, :integer
t.column :associated_type, :string
t.column :user_id, :<%= options[:audited_user_id_column_type] %>
t.column :user_type, :string
t.column :username, :string
Expand All @@ -18,13 +16,22 @@ def self.up
end

add_index :audits, [:auditable_type, :auditable_id], :name => 'auditable_index'
add_index :audits, [:associated_type, :associated_id], :name => 'associated_index'
add_index :audits, [:user_id, :user_type], :name => 'user_index'
add_index :audits, :request_uuid
add_index :audits, :created_at

create_table :audited_audit_associates, :force => true do |t|
t.column :audit_id, :integer
t.column :associated_id, :integer
t.column :associated_type, :string
end

add_index :audited_audit_associates, :audit_id, :name => 'index_audited_audit_associates_on_audit_id'
add_index :audited_audit_associates, [:associated_type, :associated_id], :name => 'index_audited_audit_associates_on_associated'
end

def self.down
drop_table :audited_audit_associates
drop_table :audits
end
end
4 changes: 4 additions & 0 deletions lib/generators/audited/upgrade_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ def migrations_to_be_applied
if indexes.any? { |i| i.columns == %w[associated_id associated_type] }
yield :revert_polymorphic_indexes_order
end

unless Audited::Audit.connection.table_exists?(Audited::AuditAssociate.table_name)
yield :create_audit_associates
end
end
end
end
Expand Down
15 changes: 10 additions & 5 deletions spec/audited/auditor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -302,30 +302,35 @@ def non_column_attr=(val)

describe "associated with" do
let(:owner) { Models::ActiveRecord::Owner.create(name: 'Models::ActiveRecord::Owner') }
let(:owned_company) { Models::ActiveRecord::OwnedCompany.create!(name: 'The auditors', owner: owner) }
let(:country) { Models::ActiveRecord::Country.create(name: 'Models::ActiveRecord::Country') }
let(:owned_company) { Models::ActiveRecord::OwnedCompany.create!(name: 'The auditors', owner: owner, country: country) }

it "should record the associated object on create" do
expect(owned_company.audits.first.associated).to eq(owner)
expect(owned_company.audits.first.associates).to contain_exactly(owner, country)
end

it "should store the associated object on update" do
owned_company.update_attribute(:name, 'The Auditors')
expect(owned_company.audits.last.associated).to eq(owner)
expect(owned_company.audits.last.associates).to contain_exactly(owner, country)
end

it "should store the associated object on destroy" do
owned_company.destroy
expect(owned_company.audits.last.associated).to eq(owner)
expect(owned_company.audits.last.associates).to contain_exactly(owner, country)
end
end

describe "has associated audits" do
let!(:owner) { Models::ActiveRecord::Owner.create!(name: 'Models::ActiveRecord::Owner') }
let!(:owned_company) { Models::ActiveRecord::OwnedCompany.create!(name: 'The auditors', owner: owner) }
let!(:country) { Models::ActiveRecord::Country.create(name: 'Models::ActiveRecord::Country') }
let!(:owned_company) { Models::ActiveRecord::OwnedCompany.create!(name: 'The auditors', owner: owner, country: country) }

it "should list the associated audits" do
expect(owner.associated_audits.length).to eq(1)
expect(owner.associated_audits.first.auditable).to eq(owned_company)

expect(country.associated_audits.length).to eq(1)
expect(country.associated_audits.first.auditable).to eq(owned_company)
end
end

Expand Down
10 changes: 8 additions & 2 deletions spec/support/active_record/models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ class Company < ::ActiveRecord::Base
class Company::STICompany < Company
end

class Country < ::ActiveRecord::Base
has_associated_audits
has_many :companies, class_name: "OwnedCompany", dependent: :destroy
end

class Owner < ::ActiveRecord::Base
self.table_name = 'users'
has_associated_audits
Expand All @@ -73,8 +78,9 @@ class Owner < ::ActiveRecord::Base
class OwnedCompany < ::ActiveRecord::Base
self.table_name = 'companies'
belongs_to :owner, class_name: "Owner"
attr_accessible :name, :owner if respond_to?(:attr_accessible) # declare attr_accessible before calling aaa
audited associated_with: :owner
belongs_to :country
attr_accessible :name, :owner, :country if respond_to?(:attr_accessible) # declare attr_accessible before calling aaa
audited associated_with: [:country, :owner]
end

class OnUpdateDestroy < ::ActiveRecord::Base
Expand Down
17 changes: 14 additions & 3 deletions spec/support/active_record/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,15 @@
t.column :favourite_device, :string
end

create_table :countries do |t|
t.column :name, :string
end

create_table :companies do |t|
t.column :name, :string
t.column :owner_id, :integer
t.column :type, :string
t.column :country_id, :integer
end

create_table :authors do |t|
Expand All @@ -60,8 +65,6 @@
create_table :audits do |t|
t.column :auditable_id, :integer
t.column :auditable_type, :string
t.column :associated_id, :integer
t.column :associated_type, :string
t.column :user_id, :integer
t.column :user_type, :string
t.column :username, :string
Expand All @@ -75,8 +78,16 @@
end

add_index :audits, [:auditable_id, :auditable_type], name: 'auditable_index'
add_index :audits, [:associated_id, :associated_type], name: 'associated_index'
add_index :audits, [:user_id, :user_type], name: 'user_index'
add_index :audits, :request_uuid
add_index :audits, :created_at

create_table :audited_audit_associates, :force => true do |t|
t.column :audit_id, :integer
t.column :associated_id, :integer
t.column :associated_type, :string
end

add_index :audited_audit_associates, :audit_id, :name => 'index_audited_audit_associates_on_audit_id'
add_index :audited_audit_associates, [:associated_type, :associated_id], :name => 'index_audited_audit_associates_on_associated'
end
20 changes: 20 additions & 0 deletions test/db/version_7.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
ActiveRecord::Schema.define do
create_table :audits, :force => true do |t|
t.column :auditable_id, :integer
t.column :auditable_type, :string
t.column :associated_id, :integer
t.column :associated_type, :string
t.column :user_id, :integer
t.column :user_type, :string
t.column :username, :string
t.column :action, :string
t.column :audited_changes, :text
t.column :version, :integer, :default => 0
t.column :comment, :string
t.column :remote_address, :string
t.column :request_uuid, :string
t.column :created_at, :datetime
end

drop_table :audited_audit_associates if table_exists?(:audited_audit_associates)
end
20 changes: 20 additions & 0 deletions test/upgrade_generator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,26 @@ class UpgradeGeneratorTest < Rails::Generators::TestCase
end
end

test "should create audited_audit_associates table" do
load_schema 7

run_generator %w(upgrade)

assert_migration "db/migrate/create_audit_associates.rb" do |content|
assert_match(/create_table :audited_audit_associates/, content)
assert_match(/t.column :audit_id, :integer/, content)
assert_match(/t.column :associated_id, :integer/, content)
assert_match(/t.column :associated_type, :string/, content)

assert_match(/add_index :audited_audit_associates, :audit_id/, content)
assert_match(/add_index :audited_audit_associates, \[:associated_type, :associated_id\]/, content)

assert_match(/remove_index :audits, name: 'associated_index'/, content)
assert_match(/remove_column :audits, :associated_id/, content)
assert_match(/remove_column :audits, :associated_type/, content)
end
end

test "generate migration with correct AR migration parent" do
load_schema 1

Expand Down