acts_as_tenant icon indicating copy to clipboard operation
acts_as_tenant copied to clipboard

`NoTenantSet` error raised within an `ActsAsTenant.without_tenant` block (RSpec + Rails)

Open its-fern opened this issue 1 year ago • 5 comments

Currently running ActsAsTenant v 1.0.1 on a Rails 7.1 app. We use rspec for testing and recently upgraded rspec-rails from 6.1.2 to 6.1.3. We started running into NoTenantSet errors on this upgrade when we were not before.

For some context, we have a helper set up that allows devs to include a tag in the rspec context to disable the tenant check:

RSpec.configure do |config|
  # Acts as tenant support - check out documenation here:
  # https://github.com/ErwinM/acts_as_tenant#testing
  config.around do |example|
    if example.metadata[:skip_multitenancy]
      ActsAsTenant.without_tenant do
        # ActsAsTenant::Errors::NoTenantSet error raised here
        example.run
      end
    else
      ## ...
    end
  end
end

Devs can then write specs that ignore the tenant check like so:


require "rails_helper"

RSpec.describe SomeModel, type: :model do
  context "without multi-tenancy", skip_multitenancy: true do
    it "allows creating models" do
      expect { SomeModel.create!(name: "foo") }.to change(SomeModel, :count).by(1)
    end
  end

  context "with multi-tenancy" do 
    it "raises an error if no tenant is set" do
      expect { SomeModel.create!(name: "foo") }.to raise_error(ActsAsTenant::Errors::NoTenantSet)
    end
  end
end

I did some digging and I think this might be related to this PR https://github.com/rspec/rspec-rails/pull/2752 (full changelog here). The ActsAsTenant.unscoped? flag is being reset somehow (confirmed this by stepping through the code) so this code block raises:

https://github.com/ErwinM/acts_as_tenant/blob/7e3bd8a534f309061a9c34b4ed3eb3576e2e065d/lib/acts_as_tenant/model_extensions.rb#L20-L22

Any help is appreciated here, and I'm happy to provide more info where I can!

its-fern avatar Jun 30 '24 19:06 its-fern

This happens because of a bug introduced in RSpec v6.1.3. Temporarily locking your Gemfile to = 6.1.2 will work around it for now. I've submitted https://github.com/rspec/rspec-rails/issues/2773 with more information and an example test case.

pond avatar Jul 02 '24 04:07 pond

This happens because of a bug introduced in RSpec v6.1.3. Temporarily locking your Gemfile to = 6.1.2 will work around it for now. I've submitted rspec/rspec-rails#2773 with more information and an example test case.

Amazing, thanks @pond!

its-fern avatar Jul 02 '24 13:07 its-fern

@its-fern Unfortunately things were going really well until a dev stepped in and seems to say that this bug is minor; we're holding it wrong by using hooks as they've been documented in RSpec. I'm not sure I agree, but you well might. Please could you check Jon Rowe's comment and vote accordingly? If ultimately I'm in the minority in my assessment of what should be done here, then of course, the community wins.

EDITED TO ADD: They're still interested in a resolution PR, so I'll work on that today (NZ time).

pond avatar Jul 03 '24 20:07 pond

@pond I appreciate the work you did in the PR attempting a fix. It's unfortunate that it got stalled, though it seems like https://github.com/rspec/rspec-rails/pull/2753 might be a similar fix. Hopefully that gets straightened out soon.

In order to not be pinned to that particular version of rspec-rails, I ended up working around this issue by defining a lambda for the require_tenant config and defining a global variable:

# config/initializers/acts_as_tenant.rb

ActsAsTenant.configure do |config|
  config.require_tenant = if Rails.env.test?
    lambda do
      # rubocop:disable Style/GlobalVars
      if defined?($skip_multitenancy) && $skip_multitenancy
        false
      else
        true
      end
      # rubocop:enable Style/GlobalVars
    end
  else
    true
  end
end

And then in the Rspec helper:

RSpec.configure do |config|
  # Acts as tenant support - check out documenation here:
  # https://github.com/ErwinM/acts_as_tenant#testing
  config.around do |example|
    if example.metadata[:skip_multitenancy]
      # rubocop:disable Style/GlobalVars
      $skip_multitenancy = true
      example.run
      $skip_multitenancy = false
      # rubocop:enable Style/GlobalVars
    else
      # ...
    end
  end
end

I don't particularly love using global variables, but this works as a decent workaround for now.

its-fern avatar Jan 20 '25 22:01 its-fern

@its-fern Yes, there are workarounds. The company I worked for that was using AAT failed at the end of last year (owing me a lot of unpaid salary, too - gah!) and given that I spent all the time on the bug report, debate, a subsequent PR which eventually got approved, but then got closed regardless... Well, you can imagine that I'm not inclined to spend any more time on any of this. People in those discussions have said they want to solve it another way so it now sits squarely on their shoulders.

pond avatar Jan 20 '25 23:01 pond