shoulda-matchers icon indicating copy to clipboard operation
shoulda-matchers copied to clipboard

HABTM with composite keys

Open ghost opened this issue 3 years ago • 3 comments

Hello,

I'm trying to validate the existence of a HABTM relationship on my models, having one of the models defining is primary key as a composite key (using rails_composite_keys).

My models are set up as follows

class ModelA < ApplicationRecord
  has_and_belongs_to_many :model_bs, association_foreign_key: ["model_b_id", "some_other_key"]
end
class ModelB < ApplicationRecord
  self.primary_keys = :model_b_id, :some_other_key

  has_and_belongs_to_many :model_as, foreign_key: ["model_b_id", "some_other_key"]
end

The database table is created as

create_table :model_as_model_bs, id: false do |t|
  t.bigint :model_a_id
  t.bigint :model_b_id
  t.bigint :some_other_key
end

execute <<-SQL
  ALTER TABLE model_as_model_bs
  ADD CONSTRAINT model_as_model_bs_foreign_key
  FOREIGN KEY (some_other_key, model_b_id)
  REFERENCES model_bs (some_other_key, model_b_id);
SQL

Running a rails console, I can verify that the HABTM works as expected. However, when trying to run my spec to verify that the association actually exists on the model, defined as

RSpec.describe ModelA, type: :model do
  let(:subject) { build(:model_a) }

  context "associations" do
    it { is_expected.to have_and_belong_to_many(:model_bs) }
  end
end

my spec fails, giving me the error

Failure/Error: it { is_expected.to have_and_belong_to_many :model_bs}
  Expected ModelA to have a has_and_belongs_to_many association called model_bs (join table model_as_model_bs missing column: model_b_id, some_other_key)

I'm not exactly sure if I'm not making a mistake here in my specs, so advice is welcome, but it feels like the AssociationMatcher actually tries to find a single column called model_b_id, some_other_key.

ghost avatar Jun 21 '21 07:06 ghost

I tested in my private fork, and it seems that if you flatten the array of the expected columns in the JoinTableMatcher it works just fine, but it was just a quick test and I did not verify that this change does not break any existing functionality.

However, if it helps, I'd make a PR for this patch

ghost avatar Jun 21 '21 08:06 ghost

Yeah, if you've figured out a fix, a PR would be appreciated!

mcmire avatar Jun 21 '21 15:06 mcmire

@mcmire Added PR #1448

ghost avatar Jun 22 '21 00:06 ghost