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.

Sign up for GitHub

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

Jump to bottom

allow private methods on audited condition #454

Merged
merged 1 commit into from
Jun 24, 2018
Merged

allow private methods on audited condition #454

merged 1 commit into from
Jun 24, 2018

Conversation

rauann
Copy link
Contributor

@rauann rauann commented May 17, 2018 •
edited
Loading

Adding trueas a second parameter to include private methods on conditional verification.

@rauann rauann mentioned this pull request May 17, 2018
Closed
tbrisker
tbrisker requested changes Jun 17, 2018
View reviewed changes
Copy link
Collaborator

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, see my comments inline.

lib/audited/auditor.rb Outdated

true
false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you change the default to false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the second parameter as true on respond_to? at last return, the default will never be raised I think. Anyway, I'll rollback it.

lib/audited/auditor.rb Outdated
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, include_all: true)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually isn't a kwarg, so just true as the second parameter is enough. (this still works since you are creating a new hash here and passing it to the function, and since a hash is non-falsy the check in respond_to? still passes, but every check of this function creates an extra object allocation)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, good catch. Updated.

@rauann rauann force-pushed the bugfix/check_private_methods branch from e251adf to 45971e2 Compare June 20, 2018 14:10
@rauann rauann force-pushed the bugfix/check_private_methods branch from 45971e2 to f130d86 Compare June 20, 2018 14:13
tbrisker
tbrisker approved these changes Jun 24, 2018
View reviewed changes
Copy link
Collaborator

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rauann !

