Raise exception when Version creation fails? (Version.create vs Version.create!)

[JamesCropcho]

Hello,

I have a versions table with additional custom fields, via #info_for_paper_trail.

Ideally for me, an exception would be raised if the Version record is invalid (for example, due to a custom attribute missing). However, Version creation fails silently.

My question/offer: would it be generally useful (i.e. for people other than me) to have the exception raised, or to have a configuration parameter available which would do so optionally?

If so, I will be happy to provide a pull request with tests. And if not, I'd be curious to hear the rationale behind the silent failures.

By the way, Paper Trail's been great to me! Thank you very much, all contributors,

[batter]

@JamesCropcho - Can you explain the scenario that you have where version creation is failing? I assume that you must have put custom validator(s) on your Version class in config/initializers/paper_trail.rb? I think it could be useful to have a parameter to make that option available, however, I also think there are scenarios where users may want to add custom metadata fields to their versions table that may not always get populated.

If you think that this is something that you would find useful, then by all means, a pull request is welcome (especially if it contains tests!). I'm wondering if we might be able to do an implementation where save!, create!, or update_attributes! on a model that has_paper_trail raises an error/warning if a Version fails to create, a call to the non-bang versions of those methods do not raise an error.

It appears as though a call to save! or update_attributes! when a PaperTrail-tracked model gets updated will raise an error if the version fails to persist properly (because it uses a before_update callback to invoke model.versions.build), but it doesn't happen on creation or destruction of the model.

[JamesCropcho]

@fullbridge-batkins yes, version creation can fail when there are validations on Version, and when the table schema does not allow certain values to be NULL.

Thanks for the sleuthing on #save! versus #create.

In my case, I'm adding, for example, and IP address column, which I would in fact always expect to be present.

Anyway, I have found a decent workaround. Here is my Version class:

class Version
  attr_accessible :ip, :headers

  validates_presence_of :ip, :headers

  # Paper Trail calls #create and not #create! ; below, we make up for this.
  after_validation { raise ActiveRecord::RecordInvalid.new(self) if self.errors.present? }
end

This seems to do the trick, manually raising a RecordInvalid exception when the validation process detects errors. But by all means, if you (or anyone else) sees an issue with this implementation, let me know.

I'm no longer inclined to make a pull request, as this appears to work well. Free free to weigh in otherwise.

@JamesCropcho

[batter]

Thanks for reporting back. I think this seems like the best solution for the time being, but I think I'll try to revisit this concept at some point, perhaps for the 3.0.0 release. Now if I could just find some more time to work on this stuff!

[JamesCropcho]

Happy to help!

[brodock]

this is related to #231

[batter]

@brodock - how do you figure?