Merge pull request #2 from hazelsparrow/overlapping-tenants

Overlapping tenants

lib/acts_as_tenant/model_extensions.rb

@@ -1,15 +1,6 @@
 module ActsAsTenant
-  @@tenant_klass = nil
   @@models_with_global_records = []
 
-  def self.set_tenant_klass(klass)
-    @@tenant_klass = klass
-  end
-
-  def self.tenant_klass
-    @@tenant_klass
-  end
-
   def self.models_with_global_records
     @@models_with_global_records
   end
@@ -18,14 +9,6 @@ def self.add_global_record_model model
     @@models_with_global_records.push(model)
   end
 
-  def self.fkey
-    "#{@@tenant_klass.to_s}_id"
-  end
-
-  def self.polymorphic_type
-    "#{@@tenant_klass.to_s}_type"
-  end
-
   def self.current_tenant=(tenant)
     RequestStore.store[:current_tenant] = tenant
   end
@@ -46,9 +29,13 @@ def self.unscoped?
     !!unscoped
   end
 
+  def self.klass_for_current_tenant
+    raise ActsAsTenant::Errors::NoTenantSet unless current_tenant
+
+    current_tenant.class.name.demodulize.downcase.to_sym
+  end
+
   class << self
-    #attr_accessor :default_tenant
-    #
     def default_tenant=(tenant)
       @default_tenant = tenant
     end
@@ -64,7 +51,6 @@ def default_tenant
 
 
   def self.with_tenant(tenant, &block)
-
     if block.nil?
       raise ArgumentError, "block required"
     end
@@ -73,7 +59,6 @@ def self.with_tenant(tenant, &block)
     self.current_tenant = tenant
     value = block.call
     return value
-
   ensure
     self.current_tenant = old_tenant
   end
@@ -90,7 +75,6 @@ def self.without_tenant(&block)
     self.unscoped = true
     value = block.call
     return value
-
   ensure
     self.current_tenant = old_tenant
     self.unscoped = old_unscoped
@@ -103,21 +87,19 @@ def self.included(base)
 
     module ClassMethods
       def acts_as_tenant(tenant = :account, options = {})
-        ActsAsTenant.set_tenant_klass(tenant)
-
         ActsAsTenant.add_global_record_model(self) if options[:has_global_records]
 
         # Create the association
         valid_options = options.slice(:foreign_key, :class_name, :inverse_of, :optional)
