Skip to content

Commit 20d09a8

Browse files
Fixes Model.limit(n).delete_all & Model.limit(n).update_all generates incorrect query (#200)
* Adds initial implementation but still failing * Users/amit909sin/issue 195 (#219) * Fix the tenant scoping for delete_all --------- Co-authored-by: amit909singh <[email protected]>
1 parent d91e62c commit 20d09a8

File tree

4 files changed

+67
-1
lines changed

4 files changed

+67
-1
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# frozen_string_literal: true
2+
3+
module Arel
4+
module ActiveRecordRelationExtension
5+
# Overrides the delete_all method to include tenant scoping
6+
def delete_all
7+
# Call the original delete_all method if the current tenant is identified by an ID
8+
return super if MultiTenant.current_tenant_is_id? || MultiTenant.current_tenant.nil?
9+
10+
tenant_key = MultiTenant.partition_key(MultiTenant.current_tenant_class)
11+
tenant_id = MultiTenant.current_tenant_id
12+
arel = eager_loading? ? apply_join_dependency.arel : build_arel
13+
arel.source.left = table
14+
15+
if tenant_id && klass.column_names.include?(tenant_key)
16+
# Check if the tenant key is present in the model's column names
17+
tenant_condition = table[tenant_key].eq(tenant_id)
18+
# Add the tenant condition to the arel query if it is not already present
19+
unless arel.constraints.any? { |node| node.to_sql.include?(tenant_condition.to_sql) }
20+
arel = arel.where(tenant_condition)
21+
end
22+
end
23+
24+
subquery = arel.clone
25+
subquery.projections.clear
26+
subquery = subquery.project(table[primary_key])
27+
in_condition = Arel::Nodes::In.new(table[primary_key], subquery.ast)
28+
stmt = Arel::DeleteManager.new.from(table)
29+
stmt.wheres = [in_condition]
30+
31+
# Execute the delete statement using the connection and return the result
32+
klass.connection.delete(stmt, "#{klass} Delete All").tap { reset }
33+
end
34+
end
35+
end
36+
37+
# Patch ActiveRecord::Relation with the extension module
38+
ActiveRecord::Relation.prepend(Arel::ActiveRecordRelationExtension)

lib/activerecord-multi-tenant/multi_tenant.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ def self.tenant_klass_defined?(tenant_name, options = {})
1717
end
1818

1919
def self.partition_key(tenant_name)
20-
"#{tenant_name}_id"
20+
"#{tenant_name.to_s.underscore}_id"
2121
end
2222

2323
# rubocop:disable Style/ClassVars

lib/activerecord_multi_tenant.rb

+1
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@
1111
require_relative 'activerecord-multi-tenant/table_node'
1212
require_relative 'activerecord-multi-tenant/version'
1313
require_relative 'activerecord-multi-tenant/habtm'
14+
require_relative 'activerecord-multi-tenant/delete_operations'

spec/activerecord-multi-tenant/query_rewriter_spec.rb

+27
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,39 @@
4545
let!(:manager1) { Manager.create(name: 'Manager 1', project: project1, account: account) }
4646
let!(:manager2) { Manager.create(name: 'Manager 2', project: project2, account: account) }
4747

48+
before(:each) do
49+
@queries = []
50+
ActiveSupport::Notifications.subscribe('sql.active_record') do |_name, _started, _finished, _unique_id, payload|
51+
@queries << payload[:sql]
52+
end
53+
end
54+
55+
after(:each) do
56+
ActiveSupport::Notifications.unsubscribe('sql.active_record')
57+
end
58+
4859
it 'delete_all the records' do
60+
expected_query = <<-SQL.strip
61+
DELETE FROM "projects" WHERE "projects"."id" IN
62+
(SELECT "projects"."id" FROM "projects"
63+
INNER JOIN "managers" ON "managers"."project_id" = "projects"."id"
64+
and "managers"."account_id" = :account_id
65+
WHERE "projects"."account_id" = :account_id
66+
)
67+
AND "projects"."account_id" = :account_id
68+
SQL
69+
4970
expect do
5071
MultiTenant.with(account) do
5172
Project.joins(:manager).delete_all
5273
end
5374
end.to change { Project.count }.from(3).to(1)
75+
76+
@queries.each do |actual_query|
77+
next unless actual_query.include?('DELETE FROM ')
78+
79+
expect(format_sql(actual_query)).to eq(format_sql(expected_query.gsub(':account_id', account.id.to_s)))
80+
end
5481
end
5582

5683
it 'delete_all the records without a current tenant' do

0 commit comments

Comments
 (0)