conditional tracking of only/ignore

[razum2um]

hi, this patch add some ability to customize only/ignore attributes behaviour

[batter]

@razum2um - Thanks for the pull request. I'm trying to merge this but I'm not sure I fully understand the functionality. The purpose of the ignore is to assign a set of attributes for which new versions won't be generated if these attributes change. The purpose of only is to assign a set of attributes for which new versions will only be generated if these attributes change (all other attributes are then assumed to be ignored at that point).

Looking at your examples that you've added to the README in your commit, can't these same things be accomplished with the if and unless options? I also think that it's more clear that way. If you add a hash to the ignore attribute set, then it's assumed that the modifications to the attribute should be ignored if the proc returns true, right? So your examples kind of work, but you could accomplish the same thing with the existing options in a less confusing manner IMHO.

Here is the example you provide in the README:

class Article < ActiveRecord::Base
  has_paper_trail :ignore => [:title, :rating => Proc.new { |obj| obj.rating == 0 } ]
end

> a = Article.create
> a.versions.length                  #1
> a.update_attributes title: 'My title', rating: 0
> a.versions.length                  #1
> a.update_attributes title 'A Title', rating: 3
> a.versions.length                  #2
> a.previous_version.rating          #3

But couldn't the same behavior be accomplished with the unless option, like so:

class Article < ActiveRecord::Base
  has_paper_trail ignore: :title, unless: Proc.new { |obj| obj.rating == 0 }
end

If I'm not mistaken, this accomplishes the exact same behavior, and actually seems more clear to me. Furthermore, allowing hashes to be passed into the ignore or only arguments could get quite confusing if someone starts putting conditionals that are unrelated to the attribute in question into the Proc they are passing as the value for the hash, like so:

class Article < ActiveRecord::Base
  has_paper_trail :ignore => [:title, :rating => Proc.new { |obj| obj.title.blank? } ]
end

What happens in a scenario like this? If I'm reading the code correctly, it basically means that any changes to rating should be ignored if the title is blank, but not under any other circumstances? So I guess this does present a set of circumstances that can't currently be produced with the current options that can be passed into the has_paper_trail method. Is this the type of scenario you are envisioning?

I think at the least, in order to merge this, we need to make the examples in the README will need to be expanded and/or clarified to make it a little more clear what this type of thing could be used for, as the current examples you've provided don't really accomplish anything different beyond what could've already been accomplished before this change.

[razum2um]

perhaps examples in README are not the best.
original use case was:
given model with description field we have to track changes in that field (apart from others) only if there were some big changes, not punctuation.

now achieved by

:ignore => [..., :description => Proc.new { |a|
  a.description.to_s.gsub(/\W/, '') == a.description_was.to_s.gsub(/\W/, '')
}]

but if, unless are model-wide options, if other attribute changed aside of description, version should be created.
i'd rather revert README if it confuses you

[batter]

@razum2um - Thanks a lot for this! It's a very neat feature, I just didn't fully understand the use case when I originally read the examples, but after playing with it for a while I see that this adds a level of flexibility that wasn't there before. I condensed and cleaned up the code and the examples on the README in 22dd0cc if you want to take a look.

[razum2um]

yep, it's much prettier now, thanks