shoulda-matchers

shoulda-matchers copied to clipboard

Hi! I have one model with two column (applicant_id and career_id) and both are foreign keys. When I a try to validate applicant_id or career_id is uniqueness

should validate_uniqueness_of(:applicant_id) or should validate_uniqueness_of(:career_id)

I got follow exception

Failure/Error: should validate_uniqueness_of(:applicant_id) ActiveRecord::InvalidForeignKey: PG::ForeignKeyViolation: ERROR: insert or update on table "degrees" violates foreign key constraint "fk_rails_25163bce0a" DETAIL: Key (applicant_id)=(0) is not present in table "applicants". : INSERT INTO "degrees" ("applicant_id", "created_at", "updated_at") VALUES ($1, $2, $3) RETURNING "id"

But if I delete the foreing key constraint this problem does not occur

The foreign key constraint says that in order for a Degree to refer to an Applicant, that Applicant must already exist with the same id. The constraint is failing when validate_uniqueness_of attempts to save your Degree record (it needs an existing record to compare a new one against). Try creating an Applicant before using validate_uniqueness_of.

I'm closing this as I don't think this is a bug with shoulda-matchers, but feel free to continue the discussion.

It is a bug in shoulda-matchers, since it assign an id not in the database. If not, what causes this error?

FactoryGirl.define do
  factory :document_publishing_stage do
    closed false

    association :document
    association :publishing_stage

    factory :closed_document_status do
      closed true
    end
  end
end


RSpec.describe DocumentPublishingStage, type: :model do
  describe 'with validation' do
    subject { build(:document_publishing_stage) }

    it { should validate_presence_of(:document) }
    it { should validate_presence_of(:publishing_stage) }
    it { should validate_uniqueness_of(:document_id).scoped_to(:publishing_stage_id, :ref_id) }
  end
end

@itsmechlark What's the error message you're getting?

Hopefully this stack-trace will help you. Thanks.

Failure/Error: it { should validate_uniqueness_of(:document_id).scoped_to(:publishing_stage_id, :ref_id) }
     ActiveRecord::InvalidForeignKey:
       PG::ForeignKeyViolation: ERROR:  insert or update on table "document_publishing_stages" violates foreign key constraint "fk_rails_b88661929f"
       DETAIL:  Key (document_id)=(0) is not present in table "documents".
       : INSERT INTO "document_publishing_stages" ("document_id", "publishing_stage_id", "created_at", "updated_at") VALUES ($1, $2, $3, $4) RETURNING "id" /*application:CDAsiaOnline,line:/spec/models/document_publishing_stage_spec.rb:21:in `block (3 levels) in <top (required)>'*/
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/postgresql_adapter.rb:606:in `exec_prepared'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/postgresql_adapter.rb:606:in `block in exec_cache'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/abstract_adapter.rb:473:in `block in log'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/notifications/instrumenter.rb:20:in `instrument'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/abstract_adapter.rb:467:in `log'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/postgresql_adapter.rb:605:in `exec_cache'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/postgresql_adapter.rb:589:in `execute_and_clear'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:160:in `exec_query'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/marginalia-1.3.0/lib/marginalia.rb:56:in `exec_query_with_marginalia'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:192:in `exec_insert'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/abstract/database_statements.rb:108:in `insert'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/abstract/query_cache.rb:14:in `insert'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/relation.rb:64:in `insert'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/persistence.rb:524:in `_create_record'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/counter_cache.rb:139:in `_create_record'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/locking/optimistic.rb:75:in `_create_record'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/attribute_methods/dirty.rb:132:in `_create_record'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/callbacks.rb:306:in `block in _create_record'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:117:in `call'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:117:in `call'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:555:in `block (2 levels) in compile'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:505:in `call'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:505:in `call'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:92:in `__run_callbacks__'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:778:in `_run_create_callbacks'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/callbacks.rb:306:in `_create_record'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/timestamp.rb:57:in `_create_record'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/persistence.rb:504:in `create_or_update'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/callbacks.rb:302:in `block in create_or_update'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:117:in `call'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:117:in `call'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:555:in `block (2 levels) in compile'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:505:in `call'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:505:in `call'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:92:in `__run_callbacks__'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activesupport-4.2.4/lib/active_support/callbacks.rb:778:in `_run_save_callbacks'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/callbacks.rb:302:in `create_or_update'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/persistence.rb:120:in `save'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/validations.rb:37:in `save'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/attribute_methods/dirty.rb:21:in `save'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/transactions.rb:286:in `block (2 levels) in save'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/transactions.rb:351:in `block in with_transaction_returning_status'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `block in transaction'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/abstract/transaction.rb:184:in `within_new_transaction'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/connection_adapters/abstract/database_statements.rb:213:in `transaction'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/transactions.rb:220:in `transaction'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/transactions.rb:348:in `with_transaction_returning_status'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/transactions.rb:286:in `block in save'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/transactions.rb:301:in `rollback_active_record_state!'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/activerecord-4.2.4/lib/active_record/transactions.rb:285:in `save'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:356:in `block in create_record_in_database'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:353:in `tap'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:353:in `create_record_in_database'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:329:in `first_instance'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:325:in `existing_record'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:385:in `block in set_scoped_attributes'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:382:in `each'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:382:in `all?'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:382:in `set_scoped_attributes'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/shoulda-matchers-3.0.0/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb:263:in `matches?'
     # ./spec/models/document_publishing_stage_spec.rb:21:in `block (3 levels) in <top (required)>'
     # ./spec/config/database_cleaner.rb:11:in `block (3 levels) in <top (required)>'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/database_cleaner-1.5.1/lib/database_cleaner/generic/base.rb:16:in `cleaning'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/database_cleaner-1.5.1/lib/database_cleaner/base.rb:92:in `cleaning'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/database_cleaner-1.5.1/lib/database_cleaner/configuration.rb:86:in `block (2 levels) in cleaning'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/database_cleaner-1.5.1/lib/database_cleaner/configuration.rb:87:in `call'
     # /home/itsmechlark/.rvm/gems/ruby-2.2.3/gems/database_cleaner-1.5.1/lib/database_cleaner/configuration.rb:87:in `cleaning'
     # ./spec/config/database_cleaner.rb:10:in `block (2 levels) in <top (required)>'

