Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No Migration Path from Earlier Versions #257

Closed
MattRogish opened this issue Mar 28, 2012 · 3 comments
Closed

No Migration Path from Earlier Versions #257

MattRogish opened this issue Mar 28, 2012 · 3 comments

Comments

@MattRogish
Copy link

The earlier version used a slugs table that is not compatible with the 'history' module (the earlier version had a sequence to handle collisions).

We're trying to use the latest version to stay in sync, but there's no clear migration from the old slugs table to the new friendly_id_slugs. Or, no easy migration from the old slugs to the per-db-table method currently in favor.

This is a major bummer. A clearly defined "Here's how to upgrade" would be most welcome. Even better, a script to migrate from slugs to friendly_id_slugs would be best. My suspicion is that we'd want to re-name the slugs table, but I'm not sure what to do with the sequence column. Does friendly_id simply look at created_at ASC to figure out the sequence?

@MattRogish
Copy link
Author

Here's my first stab at a suitable migration to upgrade. An issue is that models that don't have any collisions (e.g. sequence == 1 but no other records) will have --1 stuck to the end of it. I've tried to determine what friendly_id does in the case of a model that didn't have a collision but then does, but I can't quite tell if this will break... I'll do some testing, but figured I'd put this up here as a start.

class UpgradeToFriendlyIdInTableSlug < ActiveRecord::Migration
  def up
    models = get_models

    models.each do |model|
      table_name = model.first.tableize

      add_column table_name, :slug, :string

      connection.execute("UPDATE #{table_name} t
                               , slugs s
                             SET slug           = CONCAT(s.name, '--', sequence)
                           WHERE sluggable_type = '#{model}'
                             AND sluggable_id   = t.id")

      add_index table_name, :slug
    end
  end

  def down
    models = get_models

    models.each do |model|
      table_name = model.first.tableize

      remove_column table_name, :slug
    end
  end

  private
  def get_models
    conn = ActiveRecord::Base.connection()

    conn.execute("SELECT DISTINCT sluggable_type FROM slugs ORDER BY sluggable_type")
  end
end

@bronson
Copy link

bronson commented May 3, 2012

Great migration @MattRogish. I cribbed off yours to write mine. I don't think this one has the --1 problem but no doubt it has others...

class CreateFriendlyIdSlugs < ActiveRecord::Migration
  # do migration in 2 steps so we can verify slugs are working properly
  # side-by-side with old app.  A later migration will delete the slugs table.

  class Slug < ActiveRecord::Base
  end

  class FriendlyIdSlug < ActiveRecord::Base
    validates_presence_of :slug, :sluggable_id
    validates_uniqueness_of :slug, :scope => :sluggable_type
  end

  def self.up
    create_table :friendly_id_slugs do |t|
      t.string   :slug,           :null => false
      t.integer  :sluggable_id,   :null => false
      t.string   :sluggable_type, :limit => 40
      t.datetime :created_at
    end

    get_tables.each do |table|
      add_column table, :slug, :string, :null => false
    end

    migrate_slugs

    add_index :friendly_id_slugs, :sluggable_id
    add_index :friendly_id_slugs, [:slug, :sluggable_type], :unique => true
    add_index :friendly_id_slugs, :sluggable_type
    get_tables.each do |table|
      add_index table, :slug, :unique => true
    end
  end

  def self.down
    get_tables.each do |table|
      remove_column table, :slug
    end

    drop_table :friendly_id_slugs
  end

  private
  def get_tables
    # good idea, thanks to https://github.com/norman/friendly_id/issues/257
    ActiveRecord::Base.connection().execute(
      "SELECT DISTINCT sluggable_type FROM slugs ORDER BY sluggable_type"
    ).map { |m| m.first.tableize }
  end

  def migrate_slugs
    Slug.all.each do |old|
      slug = FriendlyIdSlug.create { |new|
        new.slug           = old.name
        new.sluggable_id   = old.sluggable_id
        new.sluggable_type = old.sluggable_type
        new.created_at     = old.created_at
      }

      raise "no slug for " + old.inspect if old.name.blank?

      model = old.sluggable_type.constantize.send(:find_by_id, old.sluggable_id)
      if slug.valid?
        model.update_attribute :slug, old.name if model
      else
        # handle duplicate slugs (errors) by dropping them and regenerating
        STDERR.puts "Dropping Slug #{old.id} #{old.name}/#{old.sluggable_type}: " + slug.errors.full_messages.join(", ") + "\n  " + old.attributes.inspect
        model.send :set_slug    # generate a new slug to replace the bad one, why is set_slug private??
        model.save!
      end
    end
  end
end

@danieldkim
Copy link

just a couple of tweaks to add to Matt's migration:

  • slug indices should be unique
  • IF() function in update sql to avoid --1 suffix

here's the modified migration:

class UpgradeToFriendlyIdInTableSlug < ActiveRecord::Migration
  def up
    models = get_models

    models.each do |model|
      table_name = model.tableize

      add_column table_name, :slug, :string

      connection.execute("UPDATE #{table_name} t
                               , slugs s
                           SET slug           = IF(sequence > 1, CONCAT(s.name, '--', sequence), s.name)
                           WHERE sluggable_type = '#{model}'
                             AND sluggable_id   = t.id")

      add_index table_name, :slug, :unique => true

      # don't technically need the part below if this migration is run only once
      # but am adding this for people who have run it before then rolled it back
      # and ran it again after my changes, in which case there could be some null
      # slugs.
      model.constantize.where(:slug => nil).each { |o| o.save! }
    end
  end

  def down
    models = get_models

    models.each do |model|
      table_name = modeltableize

      remove_column table_name, :slug
    end
  end

  private
  def get_models
    conn = ActiveRecord::Base.connection()
    # Delete old, unused slugs (we removed google map and image map)
    conn.execute("DELETE FROM slugs WHERE sluggable_type IN('GoogleMap', 'ImageMap', 'Map')")
    conn.execute("SELECT DISTINCT sluggable_type FROM slugs ORDER BY sluggable_type").map {|row| row.first}
  end
end

@parndt parndt closed this as completed Feb 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants