Skip to content

Commit d75cbbd

Browse files
MatheusRichclaude
andcommitted
Fix transaction to wrap Migration.new
Move the transaction from the instance `run` method to `self.run` so it wraps both initialization and execution. This ensures database operations in `initialize` are rolled back on failure. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent d0b21fe commit d75cbbd

2 files changed

Lines changed: 26 additions & 9 deletions

File tree

lib/data_customs/migration.rb

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,22 @@ class Migration
55
DEFAULT_BATCH_SIZE = 1000
66
DEFAULT_THROTTLE = 0.01
77

8-
def self.run(...) = new(...).run
8+
def self.run(...)
9+
ActiveRecord::Base.transaction do
10+
new(...).run
11+
end
12+
rescue => e
13+
warn "🛃 Data migration failed"
14+
raise e
15+
end
916

1017
def up = raise NotImplementedError
1118
def verify! = raise NotImplementedError
1219

1320
def run
14-
ActiveRecord::Base.transaction do
15-
up
16-
verify!
17-
puts "🛃 Data migration ran successfully!"
18-
rescue => e
19-
warn "🛃 Data migration failed"
20-
raise e
21-
end
21+
up
22+
verify!
23+
puts "🛃 Data migration ran successfully!"
2224
end
2325

2426
private

spec/data_customs/migration_spec.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,21 @@ def verify! = raise "Should not reach this"
5252
.and raise_error("Kaboom")
5353
end
5454

55+
it "rolls back db operations in initialize if up fails" do
56+
migration = Class.new(DataCustoms::Migration) do
57+
def initialize
58+
TestUser.create!(name: "Created in initialize")
59+
end
60+
61+
def up = raise "Kaboom"
62+
def verify! = nil
63+
end
64+
65+
expect { migration.run }
66+
.to raise_error("Kaboom")
67+
.and change { TestUser.count }.by(0)
68+
end
69+
5570
describe "helpers" do
5671
it "batches records" do
5772
3.times { |i| TestUser.create!(name: "User #{i}") }

0 commit comments

Comments
 (0)