@tbrisker tbrisker merged commit 02d2e24 into collectiveidea:master Jun 24, 2018
tbrisker added a commit that referenced this pull request Jun 24, 2018
@tbrisker
Update Changelog for #454
ea58b01
jeffdill2 pushed a commit to jeffdill2/audited that referenced this pull request Jul 9, 2018
@tbrisker @jeffdill2
Update Changelog for collectiveidea#454
896ec43
tbrisker added a commit to tbrisker/audited that referenced this pull request Aug 19, 2018
@tbrisker
Update Changelog for collectiveidea#454
b7442cd 
(cherry picked from commit ea58b01)
mikalai-okun pushed a commit to Fundthrough/audited that referenced this pull request Jul 30, 2021
@patrickptasinskift @jdurand
@agios @fespinoza @danielmorrison @pjohnmeyer @paneer-tikka @cluesque @tbrisker @laserlemon @vasinov @ankane @benlovell @freemanoid @bmedenwald @danabrit @jaredbeck @domcleal @nickrivadeneira @NileshPanchal @hernanat @printercu @ulfmagnetics @enriquebrgn @olegykz @mrhead @lostapathy @JonathanWThom @fatkodima @lmansur @jeffdill2 @krainboltgreene @KevinColemanInc @rauann @grosser @ipepe
[BO-1336] Update with official gem to 4.8.0 (into legacy-for-pro) (#10)
9c890f3 
* Add a Custom Auditor mention to the README

I scratched my head a bit on this one. I could only find my answer here https://github.com/collectiveidea/audited/blob/cd7065ef6fb9640a3a4e204477dd38dc0d12e50c/lib/audited/adapters/active_record/audit.rb#L40

* Use proper title

* Ruby syntax highlight

* Do not connect to db until needed fix collectiveidea#239

* Add missing `ruby` mark on README.md

* Remove Ruby 2.0 from Travis builds
2.0 is EOL so we aren't going to explicitly test it anymore.

* Remove mention of Ruby 2.0 support from README

* Upgrade appraisal gem (test dependency)

* Ensure upgrade tests don't use transactions
This was causing issues with MySQL in tests.

* Tweak travis webhook
The on_start values changed.

* Alias RSpec matcher methods for RSpec 3

Similar to pull collectiveidea#185, this pull should eliminate the deprecation
warnings RSpec emits when using the audited RSpec matchers. Aliasing
the methods should leave it compatible with earlier versions of RSpec
as well.

* Fixed README to avoid conflicts in rails 5.

* Test that demonstrates without_auditing is not thread safe

* Make temporary disabling of auditing threadsafe

For compatibility with, e.g., sidekiq

* Centralize `Audited.store` as thread safe variable store

* Test on Ruby 2.3.1 instead of 2.3.0 on Travis CI

* Update rspec-rails to 3.5.0

* Update Rails 5 tests to 5.0.0 final.

* Remove rails-observers dependency from test files
It is defined in the gemspec.

* Bump version and update README

* Tweak readme to reference new version [ci skip]

* Remove uses of Audited.audit_class accessor
Deprecate Audited.audit_class since it was in the 
README for years, but remove the setter completely.

This was needed when we had both ActiveRecord and
MongoMapper versions, but is not longer needed.

* Clean up syntax and tests

* Fix some deprecation warnings in tests

* Stop testing on jruby-head
The build isn't passing and I'm not using jruby so won't be personally working to get it passing. If anyone needs it, I'd love to see a PR.

* Correct auditing_enabled for STI models

The store used for setting and checking auditing_enabled uses
`name.tableize`, which causes STI models to check the incorrect key - if
auditing is disabled on the parent, children should also not be audited.

* Remove ambiguous argument warnings

* Update README.md

Updated all badges to be SVG and added a Hakiri security badge.

* Do not eagerly connect to database

* Normalise singular arguments for `on` option

It's idiomatic to normalise scalar arguments and I noticed this is the
case for the `except` option so I modified the handling of the `on`
option to suit. This means we can:

```ruby
class SomethingAudited < ActiveRecord::Base
  audit on: :create

  # would previously have required
  # audit on: [:create]
end
```

A small change but it adds some consistency and is less 'surprising'
perhaps.

* Allow private / protected callback declarations

Prior to this change, users would have to declare publicly scoped
callback handlers.

* Fix auditing instance attributes if "only" option specified

* Allow to pass both singular value and array to `:only` option

Allow to pass both singular value and array to `:only` option

Would previously have required audit only: [:password]

```ruby
class SomethingAudited < ActiveRecord::Base
audit only: :password
end
```

* Tiny readme tweaks

* Use new hash syntax

* Style tweaks

* Set created_at as collection cache key

* Corrected a typo: an for a

* Add 4.2.1 and 4.2.2 to changelog

* New format for changelog

This format, described by keepachangelog.com, is more common in the
ruby community. More importantly, it is hoped that this format,
especially the classification of changes, will help users make
upgrade decisions.

[ci skip]

* Document 4.3.0 in changelog

[ci skip]

* `json` and `jsonb` as possible `audited_changes` column type, fix collectiveidea#216

- update migration
- set postgres travis version to 9.4 to allow `jsonb`

* Docs: Add changelog entry for PR 295

[ci skip]

* Remove rails-observers dependency

The `Audited::Sweeper` class made limited use of the cache sweeping
functionality provided by rails-observer, mostly using it as a way to
implement `Audit#before_create` on the same class that was attached
around controller actions.

Since there is no released version of rails-observer that's compatible
with Rails 5.0 (and not in the foreseeable future without considerable
refactoring), the population of IPs, users and request UUIDs is replaced
by thread-local storage and the existing `Audit#before_create` methods.

This is a net code reduction and somewhat simplifies the "sweeper" by
removing the assumptions about how rails-observer works.

* Restore non_audited_columns= configuration setter

Fixes collectiveidea#320

* Document JSON installation support in ce9bd0b

[skip ci]

* Test Ruby on Rails 5.1, Ruby 2.4

* Update generators to use versioned AR::Migration parent

* Allow custom Audit classes

Classes that inherit from `Audited::Audit` can be used instead of the
default by assigning `Audited.audit_class` in an initializer.

Resolves collectiveidea#314

* Add .md extension to CHANGELOG

[ci skip]

* Bump version and refresh changelog for 4.4.0

* Fix AR/Rails gemspec dependencies to permit 5.1

* audited_changes - used .type instead of .sql_type to serialize - support for oracle

* Support UUID primary keys for User (collectiveidea#333)

* Call controller's current_user method during audited model creation

Restores behaviour prior to 87d402a where the sweeper would call the
controller's current_user method from the audited model callback.

Since 87d402a, the current_user method was called from an around action
callback registered on the base controller which was being called prior
to other callbacks that were authenticating the user. This caused
problems in apps where the user hadn't yet been set (so audit users were
nil), or CSRF issues because current_user was called prior to session
changes.

Fixes collectiveidea#336

* Allowing using symbol keys in non_audited_columns

* Bump version and refresh changelog for 4.5.0

* Fix typo in Audited RSpec Matchers

Currently, the audited RSpec matchers throw an error:
    undefined local variable or method 'expect' for <Audited::RspecMatchers::AuditMatcher:0x007f8627d6ae48>

when you use `.only` or `.except`.

The issue was a typo where it was referring to a non-existent variable `expect` instead of `except`.

* Remove duplicated method

* Use updated AR::Dirty API to find changed attributes

* Add functionality to undo changes

* columns order adjusted for polymorphic indexes

* Use Rails 5.1.4 instead of Rails 5.1.0.rc1

The format of gemfiles was changed by `bundle exec appraisal install`.

* Add tests for Rails 5.2

Rails 5.2 is imminent so we should add support for it.

* Require bundler in spec_helper

This allows running specs using rake.

* Simplify audited_columns

No need to get the entire columns, we only need their names. Also cache
them instead of calculating again on every call.

* Simplify audited_changes

`changes` (or `changes_to_save` in Rails>5.1) already gives us all the
changes in the format we need for saving the audit, no need to build
the hash again by ourselves.

* update to latest rubies

* don't test rails-4.x with ruby-head

* Update changelog with recent changes

* Normalize options on initialization and minor cleanup

Normalizing options passed on audited initialization simplifies code in
further steps. Also cleaned up the code a bit.

* Return nil on invalid revision number (collectiveidea#384)

Previously, when a `record.revision(version)` was called
with a version number that was greater than the total
number of revisions, it would return the last revision
in the list. This changes it to return nil.

collectiveidea#321

* Update Changelog

* Fix duplicate decleration warnings (collectiveidea#399)

* Prepare for 4.6.0 release

* Reduce db calls in #revisions method

* CI against ruby 2.5.0

* Remove references to removed allow_mass_assigment

As pointed out in issue collectiveidea#223, allow_mass_assigment has been removed and
no longer has any effect. This updates the docs to no longer talk about
it as it causes confusion to users of the gem.

* Optimize reconstructing attributes in #revisions

* Fix migrations in tests for rails 5.2

* Update Ruby versions in travis.yml

* Add inverse_of to audit relation

This allows access to the audited object's relations from inside audits
without reloading it from the database. This is useful, for example, if
you want to check has_many relations which are deleted from the database
before the audit callback is executed.

* Update Ruby versions in README

* Respect 'on' and 'except' options when used with 'comment_required' option (collectiveidea#419)

fixes collectiveidea#418

* Add conditional and exclusionary auditing feature

* Adds conditional (:if) and exclusionary (:unless) auditing feature
* Updates docs for `audited` to reflect new features

Upstream collectiveidea#167

* Allow limiting number of audits stored (collectiveidea#405)

Adds a setting to allow limiting the maximum number of audits per object.

Fixes collectiveidea#274

* fix Appriasals to match updated gemspec

* Fix RSpec matchers along with a few other things. (collectiveidea#420)

This commit fixes issues with the RSpec matchers, notably:

- For the `except` matcher code path, `model_class.default_ignored_attributes` was being called, but that method was made protected in a previous change as discussed [here](collectiveidea@8f8495d#commitcomment-28001384). This commit makes that method public again.

- Both the `on` and `except` matcher options have undergone a pretty substantial refactor. As I was fixing the original issue with `on` and `except`, I started to notice other things and addressed them. I will try to make explanatory comments on the PR to give some insight.

- The matchers as a whole have undergone pretty significant changes, and I found that many of them were not working as intended, or, for example, in the case of `on`, it was a no-op. I made the changes that I thought reflected what the matchers ought to do and I also included tests (which actually ended up being extremely useful in discovering some of the bugs!) to make sure that future changes don't break anything.

- The `requires_comment` matcher's behavior has changed to look for callbacks instead. I also changed the way that comment-required state validity is checked in the `Auditor`, but I will discuss that below. My reasoning for the two changes are related though - given an audited model where audit comments are required, we only care about the existence of the comment when an object is being created, if it has been changed (and subsequently updated), or if it is about to be destroyed (and of course we can include & exclude any combination of these three states with the `on` option). Previously, the presence of the comment was related to the overall state of the object itself, and I found this confusing since the comment only matters when we are _doing_ something to the object.

- I have decoupled the codepaths for each option so that they could be run independently of one another (and all together, of course). I was getting some interesting behavior before, but I believe I have nailed down the correct behavior.

- I will discuss anything not discussed above for the matchers in PR comments.

The `Auditor` itself:

- Changed the way non-audited columns were calculated. Will discuss in-line.

- Revised and (hopefully) simplified the way that audit comments are required & validated. Will discuss in-line.

Other comments:

- As I mentioned above, I noticed some behavioral issues that I tried to address. I will talk about them more in-line and if there are any questions about particular things I did please feel free to ask.

- Another thing I would like to do is a complete rework of the test suite itself (or if anyone else wants to take this up, feel free :P) In writing the tests for the matchers, I noticed many things such as audited columns for the `User` model being inconsistent, the `Audited.ignored_attributes` changing, and more. This is directly caused by the behavior of some of the tests, and I think it would be beneficial to figure out a way to rework the behavior such that what happens in one test doesn't have an impact on all the others. I also have a slight fear that some tests may in fact be returning false-positives, but I haven't looked too far into that yet.

- I was wondering if it would be better to not memoize `Auditor#audited_columns` and `Auditor#non_audited_columns` since the calculations are not really intensive enough to warrant memoization, but when I did that tests were breaking left and right and so I think this really ties into my point above about the test suite. Something to keep in mind / look into. I didn't do it in this PR because there is already quite a bit going on.

* Prepare for 4.7.0 release

* Backport 5.2 support to 4.7 stable

* Add ability to globally disable auditing (collectiveidea#426)


(cherry picked from commit c48893a)

* Add version to auditable_index

(cherry picked from commit dc72762)

* Add `all_audits` method to auditable models

(cherry picked from commit 8486dec)

* Rename #all_audits -> #own_and_associated_audits

(cherry picked from commit fd95166)

* Simplify the undo logic (collectiveidea#436)


(cherry picked from commit ed4e2ec)

* Fix typo in readme

(cherry picked from commit 3b76fca)

* Allow nested as_user auditing (collectiveidea#450)


(cherry picked from commit 46e560d)

* allow private methods on audited condition

(cherry picked from commit 02d2e24)

* Update Changelog for collectiveidea#454

(cherry picked from commit ea58b01)

* fix test failing and being noisy on ruby 2.5

(cherry picked from commit 9cff81b)

* avoid exceptions in threads that print their backtrace on ruby 2.5

(cherry picked from commit 250cacc)

* rename instance attribute version to audit_version (collectiveidea#443)

* rename instance attribute version to audit_version

seems dangerous to dynamically add an accessor method called version
because version is such a common name and could lead to some
unexpected behaviour in applications using audited.

* Deprecate version attribute to prepare for removal

* Update CHANGELOG

* Implement deprecation only if version not in use in audited resource

* Add version deprecation to uncovered

* Update SingleCov uncovered count

(cherry picked from commit 0296845)

* Prepare 4.8.0 release

* Add setting service name on before_create

* Add namespacing as config attribute

* [BO-1410] Loosen created_at query (#12)

* Loosen created_at query

* Add fundthrough unified changes


Co-authored-by: Jim <powerjim@gmail.com>
Co-authored-by: Alexandros Giouzenis <alexandrosg@gmail.com>
Co-authored-by: Felipe Espinoza <fespinozacast@gmail.com>
Co-authored-by: Daniel Morrison <daniel@collectiveidea.com>
Co-authored-by: Tim Bugai <tim@collectiveidea.com>
Co-authored-by: Patrick Johnmeyer <pjohnmeyer@intoxitrack.net>
Co-authored-by: paneer_tikka <neerajapte@gmail.com>
Co-authored-by: Bill Kirtley <bill@West.local>
Co-authored-by: Bill Kirtley <bill@spiffy.com>
Co-authored-by: Tomer Brisker <tbrisker@gmail.com>
Co-authored-by: Steve Richert <steve.richert@gmail.com>
Co-authored-by: Vasily Vasinov <des.elyon@gmail.com>
Co-authored-by: Andrew Kane <acekane1@gmail.com>
Co-authored-by: Ben Lovell <benjamin.lovell@gmail.com>
Co-authored-by: freemanoid <freemanoid321@gmail.com>
Co-authored-by: Brandon Medenwald <brandon@simplymadeapps.com>
Co-authored-by: Dana Jones <djones@larkfarm.com>
Co-authored-by: Jared Beck <jared@jaredbeck.com>
Co-authored-by: Dominic Cleal <dominic@cleal.org>
Co-authored-by: Nick Rivadeneira <nicholas.rivadeneira@gmail.com>
Co-authored-by: NileshPanchal <neil08.panchal@gmail.com>
Co-authored-by: Zane Shannon <zcs@smileslaughs.com>
Co-authored-by: Andrea Schiavini <metalelf0@Valhalla.local>
Co-authored-by: Anthony Hernandez <roguegdi27@gmail.com>
Co-authored-by: printercu <printercu@gmail.com>
Co-authored-by: John Berry <ulfmagnetics@gmail.com>
Co-authored-by: Enrique Barragan <ebarragan1997@gmail.com>
Co-authored-by: Oleg Yakovenko <olegykz@gmail.com>
Co-authored-by: Patrik Bóna <patrik.bona@mrhead.sk>
Co-authored-by: Joe Francis <joe@lostapathy.com>
Co-authored-by: Tomer Brisker <tbrisker@users.noreply.github.com>
Co-authored-by: Jonathan Thom <jonathan.thom1990@gmail.com>
Co-authored-by: fatkodima <fatkodima123@gmail.com>
Co-authored-by: Tekin Suleyman <tekin@freeagent.com>
Co-authored-by: Lucas Mansur <lucas.mansur2@gmail.com>
Co-authored-by: Jeffrey Dill <jeffdill2@users.noreply.github.com>
Co-authored-by: Valentino <valentino@reenhanced.com>
Co-authored-by: Kurtis Rainbolt-Greene <kurtis@rainbolt-greene.online>
Co-authored-by: Kevin Coleman <kevin.coleman@sparkstart.io>
Co-authored-by: rauan <rauann.assis@gmail.com>
Co-authored-by: Michael Grosser <michael@grosser.it>
Co-authored-by: Patryk Ptasinski <patryk@ipepe.pl>
mikalai-okun pushed a commit to Fundthrough/audited that referenced this pull request Aug 9, 2021
@patrickptasinskift @jdurand
@agios @fespinoza @danielmorrison @pjohnmeyer @paneer-tikka @cluesque @tbrisker @laserlemon @vasinov @ankane @benlovell @freemanoid @bmedenwald @danabrit @jaredbeck @domcleal @nickrivadeneira @NileshPanchal @hernanat @printercu @ulfmagnetics @enriquebrgn @olegykz @mrhead @lostapathy @JonathanWThom @fatkodima @lmansur @jeffdill2 @grosser @krainboltgreene @jsantos @KevinColemanInc @albus522 @stevendaniels @rauann @jestemkarol @stevenpetryk @jheijmans @olleolleolle @Vasfed @trevorrjohn @ipepe
[BO-1336] Update with official gem to 4.9.0 (into master) (#9)
956962b 
* Add a Custom Auditor mention to the README

I scratched my head a bit on this one. I could only find my answer here https://github.com/collectiveidea/audited/blob/cd7065ef6fb9640a3a4e204477dd38dc0d12e50c/lib/audited/adapters/active_record/audit.rb#L40

* Use proper title

* Ruby syntax highlight

* Do not connect to db until needed fix collectiveidea#239

* Add missing `ruby` mark on README.md

* Remove Ruby 2.0 from Travis builds
2.0 is EOL so we aren't going to explicitly test it anymore.

* Remove mention of Ruby 2.0 support from README

* Upgrade appraisal gem (test dependency)

* Ensure upgrade tests don't use transactions
This was causing issues with MySQL in tests.

* Tweak travis webhook
The on_start values changed.

* Alias RSpec matcher methods for RSpec 3

Similar to pull collectiveidea#185, this pull should eliminate the deprecation
warnings RSpec emits when using the audited RSpec matchers. Aliasing
the methods should leave it compatible with earlier versions of RSpec
as well.

* Fixed README to avoid conflicts in rails 5.

* Test that demonstrates without_auditing is not thread safe

* Make temporary disabling of auditing threadsafe

For compatibility with, e.g., sidekiq

* Centralize `Audited.store` as thread safe variable store

* Test on Ruby 2.3.1 instead of 2.3.0 on Travis CI

* Update rspec-rails to 3.5.0

* Update Rails 5 tests to 5.0.0 final.

* Remove rails-observers dependency from test files
It is defined in the gemspec.

* Bump version and update README

* Tweak readme to reference new version [ci skip]

* Remove uses of Audited.audit_class accessor
Deprecate Audited.audit_class since it was in the 
README for years, but remove the setter completely.

This was needed when we had both ActiveRecord and
MongoMapper versions, but is not longer needed.

* Clean up syntax and tests

* Fix some deprecation warnings in tests

* Stop testing on jruby-head
The build isn't passing and I'm not using jruby so won't be personally working to get it passing. If anyone needs it, I'd love to see a PR.

* Correct auditing_enabled for STI models

The store used for setting and checking auditing_enabled uses
`name.tableize`, which causes STI models to check the incorrect key - if
auditing is disabled on the parent, children should also not be audited.

* Remove ambiguous argument warnings

* Update README.md

Updated all badges to be SVG and added a Hakiri security badge.

* Do not eagerly connect to database

* Normalise singular arguments for `on` option

It's idiomatic to normalise scalar arguments and I noticed this is the
case for the `except` option so I modified the handling of the `on`
option to suit. This means we can:

```ruby
class SomethingAudited < ActiveRecord::Base
  audit on: :create

  # would previously have required
  # audit on: [:create]
end
```

A small change but it adds some consistency and is less 'surprising'
perhaps.

* Allow private / protected callback declarations

Prior to this change, users would have to declare publicly scoped
callback handlers.

* Fix auditing instance attributes if "only" option specified

* Allow to pass both singular value and array to `:only` option

Allow to pass both singular value and array to `:only` option

Would previously have required audit only: [:password]

```ruby
class SomethingAudited < ActiveRecord::Base
audit only: :password
end
```

* Tiny readme tweaks

* Use new hash syntax

* Style tweaks

* Set created_at as collection cache key

* Corrected a typo: an for a

* Add 4.2.1 and 4.2.2 to changelog

* New format for changelog

This format, described by keepachangelog.com, is more common in the
ruby community. More importantly, it is hoped that this format,
especially the classification of changes, will help users make
upgrade decisions.

[ci skip]

* Document 4.3.0 in changelog

[ci skip]

* `json` and `jsonb` as possible `audited_changes` column type, fix collectiveidea#216

- update migration
- set postgres travis version to 9.4 to allow `jsonb`

* Docs: Add changelog entry for PR 295

[ci skip]

* Remove rails-observers dependency

The `Audited::Sweeper` class made limited use of the cache sweeping
functionality provided by rails-observer, mostly using it as a way to
implement `Audit#before_create` on the same class that was attached
around controller actions.

Since there is no released version of rails-observer that's compatible
with Rails 5.0 (and not in the foreseeable future without considerable
refactoring), the population of IPs, users and request UUIDs is replaced
by thread-local storage and the existing `Audit#before_create` methods.

This is a net code reduction and somewhat simplifies the "sweeper" by
removing the assumptions about how rails-observer works.

* Restore non_audited_columns= configuration setter

Fixes collectiveidea#320

* Document JSON installation support in ce9bd0b

[skip ci]

* Test Ruby on Rails 5.1, Ruby 2.4

* Update generators to use versioned AR::Migration parent

* Allow custom Audit classes

Classes that inherit from `Audited::Audit` can be used instead of the
default by assigning `Audited.audit_class` in an initializer.

Resolves collectiveidea#314

* Add .md extension to CHANGELOG

[ci skip]

* Bump version and refresh changelog for 4.4.0

* Fix AR/Rails gemspec dependencies to permit 5.1

* audited_changes - used .type instead of .sql_type to serialize - support for oracle

* Support UUID primary keys for User (collectiveidea#333)

* Call controller's current_user method during audited model creation

Restores behaviour prior to 87d402a where the sweeper would call the
controller's current_user method from the audited model callback.

Since 87d402a, the current_user method was called from an around action
callback registered on the base controller which was being called prior
to other callbacks that were authenticating the user. This caused
problems in apps where the user hadn't yet been set (so audit users were
nil), or CSRF issues because current_user was called prior to session
changes.

Fixes collectiveidea#336

* Allowing using symbol keys in non_audited_columns

* Bump version and refresh changelog for 4.5.0

* Fix typo in Audited RSpec Matchers

Currently, the audited RSpec matchers throw an error:
    undefined local variable or method 'expect' for <Audited::RspecMatchers::AuditMatcher:0x007f8627d6ae48>

when you use `.only` or `.except`.

The issue was a typo where it was referring to a non-existent variable `expect` instead of `except`.

* Remove duplicated method

* Use updated AR::Dirty API to find changed attributes

* Add functionality to undo changes

* columns order adjusted for polymorphic indexes

* Use Rails 5.1.4 instead of Rails 5.1.0.rc1

The format of gemfiles was changed by `bundle exec appraisal install`.

* Add tests for Rails 5.2

Rails 5.2 is imminent so we should add support for it.

* Require bundler in spec_helper

This allows running specs using rake.

* Simplify audited_columns

No need to get the entire columns, we only need their names. Also cache
them instead of calculating again on every call.

* Simplify audited_changes

`changes` (or `changes_to_save` in Rails>5.1) already gives us all the
changes in the format we need for saving the audit, no need to build
the hash again by ourselves.

* update to latest rubies

* don't test rails-4.x with ruby-head

* Update changelog with recent changes

* Normalize options on initialization and minor cleanup

Normalizing options passed on audited initialization simplifies code in
further steps. Also cleaned up the code a bit.

* Return nil on invalid revision number (collectiveidea#384)

Previously, when a `record.revision(version)` was called
with a version number that was greater than the total
number of revisions, it would return the last revision
in the list. This changes it to return nil.

collectiveidea#321

* Update Changelog

* Fix duplicate decleration warnings (collectiveidea#399)

* Prepare for 4.6.0 release

* Reduce db calls in #revisions method

* CI against ruby 2.5.0

* Remove references to removed allow_mass_assigment

As pointed out in issue collectiveidea#223, allow_mass_assigment has been removed and
no longer has any effect. This updates the docs to no longer talk about
it as it causes confusion to users of the gem.

* Optimize reconstructing attributes in #revisions

* Fix migrations in tests for rails 5.2

* Update Ruby versions in travis.yml

* Add inverse_of to audit relation

This allows access to the audited object's relations from inside audits
without reloading it from the database. This is useful, for example, if
you want to check has_many relations which are deleted from the database
before the audit callback is executed.

* Update Ruby versions in README

* Respect 'on' and 'except' options when used with 'comment_required' option (collectiveidea#419)

fixes collectiveidea#418

* Add conditional and exclusionary auditing feature

* Adds conditional (:if) and exclusionary (:unless) auditing feature
* Updates docs for `audited` to reflect new features

Upstream collectiveidea#167

* Allow limiting number of audits stored (collectiveidea#405)

Adds a setting to allow limiting the maximum number of audits per object.

Fixes collectiveidea#274

* fix Appriasals to match updated gemspec

* Fix RSpec matchers along with a few other things. (collectiveidea#420)

This commit fixes issues with the RSpec matchers, notably:

- For the `except` matcher code path, `model_class.default_ignored_attributes` was being called, but that method was made protected in a previous change as discussed [here](collectiveidea@8f8495d#commitcomment-28001384). This commit makes that method public again.

- Both the `on` and `except` matcher options have undergone a pretty substantial refactor. As I was fixing the original issue with `on` and `except`, I started to notice other things and addressed them. I will try to make explanatory comments on the PR to give some insight.

- The matchers as a whole have undergone pretty significant changes, and I found that many of them were not working as intended, or, for example, in the case of `on`, it was a no-op. I made the changes that I thought reflected what the matchers ought to do and I also included tests (which actually ended up being extremely useful in discovering some of the bugs!) to make sure that future changes don't break anything.

- The `requires_comment` matcher's behavior has changed to look for callbacks instead. I also changed the way that comment-required state validity is checked in the `Auditor`, but I will discuss that below. My reasoning for the two changes are related though - given an audited model where audit comments are required, we only care about the existence of the comment when an object is being created, if it has been changed (and subsequently updated), or if it is about to be destroyed (and of course we can include & exclude any combination of these three states with the `on` option). Previously, the presence of the comment was related to the overall state of the object itself, and I found this confusing since the comment only matters when we are _doing_ something to the object.

- I have decoupled the codepaths for each option so that they could be run independently of one another (and all together, of course). I was getting some interesting behavior before, but I believe I have nailed down the correct behavior.

- I will discuss anything not discussed above for the matchers in PR comments.

The `Auditor` itself:

- Changed the way non-audited columns were calculated. Will discuss in-line.

- Revised and (hopefully) simplified the way that audit comments are required & validated. Will discuss in-line.

Other comments:

- As I mentioned above, I noticed some behavioral issues that I tried to address. I will talk about them more in-line and if there are any questions about particular things I did please feel free to ask.

- Another thing I would like to do is a complete rework of the test suite itself (or if anyone else wants to take this up, feel free :P) In writing the tests for the matchers, I noticed many things such as audited columns for the `User` model being inconsistent, the `Audited.ignored_attributes` changing, and more. This is directly caused by the behavior of some of the tests, and I think it would be beneficial to figure out a way to rework the behavior such that what happens in one test doesn't have an impact on all the others. I also have a slight fear that some tests may in fact be returning false-positives, but I haven't looked too far into that yet.

- I was wondering if it would be better to not memoize `Auditor#audited_columns` and `Auditor#non_audited_columns` since the calculations are not really intensive enough to warrant memoization, but when I did that tests were breaking left and right and so I think this really ties into my point above about the test suite. Something to keep in mind / look into. I didn't do it in this PR because there is already quite a bit going on.

* Prepare for 4.7.0 release

* Add ability to globally disable auditing (collectiveidea#426)

* remove unused/untested methods (collectiveidea#424)

auditing_enabled was especially dangerous since it sets on the instance and modifies the global state

* Add version to auditable_index

* drop support for deprecated rails versions

* Add `all_audits` method to auditable models

* #all_audits -> #own_and_associated_audits

* bump minimum ruby version

* Simplify the undo logic (collectiveidea#436)

* add actionable coverage reporting (collectiveidea#356)

* Add support for the final release of Rails 5.2 (collectiveidea#441)

- Fixes collectiveidea#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

* Fix typo in readme

* Remove the Gemnasium badge

This is a service we no longer use.

* 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.

* Allow nested as_user auditing (collectiveidea#450)

* allow private methods on audited condition

* Update Changelog for collectiveidea#454

* improve code coverage

* Ensure enum changes are stored consistently

* basic rubocop checks and fixes

* fix test failing and being noisy on ruby 2.5

* avoid exceptions in threads that print their backtrace on ruby 2.5

* rename instance attribute version to audit_version (collectiveidea#443)

* rename instance attribute version to audit_version

seems dangerous to dynamically add an accessor method called version
because version is such a common name and could lead to some
unexpected behaviour in applications using audited.

* Deprecate version attribute to prepare for removal

* Update CHANGELOG

* Implement deprecation only if version not in use in audited resource

* Add version deprecation to uncovered

* Update SingleCov uncovered count

* Fix Changelog for 4.7.1

* Build all stable branches on Travis CI (collectiveidea#466)

* Install bundler 1.x for Rails 4.2 builds

4.2's still _just_ about supported, install the older Bundler it expects
while keeping everything else on 2.x.

    Bundler could not find compatible versions for gem "bundler":
      In rails42.gemfile:
        appraisal was resolved to 2.2.0, which depends on
          bundler
        rails (~> 4.2.0) was resolved to 4.2.11, which depends on
          bundler (>= 1.3.0, < 2.0)
      Current Bundler version:
        bundler (2.0.1)

Can be reverted when 4.2's dropped.

* Pin sqlite3 gem to 1.3.x in development to match Rails

The Rails sqlite3 adapter requires `~> 1.3.6`, but `~> 1.3` was
installing the newer 1.4 release. Cap this to match the Rails
requirement.

    Failure/Error: require 'active_record/connection_adapters/sqlite3_adapter'
    Gem::LoadError:
      can't activate sqlite3 (~> 1.3.6), already activated sqlite3-1.4.0. Make sure all dependencies are added to Gemfile.
    # ./gemfiles/vendor/bundle/ruby/2.3.0/gems/activerecord-5.2.2/lib/active_record/connection_adapters/sqlite3_adapter.rb:12:in `<top (required)>'

* Merge back 4.8.0 changelog

* Add update_with_only_comment configuration option to not write audit if option is set to false and audit_comment is only supplied

* Add PR#327 to changelog

* Include an example of passing strings to `as_user`

For some reason, on my first read of this README, I totally missed the fact that you could pass a string in anywhere you can pass an ActiveRecord model. Having an example like this would have saved me some silly mock-object-creation. Willing to change the copy or the specificity of the example, but I have a feeling this is a common use case.

* Add Rails 6.0.0.rc1 to the support/build matrix

Add correct DB adapter gems to each version to avoid gem activation
errors, but ensure the main gemspec can match all permitted versions.

Fixes collectiveidea#488

* Replace deprecated update_attribute with update!

Soft-deprecated since Rails 4.0 but in Rails 6.0 it now prints a
deprecation warning, so switch it out for `update!` (preferring the bang
version to pick up on any errors).

* Add empty ApplicationController as actiontext expects it now

* Set database charset for MySQL, required by adapter

The adapter only defaults to utf8mb4 on newer MySQL versions, else
requires a specific charset under Rails 6.0 for our version.

* Add Ruby 2.6.3 to build matrix

No support for 2.6 (or HEAD) on Rails 4.2, exclude it.

* Update CHANGELOG

* Add 2.6.3 to set of tested Ruby versions

* Travis: Drop unused directive sudo: false


    See https://blog.travis-ci.com/2018-11-19-required-linux-infrastructure-migration for more details

* CI: Enable MySQL service

* Add info about mysql 5.7+ json type to readme

* Allow for Audting to be enabled temporarily

- allows for models to be disabled by default
- behaves exactly like #without_auditing

* Bump version 4.9.0

* Added created_at filtering for has_many :audits

* Fix Set problems when accessing audits

* Add namespacing as config attribute

* Fix failing tests with edge cases

* Handle models without created_at

* Remove created_at from test schema

* Add dev_test_schema

* Bump VERSION to 4.9.1

* Add draft circle ci config

* Test minimal circle-ci config

* Add rubocop to CI

* Change _id fields to string type

* Increase version

* Add json_audited_changes column. Version 4.9.3

* Add namespaced to set_version_number

* Add not_before_created_at to set_version_number

* Increase version number to indicate changes

Co-authored-by: Jim <powerjim@gmail.com>
Co-authored-by: Alexandros Giouzenis <alexandrosg@gmail.com>
Co-authored-by: Felipe Espinoza <fespinozacast@gmail.com>
Co-authored-by: Daniel Morrison <daniel@collectiveidea.com>
Co-authored-by: Tim Bugai <tim@collectiveidea.com>
Co-authored-by: Patrick Johnmeyer <pjohnmeyer@intoxitrack.net>
Co-authored-by: paneer_tikka <neerajapte@gmail.com>
Co-authored-by: Bill Kirtley <bill@West.local>
Co-authored-by: Bill Kirtley <bill@spiffy.com>
Co-authored-by: Tomer Brisker <tbrisker@gmail.com>
Co-authored-by: Steve Richert <steve.richert@gmail.com>
Co-authored-by: Vasily Vasinov <des.elyon@gmail.com>
Co-authored-by: Andrew Kane <acekane1@gmail.com>
Co-authored-by: Ben Lovell <benjamin.lovell@gmail.com>
Co-authored-by: freemanoid <freemanoid321@gmail.com>
Co-authored-by: Brandon Medenwald <brandon@simplymadeapps.com>
Co-authored-by: Dana Jones <djones@larkfarm.com>
Co-authored-by: Jared Beck <jared@jaredbeck.com>
Co-authored-by: Dominic Cleal <dominic@cleal.org>
Co-authored-by: Nick Rivadeneira <nicholas.rivadeneira@gmail.com>
Co-authored-by: NileshPanchal <neil08.panchal@gmail.com>
Co-authored-by: Zane Shannon <zcs@smileslaughs.com>
Co-authored-by: Andrea Schiavini <metalelf0@Valhalla.local>
Co-authored-by: Anthony Hernandez <roguegdi27@gmail.com>
Co-authored-by: printercu <printercu@gmail.com>
Co-authored-by: John Berry <ulfmagnetics@gmail.com>
Co-authored-by: Enrique Barragan <ebarragan1997@gmail.com>
Co-authored-by: Oleg Yakovenko <olegykz@gmail.com>
Co-authored-by: Patrik Bóna <patrik.bona@mrhead.sk>
Co-authored-by: Joe Francis <joe@lostapathy.com>
Co-authored-by: Tomer Brisker <tbrisker@users.noreply.github.com>
Co-authored-by: Jonathan Thom <jonathan.thom1990@gmail.com>
Co-authored-by: fatkodima <fatkodima123@gmail.com>
Co-authored-by: Tekin Suleyman <tekin@freeagent.com>
Co-authored-by: Lucas Mansur <lucas.mansur2@gmail.com>
Co-authored-by: Jeffrey Dill <jeffdill2@users.noreply.github.com>
Co-authored-by: Valentino <valentino@reenhanced.com>
Co-authored-by: Michael Grosser <michael@grosser.it>
Co-authored-by: Kurtis Rainbolt-Greene <kurtis@rainbolt-greene.online>
Co-authored-by: Jorge Oliveira Santos <jorgeoliveirasantos@gmail.com>
Co-authored-by: Kevin Coleman <kevin.coleman@sparkstart.io>
Co-authored-by: David Genord II <david@collectiveidea.com>
Co-authored-by: Steven Daniels <stevendaniels@users.noreply.github.com>
Co-authored-by: rauan <rauann.assis@gmail.com>
Co-authored-by: Dominic Cleal <dominic.cleal@freeagent.com>
Co-authored-by: Karol Wyliziński <kwylizinski@gmail.com>
Co-authored-by: Steven Petryk <petryk.steven@gmail.com>
Co-authored-by: Jeroen Heijmans <jheijmans@users.noreply.github.com>
Co-authored-by: Olle Jonsson <olle.jonsson@gmail.com>
Co-authored-by: Vasily Fedoseyev <vasilyfedoseyev@gmail.com>
Co-authored-by: Trevor John <trevor@john.tj>
Co-authored-by: Patryk Ptasinski <patryk@ipepe.pl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbrisker tbrisker tbrisker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rauann @tbrisker