Remove :_type_disabled for models in migrations, where type column doesn't exist

[lpichler]

related #6739
based on comments:
#5554 (comment) or #6739 (comment)

this script helpded me:

# prepare
system("mkdir db/migrate_temp")
print "."
Dir.glob("db/migrate/*").each do |f|
  next if File.directory?(f)
  next if f.include?("20130923182042_collapsed_initial_migration.rb")
  f_with_new_path = f.gsub('migrate', 'migrate_temp')
  system("mv #{f} #{f_with_new_path}")
  print "."
end
system("rake db:migrate &>/dev/null")
puts ".!"
def get_migration_info(f)
  type_column_exists = lambda { |t| ActiveRecord::Base.connection.column_exists?(t, :type) }
  table_exist = lambda { |t| ActiveRecord::Base.connection.data_source_exists?(t) }
  sti_disabled_in_stub = lambda do |content|
    content.include?("self.inheritance_column = :_type_disabled")
  end
  matches = File.read(f).scan(/\s+class (.*) < ActiveRecord::Base([\s\S]*?)end/)
  if matches.present?
    out_array = []
    matches.each do |match|
      klass_stub, klass_content = match
      tbl_exist                 = table_exist.call(klass_stub.tableize.to_sym)
      sti_dis_in_stub           = sti_disabled_in_stub.call(klass_content)
      sti_should_be_disabled    = tbl_exist ? type_column_exists.call(klass_stub.tableize.to_sym) : false

      unless sti_should_be_disabled == sti_dis_in_stub
        out_array.push({:f => f, :klass_stub    => klass_stub,
                        :table_exist            => tbl_exist,
                        :sti_disabled_stub      => sti_dis_in_stub,
                        :sti_should_be_disabled => sti_should_be_disabled})
      end
    end
    out_array
  else
    []
  end
end
i = 1
cnt = Dir.glob("db/migrate_temp/*").count
Dir.glob("db/migrate_temp/*").each do |f|
  next if File.directory?(f)

  f_with_new_path = f.gsub('migrate_temp', 'migrate')
  system("cp #{f} #{f_with_new_path}")
  system("rake db:migrate &>/dev/null")
  reload!
  migration_info = get_migration_info(f_with_new_path)
  if migration_info.present?
    migration_info.map! do |klass_stub|
      klass_stub[:suggestion] = klass_stub[:sti_should_be_disabled] ? "ADD it -> " : "REMOVE it -> "
      klass_stub[:where] = "#{klass_stub[:f]} for #{klass_stub[:klass_stub]}"
      puts klass_stub[:suggestion] + klass_stub[:where]
    end
    puts "."
  end
  print "*" + (cnt - i).to_s
  i = i + 1
end

cc @Fryguy @jrafanie

[Fryguy]

Really surprised on this one :)

[Fryguy]

@lpichler So does your script there check based on the latest tables, or what the state of the tables are at that time.

[lpichler]

@Fryguy no but now yes, I updated it in way that it based on state of tables at that time.

I also removed self.inheritance_column = :_type_disabled from MigrationStubHelper because only ExtManagementSystem(20131107000917_expand_dialog_field_default_value_size.rb) and DialogField(
20150407144345_add_kerberos_to_ext_management_system.rb) have needed it

[miq-bot]

Checked commit lpichler@64af459 with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1
60 files checked, 0 offenses detected
Everything looks good. 🏆

[jrafanie]

We might want to keep this for now...

The migration on the following day adds a type column:

db/migrate/20140410132430_subclass_file_depot_by_protocol.rb-  def up
db/migrate/20140410132430_subclass_file_depot_by_protocol.rb:    add_column :file_depots, :type, :string

[lpichler]

but when was this migration created the column was not here, so should I respect state of DB in that time or state of latest DB ?

[jrafanie]

@lpichler When this migration was created, it would have been possible to run these migrations out of order if you were the author of the other migration. Since we're long past this, I think it's fine to drop this as you have done... 👍

[jrafanie]

@lpichler @matthewd @Fryguy I wonder if the migration specs could monkey patch/module prepend AR::Base's inheritance_column = to check if the table for the stub model already has a type column and warn about it if not (calling super regardless)