Putting update logic in your migrations

Question

A couple of times I've been in the situation where I've wanted to refactor the design of some model and have ended up putting update logic in migrations. However, as far as I've understood, this is not good practice (especially since you are encouraged to use your schema file for deployment, and not your migrations). How do you deal with these kind of problems?

To clearify what I mean, say I have a User model. Since I thought there would only be two kinds of users, namely a "normal" user and an administrator, I chose to use a simple boolean field telling whether the user was an adminstrator or not.

However, after I while I figured I needed some third kind of user, perhaps a moderator or something similar. In this case I add a UserType model (and the corresponding migration), and a second migration for removing the "admin" flag from the user table. And here comes the problem. In the "add_user_type_to_users" migration I have to map the admin flag value to a user type. Additionally, in order to do this, the user types have to exist, meaning I can not use the seeds file, but rather create the user types in the migration (also considered bad practice). Here comes some fictional code representing the situation:

class CreateUserTypes < ActiveRecord::Migration
    def self.up
        create_table :user_types do |t|
            t.string :name, :nil => false, :unique => true
        end

        #Create basic types (can not put in seed, because of future migration dependency)
        UserType.create!(:name => "BASIC")
        UserType.create!(:name => "MODERATOR")
        UserType.create!(:name => "ADMINISTRATOR")
    end

    def self.down
        drop_table :user_types
    end
end

class AddTypeIdToUsers < ActiveRecord::Migration
    def self.up
        add_column :users, :type_id, :integer

        #Determine type via the admin flag
        basic = UserType.find_by_name("BASIC")
        admin = UserType.find_by_name("ADMINISTRATOR")
        User.all.each {|u| u.update_attribute(:type_id, (u.admin?) ? admin.id : basic.id)}

        #Remove the admin flag
        remove_column :users, :admin

        #Add foreign key
        execute "alter table users add constraint fk_user_type_id
            foreign key (type_id) references user_types (id)"
    end

    def self.down
        #Re-add the admin flag
        add_column :users, :admin, :boolean, :default => false

        #Reset the admin flag (this is the problematic update code)
        admin = UserType.find_by_name("ADMINISTRATOR")

        execute "update users set admin=true where type_id=#{admin.id}"

        #Remove foreign key constraint
        execute "alter table users drop foreign key fk_user_type_id"

        #Drop the type_id column
        remove_column :users, :type_id
    end
end

As you can see there are two problematic parts. First the row creation part in the first model, which is necessary if I would like to run all migrations in a row, then the "update" part in the second migration that maps the "admin" column to the "type_id" column.

Any advice?

Answer 1

I find it more 'unconventional' that you use fk's than that you load UserType with the old User.admin, which I guess happens pretty often.

If you use fk's you get ugly mysql errors that confuse the user. If otherwise you use AR validations and hooks to enforce the referential integrity you get pretty and well integrted error messages that do not break the user experience flow of your app.

Don't worry for a migration that will run once and think about the business logic that you're putting outside your code.

This is all matter of opinion/convention, but I hope you find my insights someway helpful.