Backfilling data in the same migration as adding a column not being caught?

[Rockster160]

Using the example provided by the documentation as an example of a bad migration:

def change
  add_column :users, :some_column, :text
  User.update_all some_column: "default_value"
end

strong_migrations does not catch this and allows it to run.

We accidentally let one of these slip through and it caused the DB to lock while running the backfill.
I thought this used to be caught- we moved to strong_migrations from zero_downtime_migrations which definitely caught this.

If it's not built-in, is there a a way to set up a custom check to raise an error when these are detected?

Rails 6.1.7.3
ruby 3.2.2
strong_migrations-1.4.1

[ankane]

Hi @Rockster160, Strong Migrations won't catch this (and hasn't in the past). You could use a custom check to track that add_column was run and patch update/update_all, but Strong Migrations doesn't currently patch models.

[Rockster160]

That's unfortunate. 😞
It may be beneficial to communicate that in the documentation- reading through, especially with the examples, lead us to believe that it would check this.

For now, we added a very hacky solution where we just parse the file and look for a few blacklisted key words.

StrongMigrations.add_check do |method, args|
  filename = Dir["db/migrate/#{version}_*.rb"].first
  contents = File.read(filename)
  blacklist_updates = [:update, :save, :create]
  blacklist_updates.each do |word|
    # Check if any uncommented lines contain any of the blacklisted words
    blacklist_regex = /^(?!.*#.*#{word})(?!.*#{word}.*#).*#{word}.*?/
    next unless contents.match?(blacklist_regex)

    message = "Don't backfill data in the same migration as adding columns!"
    stop!(message, header: "Unsafe Data Migration")
  end
end

This looks for update, save, and create keywords whenever regular actions are run and raises an error in the case any are found.
I'm sure there are caveats, but for now, it's more worth it to be extra safe.

[Capncavedan]

@Rockster160 thanks for this example! I'm adapting it for our own use.

In walking through the regex, I noticed that it passes the following example with a comment after the keyword, when I think one might want it to call stop!.

  def change
    User.update_all("full_name = 'oops'") # this should be stopped, but it is not
  end

It's the highlighted portion here:

Thanks again - very helpful.
Cheers!

[Rockster160]

You're right! I'm unsure what the reasoning behind that was!
Also another change was allowing create* by changing the list to this:

blacklist_updates = [:update, :save, "create\b"]