rspec model testing; method doesn't work

Question

The method itself works as expected, but when I try to test it in the model spec the create part fails. The find part works well. What did I miss?

conversation.rb

scope :between, -> (sender_id, recipient_id) do
  where("(conversations.sender_id = ? AND conversations.recipient_id = ?) OR (conversations.sender_id = ? AND conversations.recipient_id = ?)", sender_id, recipient_id, recipient_id, sender_id)
end

def self.create_or_find_conversation(task_assigner_id, task_executor_id)
  Conversation.between(task_assigner_id, task_executor_id).first_or_create do |conversation|
    conversation.sender_id = task_assigner_id
    conversation.recipient_id = task_executor_id
  end
end

conversation_spec.rb

describe "class methods" do

  let(:sender) { create(:user) }
  let(:recipient) { create(:user) }
  let(:other_recipient) { create(:user) }
  let!(:conversation) { create(:conversation, sender: sender, recipient: recipient) }

  context "create_of_find_conversation" do

   #this one throws Failure/Error: expect{conv}.to change{Conversation.count}.by(1)
   #expected result to have changed by 1, but was changed by 0
    it "creates conversation" do
      conv = Conversation.create_or_find_conversation(sender, other_recipient)
      expect{conv}.to change{Conversation.count}.by(1)
    end

    #this one is working as expected
    it "finds conversation" do
      conv = Conversation.create_or_find_conversation(sender, recipient)
      expect(conv).to eq(conversation)
    end
  end
end

Answer 1

I think these code:

it "creates conversation" do
  conv = Conversation.create_or_find_conversation(sender, other_recipient)
  expect{conv}.to change{Conversation.count}.by(1)
end

should be changed to:

it "creates conversation" do
  expect{
      Conversation.create_or_find_conversation(sender.id, other_recipient.id)
  }.to change{Conversation.count}.by(1)
end

because it was't the value changed the count, but the process.

Answer 2

Regular variables in Ruby are not lazy loading - the right side of the assignment is processed when the variable is assigned.

def do_something(val)
  puts "do_something called"
  val
end

a = do_something(hello_world)
puts a
# do_something called
# hello world

You would either need to change expection so that the action is called inside the block passed to expect:

it "creates conversation" do
  expect do
    Conversation.create_or_find_conversation(sender, other_recipient)
  end.to change{Conversation.count}.by(1)
end

Or use RSpec's let to create a lazy loading variable:

let(:conv) { Conversation.create_or_find_conversation(sender, other_recipient) }

it "creates conversation" do
  expect { conv }.to change{Conversation.count}.by(1)
end

But that does not fix the underlying problem that you are modeling the domain wrong. In a conversation both the sides take turns speaking - so using a recipient_id and sender_id is just plain wrong. Rather its messages that have a sender and receiver.

You can change them to be called whatever you want but it is much simpler to use a proper many to many relation so that you don't need a complex AND OR query.

class User < ActiveRecord::Base
  has_many :user_conversations
  has_many :conversations, through: user_conversations
end

class Conversation < ActiveRecord::Base
  has_many :user_conversations
  has_many :users, through: user_conversations
end

# the m2m join model.
class UserCoversation < ActiveRecord::Base
  belongs_to :user
  belongs_to :conversation
end

You can then simply query:

Conversation.where(users: [a, b])