Skip creating version for timestamp when ignoring attribute with a Hash #1256
Conversation
| expect(gadget.versions.last.changeset.keys).to eq %w[color updated_at] | ||
| end | ||
|
|
||
| it "doesn't generate a version when the ignored attribute is true" do |
There was a problem hiding this comment.
This test fails. This should succeed right?
There was a problem hiding this comment.
Hi @jaredbeck, so when using ignored with a Hash, it still creates a version for the timestamp.
There was a problem hiding this comment.
This test fails. This should succeed right?
Yeah, I think so. Nice test. 👍
| expect { gadget.update_attribute(:brand, "Stanley") }.not_to(change { gadget.versions.size }) | ||
| context "ignored via symbol" do | ||
| it "doesn't generate a version" do | ||
| expect { gadget.update_attribute(:brand, "Picard") }.not_to(change { gadget.versions.size }) |
There was a problem hiding this comment.
Updated name to Picard so the line doesn't exceed the maximum for Rubocop.
There was a problem hiding this comment.
That's fine. You can also break long lines like this:
| expect { gadget.update_attribute(:brand, "Picard") }.not_to(change { gadget.versions.size }) | |
| expect { | |
| gadget.update_attribute(:brand, "Stanley") | |
| }.not_to(change { gadget.versions.size }) |
| @@ -148,7 +152,7 @@ def changes_in_latest_version | |||
| # | |||
| # @api private | |||
| def ignored_attr_has_changed? | |||
| ignored = @record.paper_trail_options[:ignore] + @record.paper_trail_options[:skip] | |||
There was a problem hiding this comment.
Here it was only considering ignored attributes defined in the array as symbols and not hashes.
| @@ -148,7 +152,7 @@ def changes_in_latest_version | |||
| # | |||
| # @api private | |||
| def ignored_attr_has_changed? | |||
| ignored = @record.paper_trail_options[:ignore] + @record.paper_trail_options[:skip] | |||
| ignored = calculated_ignored_array + @record.paper_trail_options[:skip] | |||
There was a problem hiding this comment.
This ignores when attributes are either defined as symbols or hashes.
| @@ -107,7 +107,7 @@ def attribute_in_previous_version(attr_name, is_touch) | |||
| end | |||
|
|
|||
| # @api private | |||
| def changed_and_not_ignored | |||
| def calculated_ignored_array | |||
There was a problem hiding this comment.
Extracts this calculation to be used by changed_and_not_ignored as well as ignored_attr_has_changed?
|
Hi @jaredbeck, hope you're well. Please let me know if I should go into more detail on this change. |
tlynam
force-pushed
the
fix-ignore-via-hash-creating-version
branch
from
8dec5c6
to
24982ae
Compare
…d via Hash - It was only ignoring attributes defined as symbols. - It now ignores when attributes are either defined as symbols or Hashes. - Consolidates calculation to be shared when determining if changed and not ignored. Resolves paper-trail-gem#1240
tlynam
force-pushed
the
fix-ignore-via-hash-creating-version
branch
from
24982ae
to
f7a94d0
Compare
updated_at.Thank you for your contribution!
Check the following boxes:
master(if not - rebase it).code introduces user-observable changes.
and description in grammatically correct, complete sentences.