Skip to content

Commit

Permalink
add PaperTrail.config.association_reify_error_behaviour
Browse files Browse the repository at this point in the history
  • Loading branch information
westonganger committed May 19, 2018
1 parent 547dd11 commit f9a8a77
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 12 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/).

### Added

- None
- [#1089] Sometimes the has_one association will find more than one possible candidate and will raise a `PaperTrail::Reifiers::HasOne::FoundMoreThanOne` when NOT using STI. You may want to just assume the first result (of multiple) is the correct one and continue. Versions pre v8.1.2 and below did this without error or warning. To do so add the following line to your initializer: `PaperTrail.config.association_reify_error_behaviour = :warn`. Valid options are: `[:error, :warn, :ignore]`

### Fixed

Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -948,8 +948,9 @@ Associations are an **experimental feature** and have the following known
issues, in order of descending importance.

1. PaperTrail only reifies the first level of associations.
1. Does not fully support STI (For example, see `spec/models/person_spec.rb` and
`PaperTrail::Reifiers::HasOne::FoundMoreThanOne` error)
1. Sometimes the has_one association will find more than one possible candidate and will raise a `PaperTrail::Reifiers::HasOne::FoundMoreThanOne` error. For example, see `spec/models/person_spec.rb`
- If you are not using STI, you may want to just assume the first result (of multiple) is the correct one and continue. Versions pre v8.1.2 and below did this without error or warning. To do so add the following line to your initializer: `PaperTrail.config.association_reify_error_behaviour = :warn`. Valid options are: `[:error, :warn, :ignore]`
- When using STI, even if you enable :warn you will likely still end up recieving an `ActiveRecord::AssociationTypeMismatch` error in which case your SOL
1. [#542](https://github.com/paper-trail-gem/paper_trail/issues/542) -
Not compatible with [transactional tests][34], aka. transactional fixtures.
1. Requires database timestamp columns with fractional second precision.
Expand Down
11 changes: 7 additions & 4 deletions lib/generators/paper_trail/install_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,13 @@ def create_migration_file
end

def create_initializer
create_file(
"config/initializers/paper_trail.rb",
"PaperTrail.config.track_associations = #{!!options.with_associations?}\n"
)
if options.with_associations?
create_file(
"config/initializers/paper_trail.rb",
"PaperTrail.config.track_associations = true\n",
"PaperTrail.config.association_reify_error_behaviour = :error"
)
end
end

def self.next_migration_number(dirname)
Expand Down
2 changes: 1 addition & 1 deletion lib/paper_trail/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Config
STR

include Singleton
attr_accessor :serializer, :version_limit
attr_accessor :serializer, :version_limit, :association_reify_error_behaviour

def initialize
# Variables which affect all threads, whose access is synchronized.
Expand Down
13 changes: 12 additions & 1 deletion lib/paper_trail/reifiers/has_one.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,18 @@ def load_version(assoc, model, transaction_id, version_at)
when 1
versions.first
else
raise FoundMoreThanOne.new(base_class_name, versions.length)
case PaperTrail.config.association_reify_error_behaviour.to_s
when "warn"
version = versions.first
version.logger&.warn(
format(FoundMoreThanOne::MESSAGE_FMT, base_class_name, versions.length)
)
version
when "ignore"
versions.first
else # "error"
raise FoundMoreThanOne.new(base_class_name, versions.length)
end
end
end

Expand Down
3 changes: 3 additions & 0 deletions spec/dummy_app/app/models/person.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ class Person < ActiveRecord::Base
has_one :car, foreign_key: :owner_id
has_one :bicycle, foreign_key: :owner_id

has_one :thing
has_one :thing_2, class_name: "Thing"

if ActiveRecord.gem_version >= Gem::Version.new("5.0")
belongs_to :mentor, class_name: "Person", foreign_key: :mentor_id, optional: true
else
Expand Down
2 changes: 2 additions & 0 deletions spec/dummy_app/app/models/thing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@

class Thing < ActiveRecord::Base
has_paper_trail save_changes: false

belongs_to :person, optional: true
end
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ def up

create_table :things, force: true do |t|
t.string :name
t.references :person
end

create_table :translations, force: true do |t|
Expand Down
54 changes: 51 additions & 3 deletions spec/models/person_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
expect(Person.new).to be_versioned
end

describe "#cars and bicycles" do
it "can be reified" do
# See https://github.com/paper-trail-gem/paper_trail/issues/594
describe "#association reify error behaviour" do
it "association reify error behaviour = :error" do
person = Person.create(name: "Frank")
car = Car.create(name: "BMW 325")
bicycle = Bicycle.create(name: "BMX 1.0")
Expand All @@ -23,7 +24,6 @@

expect(person.reload.versions.length).to(eq(3))

# See https://github.com/paper-trail-gem/paper_trail/issues/594
expect {
person.reload.versions.second.reify(has_one: true)
}.to(
Expand All @@ -34,5 +34,53 @@
end
)
end

# it "association reify error behaviour = :warn" do
# #::PaperTrail.config.association_reify_error_behaviour = :warn

# person = Person.create(name: "Frank")
# thing = Thing.create(name: "BMW 325")
# thing = Thing.create(name: "BMX 1.0")

# person.thing = thing
# person.thing = thing
# person.update_attributes(name: "Steve")

# thing.update_attributes(name: "BMW 330")
# thing.update_attributes(name: "BMX 2.0")
# person.update_attributes(name: "Peter")

# expect(person.reload.versions.length).to(eq(3))

# expect(person.versions.first.logger).to(
# receive(:warn).with(/Unable to reify has_one association/).twice
# )

# person.reload.versions.second.reify(has_one: true)
# end

# it "association reify error behaviour = :ignore" do
# #::PaperTrail.config.association_reify_error_behaviour = :ignore

# person = Person.create(name: "Frank")
# thing = Thing.create(name: "BMW 325")
# thing = Thing.create(name: "BMX 1.0")

# person.thing = thing
# person.thing = thing
# person.update_attributes(name: "Steve")

# thing.update_attributes(name: "BMW 330")
# thing.update_attributes(name: "BMX 2.0")
# person.update_attributes(name: "Peter")

# expect(person.reload.versions.length).to(eq(3))

# expect(person.versions.first.logger).to_not(
# receive(:warn).with(/Unable to reify has_one association/)
# )

# person.reload.versions.second.reify(has_one: true)
# end
end
end
8 changes: 8 additions & 0 deletions spec/paper_trail/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ module PaperTrail
end
end

# describe "association_reify_error_behaviour" do
# it "accepts a value" do
# config = described_class.instance
# config.association_reify_error_behaviour = :warn
# expect(config.association_reify_error_behaviour).to eq(:warn)
# end
# end

describe ".version_limit", versioning: true do
after { PaperTrail.config.version_limit = nil }

Expand Down

0 comments on commit f9a8a77

Please sign in to comment.