No need to calculate previous values of skipped attributes

[khiav223577]

I'm going to remove a column of one model. I want to make sure that no one calls it anymore before removing it, so I overwritted it as below:

def my_column
  fail 'should not be called'
end

But paper_trail will call it even the attribute is skipped.
It calculates all attributes and then filters out skipped attributes.

paper_trail/lib/paper_trail/events/base.rb

Lines 219 to 221 in 85884ba

	def object_attrs_for_paper_trail(is_touch)
	attrs = attributes_before_change(is_touch).
	except(*@record.paper_trail_options[:skip])

The flow can be improved to calculate only non-skipped attributes.

• Wrote good commit messages.
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new
  code introduces user-observable changes.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.

[jaredbeck]

Hi Rumble, thanks for the contribution. It looks like a good start.

Please add at least one test in spec/paper_trail/events. You may want to use spec/dummy_app/app/models/skipper.rb. Feel free to add your fail to that class.

Please add some comments, at least in events/base.rb, explaining the importance of avoiding calling skipped attributes.

Thanks!

[jaredbeck]

I don't understand the purpose of the skip argument. This method is only called from one place, and we always want to skip, so what about renaming this method to nonskipped_attributes_before_change, and having it always respect skip?

[khiav223577]

I was no sure if it is ok to change the behavior of attributes_before_change method or not, so I added an extra argument to control it.

Renaming this method sounds to be a good idea, too. 👍

[jaredbeck]

Thanks, Rumble.