Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
its-fern opened this issue Jun 30, 2024 · 5 comments

Comments

@its-fern
Copy link

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 rspec/rspec-rails#2752 (full changelog here). The ActsAsTenant.unscoped? flag is being reset somehow (confirmed this by stepping through the code) so this code block raises:

if ActsAsTenant.should_require_tenant? && ActsAsTenant.current_tenant.nil? && !ActsAsTenant.unscoped?
raise ActsAsTenant::Errors::NoTenantSet
end

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

@its-fern its-fern changed the title NoTenantSet error raised within an ActsAsTenant.without_tenant block NoTenantSet error raised within an ActsAsTenant.without_tenant block (RSpec + Rails) Jun 30, 2024
@pond
Copy link

pond commented Jul 2, 2024

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.

@its-fern
Copy link
Author

its-fern commented Jul 2, 2024

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!

@pond
Copy link

pond commented Jul 3, 2024

@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).

@its-fern
Copy link
Author

@pond I appreciate the work you did in the PR attempting a fix. It's unfortunate that it got stalled, though it seems like rspec/rspec-rails#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.

@pond
Copy link

pond commented Jan 20, 2025

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants