From c48893a680f23d27f7e255dfec2af91e7a00e350 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Sun, 25 Mar 2018 22:13:50 +0300 Subject: [PATCH 01/21] Add ability to globally disable auditing (#426) --- CHANGELOG.md | 3 ++- README.md | 6 ++++++ lib/audited.rb | 3 ++- lib/audited/auditor.rb | 2 +- spec/audited/auditor_spec.rb | 15 +++++++++++++++ 5 files changed, 26 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff2c18d14..fc5979bab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ Breaking changes Added -- None +- Add ability to globally disable auditing + [#426](https://github.com/collectiveidea/audited/pull/426) Changed diff --git a/README.md b/README.md index 9c09f502b..680886732 100644 --- a/README.md +++ b/README.md @@ -312,6 +312,12 @@ To disable auditing on an entire model: User.auditing_enabled = false ``` +To disable auditing on all models: + +```ruby +Audited.auditing_enabled = false +``` + ### Custom `Audit` model If you want to extend or modify the audit model, create a new class that diff --git a/lib/audited.rb b/lib/audited.rb index 904bfd28d..36baff418 100644 --- a/lib/audited.rb +++ b/lib/audited.rb @@ -2,7 +2,7 @@ module Audited class << self - attr_accessor :ignored_attributes, :current_user_method, :max_audits + attr_accessor :ignored_attributes, :current_user_method, :max_audits, :auditing_enabled attr_writer :audit_class def audit_class @@ -21,6 +21,7 @@ def config @ignored_attributes = %w(lock_version created_at updated_at created_on updated_on) @current_user_method = :current_user + @auditing_enabled = true end require 'audited/auditor' diff --git a/lib/audited/auditor.rb b/lib/audited/auditor.rb index 716cefe5f..10f49ac8a 100644 --- a/lib/audited/auditor.rb +++ b/lib/audited/auditor.rb @@ -355,7 +355,7 @@ def audit_as(user, &block) end def auditing_enabled - Audited.store.fetch("#{table_name}_auditing_enabled", true) + Audited.store.fetch("#{table_name}_auditing_enabled", true) && Audited.auditing_enabled end def auditing_enabled=(val) diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index f6c4543d7..45fc72695 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -723,6 +723,21 @@ def stub_global_max_audits(max_audits) STDERR.puts "Thread safety tests cannot be run with SQLite" end end + + it "should not save an audit when auditing is globally disabled" do + expect(Audited.auditing_enabled).to eq(true) + Audited.auditing_enabled = false + expect(Models::ActiveRecord::User.auditing_enabled).to eq(false) + + user = create_user + expect(user.audits.count).to eq(0) + + Audited.auditing_enabled = true + expect(Models::ActiveRecord::User.auditing_enabled).to eq(true) + + user.update_attributes(name: 'Test') + expect(user.audits.count).to eq(1) + end end describe "comment required" do From 9f6619ca48d3f8e0809ca7d02451a95b7f530be3 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Sun, 1 Apr 2018 06:58:15 -0700 Subject: [PATCH 02/21] remove unused/untested methods (#424) auditing_enabled was especially dangerous since it sets on the instance and modifies the global state --- CHANGELOG.md | 4 +++- lib/audited/auditor.rb | 18 +++--------------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc5979bab..2f885fe61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,9 @@ Breaking changes -- None +- removed `audited_columns`, `non_audited_columns`, `auditing_enabled=` instance methods, + use class methods instead + [#424](https://github.com/collectiveidea/audited/pull/424) Added diff --git a/lib/audited/auditor.rb b/lib/audited/auditor.rb index 10f49ac8a..c3c692135 100644 --- a/lib/audited/auditor.rb +++ b/lib/audited/auditor.rb @@ -142,7 +142,7 @@ def revision_at(date_or_time) # List of attributes that are audited. def audited_attributes - attributes.except(*non_audited_columns) + attributes.except(*self.class.non_audited_columns) end # Combine multiple audits into one. @@ -159,14 +159,6 @@ def combine_audits(audits_to_combine) protected - def non_audited_columns - self.class.non_audited_columns - end - - def audited_columns - self.class.audited_columns - end - def revision_with(attributes) dup.tap do |revision| revision.id = id @@ -202,9 +194,9 @@ def rails_below?(rails_version) def audited_changes all_changes = respond_to?(:changes_to_save) ? changes_to_save : changes if audited_options[:only].present? - all_changes.slice(*audited_columns) + all_changes.slice(*self.class.audited_columns) else - all_changes.except(*non_audited_columns) + all_changes.except(*self.class.non_audited_columns) end end @@ -297,10 +289,6 @@ def run_conditional_check(condition, matching: true) true end - def auditing_enabled=(val) - self.class.auditing_enabled = val - end - def reconstruct_attributes(audits) attributes = {} audits.each { |audit| attributes.merge!(audit.new_attributes) } From dc727627a645472a82d7678efb335bdd4d481167 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Thu, 29 Mar 2018 21:21:57 +0300 Subject: [PATCH 03/21] Add version to auditable_index --- CHANGELOG.md | 3 ++- lib/audited/audit.rb | 2 +- .../add_version_to_auditable_index.rb | 21 +++++++++++++++++++ lib/generators/audited/templates/install.rb | 2 +- lib/generators/audited/upgrade_generator.rb | 4 ++++ test/db/version_6.rb | 2 ++ test/upgrade_generator_test.rb | 10 +++++++++ 7 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 lib/generators/audited/templates/add_version_to_auditable_index.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f885fe61..f8a4a5bf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ Added Changed -- None +- Add version to auditable_index + [#427](https://github.com/collectiveidea/audited/pull/427) Fixed diff --git a/lib/audited/audit.rb b/lib/audited/audit.rb index 4d96d02c1..ff82e4e35 100644 --- a/lib/audited/audit.rb +++ b/lib/audited/audit.rb @@ -171,7 +171,7 @@ def self.collection_cache_key(collection = all, timestamp_column = :created_at) private def set_version_number - max = self.class.auditable_finder(auditable_id, auditable_type).descending.first.try(:version) || 0 + max = self.class.auditable_finder(auditable_id, auditable_type).maximum(:version) || 0 self.version = max + 1 end diff --git a/lib/generators/audited/templates/add_version_to_auditable_index.rb b/lib/generators/audited/templates/add_version_to_auditable_index.rb new file mode 100644 index 000000000..79a4045d6 --- /dev/null +++ b/lib/generators/audited/templates/add_version_to_auditable_index.rb @@ -0,0 +1,21 @@ +class <%= migration_class_name %> < <%= migration_parent %> + def self.up + if index_exists?(:audits, [:auditable_type, :auditable_id], name: index_name) + remove_index :audits, name: index_name + add_index :audits, [:auditable_type, :auditable_id, :version], name: index_name + end + end + + def self.down + if index_exists?(:audits, [:auditable_type, :auditable_id, :version], name: index_name) + remove_index :audits, name: index_name + add_index :audits, [:auditable_type, :auditable_id], name: index_name + end + end + + private + + def index_name + 'auditable_index' + end +end diff --git a/lib/generators/audited/templates/install.rb b/lib/generators/audited/templates/install.rb index 77bf46aa3..1d43f093c 100644 --- a/lib/generators/audited/templates/install.rb +++ b/lib/generators/audited/templates/install.rb @@ -17,7 +17,7 @@ def self.up t.column :created_at, :datetime end - add_index :audits, [:auditable_type, :auditable_id], :name => 'auditable_index' + add_index :audits, [:auditable_type, :auditable_id, :version], :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 diff --git a/lib/generators/audited/upgrade_generator.rb b/lib/generators/audited/upgrade_generator.rb index 91c87444c..d2b019704 100644 --- a/lib/generators/audited/upgrade_generator.rb +++ b/lib/generators/audited/upgrade_generator.rb @@ -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 + + if indexes.any? { |i| i.columns == %w[auditable_type auditable_id] } + yield :add_version_to_auditable_index + end end end end diff --git a/test/db/version_6.rb b/test/db/version_6.rb index 776569606..281ea9227 100644 --- a/test/db/version_6.rb +++ b/test/db/version_6.rb @@ -14,4 +14,6 @@ t.column :associated_id, :integer t.column :associated_type, :string end + + add_index :audits, [:auditable_type, :auditable_id], name: 'auditable_index' end diff --git a/test/upgrade_generator_test.rb b/test/upgrade_generator_test.rb index 53ada067c..376710804 100644 --- a/test/upgrade_generator_test.rb +++ b/test/upgrade_generator_test.rb @@ -79,6 +79,16 @@ class UpgradeGeneratorTest < Rails::Generators::TestCase end end + test "should add 'version' to auditable_index" do + load_schema 6 + + run_generator %w(upgrade) + + assert_migration "db/migrate/add_version_to_auditable_index.rb" do |content| + assert_match(/add_index :audits, \[:auditable_type, :auditable_id, :version\]/, content) + end + end + test "generate migration with correct AR migration parent" do load_schema 1 From 48f968d24340b7b0f8766c3cc0a2c269733279d0 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Sun, 1 Apr 2018 12:25:20 -0700 Subject: [PATCH 04/21] drop support for deprecated rails versions --- .travis.yml | 14 -------------- Appraisals | 11 ----------- CHANGELOG.md | 2 ++ README.md | 2 +- audited.gemspec | 5 ++--- gemfiles/rails40.gemfile | 9 --------- gemfiles/rails41.gemfile | 8 -------- lib/audited/auditor.rb | 5 ----- spec/audited/auditor_spec.rb | 2 +- 9 files changed, 6 insertions(+), 52 deletions(-) delete mode 100644 gemfiles/rails40.gemfile delete mode 100644 gemfiles/rails41.gemfile diff --git a/.travis.yml b/.travis.yml index bc111c075..0f6537102 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,8 +18,6 @@ before_install: - "travis_retry gem update --system" - "travis_retry gem install bundler" gemfile: - - gemfiles/rails40.gemfile - - gemfiles/rails41.gemfile - gemfiles/rails42.gemfile - gemfiles/rails50.gemfile - gemfiles/rails51.gemfile @@ -34,18 +32,6 @@ matrix: gemfile: gemfiles/rails51.gemfile - rvm: 2.1 gemfile: gemfiles/rails52.gemfile - - rvm: 2.4.3 - gemfile: gemfiles/rails40.gemfile - - rvm: 2.4.3 - gemfile: gemfiles/rails41.gemfile - - rvm: 2.5.0 - gemfile: gemfiles/rails40.gemfile - - rvm: 2.5.0 - gemfile: gemfiles/rails41.gemfile - - rvm: ruby-head - gemfile: gemfiles/rails40.gemfile - - rvm: ruby-head - gemfile: gemfiles/rails41.gemfile fast_finish: true branches: only: diff --git a/Appraisals b/Appraisals index 9e8ef4cf3..189cf4aff 100644 --- a/Appraisals +++ b/Appraisals @@ -1,14 +1,3 @@ -appraise 'rails40' do - gem 'rails', '~> 4.0.0' - gem 'protected_attributes' - gem 'test-unit' -end - -appraise 'rails41' do - gem 'rails', '~> 4.1.0' - gem 'protected_attributes' -end - appraise 'rails42' do gem 'rails', '~> 4.2.0' gem 'protected_attributes' diff --git a/CHANGELOG.md b/CHANGELOG.md index f8a4a5bf6..fb332dc25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ Breaking changes - removed `audited_columns`, `non_audited_columns`, `auditing_enabled=` instance methods, use class methods instead [#424](https://github.com/collectiveidea/audited/pull/424) +- removed rails 4.1 and 4.0 support + [#431](https://github.com/collectiveidea/audited/pull/431) Added diff --git a/README.md b/README.md index 680886732..9a483501a 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ Audited [![Build Status](https://secure.travis-ci.org/collectiveidea/audited.svg **Audited** (previously acts_as_audited) is an ORM extension that logs all changes to your models. Audited can also record who made those changes, save comments and associate models related to the changes. -Audited currently (4.x) works with Rails 5.1, 5.0 and 4.2. It may work with 4.1 and 4.0, but this is not guaranteed. +Audited currently (4.x) works with Rails 5.1, 5.0 and 4.2. For Rails 3, use gem version 3.0 or see the [3.0-stable branch](https://github.com/collectiveidea/audited/tree/3.0-stable). diff --git a/audited.gemspec b/audited.gemspec index 99527a8b6..adb0ff88e 100644 --- a/audited.gemspec +++ b/audited.gemspec @@ -15,12 +15,11 @@ Gem::Specification.new do |gem| gem.license = 'MIT' gem.files = `git ls-files`.split($\).reject{|f| f =~ /(\.gemspec)/ } - gem.require_paths = ['lib'] - gem.add_dependency 'activerecord', '>= 4.0', '< 5.2' + gem.add_dependency 'activerecord', '>= 4.2', '< 5.2' gem.add_development_dependency 'appraisal' - gem.add_development_dependency 'rails', '>= 4.0', '< 5.2' + gem.add_development_dependency 'rails', '>= 4.2', '< 5.2' gem.add_development_dependency 'rspec-rails', '~> 3.5' # JRuby support for the test ENV diff --git a/gemfiles/rails40.gemfile b/gemfiles/rails40.gemfile deleted file mode 100644 index 3748cc964..000000000 --- a/gemfiles/rails40.gemfile +++ /dev/null @@ -1,9 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "rails", "~> 4.0.0" -gem "protected_attributes" -gem "test-unit" - -gemspec name: "audited", path: "../" diff --git a/gemfiles/rails41.gemfile b/gemfiles/rails41.gemfile deleted file mode 100644 index 31512f1cd..000000000 --- a/gemfiles/rails41.gemfile +++ /dev/null @@ -1,8 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "rails", "~> 4.1.0" -gem "protected_attributes" - -gemspec name: "audited", path: "../" diff --git a/lib/audited/auditor.rb b/lib/audited/auditor.rb index c3c692135..c4b1981fa 100644 --- a/lib/audited/auditor.rb +++ b/lib/audited/auditor.rb @@ -162,7 +162,6 @@ def combine_audits(audits_to_combine) def revision_with(attributes) dup.tap do |revision| revision.id = id - revision.send :instance_variable_set, '@attributes', self.attributes if rails_below?('4.2.0') revision.send :instance_variable_set, '@new_record', destroyed? revision.send :instance_variable_set, '@persisted', !destroyed? revision.send :instance_variable_set, '@readonly', false @@ -185,10 +184,6 @@ def revision_with(attributes) end end - def rails_below?(rails_version) - Gem::Version.new(Rails::VERSION::STRING) < Gem::Version.new(rails_version) - end - private def audited_changes diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index 45fc72695..50c8aea21 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -192,7 +192,7 @@ def non_column_attr=(val) expect(user.audits.last.audited_changes.keys).to eq(%w{non_column_attr}) end - if ActiveRecord::Base.connection.adapter_name == 'PostgreSQL' && Rails.version >= "4.2.0.0" # Postgres json and jsonb support was added in Rails 4.2 + if ActiveRecord::Base.connection.adapter_name == 'PostgreSQL' describe "'json' and 'jsonb' audited_changes column type" do let(:migrations_path) { SPEC_ROOT.join("support/active_record/postgres") } From 8486dec73b0e64ce7f465640980cce82154b9d21 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Fri, 30 Mar 2018 02:59:27 +0300 Subject: [PATCH 05/21] Add `all_audits` method to auditable models --- CHANGELOG.md | 2 ++ README.md | 5 +++++ lib/audited/auditor.rb | 8 ++++++++ spec/audited/auditor_spec.rb | 27 +++++++++++++++++++++++++++ spec/support/active_record/models.rb | 1 + 5 files changed, 43 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb332dc25..51c1f3781 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ Added - Add ability to globally disable auditing [#426](https://github.com/collectiveidea/audited/pull/426) +- Add `all_audits` method to auditable models + [#428](https://github.com/collectiveidea/audited/pull/428) Changed diff --git a/README.md b/README.md index 9a483501a..6dabc5e07 100644 --- a/README.md +++ b/README.md @@ -255,6 +255,11 @@ user.audits.last.associated # => # company.associated_audits.last.auditable # => # ``` +You can access records' own audits and associated audits in one go: +```ruby +company.all_audits +``` + ### Conditional auditing If you want to audit only under specific conditions, you can provide conditional options (similar to ActiveModel callbacks) that will ensure your model is only audited for these conditions. diff --git a/lib/audited/auditor.rb b/lib/audited/auditor.rb index c4b1981fa..f5d84e8e7 100644 --- a/lib/audited/auditor.rb +++ b/lib/audited/auditor.rb @@ -145,6 +145,14 @@ def audited_attributes attributes.except(*self.class.non_audited_columns) end + # Returns a list combined of record audits and associated audits. + def all_audits + Audited.audit_class.unscoped + .where('(auditable_type = :type AND auditable_id = :id) OR (associated_type = :type AND associated_id = :id)', + type: self.class.name, id: id) + .order(created_at: :desc) + end + # Combine multiple audits into one. def combine_audits(audits_to_combine) combine_target = audits_to_combine.last diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index 50c8aea21..1a534e60a 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -677,6 +677,33 @@ def stub_global_max_audits(max_audits) end end + describe "all_audits" do + it "should return audits for self and associated audits" do + owner = Models::ActiveRecord::Owner.create! + company = owner.companies.create! + company.update!(name: "Collective Idea") + + other_owner = Models::ActiveRecord::Owner.create! + other_company = other_owner.companies.create! + + expect(owner.all_audits).to match_array(owner.audits + company.audits) + end + + it "should order audits by creation time" do + owner = Models::ActiveRecord::Owner.create! + first_audit = owner.audits.first + first_audit.update_column(:created_at, 1.year.ago) + + company = owner.companies.create! + second_audit = company.audits.first + second_audit.update_column(:created_at, 1.month.ago) + + company.update!(name: "Collective Idea") + third_audit = company.audits.last + expect(owner.all_audits.to_a).to eq([third_audit, second_audit, first_audit]) + end + end + describe "without auditing" do it "should not save an audit when calling #save_without_auditing" do expect { diff --git a/spec/support/active_record/models.rb b/spec/support/active_record/models.rb index 2fa4eeb0a..701445f48 100644 --- a/spec/support/active_record/models.rb +++ b/spec/support/active_record/models.rb @@ -91,6 +91,7 @@ class Company::STICompany < Company class Owner < ::ActiveRecord::Base self.table_name = 'users' + audited has_associated_audits has_many :companies, class_name: "OwnedCompany", dependent: :destroy end From fd951661fa6410f94194a93059a69e805f14595f Mon Sep 17 00:00:00 2001 From: fatkodima Date: Wed, 4 Apr 2018 17:57:10 +0300 Subject: [PATCH 06/21] #all_audits -> #own_and_associated_audits --- CHANGELOG.md | 4 ++-- README.md | 2 +- lib/audited/auditor.rb | 2 +- spec/audited/auditor_spec.rb | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51c1f3781..d7f75be45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ Breaking changes -- removed `audited_columns`, `non_audited_columns`, `auditing_enabled=` instance methods, +- removed `audited_columns`, `non_audited_columns`, `auditing_enabled=` instance methods, use class methods instead [#424](https://github.com/collectiveidea/audited/pull/424) - removed rails 4.1 and 4.0 support @@ -14,7 +14,7 @@ Added - Add ability to globally disable auditing [#426](https://github.com/collectiveidea/audited/pull/426) -- Add `all_audits` method to auditable models +- Add `own_and_associated_audits` method to auditable models [#428](https://github.com/collectiveidea/audited/pull/428) Changed diff --git a/README.md b/README.md index 6dabc5e07..cb2dc1784 100644 --- a/README.md +++ b/README.md @@ -257,7 +257,7 @@ company.associated_audits.last.auditable # => # You can access records' own audits and associated audits in one go: ```ruby -company.all_audits +company.own_and_associated_audits ``` ### Conditional auditing diff --git a/lib/audited/auditor.rb b/lib/audited/auditor.rb index f5d84e8e7..9ff0b3b93 100644 --- a/lib/audited/auditor.rb +++ b/lib/audited/auditor.rb @@ -146,7 +146,7 @@ def audited_attributes end # Returns a list combined of record audits and associated audits. - def all_audits + def own_and_associated_audits Audited.audit_class.unscoped .where('(auditable_type = :type AND auditable_id = :id) OR (associated_type = :type AND associated_id = :id)', type: self.class.name, id: id) diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index 1a534e60a..174dbe683 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -677,7 +677,7 @@ def stub_global_max_audits(max_audits) end end - describe "all_audits" do + describe "own_and_associated_audits" do it "should return audits for self and associated audits" do owner = Models::ActiveRecord::Owner.create! company = owner.companies.create! @@ -686,7 +686,7 @@ def stub_global_max_audits(max_audits) other_owner = Models::ActiveRecord::Owner.create! other_company = other_owner.companies.create! - expect(owner.all_audits).to match_array(owner.audits + company.audits) + expect(owner.own_and_associated_audits).to match_array(owner.audits + company.audits) end it "should order audits by creation time" do @@ -700,7 +700,7 @@ def stub_global_max_audits(max_audits) company.update!(name: "Collective Idea") third_audit = company.audits.last - expect(owner.all_audits.to_a).to eq([third_audit, second_audit, first_audit]) + expect(owner.own_and_associated_audits.to_a).to eq([third_audit, second_audit, first_audit]) end end From 4116a7879eee2cef65d34efad0cc45c2516a5615 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Sun, 1 Apr 2018 17:28:32 -0700 Subject: [PATCH 07/21] bump minimum ruby version --- .travis.yml | 15 +++------------ README.md | 8 +++----- audited.gemspec | 3 ++- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0f6537102..d9072d0e9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,9 @@ language: ruby cache: bundler rvm: - - 2.1 - - 2.2.9 - - 2.3.6 - - 2.4.3 - - 2.5.0 + - 2.3.7 + - 2.4.4 + - 2.5.1 - ruby-head env: - DB=SQLITE @@ -25,13 +23,6 @@ gemfile: matrix: allow_failures: - rvm: ruby-head - exclude: - - rvm: 2.1 - gemfile: gemfiles/rails50.gemfile - - rvm: 2.1 - gemfile: gemfiles/rails51.gemfile - - rvm: 2.1 - gemfile: gemfiles/rails52.gemfile fast_finish: true branches: only: diff --git a/README.md b/README.md index cb2dc1784..e8194f707 100644 --- a/README.md +++ b/README.md @@ -11,11 +11,9 @@ For Rails 3, use gem version 3.0 or see the [3.0-stable branch](https://github.c Audited supports and is [tested against](http://travis-ci.org/collectiveidea/audited) the following Ruby versions: -* 2.1.10 -* 2.2.9 -* 2.3.6 -* 2.4.3 -* 2.5.0 +* 2.3.7 +* 2.4.4 +* 2.5.1 Audited may work just fine with a Ruby version not listed above, but we can't guarantee that it will. If you'd like to maintain a Ruby that isn't listed, please let us know with a [pull request](https://github.com/collectiveidea/audited/pulls). diff --git a/audited.gemspec b/audited.gemspec index adb0ff88e..f88578174 100644 --- a/audited.gemspec +++ b/audited.gemspec @@ -5,7 +5,6 @@ require "audited/version" Gem::Specification.new do |gem| gem.name = 'audited' gem.version = Audited::VERSION - gem.platform = Gem::Platform::RUBY gem.authors = ['Brandon Keepers', 'Kenneth Kalmer', 'Daniel Morrison', 'Brian Ryckbost', 'Steve Richert', 'Ryan Glover'] gem.email = 'info@collectiveidea.com' @@ -16,6 +15,8 @@ Gem::Specification.new do |gem| gem.files = `git ls-files`.split($\).reject{|f| f =~ /(\.gemspec)/ } + gem.required_ruby_version = '>= 2.3.0' + gem.add_dependency 'activerecord', '>= 4.2', '< 5.2' gem.add_development_dependency 'appraisal' From ed4e2ecc1899771784b660d3e9146ffd694c7ce4 Mon Sep 17 00:00:00 2001 From: Kurtis Rainbolt-Greene Date: Sun, 8 Apr 2018 03:31:17 -0700 Subject: [PATCH 08/21] Simplify the undo logic (#436) --- lib/audited/audit.rb | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/audited/audit.rb b/lib/audited/audit.rb index ff82e4e35..94ff4afa7 100644 --- a/lib/audited/audit.rb +++ b/lib/audited/audit.rb @@ -88,20 +88,18 @@ def old_attributes # Allows user to undo changes def undo - model = self.auditable_type.constantize - if action == 'create' + case action + when 'create' # destroys a newly created record - model.find(auditable_id).destroy! - elsif action == 'destroy' + auditable.destroy! + when 'destroy' # creates a new record with the destroyed record attributes - model.create(audited_changes) - else + auditable_type.constantize.create!(audited_changes) + when 'update' # changes back attributes - audited_object = model.find(auditable_id) - self.audited_changes.each do |k, v| - audited_object[k] = v[0] - end - audited_object.save + auditable.update_attributes!(audited_changes.transform_values(&:first)) + else + raise StandardError, "invalid action given #{action}" end end From 32982cb1bba4b886cfb71459dac436cc61abd289 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Sun, 8 Apr 2018 10:21:47 -0700 Subject: [PATCH 09/21] add actionable coverage reporting (#356) --- .gitignore | 1 - audited.gemspec | 1 + spec/audited/audit_spec.rb | 2 ++ spec/audited/auditor_spec.rb | 2 ++ spec/audited/sweeper_spec.rb | 2 ++ spec/spec_helper.rb | 4 +++- 6 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 6c9ec7593..dff6ab0ad 100644 --- a/.gitignore +++ b/.gitignore @@ -9,7 +9,6 @@ .ruby-version .rvmrc .yardoc -coverage/ doc/ Gemfile.lock gemfiles/*.lock diff --git a/audited.gemspec b/audited.gemspec index f88578174..6d2eccf8a 100644 --- a/audited.gemspec +++ b/audited.gemspec @@ -22,6 +22,7 @@ Gem::Specification.new do |gem| gem.add_development_dependency 'appraisal' gem.add_development_dependency 'rails', '>= 4.2', '< 5.2' gem.add_development_dependency 'rspec-rails', '~> 3.5' + gem.add_development_dependency 'single_cov' # JRuby support for the test ENV if defined?(JRUBY_VERSION) diff --git a/spec/audited/audit_spec.rb b/spec/audited/audit_spec.rb index 3260072eb..8ffbe4277 100644 --- a/spec/audited/audit_spec.rb +++ b/spec/audited/audit_spec.rb @@ -1,5 +1,7 @@ require "spec_helper" +SingleCov.covered! uncovered: 7 # not testing json object and collection_cache_key + describe Audited::Audit do let(:user) { Models::ActiveRecord::User.new name: "Testing" } diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index 174dbe683..10e8fb533 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -1,5 +1,7 @@ require "spec_helper" +SingleCov.covered! uncovered: 9 # not testing proxy_respond_to? hack / 2 methods + describe Audited::Auditor do describe "configuration" do diff --git a/spec/audited/sweeper_spec.rb b/spec/audited/sweeper_spec.rb index 07093fb40..48cae7eba 100644 --- a/spec/audited/sweeper_spec.rb +++ b/spec/audited/sweeper_spec.rb @@ -1,5 +1,7 @@ require "spec_helper" +SingleCov.covered! uncovered: 3 + class AuditsController < ActionController::Base before_action :populate_user diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1818d25e2..9b1993f0d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,8 @@ ENV['RAILS_ENV'] = 'test' +require 'bundler/setup' +require 'single_cov' +SingleCov.setup :rspec -require 'bundler' if Bundler.definition.dependencies.map(&:name).include?('protected_attributes') require 'protected_attributes' end From 0e7975426acf641edfefac5da7806d6faf767ebb Mon Sep 17 00:00:00 2001 From: Jorge Oliveira Santos Date: Tue, 10 Apr 2018 13:42:51 +0800 Subject: [PATCH 10/21] Add support for the final release of Rails 5.2 (#441) - Fixes #440 - Bumped Rails version support on audited.gemspec - Bumped Rails version support on gemfiles/rails52.gemfile - Bumped Rails version support on Appraisals - Added Rails 5.2 support to README --- Appraisals | 2 +- README.md | 2 +- audited.gemspec | 4 ++-- gemfiles/rails52.gemfile | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Appraisals b/Appraisals index 189cf4aff..545453620 100644 --- a/Appraisals +++ b/Appraisals @@ -12,6 +12,6 @@ appraise 'rails51' do end appraise 'rails52' do - gem 'rails', '>= 5.2.0.rc1', '< 5.3' + gem 'rails', '>= 5.2.0', '< 5.3' gem 'mysql2', '~> 0.4.4' end diff --git a/README.md b/README.md index e8194f707..813daf445 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ Audited [![Build Status](https://secure.travis-ci.org/collectiveidea/audited.svg **Audited** (previously acts_as_audited) is an ORM extension that logs all changes to your models. Audited can also record who made those changes, save comments and associate models related to the changes. -Audited currently (4.x) works with Rails 5.1, 5.0 and 4.2. +Audited currently (4.x) works with Rails 5.2, 5.1, 5.0 and 4.2. For Rails 3, use gem version 3.0 or see the [3.0-stable branch](https://github.com/collectiveidea/audited/tree/3.0-stable). diff --git a/audited.gemspec b/audited.gemspec index 6d2eccf8a..58d2180d7 100644 --- a/audited.gemspec +++ b/audited.gemspec @@ -17,10 +17,10 @@ Gem::Specification.new do |gem| gem.required_ruby_version = '>= 2.3.0' - gem.add_dependency 'activerecord', '>= 4.2', '< 5.2' + gem.add_dependency 'activerecord', '>= 4.2', '< 5.3' gem.add_development_dependency 'appraisal' - gem.add_development_dependency 'rails', '>= 4.2', '< 5.2' + gem.add_development_dependency 'rails', '>= 4.2', '< 5.3' gem.add_development_dependency 'rspec-rails', '~> 3.5' gem.add_development_dependency 'single_cov' diff --git a/gemfiles/rails52.gemfile b/gemfiles/rails52.gemfile index c23ace03b..68510e9d3 100644 --- a/gemfiles/rails52.gemfile +++ b/gemfiles/rails52.gemfile @@ -2,7 +2,7 @@ source "https://rubygems.org" -gem "rails", ">= 5.2.0.rc1", "< 5.3" +gem "rails", ">= 5.2.0", "< 5.3" gem "mysql2", "~> 0.4.4" gemspec name: "audited", path: "../" From 3b76fcaee8ee00a1b6a4cfa97c5d79fb00ab985a Mon Sep 17 00:00:00 2001 From: Kevin Coleman Date: Wed, 9 May 2018 11:49:04 +0700 Subject: [PATCH 11/21] Fix typo in readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 813daf445..421d0b276 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ Outside of a request, Audited can still record the user with the `as_user` metho ```ruby Audited.audit_class.as_user(User.find(1)) do - post.update_attribute!(title: "Hello, world!") + post.update_attributes!(title: "Hello, world!") end post.audits.last.user # => # ``` From dbac058631fd9a4cd9ed8b426ab2b4681c90360e Mon Sep 17 00:00:00 2001 From: David Genord II Date: Fri, 8 Jun 2018 11:20:10 -0400 Subject: [PATCH 12/21] Remove the Gemnasium badge This is a service we no longer use. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 421d0b276..2ca981858 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -Audited [![Build Status](https://secure.travis-ci.org/collectiveidea/audited.svg)](http://travis-ci.org/collectiveidea/audited) [![Dependency Status](https://gemnasium.com/collectiveidea/audited.svg)](https://gemnasium.com/collectiveidea/audited)[![Code Climate](https://codeclimate.com/github/collectiveidea/audited.svg)](https://codeclimate.com/github/collectiveidea/audited) [![Security](https://hakiri.io/github/collectiveidea/audited/master.svg)](https://hakiri.io/github/collectiveidea/audited/master) +Audited [![Build Status](https://secure.travis-ci.org/collectiveidea/audited.svg)](http://travis-ci.org/collectiveidea/audited) [![Code Climate](https://codeclimate.com/github/collectiveidea/audited.svg)](https://codeclimate.com/github/collectiveidea/audited) [![Security](https://hakiri.io/github/collectiveidea/audited/master.svg)](https://hakiri.io/github/collectiveidea/audited/master) ======= **Audited** (previously acts_as_audited) is an ORM extension that logs all changes to your models. Audited can also record who made those changes, save comments and associate models related to the changes. From 88f2779746bf4ee177df99b06115cd91f0bb9d92 Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Fri, 1 Jun 2018 09:41:55 -0400 Subject: [PATCH 13/21] include 4.7.1 changelog updates [ci skip] I noticed the 4.7.1 changes were missing from the Changelog.md on master. This PR adds them back. --- CHANGELOG.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7f75be45..2d8ebd2de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,24 @@ Fixed - None +## 4.7.1 (2018-04-10) + +Breaking changes + +- None + +Added + +- None + +Changed + +- None + +Fixed + +- Allow use with Rails 5.2 final + ## 4.7.0 (2018-03-14) Breaking changes From 46e560ddb63144c7f9741c8b8b99e99b43839d89 Mon Sep 17 00:00:00 2001 From: Kevin Coleman Date: Sun, 17 Jun 2018 19:47:27 +0700 Subject: [PATCH 14/21] Allow nested as_user auditing (#450) --- CHANGELOG.md | 2 ++ lib/audited/audit.rb | 3 ++- spec/audited/audit_spec.rb | 19 +++++++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d8ebd2de..adebd7515 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ Added [#426](https://github.com/collectiveidea/audited/pull/426) - Add `own_and_associated_audits` method to auditable models [#428](https://github.com/collectiveidea/audited/pull/428) +- Ability to nest `as_user` within itself + [#450](https://github.com/collectiveidea/audited/pull/450) Changed diff --git a/lib/audited/audit.rb b/lib/audited/audit.rb index 94ff4afa7..2eca03ab0 100644 --- a/lib/audited/audit.rb +++ b/lib/audited/audit.rb @@ -131,10 +131,11 @@ def self.audited_classes # by +user+. This method is hopefully threadsafe, making it ideal # for background operations that require audit information. def self.as_user(user, &block) + last_audited_user = ::Audited.store[:audited_user] ::Audited.store[:audited_user] = user yield ensure - ::Audited.store[:audited_user] = nil + ::Audited.store[:audited_user] = last_audited_user end # @private diff --git a/spec/audited/audit_spec.rb b/spec/audited/audit_spec.rb index 8ffbe4277..0f9169c34 100644 --- a/spec/audited/audit_spec.rb +++ b/spec/audited/audit_spec.rb @@ -215,6 +215,25 @@ class Models::ActiveRecord::CustomUserSubclass < Models::ActiveRecord::CustomUse end end + it "should support nested as_user" do + Audited::Audit.as_user("sidekiq") do + company = Models::ActiveRecord::Company.create name: "The auditors" + company.name = "The Auditors, Inc" + company.save + expect(company.audits[-1].user).to eq("sidekiq") + + Audited::Audit.as_user(user) do + company.name = "NEW Auditors, Inc" + company.save + expect(company.audits[-1].user).to eq(user) + end + + company.name = "LAST Auditors, Inc" + company.save + expect(company.audits[-1].user).to eq("sidekiq") + end + end + it "should record usernames" do Audited::Audit.as_user(user.name) do company = Models::ActiveRecord::Company.create name: "The auditors" From 02d2e24a55a4eee42f1baa9ba5f0083ef8793ef9 Mon Sep 17 00:00:00 2001 From: rauan Date: Wed, 16 May 2018 17:20:41 -0300 Subject: [PATCH 15/21] allow private methods on audited condition --- lib/audited/auditor.rb | 3 +-- spec/audited/auditor_spec.rb | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/audited/auditor.rb b/lib/audited/auditor.rb index 9ff0b3b93..2eeaa76ea 100644 --- a/lib/audited/auditor.rb +++ b/lib/audited/auditor.rb @@ -285,9 +285,8 @@ def auditing_enabled def run_conditional_check(condition, matching: true) return true if condition.blank? - return condition.call(self) == matching if condition.respond_to?(:call) - return send(condition) == matching if respond_to?(condition.to_sym) + return send(condition) == matching if respond_to?(condition.to_sym, true) true end diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index 10e8fb533..a78729ddf 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -22,6 +22,24 @@ context "should be configurable which conditions are audited" do subject { ConditionalCompany.new.send(:auditing_enabled) } + context "when condition method is private" do + subject { ConditionalPrivateCompany.new.send(:auditing_enabled) } + + before do + class ConditionalPrivateCompany < ::ActiveRecord::Base + self.table_name = 'companies' + + audited if: :foo? + + private def foo? + true + end + end + end + + it { is_expected.to be_truthy } + end + context "when passing a method name" do before do class ConditionalCompany < ::ActiveRecord::Base From ea58b019b5f4b390f76c455538b42b144d532ee3 Mon Sep 17 00:00:00 2001 From: Tomer Brisker Date: Sun, 24 Jun 2018 16:19:38 +0300 Subject: [PATCH 16/21] Update Changelog for #454 --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index adebd7515..e4e509fd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,10 +18,12 @@ Added [#428](https://github.com/collectiveidea/audited/pull/428) - Ability to nest `as_user` within itself [#450](https://github.com/collectiveidea/audited/pull/450) +- Private methods can now be used for conditional auditing + [#454](https://github.com/collectiveidea/audited/pull/454) Changed -- Add version to auditable_index +- Add version to `auditable_index` [#427](https://github.com/collectiveidea/audited/pull/427) Fixed From 9c40fcab35171937ce8a9aa0b3df86609c445851 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Sun, 8 Apr 2018 16:51:50 -0700 Subject: [PATCH 17/21] improve code coverage --- CHANGELOG.md | 2 + lib/audited/audit.rb | 10 ++-- spec/audited/audit_spec.rb | 90 +++++++++++++++++++++++++++--------- spec/audited/sweeper_spec.rb | 16 +++++-- 4 files changed, 85 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4e509fd4..f52fed165 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Breaking changes +- removed block support for `Audit.reconstruct_attributes` + [#437](https://github.com/collectiveidea/audited/pull/437) - removed `audited_columns`, `non_audited_columns`, `auditing_enabled=` instance methods, use class methods instead [#424](https://github.com/collectiveidea/audited/pull/424) diff --git a/lib/audited/audit.rb b/lib/audited/audit.rb index 2eca03ab0..d7d56dea8 100644 --- a/lib/audited/audit.rb +++ b/lib/audited/audit.rb @@ -140,12 +140,10 @@ def self.as_user(user, &block) # @private def self.reconstruct_attributes(audits) - attributes = {} - result = audits.collect do |audit| - attributes.merge!(audit.new_attributes)[:version] = audit.version - yield attributes if block_given? + audits.each_with_object({}) do |audit, all| + all.merge!(audit.new_attributes) + all[:version] = audit.version end - block_given? ? result : attributes end # @private @@ -163,7 +161,7 @@ def self.assign_revision_attributes(record, attributes) end # use created_at as timestamp cache key - def self.collection_cache_key(collection = all, timestamp_column = :created_at) + def self.collection_cache_key(collection = all, *) super(collection, :created_at) end diff --git a/spec/audited/audit_spec.rb b/spec/audited/audit_spec.rb index 0f9169c34..2f0008557 100644 --- a/spec/audited/audit_spec.rb +++ b/spec/audited/audit_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -SingleCov.covered! uncovered: 7 # not testing json object and collection_cache_key +SingleCov.covered! describe Audited::Audit do let(:user) { Models::ActiveRecord::User.new name: "Testing" } @@ -40,43 +40,64 @@ class TempModel < ::ActiveRecord::Base end end - it "should undo changes" do - user = Models::ActiveRecord::User.create(name: "John") + context "when a custom audit class is not configured" do + it "should default to #{described_class}" do + TempModel.audited + + record = TempModel.create + + audit = record.audits.first + expect(audit).to be_a Audited::Audit + expect(audit.respond_to?(:custom_method)).to be false + end + end + end + + describe "#audited_changes" do + let(:audit) { Audited.audit_class.new } + + it "can unserialize yaml from text columns" do + audit.audited_changes = {foo: "bar"} + expect(audit.audited_changes).to eq foo: "bar" + end + + it "does not unserialize from binary columns" do + allow(Audited.audit_class.columns_hash["audited_changes"]).to receive(:type).and_return("foo") + audit.audited_changes = {foo: "bar"} + expect(audit.audited_changes).to eq "{:foo=>\"bar\"}" + end + end + + describe "#undo" do + let(:user) { Models::ActiveRecord::User.create(name: "John") } + + it "undos changes" do user.update_attribute(:name, 'Joe') user.audits.last.undo user.reload - expect(user.name).to eq("John") end - it "should undo destroyed model" do - user = Models::ActiveRecord::User.create(name: "John") + it "undos destroy" do user.destroy user.audits.last.undo user = Models::ActiveRecord::User.find_by(name: "John") expect(user.name).to eq("John") end - it "should undo created model" do - user = Models::ActiveRecord::User.create(name: "John") + it "undos creation" do + user # trigger create expect {user.audits.last.undo}.to change(Models::ActiveRecord::User, :count).by(-1) end - context "when a custom audit class is not configured" do - it "should default to #{described_class}" do - TempModel.audited - - record = TempModel.create - - audit = record.audits.first - expect(audit).to be_a Audited::Audit - expect(audit.respond_to?(:custom_method)).to be false - end + it "fails when trying to undo unknown" do + audit = user.audits.last + audit.action = 'oops' + expect { audit.undo }.to raise_error("invalid action given oops") end end describe "user=" do - it "should be able to set the user to a model object" do subject.user = user expect(subject.user).to eq(user) @@ -112,11 +133,9 @@ class TempModel < ::ActiveRecord::Base subject.user = user expect(subject.username).to be_nil end - end describe "revision" do - it "should recreate attributes" do user = Models::ActiveRecord::User.create name: "1" 5.times {|i| user.update_attribute :name, (i + 2).to_s } @@ -150,6 +169,34 @@ class TempModel < ::ActiveRecord::Base end end + describe ".collection_cache_key" do + if ActiveRecord::VERSION::MAJOR >= 5 + it "uses created at" do + Audited::Audit.delete_all + audit = Models::ActiveRecord::User.create(name: "John").audits.last + audit.update_columns(created_at: Time.parse('2018-01-01')) + expect(Audited::Audit.collection_cache_key).to match(/-20180101\d+$/) + end + else + it "is not defined" do + expect { Audited::Audit.collection_cache_key }.to raise_error(NoMethodError) + end + end + end + + describe ".assign_revision_attributes" do + it "dups when frozen" do + user.freeze + assigned = Audited::Audit.assign_revision_attributes(user, name: "Bar") + expect(assigned.name).to eq "Bar" + end + + it "ignores unassignable attributes" do + assigned = Audited::Audit.assign_revision_attributes(user, oops: "Bar") + expect(assigned.name).to eq "Testing" + end + end + it "should set the version number on create" do user = Models::ActiveRecord::User.create! name: "Set Version Number" expect(user.audits.first.version).to eq(1) @@ -284,6 +331,5 @@ class Models::ActiveRecord::CustomUserSubclass < Models::ActiveRecord::CustomUse }.to raise_exception('expected') expect(Audited.store[:audited_user]).to be_nil end - end end diff --git a/spec/audited/sweeper_spec.rb b/spec/audited/sweeper_spec.rb index 48cae7eba..975d5255d 100644 --- a/spec/audited/sweeper_spec.rb +++ b/spec/audited/sweeper_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -SingleCov.covered! uncovered: 3 +SingleCov.covered! uncovered: 2 # 2 conditional on_load conditions class AuditsController < ActionController::Base before_action :populate_user @@ -29,14 +29,13 @@ def populate_user; end include RSpec::Rails::ControllerExampleGroup render_views - before(:each) do + before do Audited.current_user_method = :current_user end let(:user) { create_user } describe "POST audit" do - it "should audit user" do controller.send(:current_user=, user) expect { @@ -46,6 +45,15 @@ def populate_user; end expect(controller.company.audits.last.user).to eq(user) end + it "does not audit when method is not found" do + controller.send(:current_user=, user) + Audited.current_user_method = :nope + expect { + post :create + }.to change( Audited::Audit, :count ) + expect(controller.company.audits.last.user).to eq(nil) + end + it "should support custom users for sweepers" do controller.send(:custom_user=, user) Audited.current_user_method = :custom_user @@ -86,7 +94,6 @@ def populate_user; end expect(controller.company.audits.last.user).to eq(user) end - end describe "PUT update" do @@ -100,7 +107,6 @@ def populate_user; end end end - describe Audited::Sweeper do it "should be thread-safe" do From fab11709fb41b8a473b099b1c94651a5c7087074 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Sat, 31 Mar 2018 02:55:56 +0300 Subject: [PATCH 18/21] Ensure enum changes are stored consistently --- CHANGELOG.md | 3 ++- lib/audited/auditor.rb | 35 ++++++++++++++++++++++++---- spec/audited/auditor_spec.rb | 30 +++++++++++++++++++++--- spec/support/active_record/models.rb | 1 + spec/support/active_record/schema.rb | 1 + 5 files changed, 61 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f52fed165..b51f7659f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,8 @@ Changed Fixed -- None +- Ensure enum changes are stored consistently + [#429](https://github.com/collectiveidea/audited/pull/429) ## 4.7.1 (2018-04-10) diff --git a/lib/audited/auditor.rb b/lib/audited/auditor.rb index 2eeaa76ea..2196407b8 100644 --- a/lib/audited/auditor.rb +++ b/lib/audited/auditor.rb @@ -142,7 +142,8 @@ def revision_at(date_or_time) # List of attributes that are audited. def audited_attributes - attributes.except(*self.class.non_audited_columns) + audited_attributes = attributes.except(*self.class.non_audited_columns) + normalize_enum_changes(audited_attributes) end # Returns a list combined of record audits and associated audits. @@ -196,11 +197,35 @@ def revision_with(attributes) def audited_changes all_changes = respond_to?(:changes_to_save) ? changes_to_save : changes - if audited_options[:only].present? - all_changes.slice(*self.class.audited_columns) - else - all_changes.except(*self.class.non_audited_columns) + filtered_changes = \ + if audited_options[:only].present? + all_changes.slice(*self.class.audited_columns) + else + all_changes.except(*self.class.non_audited_columns) + end + + filtered_changes = normalize_enum_changes(filtered_changes) + filtered_changes.to_hash + end + + def normalize_enum_changes(changes) + self.class.defined_enums.each do |name, values| + if changes.has_key?(name) + changes[name] = \ + if changes[name].is_a?(Array) + changes[name].map { |v| values[v] } + elsif rails_below?('5.0') + changes[name] + else + values[changes[name]] + end + end end + changes + end + + def rails_below?(rails_version) + Gem::Version.new(Rails::VERSION::STRING) < Gem::Version.new(rails_version) end def audits_to(version = nil) diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index a78729ddf..b30e871ea 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -266,7 +266,7 @@ def non_column_attr=(val) end describe "on create" do - let( :user ) { create_user audit_comment: "Create" } + let( :user ) { create_user status: :reliable, audit_comment: "Create" } it "should change the audit count" do expect { @@ -290,6 +290,10 @@ def non_column_attr=(val) expect(user.audits.first.audited_changes).to eq(user.audited_attributes) end + it "should store enum value" do + expect(user.audits.first.audited_changes["status"]).to eq(1) + end + it "should store comment" do expect(user.audits.first.comment).to eq('Create') end @@ -308,7 +312,7 @@ def non_column_attr=(val) describe "on update" do before do - @user = create_user( name: 'Brandon', audit_comment: 'Update' ) + @user = create_user( name: 'Brandon', status: :active, audit_comment: 'Update' ) end it "should save an audit" do @@ -332,6 +336,11 @@ def non_column_attr=(val) expect(@user.audits.last.audited_changes).to eq({ 'name' => ['Brandon', 'Changed'] }) end + it "should store changed enum values" do + @user.update_attributes status: 1 + expect(@user.audits.last.audited_changes["status"]).to eq([0, 1]) + end + it "should store audit comment" do expect(@user.audits.last.comment).to eq('Update') end @@ -368,7 +377,7 @@ def non_column_attr=(val) describe "on destroy" do before do - @user = create_user + @user = create_user(status: :active) end it "should save an audit" do @@ -393,6 +402,11 @@ def non_column_attr=(val) expect(@user.audits.last.audited_changes).to eq(@user.audited_attributes) end + it "should store enum value" do + @user.destroy + expect(@user.audits.last.audited_changes["status"]).to eq(0) + end + it "should be able to reconstruct a destroyed record without history" do @user.audits.delete_all @user.destroy @@ -648,6 +662,16 @@ def stub_global_max_audits(max_audits) expect(u.revision(1).username).to eq('brandon') end + it "should correctly restore revision with enum" do + u = Models::ActiveRecord::User.create(status: :active) + u.update_attribute(:status, :reliable) + u.update_attribute(:status, :banned) + + expect(u.revision(3)).to be_banned + expect(u.revision(2)).to be_reliable + expect(u.revision(1)).to be_active + end + it "should be able to get time for first revision" do suspended_at = Time.zone.now u = Models::ActiveRecord::User.create(suspended_at: suspended_at) diff --git a/spec/support/active_record/models.rb b/spec/support/active_record/models.rb index 701445f48..12c851496 100644 --- a/spec/support/active_record/models.rb +++ b/spec/support/active_record/models.rb @@ -7,6 +7,7 @@ class User < ::ActiveRecord::Base audited except: :password attribute :non_column_attr if Rails.version >= '5.1' attr_protected :logins if respond_to?(:attr_protected) + enum status: { active: 0, reliable: 1, banned: 2 } def name=(val) write_attribute(:name, CGI.escapeHTML(val)) diff --git a/spec/support/active_record/schema.rb b/spec/support/active_record/schema.rb index f340ff0c4..31319e022 100644 --- a/spec/support/active_record/schema.rb +++ b/spec/support/active_record/schema.rb @@ -35,6 +35,7 @@ t.column :username, :string t.column :password, :string t.column :activated, :boolean + t.column :status, :integer, default: 0 t.column :suspended_at, :datetime t.column :logins, :integer, default: 0 t.column :created_at, :datetime From 3057aecc8854df928d9a46de75fd99c77e7a5038 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Sun, 1 Apr 2018 11:59:27 -0700 Subject: [PATCH 19/21] basic rubocop checks and fixes --- .rubocop.yml | 25 +++++++++++++++++++++++++ .travis.yml | 4 ++++ audited.gemspec | 1 + lib/audited/audit.rb | 2 +- spec/audited/auditor_spec.rb | 6 +++--- spec/support/active_record/schema.rb | 2 +- 6 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 .rubocop.yml diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 000000000..27fda3aee --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,25 @@ +AllCops: + DisplayCopNames: true + TargetRubyVersion: 2.3 + Exclude: + - lib/generators/audited/templates/**/* + - vendor/bundle/**/* + - gemfiles/vendor/bundle/**/* + +Bundler/OrderedGems: + Enabled: false + +Gemspec/OrderedDependencies: + Enabled: false + +Layout: + Enabled: false + +Metrics: + Enabled: false + +Naming: + Enabled: false + +Style: + Enabled: false diff --git a/.travis.yml b/.travis.yml index d9072d0e9..2130bb8f3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,6 +21,10 @@ gemfile: - gemfiles/rails51.gemfile - gemfiles/rails52.gemfile matrix: + include: + - rvm: 2.3.7 + script: bundle exec rubocop --parallel + env: DB=rubocop # make travis build display nicer allow_failures: - rvm: ruby-head fast_finish: true diff --git a/audited.gemspec b/audited.gemspec index 58d2180d7..d05f7fcb7 100644 --- a/audited.gemspec +++ b/audited.gemspec @@ -21,6 +21,7 @@ Gem::Specification.new do |gem| gem.add_development_dependency 'appraisal' gem.add_development_dependency 'rails', '>= 4.2', '< 5.3' + gem.add_development_dependency 'rubocop', '~> 0.54.0' gem.add_development_dependency 'rspec-rails', '~> 3.5' gem.add_development_dependency 'single_cov' diff --git a/lib/audited/audit.rb b/lib/audited/audit.rb index d7d56dea8..a79a39eaa 100644 --- a/lib/audited/audit.rb +++ b/lib/audited/audit.rb @@ -130,7 +130,7 @@ def self.audited_classes # All audits made during the block called will be recorded as made # by +user+. This method is hopefully threadsafe, making it ideal # for background operations that require audit information. - def self.as_user(user, &block) + def self.as_user(user) last_audited_user = ::Audited.store[:audited_user] ::Audited.store[:audited_user] = user yield diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index b30e871ea..5dfc530ac 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -728,7 +728,7 @@ def stub_global_max_audits(max_audits) company.update!(name: "Collective Idea") other_owner = Models::ActiveRecord::Owner.create! - other_company = other_owner.companies.create! + other_owner.companies.create! expect(owner.own_and_associated_audits).to match_array(owner.audits + company.audits) end @@ -933,7 +933,7 @@ def stub_global_max_audits(max_audits) end describe "after_audit" do - let( :user ) { user = Models::ActiveRecord::UserWithAfterAudit.new } + let( :user ) { Models::ActiveRecord::UserWithAfterAudit.new } it "should invoke after_audit callback on create" do expect(user.bogus_attr).to be_nil @@ -943,7 +943,7 @@ def stub_global_max_audits(max_audits) end describe "around_audit" do - let( :user ) { user = Models::ActiveRecord::UserWithAfterAudit.new } + let( :user ) { Models::ActiveRecord::UserWithAfterAudit.new } it "should invoke around_audit callback on create" do expect(user.around_attr).to be_nil diff --git a/spec/support/active_record/schema.rb b/spec/support/active_record/schema.rb index 31319e022..ab04d4082 100644 --- a/spec/support/active_record/schema.rb +++ b/spec/support/active_record/schema.rb @@ -18,7 +18,7 @@ adapter.recreate_database db_name adapter.disconnect! end -rescue Exception => e +rescue => e Kernel.warn e end From 9cff81b40fbc71e2455a00ef50f10bc200784af2 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Mon, 25 Jun 2018 17:23:17 -0700 Subject: [PATCH 20/21] fix test failing and being noisy on ruby 2.5 --- spec/rails_app/config/application.rb | 5 +++++ test/test_helper.rb | 3 +-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/rails_app/config/application.rb b/spec/rails_app/config/application.rb index 0df2498d8..6b6bbd24f 100644 --- a/spec/rails_app/config/application.rb +++ b/spec/rails_app/config/application.rb @@ -6,3 +6,8 @@ class Application < Rails::Application config.i18n.enforce_available_locales = true end end + +require 'active_record/connection_adapters/sqlite3_adapter' +if ActiveRecord::ConnectionAdapters::SQLite3Adapter.respond_to?(:represent_boolean_as_integer) + ActiveRecord::ConnectionAdapters::SQLite3Adapter.represent_boolean_as_integer = true +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 1cdb04dcb..15e7e3adb 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,6 +1,6 @@ ENV['RAILS_ENV'] = 'test' -$:.unshift File.dirname(__FILE__) +$LOAD_PATH.unshift File.dirname(__FILE__) require File.expand_path('../../spec/rails_app/config/environment', __FILE__) require 'rails/test_help' @@ -8,7 +8,6 @@ require 'audited' class ActiveSupport::TestCase - setup do ActiveRecord::Migration.verbose = false end From 250cacc1a4b3cd80f581f6436d55f3286d47fab4 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Mon, 25 Jun 2018 17:30:33 -0700 Subject: [PATCH 21/21] avoid exceptions in threads that print their backtrace on ruby 2.5 --- spec/audited/auditor_spec.rb | 42 +++++++++++++++++------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/spec/audited/auditor_spec.rb b/spec/audited/auditor_spec.rb index 5dfc530ac..a2ed2383f 100644 --- a/spec/audited/auditor_spec.rb +++ b/spec/audited/auditor_spec.rb @@ -768,31 +768,29 @@ def stub_global_max_audits(max_audits) end it "should be thread safe using a #without_auditing block" do - begin - t1 = Thread.new do - expect(Models::ActiveRecord::User.auditing_enabled).to eq(true) - Models::ActiveRecord::User.without_auditing do - expect(Models::ActiveRecord::User.auditing_enabled).to eq(false) - Models::ActiveRecord::User.create!( name: 'Bart' ) - sleep 1 - expect(Models::ActiveRecord::User.auditing_enabled).to eq(false) - end - expect(Models::ActiveRecord::User.auditing_enabled).to eq(true) - end - - t2 = Thread.new do - sleep 0.5 - expect(Models::ActiveRecord::User.auditing_enabled).to eq(true) - Models::ActiveRecord::User.create!( name: 'Lisa' ) + skip if Models::ActiveRecord::User.connection.class.name.include?("SQLite") + + t1 = Thread.new do + expect(Models::ActiveRecord::User.auditing_enabled).to eq(true) + Models::ActiveRecord::User.without_auditing do + expect(Models::ActiveRecord::User.auditing_enabled).to eq(false) + Models::ActiveRecord::User.create!( name: 'Bart' ) + sleep 1 + expect(Models::ActiveRecord::User.auditing_enabled).to eq(false) end - t1.join - t2.join + expect(Models::ActiveRecord::User.auditing_enabled).to eq(true) + end - expect(Models::ActiveRecord::User.find_by_name('Bart').audits.count).to eq(0) - expect(Models::ActiveRecord::User.find_by_name('Lisa').audits.count).to eq(1) - rescue ActiveRecord::StatementInvalid - STDERR.puts "Thread safety tests cannot be run with SQLite" + t2 = Thread.new do + sleep 0.5 + expect(Models::ActiveRecord::User.auditing_enabled).to eq(true) + Models::ActiveRecord::User.create!( name: 'Lisa' ) end + t1.join + t2.join + + expect(Models::ActiveRecord::User.find_by_name('Bart').audits.count).to eq(0) + expect(Models::ActiveRecord::User.find_by_name('Lisa').audits.count).to eq(1) end it "should not save an audit when auditing is globally disabled" do