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

Multi-tenant class not found if defined inside a module #105

Open
serch opened this issue Dec 4, 2020 · 1 comment
Open

Multi-tenant class not found if defined inside a module #105

serch opened this issue Dec 4, 2020 · 1 comment
Labels

Comments

@serch
Copy link
Contributor

serch commented Dec 4, 2020

The following code works as expected and the call to multi_tenant :tenant automatically calls belongs_to :tenant and defines both tenant and tenant= methods.

class Tenant < ApplicationRecord; end

class Foo < ApplicationRecord
  multi_tenant :tenant
end

However, if both classes are inside a module

module MyModule
  class Tenant < ApplicationRecord; end

  class Foo < ApplicationRecord
    multi_tenant :tenant
  end
end

the multi_tenant :tenant call does not automatically call belongs_to :tenant or define tenant and tenant= methods.
This is due to the implementation of tenant_klass_defined?

def self.tenant_klass_defined?(tenant_name)
  !!tenant_name.to_s.classify.safe_constantize
end

so only the top-level constant ::Tenant is expected to be the tenant class for tenant name :tenant, ::MyModule::Tenant is not considered.

I thought about copying Rails' implementation of compute_type

https://github.com/rails/rails/blob/6ef39975d60cc9dafd1728c49e394dad11d12327/activerecord/lib/active_record/inheritance.rb#L224-L235

that in our example would search for the following 3 classes:

::MyModule::Foo::Tenant, ::MyModule::Tenant, ::Tenant

in that order.

Another solution would be to pass class_name just like we do in Rails.

multi_tenant :tenant, class_name: "::MyModule::Tenant"

What do you guys think?

I'm willing to implement this if we can agree on a solution.

@serprex
Copy link
Collaborator

serprex commented Mar 29, 2022

Both solutions could work. Update search order, & add optional class_name parameter

@serprex serprex added the 2.0 label Mar 29, 2022
msasaki666 added a commit to msasaki666/activerecord-multi-tenant that referenced this issue Jun 15, 2023
gurkanindibay added a commit that referenced this issue Jun 16, 2023
* fix logic inside the tenant_klass_defined method

* add tests for tenant_klass_defined?

* fix failed tests

* rubocop -a

* remove unnecessary word

* add a test case for the following
#105

* test a more appropriate class

* add multi_tenant

* Fixes rubocop warnings
---------

Co-authored-by: Gürkan İndibay <gindibay@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants