Commit 75244061 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'dm-add-column-with-default-cop' into 'master'

Add cop that checks if add_column_with_default is used with up/down methods

See merge request !9077
parents e8f1bfaf 29a32cfc
class AddDevelopersCanMergeToProtectedBranches < ActiveRecord::Migration class AddDevelopersCanMergeToProtectedBranches < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction! disable_ddl_transaction!
def change def up
add_column_with_default :protected_branches, :developers_can_merge, :boolean, default: false, allow_null: false add_column_with_default :protected_branches, :developers_can_merge, :boolean, default: false, allow_null: false
end end
def down
remove_column :protected_branches, :developers_can_merge
end
end end
...@@ -14,7 +14,11 @@ class AddSubmittedAsHamToSpamLogs < ActiveRecord::Migration ...@@ -14,7 +14,11 @@ class AddSubmittedAsHamToSpamLogs < ActiveRecord::Migration
disable_ddl_transaction! disable_ddl_transaction!
def change def up
add_column_with_default :spam_logs, :submitted_as_ham, :boolean, default: false add_column_with_default :spam_logs, :submitted_as_ham, :boolean, default: false
end end
def down
remove_column :spam_logs, :submitted_as_ham
end
end end
require_relative '../../migration_helpers'
module RuboCop module RuboCop
module Cop module Cop
module Migration module Migration
# Cop that checks if columns are added in a way that doesn't require # Cop that checks if columns are added in a way that doesn't require
# downtime. # downtime.
class ColumnWithDefault < RuboCop::Cop::Cop class AddColumn < RuboCop::Cop::Cop
include MigrationHelpers include MigrationHelpers
WHITELISTED_TABLES = [:application_settings] WHITELISTED_TABLES = [:application_settings]
MSG = 'add_column with a default value requires downtime, ' \ MSG = '`add_column` with a default value requires downtime, ' \
'use add_column_with_default instead' 'use `add_column_with_default` instead'
def on_send(node) def on_send(node)
return unless in_migration?(node) return unless in_migration?(node)
......
require_relative '../../migration_helpers'
module RuboCop
module Cop
module Migration
# Cop that checks if `add_column_with_default` is used with `up`/`down` methods
# and not `change`.
class AddColumnWithDefault < RuboCop::Cop::Cop
include MigrationHelpers
MSG = '`add_column_with_default` is not reversible so you must manually define ' \
'the `up` and `down` methods in your migration class, using `remove_column` in `down`'
def on_send(node)
return unless in_migration?(node)
name = node.children[1]
return unless name == :add_column_with_default
node.each_ancestor(:def) do |def_node|
next unless method_name(def_node) == :change
add_offense(def_node, :name)
end
end
def method_name(node)
node.children.first
end
end
end
end
end
require_relative '../../migration_helpers'
module RuboCop module RuboCop
module Cop module Cop
module Migration module Migration
...@@ -5,7 +7,7 @@ module RuboCop ...@@ -5,7 +7,7 @@ module RuboCop
class AddIndex < RuboCop::Cop::Cop class AddIndex < RuboCop::Cop::Cop
include MigrationHelpers include MigrationHelpers
MSG = 'add_index requires downtime, use add_concurrent_index instead' MSG = '`add_index` requires downtime, use `add_concurrent_index` instead'
def on_def(node) def on_def(node)
return unless in_migration?(node) return unless in_migration?(node)
......
require_relative 'migration_helpers'
require_relative 'cop/migration/add_index' require_relative 'cop/migration/add_index'
require_relative 'cop/migration/column_with_default' require_relative 'cop/migration/add_column'
require_relative 'cop/migration/add_column_with_default'
require_relative 'cop/gem_fetcher' require_relative 'cop/gem_fetcher'
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/migration/add_column_with_default'
describe RuboCop::Cop::Migration::AddColumnWithDefault do
include CopHelper
subject(:cop) { described_class.new }
context 'in migration' do
before do
allow(cop).to receive(:in_migration?).and_return(true)
end
it 'registers an offense when add_column_with_default is used inside a change method' do
inspect_source(cop, 'def change; add_column_with_default :table, :column, default: false; end')
aggregate_failures do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([1])
end
end
it 'registers no offense when add_column_with_default is used inside an up method' do
inspect_source(cop, 'def up; add_column_with_default :table, :column, default: false; end')
expect(cop.offenses.size).to eq(0)
end
end
context 'outside of migration' do
it 'registers no offense' do
inspect_source(cop, 'def change; add_column_with_default :table, :column, default: false; end')
expect(cop.offenses.size).to eq(0)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment