Project

General

Profile

Bug #13266

Migrations redefining actual model

Added by Partha Aji over 3 years ago. Updated about 1 year ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
-
Target version:
Difficulty:
Triaged:
Yes
Bugzilla link:
Pull request:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

It is a common practice that in a migration whenever we add an attribute to a AR model and need a default value, we
  1. Need to create a new class that extends ActiveRecord::Base
  2. Change the self.table_name attribute to point to the right table
  3. Only use this new class to update.

For example:

class AddAttributeFooToRepositories < ActiveRecord::Migration

 class MyRepoModel < ActiveRecordBase
   self.table_name = "katello_repositories" 
 end

 def change
    add_column :katello_repositories, :foo, :boolean, :default => true, :null => false

    # want the existing repos to have false
    MyRepoModel.each do |repo|
     repo.update_attributes!(:foo =>  false)
    end 
    #note  this is just an example MyRepoModel.update_all(:foo => false) would be preferred....
 end

end

In the above example we created a new class to represent "katello_repositories" instead of doing Katello::Repository.each . This is done to keep this change local to only this migration. Also it will not trigger a save in candlepin/pulp and run through that orchestration . Also If the Repository model got changed to something or the attribute got renamed in future migrations it will not affect this migration..
check http://blog.makandra.com/2010/03/how-to-use-models-in-your-migrations-without-killing-kittens/ for more info.

Any way problem is bunch of existing migrations follow the template below

class AddAttributeFooToRepositories < ActiveRecord::Migration

 class Katello::Repository < ActiveRecordBase
   self.table_name = "katello_repositories" 
 end

 def change
    add_column :katello_repositories, :foo, :boolean, :default => true, :null => false

    # want the existing repos to have false
    Katello::Repository.each do |repo|
     repo.update_attributes!(:foo =>  false)
    end 
 end

end

They are just unnecessarily redefining the same Model not creating a New class encapsulating the repositories table.

The migrations below need to be fixed to NOT redefine existing model but rather create a new classes encapsulating those tables.

20150901213759_remove_distributors.rb
2:  class Katello::Distributor < ActiveRecord::Base

20150908222711_drop_marketing_engineering_products.rb
2:  class Katello::MarketingProduct < ActiveRecord::Base

20150930183738_migrate_content_hosts.rb
2:  class Katello::HostCollectionAssociation < ActiveRecord::Base
6:  class Katello::ErrataAssociation < ActiveRecord::Base
10:  class Katello::RepositoryAssociation < ActiveRecord::Base
14:  class Katello::System < ActiveRecord::Base

20150507131145_update_composite_default_for_content_view.rb
2:  class Katello::ContentView

20140422000001_update_products_add_organization.rb
2:  class Katello::Product < ActiveRecord::Base

History

#1 Updated by Eric Helms over 3 years ago

  • Legacy Backlogs Release (now unused) set to 114

Also available in: Atom PDF