From 60bfde1ea655189dec7f41fbba59b3adaf19bf21 Mon Sep 17 00:00:00 2001 From: amit909singh Date: Sun, 3 Dec 2023 05:36:32 -0800 Subject: [PATCH] Users/amit909sin/issue 195 (#219) * Fix the tenant scoping for delete_all --- .../delete_operations.rb | 41 ++++++++++--------- lib/activerecord-multi-tenant/multi_tenant.rb | 2 +- .../query_rewriter_spec.rb | 7 ++-- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/lib/activerecord-multi-tenant/delete_operations.rb b/lib/activerecord-multi-tenant/delete_operations.rb index 3508ad5..b2bfbf3 100644 --- a/lib/activerecord-multi-tenant/delete_operations.rb +++ b/lib/activerecord-multi-tenant/delete_operations.rb @@ -1,31 +1,34 @@ +# frozen_string_literal: true + module Arel module ActiveRecordRelationExtension - def delete_all(conditions = nil) - tenant_id = MultiTenant.current_tenant_id - tenant_key = MultiTenant.partition_key(MultiTenant.current_tenant_class) + # Overrides the delete_all method to include tenant scoping + def delete_all + # Call the original delete_all method if the current tenant is identified by an ID + return super if MultiTenant.current_tenant_is_id? || MultiTenant.current_tenant.nil? + tenant_key = MultiTenant.partition_key(MultiTenant.current_tenant_class) + tenant_id = MultiTenant.current_tenant_id arel = eager_loading? ? apply_join_dependency.arel : build_arel arel.source.left = table - group_values_arel_columns = arel_columns(group_values.uniq) - having_clause_ast = having_clause.ast unless having_clause.empty? - stmt = arel.compile_delete(table[primary_key], having_clause_ast, group_values_arel_columns) - - if tenant_id - tenant_condition = table[tenant_key.downcase].eq(tenant_id) - account_condition = table["account_id"].eq(tenant_id) - conditions = Arel::Nodes::And.new([tenant_condition, conditions].compact) - puts "conditions: #{conditions.to_sql}" - puts "tenant_id: #{tenant_id}" + if tenant_id && klass.column_names.include?(tenant_key) + # Check if the tenant key is present in the model's column names + tenant_condition = table[tenant_key].eq(tenant_id) + # Add the tenant condition to the arel query if it is not already present + unless arel.constraints.any? { |node| node.to_sql.include?(tenant_condition.to_sql) } + arel = arel.where(tenant_condition) + end end - puts "stmt klass: #{stmt.class}" - - if conditions - stmt.where(conditions) - end + subquery = arel.clone + subquery.projections.clear + subquery = subquery.project(table[primary_key]) + in_condition = Arel::Nodes::In.new(table[primary_key], subquery.ast) + stmt = Arel::DeleteManager.new.from(table) + stmt.wheres = [in_condition] - puts "stmtt: #{stmt.to_sql}" + # Execute the delete statement using the connection and return the result klass.connection.delete(stmt, "#{klass} Delete All").tap { reset } end end diff --git a/lib/activerecord-multi-tenant/multi_tenant.rb b/lib/activerecord-multi-tenant/multi_tenant.rb index 91abba8..e03008d 100644 --- a/lib/activerecord-multi-tenant/multi_tenant.rb +++ b/lib/activerecord-multi-tenant/multi_tenant.rb @@ -17,7 +17,7 @@ def self.tenant_klass_defined?(tenant_name, options = {}) end def self.partition_key(tenant_name) - "#{tenant_name}_id" + "#{tenant_name.to_s.underscore}_id" end # rubocop:disable Style/ClassVars diff --git a/spec/activerecord-multi-tenant/query_rewriter_spec.rb b/spec/activerecord-multi-tenant/query_rewriter_spec.rb index 9c506fe..2b6410e 100644 --- a/spec/activerecord-multi-tenant/query_rewriter_spec.rb +++ b/spec/activerecord-multi-tenant/query_rewriter_spec.rb @@ -57,7 +57,6 @@ end it 'delete_all the records' do - expected_query = <<-SQL.strip DELETE FROM "projects" WHERE "projects"."id" IN (SELECT "projects"."id" FROM "projects" @@ -65,6 +64,7 @@ and "managers"."account_id" = :account_id WHERE "projects"."account_id" = :account_id ) + AND "projects"."account_id" = :account_id SQL expect do @@ -72,12 +72,11 @@ Project.joins(:manager).delete_all end end.to change { Project.count }.from(3).to(1) - query_index = 0 - @queries.each_with_index do |actual_query, index| + + @queries.each do |actual_query| next unless actual_query.include?('DELETE FROM ') expect(format_sql(actual_query)).to eq(format_sql(expected_query.gsub(':account_id', account.id.to_s))) - query_index += 1 end end