Skip any changed attributes marked to skip in the PaperTrail options

[joshmcarthur]

I came across the same problem described in #224 when using paper_trail with acts-as-taggable-on, and have a (partial) solution.

The reason this happens is that acts-as-taggable-on defines some code that manually adds a non-database field to the changed_attributes collection. paper_trail then tries to assign this value directly to the record using the column accessor, resulting in the following error:

ActiveModel::MissingAttributeError (can't write unknown attribute `tag_list')

My solution to this is to exclude from changed_attributes any keys defined in the :skip options of paper_trail - identically to what happens soon after that in the process, when the object is serialised. The reason this is just a partial fix is that the tag_list attribute still needs to be specified in the "skip" options for paper_trail. I don't really see a way around this short of acts-as-taggable-on changing how it behaves, but I'm very open to any suggestions.

[batter]

Thanks for the pull request. I'm happy to pull this in if this helps as a fix for some, although, I'm wondering if we might be able to do a fix that would actually prevent this error from being raised entirely.

I'm envisioning restricting the arguments here to only attributes which have corresponding columns in the database.

Here's what I'm envisioning, instead of:

# whats currently being used now
changed_attributes.each { |attr, before| prev.send[attr] = before }

use this:

changed_attributes.select { |k,v| self.class.column_names.include?(k) }.each { |attr, before| prev[attr] = before }

This both alleviates the issue entirely as far as I can tell, and prevents future incidents from occurring in similar scenarios. Thoughts?

[joshmcarthur]

@fullbridge-batkins:

1. I'm not sure why the splat operator is for either - I had assumed that because it was used a few lines down, it was there because it was necessary - perhaps for an older version of Rails?
2. Your approach is much, much better and exactly why I submitted a pull - I was hoping someone would be able to chip in with a more universal solution! I'll make that change on my branch now.

[joshmcarthur]

I've adjusted how the changed_attributes are excluded now as per your suggestion. I also tried removing the splat operator from the object_to_string method, but found that it resulted in a test failure, so put it back - it's probably not a change that should be made in this pull anyway.