@itsmechlark So here's what the matcher is doing (or supposed to do anyway):

1. The matcher takes a new instance of DocumentPublishingStage with document_id pointing to a real Document
2. The matcher takes this instance and sets document_id to some default value, then saves it
3. The matcher makes a new instance of DocumentPublishingStage, sets its document_id to the same value, and asserts that it fails the uniqueness validation

The problem is that it's failing on step 2, because the default value it sets document_id to ends up being 0. Since this doesn't point to a real Document, when that first instance is saved, the foreign key constraint you have on the table fails.

So now that I've had a second look at this, I do agree that this can probably be fixed from the shoulda-matchers side. Specifically, I'm curious what would happen if we didn't override the attribute being tested with a new, default value, as we are doing. So I'll reopen this issue to remind myself to investigate that.

In the meantime, what would happen if you used an existing instance of DocumentPublishingStage instead of a new (unsaved) instance? In other words:

it do
  create(:document_publishing_stage)
  should validate_uniqueness_of(:document_id).scoped_to(:publishing_stage_id, :ref_id)
end

I tried:

subject { create(:document_publishing_stage) }
it { should validate_uniqueness_of(:document_id).scoped_to(:publishing_stage_id, :ref_id) }

it do
   create(:document_publishing_stage) }
   should validate_uniqueness_of(:document_id).scoped_to(:publishing_stage_id, :ref_id) 
end

still not working. I was thinking during validation:

subject { create(:document_publishing_stage) }
it { should validate_uniqueness_of(:document_id).scoped_to(:publishing_stage_id, :ref_id) }

1. The matcher creates new instance of DocumentPublishingStage with :document_id plus the scope :publishing_stage_id and ref_id from the subject
2. The matcher should asserts that it fails the uniqueness validation.

Yeah, the values of scopes are copied from the subject, but for some reason the value of the attribute (document_id) isn't.

So you're saying that if you create the record instead of building it you get the exact same error?

No, it didn't copy values of scope. I found out this when one of my custom attribute was assign with nil value, ref_id. We should add test regarding foreign key constraint.

Okay. I'm not sure when I'll be able to get to this, but in the meantime do you think it's possible for you to write a failing test for this and submit that as a PR?

I am not familiar neither with shoulda-matchers nor with rspec however the issue has gone away when I've rewritten the test in the next way

it do
   subject.document = FactoryGirl.build(:document)
   it { should validate_uniqueness_of(:document_id).scoped_to(:publishing_stage_id, :ref_id) }
