Ensure enum changes are stored consistently #429
Conversation
lib/audited/auditor.rb
Outdated
| end | ||
|
|
||
| def normalize_enum_changes(changes) | ||
| changes.each do |key, value| |
There was a problem hiding this comment.
instead of looping over all the changes, how about looping on the definded enums? in most cases there would be 0-2 enums while changes could be many more.
| end | ||
|
|
||
| filtered_changes = normalize_enum_changes(filtered_changes) | ||
| filtered_changes.to_hash |
There was a problem hiding this comment.
why the to_hash? shouldn't this already be a hash?
There was a problem hiding this comment.
This is a hash-like object (ActiveSupport::HashWithIndifferentAccess)
There was a problem hiding this comment.
@fatkodima a little late to the party, but this object indeed was an ActiveSupport::HashWithIndifferentAccess, but after you call to_hash on it, it's not anymore, it's simply a Hash object. The reason I'm writing this is that after upgrading from 4.8.0 to 4.9.0 I noticed that i cannot access some audited data by simbol, because they are not stored as ActiveSupport::HashWithIndifferentAccess anymore. Was this intended?
|
Updated. |
There was a problem hiding this comment.
Looks good @fatkodima, could you please also add a test for create and destroy audits to make sure the audited_changes are still stored correctly, since those audits don't use the audited_changes method?
spec/audited/auditor_spec.rb
Outdated
| @@ -290,7 +290,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: 0, audit_comment: 'Update' ) | |||
There was a problem hiding this comment.
Shouldn't this be status: :active to ensure that is converted correctly?
fatkodima
force-pushed
the
fix-enums
branch
2 times, most recently
from
40683f0 to
072ec46
Compare
|
@tbrisker done |
| if changes[name].is_a?(Array) | ||
| changes[name].map { |v| values[v] } | ||
| elsif rails_below?('5.0') | ||
| changes[name] |
There was a problem hiding this comment.
Why for rails<5 this is a noop but only if the change isn't an array?
There was a problem hiding this comment.
That is due to the fact, that rails stores enum changes differently for rails < 5 and for rails >= 5.
|
@tbrisker any idea when this will be in a published version? |
Currently, when record is saved or destroyed, their audits contain enum values ("New", "In progress" or "Complete"). So I fixed "update" audits to store enum values as well for consistency. Also storing enum values (but not keys) allows to easily change enum key namings in future without changing existing audits.
Fixes #202
Fixes #214
Fixes #412