order of acts_as_tenant declaration subverts validation errors and save exceptions when associated tenants do not match

[ybakos]

Via #11 .

I believe there may be some inconsistency to how well acts_as_tenant may be emitting validation errors or raising exceptions when saving a model with associations referring to other tenants.

Should acts_as_tenant be properly ensuring that models within a chain of associations are all of the same tenant? For example, given C belongs to B, and both B and C belong to Account via acts_as_tenant, shouldn't the gem enforce that C and B's accounts are the same?

It appears it does not.

Here's a barebones Rails 6.1.3 repo using acts_as_tenant 0.5.0: https://github.com/ybakos/acts_as_tenant_issue_tests

Notice the contents of test/models/person_test.rb

test 'acts_as_tenant: ensure all models in an association belong to the same tenant' do
    ActsAsTenant.with_tenant(accounts(:one)) do
      person = people(:one)
      assert person.account == accounts(:one)
      assert person.valid?
      first_room = rooms(:one)
      assert first_room.account == accounts(:one)
      assert person.room == first_room

      second_room = rooms(:two)
      refute first_room.account == second_room.account
      refute person.account == second_room.account
      person.room = second_room # person's account is not the same as second_room's account

      refute person.valid? # This should pass, but it fails.
      refute person.save # This should pass, but it fails.
      assert_raises(ActiveRecord::RecordInvalid) { person.save! } # This should pass, but it fails.
    end
  end

This surprised me, because in the past I have indeed see act_as_tenant emit some appropriate errors when the tenants do not match.

[ybakos]

Here is another test. It mimics acts_as_tenant:/spec/models/model_extensions.rb:194, "associations can only be made with in-scope:objects".

https://github.com/ybakos/acts_as_tenant_issue_tests/blob/main/test/models/account_test.rb

  test 'associations can be made with out-of-scope objects (but should not!)' do
    # See acts_as_tenant:/spec/models/model_extensions.rb:194,
    # "associations can only be made with in-scope objects"
    room2 = accounts(:two).rooms.create!(name: 'Account 2 room')
    ActsAsTenant.current_tenant = accounts(:one)
    room1 = Room.create!(name: "Account 1 room")
    person = room1.people.create!(name: 'First account person')
    person.room = room2
    refute person.valid? # This should pass, but it fails!
    refute person.update(room_id: room2.id) # This should pass, but it fails!
  end

Am I losing my mind or is there indeed a problem? This is a Rails 6.1.3 app.

I've run the rspec suite for acts_as_tenant and the behavior there within its "dummy" app is as expected. But I am seeing unexpected association behavior in both this example 6.1.3 app and a 5.2.4 app.

[ybakos]

Aha, found the issue. Is this a bug or did I miss the instructions that state "one must declare acts_as_tenant after all other associations" ?

ybakos/acts_as_tenant_issue_tests@56b3646

[ybakos]

^^^ That was not a smart alec comment, I truly wondered if the doc emphasized that or not. I just checked... and although it does mention the order concern in the controller for setting the tenant, there is no mention of the order concern at the model layer.

Indeed, if I switch the declaration of Task to:

class Task < ActiveRecord::Base
  acts_as_tenant :account
  belongs_to :project

and also in Project:

class Project < ActiveRecord::Base
  acts_as_tenant :account

We see the test error:

Failures:

  1) ActsAsTenant associations can only be made with in-scope objects
     Failure/Error: expect(task.update(project_id: project1.id)).to eq(false)

       expected: false
            got: true

       (compared using ==)

       Diff:
       @@ -1 +1 @@
       -false
       +true

     # ./spec/models/model_extensions_spec.rb:201:in `block (2 levels) in <top (required)>'