Skip to content

Commit

Permalink
Fix #501; The check to whether the table exists for association suppo…
Browse files Browse the repository at this point in the history
…rt should not be inside of a Proc
  • Loading branch information
batter committed Mar 20, 2015
1 parent 9528e9e commit ca09a8c
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions lib/paper_trail/version_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ module VersionConcern
included do
belongs_to :item, :polymorphic => true

# Need to inspect inside of a Proc so that tests pass even when DB is not initialized
# such as when we run on Travis (there won't be a db in `test/dummy/db/`)
if lambda { PaperTrail::VersionAssociation.table_exists? }
# Since the test suite has test coverage for this, we want to declare the
# association when the test suite is running. This makes it pass
# when DB is not initialized prior to test runs such as when we run on
# Travis CI (there won't be a db in `test/dummy/db/`)
if PaperTrail::VersionAssociation.table_exists? || ::Rails.env.test?
has_many :version_associations, :dependent => :destroy
end

Expand Down

1 comment on commit ca09a8c

@TMaYaD
Copy link
Contributor

@TMaYaD TMaYaD commented on ca09a8c Mar 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does fix the bug in production, development and most of the tests. But fails on a few request specs that actually depend on paper_trail.

I guess we need to figure out a way to differentiate gem specs from app specs. If I'm understanding this correctly, we need to force association when running gem specs.

However I would argue that the responsibility in that case lies with the one disabling the db, the test setup. Either we can mock table_exists? there, or even better wrap table_exists? in a config option and provide a way to force that. I'm currently on mobile. Will make a pull request later today.

Please sign in to comment.