FIX: Improve allowed_path column migration (PR #10321)

Because previous migration was already deployed and some databases were already migrated, I needed to add some conditions to the migration.

Previous migration - discourse/20200629232159_rename_path_whitelist_to_allowed_paths.rb at master · discourse/discourse · GitHub

What will happen in a scenario when previous migration was not run.

  1. column allowed_paths will be created
  2. allowed_path will be populated with data from path_whitelist
  3. rename column will be skipped - unless column_exists?(:embeddable_hosts, :allowed_paths)
  4. path_whitelist column will be dropped

What will happen in a scenario when previous migration was already run.

  1. column allowed_paths will not be created because already exists - unless column_exists?(:embeddable_hosts, :allowed_paths)
  2. Data will not be copied because path_whitelist is missing - if column_exists?(:embeddable_hosts, :path_whitelist) && column_exists?(:embeddable_hosts, :allowed_paths)
  3. rename column will be skipped - unless column_exists?(:embeddable_hosts, :allowed_paths)
  4. path_whitelist column deletion will be skipped - if column_exists?(:embeddable_hosts, :path_whitelist)

GitHub

I think this migration can be removed completely?

It can be done, but… some systems are already migration so we will have a number in schema_migrations without corresponding file. In theory, it is fine, but should we keep it as a reference?

I think there isn’t a good reason to keep it as a reference since it adds an overhead to all new Discourse instance.

    Migration::ColumnDropper.execute_drop(:embeddable_host, :path_whitelist)
    if column_exists?(:embeddable_hosts, :path_whitelist)
      Migration::ColumnDropper.mark_readonly('embeddable_hosts', 'path_whitelist')

      if column_exists?(:embeddable_hosts, :allowed_paths)
        DB.exec <<~SQL
          UPDATE embeddable_hosts
          SET allowed_paths = path_whitelist
        SQL
      end
  end