I’ll start this discussion with an initializer I just added to my Rails app:
ActiveSupport.on_load(:active_record) do
ActiveRecord::Persistence::ClassMethods.module_eval do
[:update, :update!].each do |m|
remove_method(m)
define_method(m) do |*_|
raise <<~TXT.squish
The class method `#{m}` has been banned.
This method will load and then mutate every single record
in the database. This is probably not what you want to be doing,
and if it is, there are safer and faster ways of doing it.
TXT
end
end
end
end
You can probably guess what happened. I was fixing a couple bits of data in production. I had my record to fix in a variable user and typed user.update(attrs). Or, that’s what I meant to type. I actually typed User.update(attrs). The next hour of my day was pulling a point-in-time database backup and recovering all the data I had clobbered.
Having update and update! as class-level persistence methods is making a dangerous weapon readily available. You’re one character away from clobbering every record in a table. If the application doesn’t have robust audit logging or you don’t have point-in-time backups, this will do irreparable damage to your database.
IMO dangerous operations should not be convenient and easy. If someone really wants to load and then update every single record in the database, let them do it with a loop. Then it’s blindingly obvious what you’re doing.
If there’s a desire to keep these methods around, how about renaming them so it’s clear what you’re doing? Perhaps mass_update or update_each? update_all might be nice, but the behaviour would not be the same as when used with scopes/relations like User.where(x: "y").update_all(x: "z") which just fires-off a single UPDATE.
