-
Notifications
You must be signed in to change notification settings - Fork 899
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Log errors when versions can't be created #809
Conversation
Just bump up the number in rubocop_todo.yml .. it's a fight for another day :) |
unless log_version_errors(version, :update) | ||
update_transaction_id(version) | ||
save_associations(version) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this conditional hard to read. I'd prefer:
if version.errors.any?
log_version_errors(version, :update)
else
update_transaction_id(version)
# etc.
If rubocop complains about method complexity, just bump up the numbers; it's a fight for another day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I had originally until RuboCop complained about AbcSize
. I've changed it back, though subsequent edits seem to have removed the need to bump AbcSize
.
I've refactored and bumped the |
Thanks Mike! |
🤘 |
As discussed in #807, this PR updates
record_update
andrecord_destroy
to log a warning listing all validation errors when aVersion
instance cannot be saved. It does not updaterecord_create
since that callsVersion.create!
instead ofVersion.create
and should raise an error for any validation failures.Due to Rubocop complaining, I have also slightly refactored
has_paper_trail.rb
to fix some of the complaints. There is still a complaint about theInstanceMethods
module being too many lines, but I didn't see any easy way to reduce the line count of these changes without going too far off topic and into the weeds.