-        fkey = valid_options[:foreign_key] || ActsAsTenant.fkey
-        polymorphic_type = valid_options[:foreign_type] || ActsAsTenant.polymorphic_type
+        fkey = valid_options[:foreign_key] || "#{tenant}_id".to_sym
+        polymorphic_type = valid_options[:foreign_type] || "#{tenant}_type"
         belongs_to tenant, valid_options
 
         default_scope lambda {
           if ActsAsTenant.configuration.require_tenant && ActsAsTenant.current_tenant.nil? && !ActsAsTenant.unscoped?
             raise ActsAsTenant::Errors::NoTenantSet
           end
-          if ActsAsTenant.current_tenant
+          if ActsAsTenant.current_tenant && (ActsAsTenant.klass_for_current_tenant == tenant || options[:polymorphic])
             keys = [ActsAsTenant.current_tenant.id]
             keys.push(nil) if options[:has_global_records]
 
@@ -138,7 +120,7 @@ def acts_as_tenant(tenant = :account, options = {})
             if options[:polymorphic]
               m.send("#{fkey}=".to_sym, ActsAsTenant.current_tenant.id)
               m.send("#{polymorphic_type}=".to_sym, ActsAsTenant.current_tenant.class.to_s)
-            else
+            elsif ActsAsTenant.klass_for_current_tenant == tenant
               m.send "#{fkey}=".to_sym, ActsAsTenant.current_tenant.id
             end
           end
@@ -173,14 +155,14 @@ def acts_as_tenant(tenant = :account, options = {})
             integer
           end
 
-          define_method "#{ActsAsTenant.tenant_klass.to_s}=" do |model|
+          define_method "#{tenant.to_s}=" do |model|
             super(model)
             raise ActsAsTenant::Errors::TenantIsImmutable if send("#{fkey}_changed?") && persisted? && !send("#{fkey}_was").nil?
             model
           end
 
-          define_method "#{ActsAsTenant.tenant_klass.to_s}" do
-            if !ActsAsTenant.current_tenant.nil? && send(fkey) == ActsAsTenant.current_tenant.id
+          define_method "#{tenant.to_s}" do
+            if !ActsAsTenant.current_tenant.nil? && send(fkey) == ActsAsTenant.current_tenant.id && ActsAsTenant.klass_for_current_tenant == tenant
               return ActsAsTenant.current_tenant
             else
               super()
@@ -196,10 +178,10 @@ def scoped_by_tenant?
         end
       end
 
-      def validates_uniqueness_to_tenant(fields, args ={})
+      def validates_uniqueness_to_tenant(fields, args = {})
         raise ActsAsTenant::Errors::ModelNotScopedByTenant unless respond_to?(:scoped_by_tenant?)
-        fkey = reflect_on_association(ActsAsTenant.tenant_klass).foreign_key
-        #tenant_id = lambda { "#{ActsAsTenant.fkey}"}.call
+        tenant = args[:tenant] || :account
+        fkey = reflect_on_association(tenant).foreign_key
         if args[:scope]
           args[:scope] = Array(args[:scope]) << fkey
         else
@@ -222,7 +204,6 @@ def validates_uniqueness_to_tenant(fields, args ={})
                                  .where.not(:id => instance.id).empty?
                   errors.add(field, 'has already been taken')
                 end
-
               end
             end
           end

spec/active_record_models.rb

@@ -81,8 +81,10 @@ class Project < ActiveRecord::Base
 end
 
 class Manager < ActiveRecord::Base
-  belongs_to :project
   acts_as_tenant :account
+  acts_as_tenant :project
+
+  validates_uniqueness_to_tenant :name, tenant: :project
 end
 
 class Task < ActiveRecord::Base

spec/acts_as_tenant/model_extensions_spec.rb

@@ -215,7 +215,7 @@
         @account = Account.create!(name: 'foo')
         @project = Project.create!(name: 'polymorphic project')
         ActsAsTenant.current_tenant = @project
-        @comment = PolymorphicTenantComment.new(account: @account)
+        @comment = PolymorphicTenantComment.create!(account: @account)
       end
 
       it 'populates commentable_type with the current tenant' do
@@ -463,4 +463,98 @@
       expect(ActsAsTenant.current_tenant).to eq(@account)
     end
   end
+
+  describe 'overlapping tenants' do
+    before(:each) do
+      ActsAsTenant.without_tenant do
+        @account1 = Account.create!
+        @account2 = Account.create!
+        @project1 = Project.create!(name: 'Project1')
+        @project2 = Project.create!(name: 'Project2')
+      end
+
+      ActsAsTenant.with_tenant(@account1) do
+        @manager1 = Manager.create!(name: 'Manager1')
+      end
+
+      ActsAsTenant.with_tenant(@project1) do
+        @manager2 = Manager.create!(account: @account1, name: 'Manager2')
+        @manager1.update!(project_id: @project1.id)
+      end
+
+      ActsAsTenant.with_tenant(@project2) do
+        @manager3 = Manager.create!(account: @account1, name: 'Manager3')
+      end
+    end
+
+    context 'when current tenant is the first tenant defined for this model' do
+      it 'correctly scopes to the first tenant' do
+        expect(ActsAsTenant.with_tenant(@account1) { Manager.count }).to eq(3)
+        expect(ActsAsTenant.with_tenant(@account2) { Manager.count }).to eq(0)
+      end
+    end
+
+    context 'when current tenant is the second tenant defined for this model' do
+      it 'correctly scopes to the second tenant' do
+        expect(ActsAsTenant.with_tenant(@project1) { Manager.count }).to eq(2)
+        expect(ActsAsTenant.with_tenant(@project2) { Manager.count }).to eq(1)
+
+        expect(ActsAsTenant.with_tenant(@project1) { Manager.first }).to eq(@manager1)
+        expect(ActsAsTenant.with_tenant(@project1) { Manager.second }).to eq(@manager2)
+        expect(ActsAsTenant.with_tenant(@project2) { Manager.first }).to eq(@manager3)
+      end
+    end
+
+    context 'when creating a new model' do
+      before do
+        ActsAsTenant.current_tenant = @project1
+        @manager4 = Manager.create!(account: @account1, name: 'Manager4')
+        @manager5 = Manager.create!(name: 'Manager5')
+      end
+
+      it 'scopes it to overlapping tenants' do
+        expect(ActsAsTenant.with_tenant(@project1) { Manager.find_by(id: @manager4.id)}).not_to be_nil
+        expect(ActsAsTenant.with_tenant(@project2) { Manager.find_by(id: @manager4.id)}).to be_nil
+        expect(ActsAsTenant.with_tenant(@account1) { Manager.find_by(id: @manager4.id)}).not_to be_nil
+
+        expect(ActsAsTenant.with_tenant(@project1) { Manager.find_by(id: @manager5.id)}).not_to be_nil
+        expect(ActsAsTenant.with_tenant(@account1) { Manager.find_by(id: @manager5.id)}).to be_nil
+      end
+    end
+
+    context 'when trying to overwrite tenant on existing model' do
+      it 'should raise an error' do
+        expect { @manager1.update!(project_id: @project2.id) }.to raise_error ActsAsTenant::Errors::TenantIsImmutable
+        expect { @manager1.update!(account_id: @account2.id) }.to raise_error ActsAsTenant::Errors::TenantIsImmutable
+        expect { @manager1.update!(project_id: @account1.id) }.not_to raise_error
+        expect { @manager1.update!(project_id: @project1.id) }.not_to raise_error
+        expect {
+          ActsAsTenant.without_tenant { @manager1.update!(project_id: @project2.id) }
+        }.to raise_error ActsAsTenant::Errors::TenantIsImmutable
+        expect {
+          ActsAsTenant.with_tenant(@project2) { @manager1.update!(project_id: @project2.id) }
+        }.to raise_error ActsAsTenant::Errors::TenantIsImmutable
+      end
+    end
+
+    context 'when validating uniqueness to tenant' do
+      it 'only enforces uniqueness to the given tenant' do
+        expect(
+          ActsAsTenant.without_tenant { @manager2.update(name: 'Manager1') }
+        ).to eq(false)
+
+        expect(
+          ActsAsTenant.without_tenant { Manager.create(project: @project1, name: 'Manager1').valid? }
+        ).to eq(false)
+
+        expect(
+          ActsAsTenant.without_tenant { Manager.create(account: @account1, name: 'Manager1').valid? }
+        ).to eq(true)
+
+        expect(
+          ActsAsTenant.without_tenant { Manager.create(project: @project2, name: 'Manager1').valid? }
+        ).to eq(true)
+      end
+    end
+  end
 end