jenseng / immigrant

Foreign key migration generator for Rails
MIT License
485 stars 24 forks source link

CI Support #22

Closed reconbot closed 9 years ago

reconbot commented 9 years ago

I added immigrant to my CI. I don't know if you'd like immigrant to include this functionality but I thought others might find it useful.

I made a rake task that checked for any missing foreign key relationships and exits with an error if it finds any.

namespace :immigrant do
  desc 'Checks for missing foreign key relationships in the database'
  task check_keys: :environment do
    Rails.application.eager_load!

    config_file = '.immigrant.yml'
    if File.exist?(config_file)
      ignore_keys = YAML.load_file(config_file)
    else
      ignore_keys = []
    end

    keys, warnings = Immigrant::KeyFinder.new.infer_keys
    warnings.values.each { |warning| $stderr.puts "WARNING: #{warning}" }

    keys.select! do |key|
      !ignore_keys.include?('from_table' => key.from_table, 'to_table' => key.to_table)
    end

    keys.each do |key|
      column = key.options[:column]
      pk = key.options[:primary_key]
      $stderr.puts "Missing foreign key relationship on '#{key.from_table}.#{column}' to '#{key.to_table}.#{pk}'"
    end

    if keys.present?
      puts 'Found missing foreign keys, run `rails generate immigration MigrationName` or add them.'
      puts "Or add them to #{config_file} to ignore them."
      exit keys.count
    end
  end
end

The .immigrant.yml looks like this


---
- from_table: shipments
  to_table: clients
- from_table: shipments
  to_table: routes

I welcome feedback and I'm happy to make a PR if you want to add the feature.

jenseng commented 9 years ago

that's not a bad idea. personally i usually create a spec/test that does this, e.g.

describe "Foreign keys" do
  it "exist for all associations" do
    Rails.application.eager_load!
    missing_keys, = Immigrant::KeyFinder.new.infer_keys
    missing_key_columns = missing_keys.map { |key| "#{key.from_table}.#{key.options[:column]}" }

    expect(missing_key_columns).to be_empty, -> {
      author = `git --no-pager show -s --format='%an'`.gsub(/\s/, "").camelize
      <<-TEXT.strip_heredoc
        expected all foreign keys to exist, no key(s) found for:
          * #{missing_key_columns.join("\n        * ")}

        Hint: run `rails g immigration #{author}TotesForgotAboutReferentialIntegrity`
              to automatically add all missing keys

        You can change the migration name if you really want to :P
      TEXT
    }
  end
end

though adding a single rake task to your CI is a little easier than copy/pasting an entire spec like that

i'd happily accept a PR along these lines, just a couple suggestions:

  1. have the .immigrant.yml stuff be a separate PR (seems like a useful feature on its own). also i'd say move that logic into Immigrant::KeyFinder#infer_keys (or wherever). that way it doesn't need to be done in the rake task, and you'd get consistent behavior both in the generator and the rake task
  2. for the .immigrant.yml, i'd maybe stick that list under an ignore: key ... that way other settings/overrides could be added down the road (e.g. if we make #19 configurable)
jenseng commented 9 years ago

v0.3.2 has been released with the new rake task. sorry about the delay :( ... thought i had already done it

reconbot commented 9 years ago

Lets keep this ticket open until I or someone else builds ignore support!

reconbot commented 9 years ago

<3