Skip to content

Commit c1d9640

Browse files
authored
Fix crash when deleting an association with composite primary keys (#450)
* Fix crash when deleting an association with composite primary keys * Skip test for older version of rails. Consistent mapping to symbol
1 parent 96254d2 commit c1d9640

File tree

2 files changed

+110
-3
lines changed

2 files changed

+110
-3
lines changed

lib/acts_as_list/active_record/acts/scope_method_definer.rb

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,16 @@ def self.call(caller_class, scope)
2121
end
2222

2323
define_method :destroyed_via_scope? do
24-
scope == (destroyed_by_association && destroyed_by_association.foreign_key.to_sym)
24+
return false unless destroyed_by_association
25+
26+
foreign_key = destroyed_by_association.foreign_key
27+
if foreign_key.is_a?(Array)
28+
# Composite foreign key - check if scope is one of the keys
29+
foreign_key.map(&:to_sym).include?(scope)
30+
else
31+
# Single foreign key
32+
scope == foreign_key.to_sym
33+
end
2534
end
2635
elsif scope.is_a?(Array)
2736
define_method :scope_condition do
@@ -44,7 +53,16 @@ def self.call(caller_class, scope)
4453
end
4554

4655
define_method :destroyed_via_scope? do
47-
scope_condition.keys.include? (destroyed_by_association && destroyed_by_association.foreign_key.to_sym)
56+
return false unless destroyed_by_association
57+
58+
foreign_key = destroyed_by_association.foreign_key
59+
if foreign_key.is_a?(Array)
60+
# Composite foreign key - check if any keys overlap with scope
61+
(scope_condition.keys & foreign_key.map(&:to_sym)).any?
62+
else
63+
# Single foreign key
64+
scope_condition.keys.include?(foreign_key.to_sym)
65+
end
4866
end
4967
else
5068
define_method :scope_condition do
@@ -68,7 +86,10 @@ def self.idify(caller_class, name)
6886
return name if name.to_s =~ /_id$/
6987

7088
if caller_class.reflections.key?(name.to_s)
71-
caller_class.reflections[name.to_s].foreign_key.to_sym
89+
foreign_key = caller_class.reflections[name.to_s].foreign_key
90+
# Handle composite foreign keys (Arrays) by returning as-is
91+
# Single foreign keys should be converted to symbols
92+
foreign_key.is_a?(Array) ? foreign_key.map(&:to_sym) : foreign_key.to_sym
7293
else
7394
ActiveSupport::Inflector.foreign_key(name).to_sym
7495
end

test/test_composite_foreign_key.rb

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# frozen_string_literal: true
2+
3+
require 'helper'
4+
5+
# Composite foreign keys were introduced in Rails 7.2
6+
if ActiveRecord::VERSION::MAJOR == 7 && ActiveRecord::VERSION::MINOR >= 2 || ActiveRecord::VERSION::MAJOR > 7
7+
class CompositeForeignKeyParent < ActiveRecord::Base
8+
self.table_name = 'composite_foreign_key_parents'
9+
end
10+
11+
class CompositeForeignKeyChild < ActiveRecord::Base
12+
self.table_name = 'composite_foreign_key_children'
13+
14+
# Composite foreign key relationship
15+
belongs_to :composite_foreign_key_parent,
16+
foreign_key: [:parent_id, :child_key],
17+
primary_key: [:id, :name],
18+
optional: false
19+
20+
acts_as_list scope: [:parent_id, :child_key, :category]
21+
end
22+
23+
class CompositeForeignKeyTestCase < Minitest::Test
24+
def setup
25+
ActiveRecord::Base.connection.create_table :composite_foreign_key_parents do |t|
26+
t.string :name
27+
t.timestamps
28+
end
29+
30+
ActiveRecord::Base.connection.create_table :composite_foreign_key_children do |t|
31+
t.integer :parent_id
32+
t.string :child_key
33+
t.string :category
34+
t.integer :position
35+
t.timestamps
36+
end
37+
38+
ActiveRecord::Base.connection.schema_cache.clear!
39+
[CompositeForeignKeyParent, CompositeForeignKeyChild].each(&:reset_column_information)
40+
41+
# Set up the has_many association dynamically
42+
CompositeForeignKeyParent.has_many :composite_foreign_key_children,
43+
foreign_key: [:parent_id, :child_key],
44+
primary_key: [:id, :name],
45+
dependent: :destroy
46+
end
47+
48+
def teardown
49+
teardown_db
50+
end
51+
52+
def test_allows_destroying_parent_with_composite_foreign_key_children
53+
parent = CompositeForeignKeyParent.create!(name: 'test')
54+
child1 = CompositeForeignKeyChild.create!(
55+
parent_id: parent.id,
56+
child_key: 'test',
57+
category: 'A',
58+
position: 1
59+
)
60+
child2 = CompositeForeignKeyChild.create!(
61+
parent_id: parent.id,
62+
child_key: 'test',
63+
category: 'A',
64+
position: 2
65+
)
66+
67+
# When parent is destroyed with dependent: :destroy, it will destroy children
68+
# The child's destroyed_via_scope? method will be called
69+
# Without the fix, this raises: NoMethodError: undefined method 'to_sym' for an instance of Array
70+
assert parent.destroy
71+
assert_equal 0, CompositeForeignKeyChild.count
72+
end
73+
74+
def test_allows_destroying_child_with_composite_foreign_key
75+
parent = CompositeForeignKeyParent.create!(name: 'test')
76+
child = CompositeForeignKeyChild.create!(
77+
parent_id: parent.id,
78+
child_key: 'test',
79+
category: 'A',
80+
position: 1
81+
)
82+
83+
assert child.destroy
84+
end
85+
end
86+
end

0 commit comments

Comments
 (0)