end

hope it helps someone at least as a workaround.

@tamatsyk Ah, interesting. You could also try creating the record in question beforehand; that might be more straightforward than setting subject.document.

I have the same problem, and any solution worked for me ):

I solved it the same way @tarasmatsyk solved it.

it do
  subject.user = FactoryGirl.create(:user, organization: subject.organization)
  is_expected.to validate_uniqueness_of(:user_id).scoped_to(:organization_id).allow_nil
end

Funnily enough, this didn't happen before, only after refactoring the my specs which contained a subtle error in my factories.

@tarasmatsyk Thank you!

I also ran into this issue, and fixed it with @tarasmatsyk's solution:

  it do
    # https://github.com/thoughtbot/shoulda-matchers/issues/682
    subject.user = create(:user)
    is_expected.to validate_uniqueness_of(:user_id).scoped_to(:organization_id)
  end

I solved the problem by creating the factory instead of building it for the specific context.

  context do
    subject { create(: document_publishing_stage) }
    it do
      is_expected.to validate_uniqueness_of(:user_id).scoped_to(:organization_id)
    end
  end

Hey folks. In an effort to lighten our load as maintainers and be able to serve you better in the future, the shoulda-matchers team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this issue.

Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised. If you feel that we should reopen this issue, then please let us know so that we can reprioritize it. Thanks!

To anyone else coming across this issue, I fixed my problem by creating a record with zero for an ID (so when shoulda-matchers assigns the zero as a dummy value, the foreign-keyed model exists in the test database)

I think it would be worth re-opening this issue. Seems like an important bug to fix and I run into it a lot in my projects. I personally usually suggest not using foreign keys in many projects for this and other issues they create, but sometimes I'm on teams that insist on the FKs. Creating a record with a zero ID seems like a clunky workaround but I could see that as being added as a step in the matcher itself.

@jcraigk Have you tried this fix? https://github.com/thoughtbot/shoulda-matchers/issues/682#issuecomment-151715507.

Edit: I'll reopen this issue but I can't make any promises about when it would get fixed. This is a tricky problem to solve and honestly I'm not quite sure what the solution should be. It could involve making a backward-incompatible change and breaking a bunch of people's tests who rely on the current behavior along the way. I'm also not sure how it fits into other fixes — there have been a few other issues raised in the past like this, but notable is #1067, which could potentially conflict with this issue solution-wise as well.

@mcmire thanks for re-opening this issue.

I did try the fix mentioned and it does work, with the caveat that I need to change the order of the validates call in the model to accommodate. This does not read well on polymorphic associations. For example the validates call in this example class

class Baz < ApplicationRecord
  belongs_to :foo
  belongs_to :bar, polymorphic: true
  validates :foo_id, uniqueness: { scope: %i[barable_type barable_id] }
end

needs to become

validates :barable_type, uniqueness: { scope: %i[foo_id barable_id] }

and then of course the extra line in the RSpec example to create the necessary bar record so we don't get something like

ActiveRecord::InvalidForeignKey:
PG::ForeignKeyViolation: ERROR:  insert or update on table "bazes" violates foreign key constraint "fk_rails_a6d5330bb7"
DETAIL:  Key (bar_id)=(0) is not present in table "bars".

Compromising model readability and adding an explicit create doesn't seem like a good long-term solution here, but thanks for pointing this out - it may come in handy. I think creating an extra record in the background makes sense, but I'm not sure how to best handle the order of the columns in the validate call.

Concerning backwards compatibility, isn't that what semantic versioning is for?

I appreciate the effort put into this. I see the issue is still open.

For me a solution to this problem is very welcome in case related models depend on related models, depending on related models...

Example: Assuming the belongs_to relations have foreign_key constraints in the db.

class Thing
  belongs_to :user, optional: false
end

class User
  has_many :things
  belongs_to :organisation, optional: false
end

class Organisation
  has_many :users
  belongs_to :location, optional: false
end

class Location
  has_many :organisations
  belongs_to :region, optional: false
end

What was supposed to be a simple (unit-)test now requires a relatively large setup with several other records that have to be created.

(Sure, this could be a red flag on the part of the relation setup, but I think that's beside the point here.)

I've also tried using instance_doubles, but that'll get you a ActiveRecord::AssociationTypeMismatch

For now I have everything working by doing the whole setup. But somehow fixing this by not actually requiring the persisted records would be awesome.